squeak: fix build with LLVM 16#268227
Conversation
Part of ZHF for 23.11: https://hydra.nixos.org/build/241300457 I bisected this failure to this first bad commit: [9ed7bfe] llvmPackages: 11 -> 16 on Linux, then looked around for other repairs made to assuage LLVM's errors. This appears to be the pattern.
delroth
added
the
12.approvals: 1
ofborg
bot
added
10.rebuild-darwin: 0
|
|
||
| squeak = callPackage ../development/compilers/squeak { }; | ||
| squeak = callPackage ../development/compilers/squeak { | ||
| stdenv = clangStdenv; |
There was a problem hiding this comment.
Does this need to be clangStdenv other than for testing?
There was a problem hiding this comment.
When omitted, nix-build -A squeak errors as follows:
Configuring Squeak 5.0-202003021730 (5.3-19459-64bit) for x86_64-pc-linux-gnu
checking whether make sets $(MAKE)... yes
checking for gcc... clang
checking whether the C compiler works... no
configure: error: in `/build/source/build.linux64x64/squeak.cog.spur/build':
configure: error: C compiler cannot create executables
See `config.log' for more details
I don't know why, in the default stdenv, it detects clang, as it's no longer passed into the environment. If I pass in gccStdenv, clang is still detected.
So, yes, I think this does have to be clangStdenv.
| "-Wno-error=implicit-function-declaration" | ||
| "-Wno-error=incompatible-function-pointer-types" | ||
| ]) | ||
| ); |
There was a problem hiding this comment.
I would prefer this to be a list at NIX_CFLAGS_COMPILE rather than a string at env.NIX_CFLAGS_COMPILE, unless we have a policy of using env.«…» that I don't know about.
There was a problem hiding this comment.
env.NIX_CFLAGS_COMPILE is definitely the majority. I also prefer NIX_CFLAGS_COMPILE, and I'll change it to that form as it works just fine.
$ rg env.NIX_CFLAGS_COMPILE | wc -l
816
$ rg NIX_CFLAGS_COMPILE | rg -v env | wc -l
236
There was a problem hiding this comment.
No, please don't. Moving environmental variables to env is a step towards __structuredAttrs support, see #263773.
There was a problem hiding this comment.
> $ rg NIX_CFLAGS_COMPILE | rg -v env | wc -l
> 236
We've just done #217206 this year, it's unbelievable there are still that many instances of NIX_CFLAGS_COMPILE. Does the command count patterns like the following?
env = {
NIX_CFLAGS_COMPILE = "...";
};
There was a problem hiding this comment.
It would count those, yes. There's likely quite a few false positives.
There was a problem hiding this comment.
Believe it because it's an undocumented practice - https://nixos.org/manual/nixpkgs/unstable/.
philiptaron
force-pushed
the
fix-squeak-build
branch
from
911e746 to
0b66391
Compare
philiptaron
force-pushed
the
fix-squeak-build
branch
from
0b66391 to
911e746
Compare
|
Based on @wegank's clarifications, I've reverted the changes I made based on @ehmry's feedback. I believe this is ready for merging. |
philiptaron
added
the
0.kind: ZHF Fixes
4 participants
Description of changes
Part of ZHF (#265948) for 23.11: https://hydra.nixos.org/build/241300457
I bisected this failure to this first bad commit: [9ed7bfe] llvmPackages: 11 -> 16 on Linux, then looked around for other repairs made to assuage LLVM's errors. This appears to be the pattern.
To fit the
lib.optionals stdenv.cc.isClangcheck, I passedclangStdenvinall-packages.nixinstead of the previous pattern of puttingclanginnativeBuildInputs.I also took this moment to make the long list of dependencies be formatted one-per-line, as per
nixpkgs-fmtstyle. And as I added another use oftoString, I added it to the list of things brought in frombuiltins.Things done
nix.conf? (See Nix manual)sandbox = relaxedsandbox = truenix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage./result/bin/)