top-level/release.nix: add patchShebangs test #46756
Conversation
copumpkin
force-pushed
the
patch-shebangs-test
branch
from
da58d6e
to
c232dbb
Compare
| @@ -77,6 +77,7 @@ let | |||
| jobs.tests.cc-wrapper-libcxx-39.x86_64-darwin | |||
| jobs.tests.stdenv-inputs.x86_64-darwin | |||
| jobs.tests.macOSSierraShared.x86_64-darwin | |||
| jobs.tests.patch-shebangs.x86_64-darwin | |||
There was a problem hiding this comment.
Do I need to list these out? I was just following the pattern but it feels sort of redundant
There was a problem hiding this comment.
We could use [] ++ builtins.getAttrValues jobs.foo everywhere instead. However the unstable jobset would update if there's an evaluation error for one of the platforms, instead of making the job disappear.
This is currently failing but nobody noticed!
copumpkin
force-pushed
the
patch-shebangs-test
branch
from
c232dbb
to
88a969d
Compare
|
I'm wondering if we should add these to a separate combined job and add only that one to unstable, etc. channel jobs. |
|
I think I'd be up for that but am not entirely clear on what you mean. Can you elaborate or push a change to my branch? |
| let | ||
| bad-shebang = stdenv.mkDerivation { | ||
| name = "bad-shebang"; | ||
| unpackPhase = ":"; |
There was a problem hiding this comment.
I think it's better to do dontUnpack instead of this kind of weird trick.
There was a problem hiding this comment.
No that's not a thing AFAIK, this is a pretty common approach to skip unpacking and it's better than setting phases directly. But we have dont* variants of most other phases, it would make sense to add it.
There was a problem hiding this comment.
I knew of the other do* and dont*s, but I thought there was none for unpack.
There was a problem hiding this comment.
Lose your unpack phase with this one weird trick. Maintainers hate it!
There was a problem hiding this comment.
Yeah that's what I was thinking too :D
|
Yeah let's please do this!! It will greatly assist Matt and I with any further experiments too :). |
|
@matthewbauer pretty sure this test will fail on current staging, FYI, even with your change |
This is currently failing but nobody noticed!
Motivation for this change
The failure causes all sorts of subtle issues so best to catch it early and loudly.