Default emit_rerun_if_env_changed to true#738
Conversation
JohnTitor
left a comment
There was a problem hiding this comment.
Thanks for the explanation, makes sense to me 👍
|
I think this is a breaking change. In my CI, I was (perhaps wrongly) setting a bunch of environment variables when doing |
I think this is a misunderstanding. Cargo's behavior is that if your build script emits neither rerun-if-changed nor rerun-if-env-changed, then it will default to rerunning when any file within the package is changed. This is the fairly good default that you mention. But whether you emit a rerun-if-changed or rerun-if-env-changed, either way the default behavior goes away. |
|
Yeah we could improve our doc here to mention the default behavior of cargo |
|
Opened #1508 |
As mentioned in #701 (comment), I don't think there's a reason to default this to false.
rerun-if-env-changedis purely additive (unlikererun-if-changed, which has a fairly good default cleared if you emit it at all), so it should never hurt.I've also added code to skip emitting it for vars that provided by cargo, otherwise every use of
ccwill end up withcargo:rerun-if-env-changedinvocations for pointless things likeDEBUG,OPT_LEVEL, and so on. I don't think these are likely to hurt anything, but they definitely don't help, and cargo's docs say not to.An alternative to this would be to add a different function for vars provided by cargo, but that sounds like a headache to maintain, as reviewers could easily not notice the wrong function getting called for a given variable.
I'd like to get this in before the release, but mostly for the change of the default (since changing this after the release would be breaking) rather than skipping the cargo vars (which is not actually necessary).