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

Ignore failures of RLS on aarch64 Windows #86607

Closed

Conversation

Mark-Simulacrum
Copy link
Member

We've been putting this patch on beta/stable for multiple cycles now, it likely makes sense to just put it on master as well to avoid the extra cherry pick on each beta promotion. I've filed #86606 to track actually fixing this but I somewhat doubt it'll happen in the short term.

r? @pietroalbini

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jun 24, 2021
@pietroalbini
Copy link
Member

I'm all for moving the workaround to master instead of cherry-picking it to beta every single time, but I'm not too happy with this specific fix landing on master. The code that disables RLS is deep into rustbuild, and I'm worried someone will forget to remove it once RLS starts building again on that target.

Personally I'd be more happy with a generic BROKEN_TOOLS=rls environment variable we can set on CI, as that's a more visible place we'll be less likely to forget about.

@pietroalbini pietroalbini added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jun 25, 2021
@Mark-Simulacrum
Copy link
Member Author

I would prefer to avoid that. Hooks like that add additional complexity to the whole system as they're external configuration we always have to think about; this fix is local. I'm happy to add a note to the relevant issue that this code was added as a stopgap and add a FIXME to the code being added; I think that's more likely to not let us forget this.

@Mark-Simulacrum Mark-Simulacrum added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jun 26, 2021
@Mark-Simulacrum
Copy link
Member Author

Updated with a FIXME and added a note to the issue.

@Xanewok
Copy link
Member

Xanewok commented Jun 29, 2021

FWIW it looks like the build will be fixed on aarch64 Windows once rust-lang/rls#1741 is merged.

@pietroalbini
Copy link
Member

@Xanewok do you think that PR is going to be merged in this cycle?

@Xanewok
Copy link
Member

Xanewok commented Jun 30, 2021

Is the cut around July 27th? If so then yes, I'd say this PR is going to be merged then.

@JohnCSimon JohnCSimon added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jul 18, 2021
@Xanewok Xanewok mentioned this pull request Jul 21, 2021
@bors bors closed this in 4a1f419 Jul 23, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants