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

fix: Make context.mode always be GoConfigInfo #4203

Merged
merged 3 commits into from
Dec 30, 2024

Conversation

dzbarsky
Copy link
Contributor

What type of PR is this?
Bug fix

What does this PR do? Why is it needed?
Providers and structs with the same fields don't compare equal

Which issues(s) does this PR fix?
Fixes #4197

I wasn't able to figure out a repro in rules_go, it seems like it may rely on some transitions? Not sure we should block landing on this, it seems like this should be the right fix

@fmeum fmeum enabled auto-merge (squash) December 24, 2024 22:39
@dzbarsky
Copy link
Contributor Author

@fmeum I think this needs help to merge

@fmeum
Copy link
Member

fmeum commented Dec 30, 2024

I added a reproducer in #4206 and then applied the fix. The history of CI checks leads me to believe that this is a genuine failure caused by this change, I just don't know why...

@dzbarsky
Copy link
Contributor Author

I added a reproducer in #4206 and then applied the fix. The history of CI checks leads me to believe that this is a genuine failure caused by this change, I just don't know why...

Hmm your repro looks pretty similar to what I tried, I wonder why I didn't have success. Sadly I don't have a laptop with me to try it right now, but it seems good. Let's ship it!

@fmeum
Copy link
Member

fmeum commented Dec 30, 2024

We unfortunately can't ship since the fix legitimately breaks existing tests. I'll look into it.

Copy link
Member

@fmeum fmeum left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I found the fix

@fmeum fmeum merged commit 00e2be6 into bazel-contrib:master Dec 30, 2024
1 check passed
peng3141 added a commit to peng3141/rules_go that referenced this pull request Jan 3, 2025
peng3141 added a commit to peng3141/rules_go that referenced this pull request Jan 3, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Archive mode does not match when pure = off
2 participants