Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should there be a rerun-if-changed for this, or does cargo rerun anyway when the cfg flags change?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A dedicated rerun-if-change definitely won't hurt, I think.
That said, presumably you actually don't want to re-run the script (and rebuild the world) when switching between --cfg=miri and not, if you've previously had it build successfully? RUST_CHECK has similar behavior where once we've built LLVM, it never gets passed down (even in check builds).
I admit I've not followed the story around cfg(miri) enough to know where we're setting that or how, can you point me to that?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
Miri sets
--cfg miri
on all things it builds: here the flag is defined, here it is added.Miri also uses a separate target dir, so for a given target dir, it should never be the case that it is sometimes built with
--cfg miri
and sometimes without.So that is some further extra magic and rustbuild implements?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, okay, if this never differs for a given target dir then it should be OK -- no need for special magic.
Yes, RUST_CHECK is only set by rustbuild (not Cargo or others); definition here.