Can we just do this unconditionally regardless of __structuredAttrs?

the current formulation avoids a mass rebuild

Did this regress recently? Otherwise I think it’s okay to wait for staging, since we’d want to clean this up on staging anyway. I’m also not sure how many rebuilds pkgsStatic is; we don’t build a full jobset on it, so maybe it’s fine to send to master?

Did this regress recently? Otherwise I think it’s okay to wait for staging, since we’d want to clean this up on staging anyway. I’m also not sure how many rebuilds pkgsStatic is; we don’t build a full jobset on it, so maybe it’s fine to send to master?

this regressed Lix static builds :'( — why not send this and clean it up on staging (by reverting this) as well?

also I don't want to know what happens if a derivation with { NIX_CFLAGS_LINK = "foo"; } is overridden to { NIX_CFLAGS_LINK = "foo"; env.NIX_CFLAGS_LINK = "foo -static"; }

Could someone link the commit / PR that's being referenced as the source of this issue?

Could someone link the commit / PR that's being referenced as the source of this issue?

#394674

also I don't want to know what happens if a derivation with { NIX_CFLAGS_LINK = "foo"; } is overridden to { NIX_CFLAGS_LINK = "foo"; env.NIX_CFLAGS_LINK = "foo -static"; }

There will be an evaluation error. However quite a few packages already override NIX_CFLAGS_LINK manually so that I think it's still enough of a footgun that it should better be avoided.

However what could work is basically inverting the current test and putting it in env by default, and only in the attrs themselves when it's already there.

that would have the effect that (stdenv.mkDerivation { NIX_CFLAGS_LINK = "foo"; }).overrideAttrs { NIX_CFLAGS_LINK = "bar"; }) would work in top level pkgs and fail to evaluate in pkgsStatic.

That would actually still work because NIX_CFLAGS_COMPILE is already in the original. I assume you mean that (stdenv.mkDerivation { }).overrideAttrs { NIX_CFLAGS_LINK = "bar"; } would break, which is indeed the case?

wouldn't the adapter move the first NIX_CFLAGS_LINK to env and the the override add it to top-level ? anyway your example also illustrates it well.

My proposal was not that the adapter would move it. It would append to NIX_CFLAGS_COMPILE if it's already there and create or append to env.NIX_CFLAGS_COMPILE otherwise. However I think your approach of checking for __structuredAttrs has even less potential for breakage and hence might be preferable.

this also breaks the configureFlags and cmakeFlags just below because the conditional has lower precedence than the attrset merge

well spotted, thanks

On behalf of Lix, thank you so much @symphorien!

Bisect leads me here: This broke pkgsStatic.libpq.

According to #429535 it also broke pkgsStatic.postgresql.

It would have been great to mention this PR in the structuredAttrs tracking issue #205690.

Note that the issue does not appear to be related to __structuredAttrs itself, but due the pre-existing precedence issue that was fixed here because the __structuredAttrs change would have made it worse. I am investigating it but it could possibly take some time because postgresql builds a full LLVM.

Note that the issue does not appear to be related to __structuredAttrs itself, but due the pre-existing precedence issue that was fixed here because the __structuredAttrs change would have made it worse.

I remember pkgsStatic for both postgresql and libpq only started to work when we enabled structuredAttrs - as a side effect of unknown reason. I think what happened is, that this bug worked in favor for this case - postgresql can't take -static in NIX_CFLAGS_LINK, it will then fail with:

       > /nix/store/0adymgz38j1xgca61fgg14lvaq0iwq1b-x86_64-unknown-linux-musl-binutils-2.44/bin/x86_64-unknown-linux-musl-ld: /nix/store/54m0dlvjd38xgj7sxzv9bpmfg39gpsgv-x86_64-unknown-linux-musl-gcc-14.3.0/lib/gcc/x86_64-unknown-linux-musl/14.3.0/crtbeginT.o: relocation R_X86_64_32 against hidden symbol `__TMC_END__' can not be used when making a shared object
       > /nix/store/0adymgz38j1xgca61fgg14lvaq0iwq1b-x86_64-unknown-linux-musl-binutils-2.44/bin/x86_64-unknown-linux-musl-ld: failed to set dynamic section sizes: bad value

I am investigating it but it could possibly take some time because postgresql builds a full LLVM.

Try pkgsStatic.libpq, which doesn't need LLVM - much quicker for me.

I confirmed that dontAddStaticConfigureFlags = true doesn't help, thus the "precedence issue" was not the problem. I think it's related to NIX_CFLAGS_LINK itself as mentioned above.

But yeah.. this just means that postgresql's build system is just not going to work well with pkgsStatic and we had just been lucky to have it work for a year...

I'm not even sure it is the same issue; the linked bug mentions the difference in flags for compiler-rt-libc which libpq does not build-depend on. So either this is a red herring or there are multiple issues.

It seems that libpq indeed is NIX_CFLAGS_LINK after all, as that's the only difference I can see in nix-diff.

I'm not even sure it is the same issue; the linked bug mentions the difference in flags for compiler-rt-libc which libpq does not build-depend on. So either this is a red herring or there are multiple issues.

Ah, I hadn't looked that close. I guess there are multiple issues then, and once compiler-rt-libc works, postgresql would hit the same issue that libpq has now.

sorry I won't be able to fix this, I will not be near computers for a few days.

Ah, I hadn't looked that close. I guess there are multiple issues then, and once compiler-rt-libc works, postgresql would hit the same issue that libpq has now.

Actually the compiler-rt-libc stuff is a red herring since it doesn't create meaningful changes, and while there are in fact multiple issues they all boil down to things only working before because they were actually dynamically linked. See the comments I posted in #429535 (in fact I think this conversation should continue there).