[pacman-dev] [PATCH 1/2] Restore ability for $PACMAN to include arguments and document it
Unmangled version: https://github.com/vadmium/pacman-arch/commit/70b1327.patch
From 70b1327c0443818d163e10b649ba882a4fd5a0f3 Mon Sep 17 00:00:00 2001 From: Martin Panter <vadmium à gmail·com> Date: Wed, 31 Oct 2012 03:05:42 +0000 Subject: [PATCH] Restore ability for $PACMAN to include arguments and document it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit
Judging by the “${PACMAN%% *}” incantation from revision 66c6d28 (makepkg: allow to specify an alternative pacman command), it looks like this was consciously intended. Currently, including arguments in $PACMAN means the --syncdeps operation may be attempted, but each Pacman invocation will probably fail. However in other cases the operation would be skipped if $PACMAN cannot be found. Looks like this inconsistency comes from revision 622326b (makepkg: fix sudo/su calling of pacman). --- doc/makepkg.8.txt | 3 ++- scripts/makepkg.sh.in | 4 ++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/doc/makepkg.8.txt b/doc/makepkg.8.txt index 9d19e38..8b440a3 100644 --- a/doc/makepkg.8.txt +++ b/doc/makepkg.8.txt @@ -196,7 +196,8 @@ Environment Variables The command that will be used to check for missing dependencies and to install and remove packages. Pacman's -Qq, -Rns, -S, -T, and -U operations must be supported by this command. If the variable is not - set or empty, makepkg will fall back to `pacman'. + set or empty, makepkg will fall back to `pacman'. The variable may + include command arguments, separated with spaces. **PKGDEST=**"/path/to/folder":: Folder where the resulting packages will be stored. Overrides the diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in index d387b7d..3b4f27f 100644 --- a/scripts/makepkg.sh.in +++ b/scripts/makepkg.sh.in @@ -873,9 +873,9 @@ source_has_signatures() { run_pacman() { local cmd if [[ ! $1 = -@(T|Qq) ]]; then - cmd=("$PACMAN" $PACMAN_OPTS "$@") + cmd=($PACMAN $PACMAN_OPTS "$@") else - cmd=("$PACMAN" "$@") + cmd=($PACMAN "$@") fi if (( ! ASROOT )) && [[ ! $1 = -@(T|Qq) ]]; then if type -p sudo >/dev/null; then -- 1.7.12
On Wed, Oct 31, 2012 at 07:02:59AM +0000, Martin Panter wrote:
Unmangled version: https://github.com/vadmium/pacman-arch/commit/70b1327.patch
From 70b1327c0443818d163e10b649ba882a4fd5a0f3 Mon Sep 17 00:00:00 2001 From: Martin Panter <vadmium à gmail·com> Date: Wed, 31 Oct 2012 03:05:42 +0000 Subject: [PATCH] Restore ability for $PACMAN to include arguments and document it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit
Judging by the “${PACMAN%% *}” incantation from revision 66c6d28 (makepkg: allow to specify an alternative pacman command), it looks like this was consciously intended.
66c6d28 is a 3 year old patch, and I strongly disagree with what it does. Other common environment variables like EDITOR do not allow options to be part of the binary name, and I think we should stick to that.
Currently, including arguments in $PACMAN means the --syncdeps operation may be attempted, but each Pacman invocation will probably fail. However in other cases the operation would be skipped if $PACMAN cannot be found. Looks like this inconsistency comes from revision 622326b (makepkg: fix sudo/su calling of pacman).
This is a two year old patch which isn't really much better given that it relies on eval (a common misspelling of evil). The current behavior using an array is what I consider to be correct and proper (and safe) bash. If you'd like the ability to extend PACMAN and invoke it with arguments, I suggest exposing the currentlly internal PACMAN_OPTS in a sane way. Thanks, Dave
--- doc/makepkg.8.txt | 3 ++- scripts/makepkg.sh.in | 4 ++-- 2 files changed, 4 insertions(+), 3 deletions(-)
diff --git a/doc/makepkg.8.txt b/doc/makepkg.8.txt index 9d19e38..8b440a3 100644 --- a/doc/makepkg.8.txt +++ b/doc/makepkg.8.txt @@ -196,7 +196,8 @@ Environment Variables The command that will be used to check for missing dependencies and to install and remove packages. Pacman's -Qq, -Rns, -S, -T, and -U operations must be supported by this command. If the variable is not - set or empty, makepkg will fall back to `pacman'. + set or empty, makepkg will fall back to `pacman'. The variable may + include command arguments, separated with spaces.
**PKGDEST=**"/path/to/folder":: Folder where the resulting packages will be stored. Overrides the diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in index d387b7d..3b4f27f 100644 --- a/scripts/makepkg.sh.in +++ b/scripts/makepkg.sh.in @@ -873,9 +873,9 @@ source_has_signatures() { run_pacman() { local cmd if [[ ! $1 = -@(T|Qq) ]]; then - cmd=("$PACMAN" $PACMAN_OPTS "$@") + cmd=($PACMAN $PACMAN_OPTS "$@") else - cmd=("$PACMAN" "$@") + cmd=($PACMAN "$@") fi if (( ! ASROOT )) && [[ ! $1 = -@(T|Qq) ]]; then if type -p sudo >/dev/null; then -- 1.7.12
On 3 November 2012 01:56, Dave Reisner <d@falconindy.com> wrote:
On Wed, Oct 31, 2012 at 07:02:59AM +0000, Martin Panter wrote:
Unmangled version: https://github.com/vadmium/pacman-arch/commit/70b1327.patch
From 70b1327c0443818d163e10b649ba882a4fd5a0f3 Mon Sep 17 00:00:00 2001 From: Martin Panter <vadmium à gmail·com> Date: Wed, 31 Oct 2012 03:05:42 +0000 Subject: [PATCH] Restore ability for $PACMAN to include arguments and document it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit
Judging by the “${PACMAN%% *}” incantation from revision 66c6d28 (makepkg: allow to specify an alternative pacman command), it looks like this was consciously intended.
66c6d28 is a 3 year old patch, and I strongly disagree with what it does. Other common environment variables like EDITOR do not allow options to be part of the binary name, and I think we should stick to that.
Currently, including arguments in $PACMAN means the --syncdeps operation may be attempted, but each Pacman invocation will probably fail. However in other cases the operation would be skipped if $PACMAN cannot be found. Looks like this inconsistency comes from revision 622326b (makepkg: fix sudo/su calling of pacman).
This is a two year old patch which isn't really much better given that it relies on eval (a common misspelling of evil). The current behavior using an array is what I consider to be correct and proper (and safe) bash.
If you'd like the ability to extend PACMAN and invoke it with arguments, I suggest exposing the currentlly internal PACMAN_OPTS in a sane way.
I don’t have any need for custom arguments. Would it be better to remove the code that strips off the arguments and make it so that $PACMAN is clearly a command name only?
On Sat, Nov 03, 2012 at 04:23:12AM +0000, Martin Panter wrote:
On 3 November 2012 01:56, Dave Reisner <d@falconindy.com> wrote:
On Wed, Oct 31, 2012 at 07:02:59AM +0000, Martin Panter wrote:
Unmangled version: https://github.com/vadmium/pacman-arch/commit/70b1327.patch
From 70b1327c0443818d163e10b649ba882a4fd5a0f3 Mon Sep 17 00:00:00 2001 From: Martin Panter <vadmium à gmail·com> Date: Wed, 31 Oct 2012 03:05:42 +0000 Subject: [PATCH] Restore ability for $PACMAN to include arguments and document it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit
Judging by the “${PACMAN%% *}” incantation from revision 66c6d28 (makepkg: allow to specify an alternative pacman command), it looks like this was consciously intended.
66c6d28 is a 3 year old patch, and I strongly disagree with what it does. Other common environment variables like EDITOR do not allow options to be part of the binary name, and I think we should stick to that.
Currently, including arguments in $PACMAN means the --syncdeps operation may be attempted, but each Pacman invocation will probably fail. However in other cases the operation would be skipped if $PACMAN cannot be found. Looks like this inconsistency comes from revision 622326b (makepkg: fix sudo/su calling of pacman).
This is a two year old patch which isn't really much better given that it relies on eval (a common misspelling of evil). The current behavior using an array is what I consider to be correct and proper (and safe) bash.
If you'd like the ability to extend PACMAN and invoke it with arguments, I suggest exposing the currentlly internal PACMAN_OPTS in a sane way.
I don’t have any need for custom arguments. Would it be better to remove the code that strips off the arguments and make it so that $PACMAN is clearly a command name only?
Modifying the variable would be worse than allowing arguments. I'd rather we just ensure that the expansion of PACMAN is always quoted so that it blows up when a user tries to pass arguments in the variable.
participants (2)
-
Dave Reisner
-
Martin Panter