Conversation
06kellyjac
left a comment
There was a problem hiding this comment.
--ro-bind-try seems appropriate for nixpkgs/nixos
Shame we're still applying patches like this but for libglycin but LGTM
What is the benefit of removing |
|
It's a compatibility hack that we shouldn't have, much like we deliberately don't have a working /lib64/ld-linux-x86-64.so.2. I don't think any software should rely on the implicit existence of /usr/bin/env. For glycin in particular, this path is entirely useless anyway. |
|
Just an update: there is a legitimate (?) use of Example: #!/usr/bin/env nix-shell
#!nix-shell -i bash -p coreutils nix nix-update curl jq
# shellcheck shell=bashBut this does seem like a compatibility scheme we can go without as one can just invoke the bash script via |
|
I independently encountered the need for this when running glycin-loaders tests it in Nix build sandbox. Incorporated your proposed solution into a larger refactoring effort in #481377 |
|
I guess glycin devs already essentially made the same remark, but just to reiterate: out-of-touch comments like "I'd consider a system without /nix/store to be far more broken than one without /usr" will just make upstream developers (in general, not just glycin's) dislike NixOS maintainers. Please don't. |
I tried to get this through upstream, but it was rejected.
I'm on a mission to get rid of /bin/sh and /usr/bin/env, and the addition of glycin to the GNOME ecosystem is the latest hiccup I've encountered. The sandboxing code wants to bind-mount /usr into the sandbox and straight up crashes when /usr doesn't exist. This PR includes a minimally invasive patch to fix my particular use case by ignoring the absence of /usr.
I've exposed the patch via
passthruas was previously done for a different patch, and applied the patch in all the places where the other patch was applied. I've also changed Fractal to also make use of the paths patch instead of prefixingPATHvia the wrapper, because I think it's cleaner to call the binary by its absolute path.Things done
passthru.tests.nixpkgs-reviewon this PR. See nixpkgs-review usage../result/bin/.Add a 👍 reaction to pull requests you find important.