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
bintools-wrapper, cc-wrapper: infer propagateDoc automatically #46119
Conversation
Timed out, unknown build status on x86_64-linux (full log) Attempted: stdenv Partial log (click to expand)
|
Success on aarch64-linux (full log) Attempted: stdenv Partial log (click to expand)
|
09721c6
to
1bb1e7a
Compare
d0270a3
to
5297be3
Compare
5297be3
to
c8ba9df
Compare
Success on aarch64-linux (full log) Attempted: stdenv Partial log (click to expand)
|
Success on x86_64-linux (full log) Attempted: stdenv Partial log (click to expand)
|
c8ba9df
to
c33cf36
Compare
Success on aarch64-linux (full log) Attempted: stdenv Partial log (click to expand)
|
Success on x86_64-linux (full log) Attempted: stdenv Partial log (click to expand)
|
c33cf36
to
afe6c1c
Compare
Success on x86_64-linux (full log) Attempted: stdenv Partial log (click to expand)
|
Success on aarch64-linux (full log) Attempted: stdenv Partial log (click to expand)
|
Yay, a noop! But look at the pinned values of |
…y, but ... ... pin the old values in everywhere to avoid a mass rebuild.
afe6c1c
to
6805be7
Compare
I bet we could simplify this, but yeah looks good for now.
I'm 👎 on merging this.
Like I said in #46115 (comment), I think this is a nice demo that shows how broken those wrappers are on master (`propagateDoc` values are pretty much random), but it makes no sense to merge this PR alone, as it changes nothing w.r.t. the brokenness, but adds a ton of useless workarounds to be a noop.
Fixing brokenness does take a mass rebuild, i.e. #46115.
|
Oh. Well I won't on principle merge a PR that the author doesn't want merged, but I still think it would be good to use this. Then you can make the other remove all the hacks, old and new, which most clearly indicates the benefit symlinks. |
Success on aarch64-linux (full log) Attempted: stdenv Partial log (click to expand)
|
Success on x86_64-linux (full log) Attempted: stdenv Partial log (click to expand)
|
Symlinks are a separate issue. My problem here is that, as it turns out, This whole issue has long run out of its allocated time, so I'm not going to change anything related unless the motivation for that is internal for a while now. Closing this, feel free to do whatever, but I think #46115 is the correct thing to do, I'm merging that to SLNOS. |
02c09e0 (NixOS#44558) was reverted in c981787 but, as it turns out, it fixed an issue I didn't know about at the time: the values of `propagateDoc` options were (and now again are) inconsistent with the underlying things those wrappers wrap (see NixOS#46119), which was (and now is) likely to produce more instances of NixOS#43547, if not now, then eventually as stdenv changes. This patch (which is a simplified version of the original reverted patch) is the simplest solution to this whole thing: it forces wrappers to directly inspect the outputs of the things they are wrapping instead of making stdenv guess the correct values.
See discussion in #43547. This is a piece of #46115.