gnumake: do not use MAKE_CXX#314716
Conversation
tie
force-pushed
the
make-cxx
branch
3 times, most recently
from
d24a15c to
d407d58
Compare
ofborg
bot
added
10.rebuild-darwin-stdenv
|
Yikes, gnumake doesn’t have a maintainer now (#324539) 🫠 @AndersonTorres, would you mind reviewing this PR? |
|
Let's start with this! |
AndersonTorres
left a comment
AndersonTorres
left a comment
There was a problem hiding this comment.
I have some extra doubts about this specific project.
From a puristic perspective, it should be bootstrapped.
However, how can it be bootstrapped, given that a truckload of things that compose stdenv depend on it?
But let's postpone this. For now we need to test.
|
P.S.: technically this PR should wait for next staging merge so that the meta info can be updated. But let's hope it does not conflict. |
tie
force-pushed
the
make-cxx
branch
2 times, most recently
from
239a8ed to
adc412c
Compare
Do we have a place that list all tracking issues? I always find myself to search them which can something be tedious as i don't remember their name well |
Search for the label "scope tracking": |
tie
force-pushed
the
make-cxx
branch
2 times, most recently
from
01ff176 to
a253799
Compare
|
I’ve rebased the PR and updated the change to use autoreconfHook with a patch to configure.ac and src/default.c. The only downside is that now we always need pkgconf/pkg-config to regenerate configure script (hence the updated nativeBuildInputs). |
|
Isn't updateAutotoolsGnuConfigScriptsHook a subset of autoreconfHook, so you only need the latter? |
philiptaron
left a comment
philiptaron
left a comment
There was a problem hiding this comment.
I built and tested the stdenv, Nix, and a few other things (gcc, clang, zig.)
LGTM.
|
I intend to merge this tomorrow; I'm holding it open for more commentary. |
Uh, I’m not that familiar with Autotools and related build infrastructure in Nixpkgs. I guess this is true if autoreconfHook regenerates the files that updateAutotoolsGnuConfigScriptsHook updates 😅 I’ll remove it today when I get home if I don’t fall asleep, otherwise in the morning. |
Removes unnecessary C++ compiler reference when CXX environment variable is set to an absolute path.
|
As suggested in #314716 (comment), recreated MAKE_CXX patch with |
| # TODO: stdenv’s setup.sh should be aware of patch directories. It’s very | ||
| # convenient to keep them in a separate directory but we can defer listing the | ||
| # directory until derivation realization to avoid unnecessary Nix evaluations. |
philiptaron
left a comment
philiptaron
left a comment
There was a problem hiding this comment.
I'm planning on merging once a test of the stdenv completes, even though I'm not sold on the patches approach.
| # TODO: stdenv’s setup.sh should be aware of patch directories. It’s very | ||
| # convenient to keep them in a separate directory but we can defer listing the | ||
| # directory until derivation realization to avoid unnecessary Nix evaluations. | ||
| patches = lib.filesystem.listFilesRecursive ./patches; |
| Purity: don't look for library dependencies (of the form `-lfoo') in | ||
| /lib and /usr/lib. It's a stupid feature anyway. Likewise, when | ||
| searching for included Makefiles, don't look in /usr/include and | ||
| friends. |
There was a problem hiding this comment.
This was taken from the comment in the Nix file (and I appreciate it here!)
5 participants
Description of changes
Removes unnecessary C++ compiler reference when CXX environment variable is set to an absolute path.
See also https://savannah.gnu.org/bugs/?63668
Parent PR:
Motivation
I’m trying to fix cross-compilation for platforms where C/C++ compiler’s
targetPrefixis identical. It’s common to setwhen cross-compiling, however, cc/bintools wrapper setup hook currently sets {CC,CXX}{,_FOR_BUILD} et cetera to program name instead of absolute path. That obviously doesn’t work because only the first program from
PATHis used (for example, this is part of the reason why static glibc doesn’t work in Nixpkgs and we have pkgsStatic use musl that has different target prefix). There are a lot of other things that start falling apart in stdenv when we set these variables to absolute paths, this is just one of them: gnumake is part of the stdenv and shouldn’t refer to the bootstrap tools.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/)Add a 👍 reaction to pull requests you find important.