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

#115269 broke builds in Fuchsia #115852

Closed
djkoloski opened this issue Sep 14, 2023 · 3 comments
Closed

#115269 broke builds in Fuchsia #115852

djkoloski opened this issue Sep 14, 2023 · 3 comments
Labels
C-bug Category: This is a bug. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@djkoloski
Copy link
Contributor

We just hit a CI failure in Fuchsia due to #115269 which corrected name resolution ordering for macro-expanded items. This change can break semver for crates. This change probably deserves a crater run to evaluate ecosystem impact, even if the old behavior is incorrect.

@djkoloski djkoloski added the C-bug Category: This is a bug. label Sep 14, 2023
@rustbot rustbot added the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Sep 14, 2023
@Urgau
Copy link
Member

Urgau commented Sep 15, 2023

Would you happen to have a minimal test case? or something we can reproduce?

Also there was a crater run: #115269 (comment)

@rustbot label +T-compiler +E-needs-mcve +regression-untriaged -needs-triage
cc @bvanjoi

@rustbot rustbot added E-needs-mcve Call for participation: This issue has a repro, but needs a Minimal Complete and Verifiable Example regression-untriaged Untriaged performance or correctness regression. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. I-prioritize Issue: Indicates that prioritization has been requested for this issue. and removed needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. labels Sep 15, 2023
@Noratrieb
Copy link
Member

I don't quite understand what you mean with "broke semver". Do you mean that it broke Fuchsias build? Or that it changes behavior? Not sure what this has to do with semver, no new releases of crates are being made.

@djkoloski
Copy link
Contributor Author

Sorry for the confusion @Nilstrieb . I say "broke semver" in the sense of changing the API surface in a way that we were able to observe (our build broke). That's mostly based on @obi1kenobi's efforts to track down unexpected semver violations, which has also found a lot of pre-existing problems with glob imports. It's probably not the best terminology for this situation, sorry for the confusion.

We got lucky in the sense that it was just a compiler error instead of a functionality change. I wanted to raise the issue since it seemed unlikely that we were the only ones who observed breakage. The crater run confirms that other crates were affected, but I see now that there was discussion about whether those breakages were acceptable. Since that discussion has already happened, I'm going to close this issue.

@djkoloski djkoloski changed the title #115269 broke semver in Fuchsia #115269 broke builds in Fuchsia Sep 15, 2023
@apiraino apiraino removed E-needs-mcve Call for participation: This issue has a repro, but needs a Minimal Complete and Verifiable Example I-prioritize Issue: Indicates that prioritization has been requested for this issue. regression-untriaged Untriaged performance or correctness regression. labels Sep 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: This is a bug. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

5 participants