bash: don't install loadables dir for static build #270975
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
veehaitch
force-pushed
the
bash-static-r13y
branch
from
76340af to
1663ed9
Compare
ofborg
bot
added
10.rebuild-darwin-stdenv
wentasah
approved these changes
Dec 3, 2023
veehaitch
added
the
12.approvals: 1
| infodir=$(infodir) htmldir=$(htmldir) DESTDIR=$(DESTDIR) $@ ) | ||
| -( cd $(DEFDIR) ; $(MAKE) $(MFLAGS) DESTDIR=$(DESTDIR) $@ ) | ||
| -( cd $(PO_DIR) ; $(MAKE) $(MFLAGS) DESTDIR=$(DESTDIR) $@ ) | ||
| +ifeq (,$(findstring -static,$(STATIC_LD))) |
There was a problem hiding this comment.
Can you try upstreaming this, too?
There was a problem hiding this comment.
Sure, I can give it a shot.
veehaitch
force-pushed
the
bash-static-r13y
branch
from
1663ed9 to
399a9b7
Compare
SuperSandro2000
approved these changes
Dec 10, 2023
delroth
added
12.approvals: 2
alyssais
requested changes
Dec 12, 2023
veehaitch
force-pushed
the
bash-static-r13y
branch
from
399a9b7 to
f679aa0
Compare
The dynamic build ships a bunch of sample loadable builtins and installs
them to `$out/lib/bash`. When building statically with musl, the target
fails as it cannot produce any of the dynamically linked loadable
builtins.
Although the failure is not fatal, it also introduces an impurity due to
the heavy parallelization: _sometimes_ the files `loadables.h` and
`Makefile.sample` end up in `$out/lib/bash` depending on how fast the
compilation of the loadable builtins fails. Try the following to
reproduce the issue:
nix build --rebuild --cores 48 nixpkgs#pkgsStatic.bash
This commit skips building and installing the loadable builtins
altogether if building statically.
veehaitch
force-pushed
the
bash-static-r13y
branch
from
f679aa0 to
12c4c49
Compare
|
Awesome, thanks! I'd like to wait a little bit to see what upstream says, in case we can avoid carrying the patch. |
delroth
removed
the
12.approvals: 2
wegank
added
the
2.status: merge conflict
wegank
added
the
2.status: stale
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The dynamic build ships a bunch of sample loadable builtins and installs them to
$out/lib/bash. When building statically with musl, the target fails as it cannot produce any of the dynamically linked loadable builtins.Although the failure is not fatal, it also introduces an impurity due to the heavy parallelization: sometimes the files
loadables.handMakefile.sampleend up in$out/lib/bashdepending on how fast the compilation of the loadable builtins fails. Try the following to reproduce the issue:This commit skips building and installing the loadable builtins altogether if building statically.
Description of changes
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/)Priorities
Add a 👍 reaction to pull requests you find important.