patchShebangs: performance/readability optimization #94642
Conversation
|
The following built for me: |
|
We might want to find a way of clearing unneeded jobs.
|
Travis has a feature to cancel previous jobs. For github there was a github action that emulates this behavior. |
|
This looks fine to me. Perhaps we should reorganize the commits once more but otherwise this looks like it deserves a shot in the staging branch. |
|
I can confirm that it builds on other Linux distributions. |
builtins for small input sizes should be faster due to less forking.
according to shellcheck [[ foo == "bla" ]] && [[ ... ]] has better posix semantics over [ foo = "bla" -a ... ]. It is also easier to read.
less commands -> faster
|
done. |
|
/marvin opt-in |
|
Hi! I'm an experimental bot. My goal is to guide this PR through its stages, hopefully ending with a merge. You can read up on the usage here. |
|
I have reverted this patch in staging-next as it apparently caused the |
This caused shebangs that were already store paths to be rewritten. Introduced by ab4c359 in NixOS#94642 Example difference: $ echo "hello world" | tail -c+3 llo world $ str="hello world"; echo ${str:3} lo world
This caused shebangs that were already store paths to be rewritten. Introduced by ab4c359 in NixOS#94642 Example difference: $ echo "hello world" | tail -c+3 llo world $ str="hello world"; echo ${str:3} lo world
| @@ -105,12 +105,12 @@ patchShebangs() { | |||
| } | |||
|
|
|||
| patchShebangsAuto () { | |||
| if [ -z "${dontPatchShebangs-}" -a -e "$prefix" ]; then | |||
| if [[ -z "${dontPatchShebangs-}" && -e "$prefix" ]]; then | |||
There was a problem hiding this comment.
I've noticed dontPatchShebangs not being respected for the androidStudioPackages.{canary,dev} a bit ago, could this have anything to do with this change?
I'm not too familair with the stdenv or bash scripting, so please disregard this comment if it does not make any sense.
| if [[ -z "${dontPatchShebangs-}" && -e "$prefix" ]]; then | |
| if [[ -z "${dontPatchShebangs-}" || -e "$prefix" ]]; then |
There was a problem hiding this comment.
The -a stands for &&, so the code is correct. Is dontPatchShebangs unset by anything in the package you mention?
Motivation for this change
need for speed
Things done
sandboxinnix.confon non-NixOS linux)nix-shell -p nixpkgs-review --run "nixpkgs-review wip"./result/bin/)nix path-info -Sbefore and after)