-
-
Notifications
You must be signed in to change notification settings - Fork 14.9k
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
python.pkgs.wrapPython: fix makeWrapperArgs #76283
Conversation
When `makeWrapperArgs` is a Bash array, we only passed the first item to `wrapProgram`. We need to use `"${makeWrapperArgs[@]}"` to extract all the items. But that breaks the common string case so we need to handle that case separately.
$ declare -a foo=(bar baz)
$ echo $foo
bar
$ declare -a bar=${foo[@]}
$ echo $bar
bar baz
$ declare -a bar=(${foo[@]})
$ echo $bar
bar
$ declare -a bar=("${foo[@]}")
$ echo $bar
bar
$ declare -a bar='("${foo[@]}")'
$ echo $bar
bar
$ qux='bar baz'
$ declare -a bar='("${qux[@]}")'
$ echo $bar
bar baz
$ declare -a bar='($qux)'
$ echo $bar
bar
|
@jtojnar I believe this works. But So all the |
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.
This change LGTM, but the larger issues needs a fix in wrapGAppsHook
(and honestly a slight rewrite 😄 )
This broke pythonPackages.setuptools:
|
Aargh, turns out there is a difference between local -a user_args="()" and local -a user_args
user_args="()" |
set -eux
a() {
local -a user_args="()"
declare -p user_args
}
b() {
local -a user_args
user_args="()"
declare -p user_args
}
a
b I long ran out of expletives for bash: + a
+ local -a 'user_args=()'
+ declare -p user_args
declare -a user_args=()
+ b
+ local -a user_args
+ user_args='()'
+ declare -p user_args
declare -a user_args=([0]="()") |
Since set -eux
c() {
if [[ "$(declare -p makeWrapperArgs)" =~ ^'declare -a makeWrapperArgs=' ]]; then
local -a user_args=("${makeWrapperArgs[@]}")
else
local -a user_args="($makeWrapperArgs)"
fi
local -a wrapProgramArgs=("${wrap_args[@]}" "${user_args[@]}")
declare -p wrapProgramArgs
}
makeWrapperArgs=
c
makeWrapperArgs=()
c → set -eux
c() {
if [[ "$(declare -p makeWrapperArgs)" =~ ^'declare -a makeWrapperArgs=' ]]; then
local -a user_args=("${makeWrapperArgs[@]}")
else
local -a user_args="($makeWrapperArgs)"
fi
local -a wrapProgramArgs=("${wrap_args[@]}" "${user_args[@]}")
declare -p wrapProgramArgs
}
makeWrapperArgs=
c
makeWrapperArgs=()
c |
Fixed in a6bb2ed, thanks for letting me know. |
Motivation for this change
When
makeWrapperArgs
is a Bash array, we only passed the first item towrapProgram
. We need to use"${makeWrapperArgs[@]}"
to extract all the items. But that breaks the common string case so we need to handle that case separately.Closes: #76158
Things done
sandbox
innix.conf
on non-NixOS)nix-shell -p nox --run "nox-review wip"
./result/bin/
)nix path-info -S
before and after)cc @worldofpeace