-
-
Notifications
You must be signed in to change notification settings - Fork 14.9k
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
tree-wide: disable doCheck
and doInstallCheck
where it fails
#39463
Conversation
For |
5b3fefd
to
7438083
Compare
I think might be interested: @grahamc @Profpatsch @Ekleog Have tried to check that all the cleanups are pure layout changes (toward more uniformity). I think I have read every change; the cleanups are pure cleanups. Please remind me in a few days where few is more than a couple, if nobody merges it before. |
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.
This PR adds doCheck = false
to GHC version 8.2.2, but not to any of the other versions we have even though they'll surely need the same setting. I suppose this PR cannot be expected to be "complete" since that would be a lot of packages to compile, but still it might be worthwhile to point out that switching to doCheck = true
would break a lot of packages -- even after these changes.
Ugh, this leads to rebuilds because |
This PR would be a rebuild anyway: there are some cleanups in the layout of the shell script parts of some expressions.
|
Interestingly, no, at least not for the older ones. I compiled a bunch of them and only this one was broken for some reason.
This PR is maybe a third of all the changes. And even then I didn't try to recompile every single thing, so there are going to be errors even after (and if) the whole patchset is merged. Also I wouldn't recommend switch
In the perfect world |
This PR is maybe a third of all the changes. And even then I didn't try to recompile every single thing, so there are going to be errors even after (and if) the whole patchset is merged.
I think breakage level below the breakage of an allegedely minor bump in a highly-used library should be considered acceptable — obviously, as a proponent of tristates I say that the current PR is useful regardless of the eventual default value of doCheck.
In the perfect world `mkDerivation` would normalize shell scripts that that would be a noop too :)
At that point we have to just go full-Guix-style with a language for builders. I mean, the builder text is accessible to the build, so technically it is possible to copy it to $out…
|
@peti do I understand your comment as «not complete but an OK step in the general direction»? Looking at the feedback I am inclined to just merge it before we get a formatting-based merge conflict or something like that. |
@7c6f434c, yes, sure. I am happy with the direction of this PR. |
Nice. I updated #39464 with most of the infrastructure. See the OP there. |
(the trivial part)
Motivation for this change
#33599. I implemented that issue for fun and giggles and now run a system that is "fully-tested". Turns out, not unexpectedly, an enormous number of packages fail to check. This is the first patch from that patchset. This patch disables many things that fail, but I couldn't (even partially) fix them. I.e. this patch contains only trivial
doCheck = false
changes and a couple of cleanups. The rest is coming later.Note that I started documenting error messages too late, hence many changes here just disable tests without comments. I'm too lazy to repeat the effort to document the "doCheck = false; # fails" lines as rebuilding and fixing everything multiple times with 33599-patchset took almost two weeks.
Note: For most packages you won't be able to check why they actually fail with just
doCheck = true;
without some more patches from the patchset because the patchset addscheckTraget
autodetection to stdenv.Things done