Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Revert "[chore] Use crosslink tidylist in make gotidy (#37142)" #37173

Merged
merged 1 commit into from
Jan 13, 2025

Conversation

TylerHelmuth
Copy link
Member

@TylerHelmuth TylerHelmuth commented Jan 13, 2025

Reverts #37142 as something with this change is not working as expected. Reverting for now to unblock PRs.

See #37170 and #37172

@TylerHelmuth TylerHelmuth requested a review from a team as a code owner January 13, 2025 17:16
@TylerHelmuth TylerHelmuth added the Skip Changelog PRs that do not require a CHANGELOG.md entry label Jan 13, 2025
@jade-guiton-dd
Copy link
Contributor

Could you point to a PR which shows non-deterministic behavior? As in, multiple runs of the CI where make tidylist has different outputs with the same inputs.

@TylerHelmuth
Copy link
Member Author

@jade-guiton-dd #37172 is an example. The CI is failing, telling me to run make tidylist locally, but when I run make tidylist locally again there are no changes.

@jade-guiton-dd
Copy link
Contributor

jade-guiton-dd commented Jan 13, 2025

@TylerHelmuth Do you have untracked changes to any go.mod files in your local copy, compared to main?

@TylerHelmuth
Copy link
Member Author

TylerHelmuth commented Jan 13, 2025

Do you have changes to any go.mod files in your local copy, compared to main?

No

@jade-guiton-dd
Copy link
Contributor

I see. In that case, it does sound like it's non-deterministic (or maybe platform-dependent).

@TylerHelmuth
Copy link
Member Author

I'm wondering if different go versions affect it. Can you keep working on a fix? I would love not to need to run make gotidy multiple times anymore.

@jade-guiton-dd
Copy link
Contributor

jade-guiton-dd commented Jan 13, 2025

Yes, I'll work on a fix tomorrow, a revert is fine in the meantime. Could you tell me what OS and version of Go you're using (the one used to compile crosslink) so I can try to replicate it?

@TylerHelmuth
Copy link
Member Author

Could you tell me what OS and version of Go you're using (the one used to compile crosslink) so I can try to replicate it?

MacOS 15.1.1 (24B91), Go 1.22.7 (was not able to get CI to match Go 1.22.10 either).

@TylerHelmuth TylerHelmuth merged commit 2c14176 into open-telemetry:main Jan 13, 2025
175 of 176 checks passed
@TylerHelmuth TylerHelmuth deleted the revert-tidylist branch January 13, 2025 17:47
@github-actions github-actions bot added this to the next release milestone Jan 13, 2025
@jade-guiton-dd
Copy link
Contributor

jade-guiton-dd commented Jan 13, 2025

I've figured out the issue: it was indeed a problem with "untracked changes to any go.mod files in your local copy", specifically whether make genotelcontribcol has been run or not (the output contains a go.mod which is .gitignored). I stripped cmd/otelcontribcol from the output of the tool, but it wasn't enough: the difference in the input dependency graph still changes the rest of the output quite significantly. The simplest option is probably for me to add an --exclude flag to the tool to make it completely ignore this module.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
internal/tools Skip Changelog PRs that do not require a CHANGELOG.md entry
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants