Re: [pacman-dev] [PATCH 1/2] Restore ability for $PACMAN to include arguments and document it

On Sat, Nov 03, 2012 at 04:23:12AM +0000, Martin Panter wrote:

On 3 November 2012 01:56, Dave Reisner 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 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.