-
-
Notifications
You must be signed in to change notification settings - Fork 15.1k
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
setup.sh: verify that find
is from findutils
#177693
Conversation
PR #168413 causes `setup.sh` to use `find`. Packages which put `busybox` into their `nativeBuildInputs` will have `$busybox/bin/find` ahead of `$findutils/bin/find` in `$PATH` when `setup.sh` is run, causing `setup.sh` to fail. Let's check that `find` is really from findutils so we can provide a more useful error message. The `setup.sh` script already does a similar check to make sure that the shell it is running under is `bash` rather than some other shell. A longer-term solution to the problem of `busybox`'s outpaths conflicting with `stdenv`s is described here: #177682 (comment)
Co-authored-by: Sandro <sandro.jaeckel@gmail.com>
Co-authored-by: Sandro <sandro.jaeckel@gmail.com>
@@ -1022,6 +1022,15 @@ configurePhase() { | |||
fixLibtool "$i" | |||
done | |||
|
|||
# Make sure `find` is the full `find` from findutils, rather | |||
# than from busybox or some other implementation. This is | |||
# analogous to `shellcheck shell=bash` at the top of this |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
shellcheck is a tool to verify if shell scripts are valid, it's unrelated to checking if the correct shell is used.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
:embarrassed:
Could there be reasons for people to want to use find from busybox? This seems quite strict, though I understand where you're coming from. |
What features of GNU findutils do we need that Busybox's find doesn't have? Would it be possible to make setup.sh compatible with both? |
Hmm, I thought it was abuild that was expecting find with printf. I don't see -printf in the fixLibtool PR. So putting it here may be at the wrong layer, but not sure where to put it then. And I agree we don't want conflicting binaries in stdemv regardless. |
Just
I'm not sure. But it can't hurt.
Sure: #177789 |
It's the
I am certainly on board with using the minimum features needed for the task, so I'm switching this to |
It would be really nice if build inputs looked at |
Hrm, is that possible? I thought that changing a package's If |
Also while we're wishing for things, I wish So for any given package you can add a list of other packages it is strictly-less-than (or strictly-greater than). No integers to remember and no running out of priority levels between e.g. "5" and "6". I can never seem to remember integer priority systems like the one in nftables. |
Description of changes
PR #168413 causes
setup.sh
to usefind
. Packages which putbusybox
into theirnativeBuildInputs
will have$busybox/bin/find
ahead of$findutils/bin/find
in$PATH
whensetup.sh
is run, causingsetup.sh
to fail.Let's check that
find
is really from findutils so we can provide a more useful error message.The
setup.sh
script already does a similar check to make sure that the shell it is running under isbash
rather than some other shell.A longer-term solution to the problem of
busybox
's outpaths conflicting withstdenv
s is described here:#177682 (comment)
Things done
sandbox = true
set innix.conf
? (See Nix manual)nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD"
. Note: all changes have to be committed, also see nixpkgs-review usage./result/bin/
)nixos/doc/manual/md-to-db.sh
to update generated release notes