[pacman-dev] [PATCH 1/2] makepkg: move pacman calls to a function
Signed-off-by: Cedric Staniewski <cedric@gmx.ca> --- I'm not sure about that part pmout=$(run_pacman -T "$@") for two reasons. First, we have to work around sudo in run_pacman, and second, most of the pacman wrappers does not support -T or do not return the same codes as pacman does. For this second reason, the following patch would be (currently) useless for the majority of pacman wrapper users. scripts/makepkg.sh.in | 39 +++++++++++++++------------------------ 1 files changed, 15 insertions(+), 24 deletions(-) diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in index fbcdc70..cfc5736 100644 --- a/scripts/makepkg.sh.in +++ b/scripts/makepkg.sh.in @@ -328,10 +328,21 @@ download_file() { fi } +run_pacman() { + local ret=0 + if (( ! ASROOT )) && [[ $1 != -T ]]; then + sudo pacman $PACMAN_OPTS "$@" || ret=$? + else + pacman $PACMAN_OPTS "$@" || ret=$? + fi + return $ret +} + check_deps() { [ $# -gt 0 ] || return - pmout=$(pacman $PACMAN_OPTS -T "$@") + local ret=0 + pmout=$(run_pacman -T "$@") ret=$? if [ $ret -eq 127 ]; then #unresolved deps echo "$pmout" @@ -356,15 +367,8 @@ handle_deps() { if [ "$DEP_BIN" -eq 1 ]; then # install missing deps from binary packages (using pacman -S) msg "$(gettext "Installing missing dependencies...")" - local ret=0 - - if [ "$ASROOT" -eq 0 ]; then - sudo pacman $PACMAN_OPTS -S --asdeps $deplist || ret=$? - else - pacman $PACMAN_OPTS -S --asdeps $deplist || ret=$? - fi - if [ $ret -ne 0 ]; then + if ! run_pacman -S --asdeps $deplist; then error "$(gettext "Pacman failed to install missing dependencies.")" exit 1 # TODO: error code fi @@ -422,15 +426,9 @@ remove_deps() { done msg "Removing installed dependencies..." - local ret=0 - if [ "$ASROOT" -eq 0 ]; then - sudo pacman $PACMAN_OPTS -Rns $deplist || ret=$? - else - pacman $PACMAN_OPTS -Rns $deplist || ret=$? - fi # Fixes FS#10039 - exit cleanly as package has built successfully - if [ $ret -ne 0 ]; then + if ! run_pacman -Rns $deplist; then warning "$(gettext "Failed to remove installed dependencies.")" return 0 fi @@ -1113,14 +1111,7 @@ install_package() { fi done - local ret=0 - if [ "$ASROOT" -eq 0 ]; then - sudo pacman $PACMAN_OPTS -U ${pkglist} || ret=$? - else - pacman $PACMAN_OPTS -U ${pkglist} || ret=$? - fi - - if [ $ret -ne 0 ]; then + if ! run_pacman -U $pkglist; then warning "$(gettext "Failed to install built package(s).")" return 0 fi -- 1.6.5.2
Implements FS#13028. Signed-off-by: Cedric Staniewski <cedric@gmx.ca> --- doc/makepkg.conf.5.txt | 6 ++++++ scripts/makepkg.sh.in | 20 ++++++++++++-------- 2 files changed, 18 insertions(+), 8 deletions(-) diff --git a/doc/makepkg.conf.5.txt b/doc/makepkg.conf.5.txt index 704dccc..617ec84 100644 --- a/doc/makepkg.conf.5.txt +++ b/doc/makepkg.conf.5.txt @@ -178,6 +178,12 @@ Options *PKGEXT*, *SRCEXT*:: Do not touch these unless you know what you are doing. +*PACMAN*:: + The command which will be used to check for missing dependencies and to + install and remove packages. Pacman's -U, -T, -S and -Rns operations + must be supported by this command. If no command is specified, makepkg + will fall back to `pacman'. + See Also -------- diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in index cfc5736..cf2b001 100644 --- a/scripts/makepkg.sh.in +++ b/scripts/makepkg.sh.in @@ -77,6 +77,7 @@ SPLITPKG=0 # when dealing with svn/cvs/etc PKGBUILDs. FORCE_VER="" +PACMAN= PACMAN_OPTS= ### SUBROUTINES ### @@ -331,9 +332,9 @@ download_file() { run_pacman() { local ret=0 if (( ! ASROOT )) && [[ $1 != -T ]]; then - sudo pacman $PACMAN_OPTS "$@" || ret=$? + sudo $PACMAN $PACMAN_OPTS "$@" || ret=$? else - pacman $PACMAN_OPTS "$@" || ret=$? + $PACMAN $PACMAN_OPTS "$@" || ret=$? fi return $ret } @@ -347,7 +348,7 @@ check_deps() { if [ $ret -eq 127 ]; then #unresolved deps echo "$pmout" elif [ $ret -ne 0 ]; then - error "$(gettext "Pacman returned a fatal error (%i): %s")" "$ret" "$pmout" + error "$(gettext "%s returned a fatal error (%i): %s")" "$PACMAN" "$ret" "$pmout" exit 1 fi } @@ -369,7 +370,7 @@ handle_deps() { msg "$(gettext "Installing missing dependencies...")" if ! run_pacman -S --asdeps $deplist; then - error "$(gettext "Pacman failed to install missing dependencies.")" + error "$(gettext "%s failed to install missing dependencies.")" "$PACMAN" exit 1 # TODO: error code fi fi @@ -1097,9 +1098,9 @@ install_package() { [ "$INSTALL" -eq 0 ] && return if [ "$SPLITPKG" -eq 0 ]; then - msg "$(gettext "Installing package ${pkgname} with pacman -U...")" + msg "$(gettext "Installing package %s with %s -U...")" "$pkgname" "$PACMAN" else - msg "$(gettext "Installing ${pkgbase} package group with pacman -U...")" + msg "$(gettext "Installing %s package group with %s -U...")" "$pkgbase" "$PACMAN" fi local pkglist @@ -1550,6 +1551,9 @@ if [ -r ~/.makepkg.conf ]; then source ~/.makepkg.conf fi +# set pacman command if not defined in config files +PACMAN=${PACMAN:-pacman} + # check if messages are to be printed using color unset ALL_OFF BOLD BLUE GREEN RED YELLOW if [ -t 2 -a ! "$USE_COLOR" = "n" -a "$(check_buildenv color)" = "y" ]; then @@ -1796,7 +1800,7 @@ if [ "$NODEPS" -eq 1 -o "$NOBUILD" -eq 1 -o "$REPKG" -eq 1 ]; then if [ "$NODEPS" -eq 1 ]; then warning "$(gettext "Skipping dependency checks.")" fi -elif [ $(type -p pacman) ]; then +elif [ $(type -p "$PACMAN") ]; then unset pkgdeps # Set by resolve_deps() and used by remove_deps() deperr=0 @@ -1811,7 +1815,7 @@ elif [ $(type -p pacman) ]; then exit 1 fi else - warning "$(gettext "pacman was not found in PATH; skipping dependency checks.")" + warning "$(gettext "%s was not found in PATH; skipping dependency checks.")" "$PACMAN" fi # ensure we have a sane umask set -- 1.6.5.2
On Fri, Nov 6, 2009 at 8:47 PM, Cedric Staniewski <cedric@gmx.ca> wrote:
Signed-off-by: Cedric Staniewski <cedric@gmx.ca> --- I'm not sure about that part
pmout=$(run_pacman -T "$@")
for two reasons. First, we have to work around sudo in run_pacman, and second, most of the pacman wrappers does not support -T or do not return the same codes as pacman does. For this second reason, the following patch would be (currently) useless for the majority of pacman wrapper users.
This comment makes more sense after reading your second patch :) where you make run_pacman use a specified $PACMAN binary I think its fine to keep calling pacman directly for -T operation, and allow a wrapper for the others.
Xavier wrote:
On Fri, Nov 6, 2009 at 8:47 PM, Cedric Staniewski <cedric@gmx.ca> wrote:
Signed-off-by: Cedric Staniewski <cedric@gmx.ca> --- I'm not sure about that part
pmout=$(run_pacman -T "$@")
for two reasons. First, we have to work around sudo in run_pacman, and second, most of the pacman wrappers does not support -T or do not return the same codes as pacman does. For this second reason, the following patch would be (currently) useless for the majority of pacman wrapper users.
This comment makes more sense after reading your second patch :) where you make run_pacman use a specified $PACMAN binary
I think its fine to keep calling pacman directly for -T operation, and allow a wrapper for the others.
I think it mostly depends on if the second patch gets applied or not. In any case, I will send patches without run_pacman -T.
Signed-off-by: Cedric Staniewski <cedric@gmx.ca> --- scripts/makepkg.sh.in | 37 ++++++++++++++----------------------- 1 files changed, 14 insertions(+), 23 deletions(-) diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in index fbcdc70..125b4e0 100644 --- a/scripts/makepkg.sh.in +++ b/scripts/makepkg.sh.in @@ -328,9 +328,20 @@ download_file() { fi } +run_pacman() { + local ret=0 + if (( ! ASROOT )); then + sudo pacman $PACMAN_OPTS "$@" || ret=$? + else + pacman $PACMAN_OPTS "$@" || ret=$? + fi + return $ret +} + check_deps() { [ $# -gt 0 ] || return + local ret=0 pmout=$(pacman $PACMAN_OPTS -T "$@") ret=$? if [ $ret -eq 127 ]; then #unresolved deps @@ -356,15 +367,8 @@ handle_deps() { if [ "$DEP_BIN" -eq 1 ]; then # install missing deps from binary packages (using pacman -S) msg "$(gettext "Installing missing dependencies...")" - local ret=0 - - if [ "$ASROOT" -eq 0 ]; then - sudo pacman $PACMAN_OPTS -S --asdeps $deplist || ret=$? - else - pacman $PACMAN_OPTS -S --asdeps $deplist || ret=$? - fi - if [ $ret -ne 0 ]; then + if ! run_pacman -S --asdeps $deplist; then error "$(gettext "Pacman failed to install missing dependencies.")" exit 1 # TODO: error code fi @@ -422,15 +426,9 @@ remove_deps() { done msg "Removing installed dependencies..." - local ret=0 - if [ "$ASROOT" -eq 0 ]; then - sudo pacman $PACMAN_OPTS -Rns $deplist || ret=$? - else - pacman $PACMAN_OPTS -Rns $deplist || ret=$? - fi # Fixes FS#10039 - exit cleanly as package has built successfully - if [ $ret -ne 0 ]; then + if ! run_pacman -Rns $deplist; then warning "$(gettext "Failed to remove installed dependencies.")" return 0 fi @@ -1113,14 +1111,7 @@ install_package() { fi done - local ret=0 - if [ "$ASROOT" -eq 0 ]; then - sudo pacman $PACMAN_OPTS -U ${pkglist} || ret=$? - else - pacman $PACMAN_OPTS -U ${pkglist} || ret=$? - fi - - if [ $ret -ne 0 ]; then + if ! run_pacman -U $pkglist; then warning "$(gettext "Failed to install built package(s).")" return 0 fi -- 1.6.5.2
Implements FS#13028. Signed-off-by: Cedric Staniewski <cedric@gmx.ca> --- doc/makepkg.conf.5.txt | 6 ++++++ scripts/makepkg.sh.in | 20 ++++++++++++-------- 2 files changed, 18 insertions(+), 8 deletions(-) diff --git a/doc/makepkg.conf.5.txt b/doc/makepkg.conf.5.txt index 704dccc..617ec84 100644 --- a/doc/makepkg.conf.5.txt +++ b/doc/makepkg.conf.5.txt @@ -178,6 +178,12 @@ Options *PKGEXT*, *SRCEXT*:: Do not touch these unless you know what you are doing. +*PACMAN*:: + The command which will be used to check for missing dependencies and to + install and remove packages. Pacman's -U, -T, -S and -Rns operations + must be supported by this command. If no command is specified, makepkg + will fall back to `pacman'. + See Also -------- diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in index 125b4e0..9463601 100644 --- a/scripts/makepkg.sh.in +++ b/scripts/makepkg.sh.in @@ -77,6 +77,7 @@ SPLITPKG=0 # when dealing with svn/cvs/etc PKGBUILDs. FORCE_VER="" +PACMAN= PACMAN_OPTS= ### SUBROUTINES ### @@ -331,9 +332,9 @@ download_file() { run_pacman() { local ret=0 if (( ! ASROOT )); then - sudo pacman $PACMAN_OPTS "$@" || ret=$? + sudo $PACMAN $PACMAN_OPTS "$@" || ret=$? else - pacman $PACMAN_OPTS "$@" || ret=$? + $PACMAN $PACMAN_OPTS "$@" || ret=$? fi return $ret } @@ -347,7 +348,7 @@ check_deps() { if [ $ret -eq 127 ]; then #unresolved deps echo "$pmout" elif [ $ret -ne 0 ]; then - error "$(gettext "Pacman returned a fatal error (%i): %s")" "$ret" "$pmout" + error "$(gettext "%s returned a fatal error (%i): %s")" "$PACMAN" "$ret" "$pmout" exit 1 fi } @@ -369,7 +370,7 @@ handle_deps() { msg "$(gettext "Installing missing dependencies...")" if ! run_pacman -S --asdeps $deplist; then - error "$(gettext "Pacman failed to install missing dependencies.")" + error "$(gettext "%s failed to install missing dependencies.")" "$PACMAN" exit 1 # TODO: error code fi fi @@ -1097,9 +1098,9 @@ install_package() { [ "$INSTALL" -eq 0 ] && return if [ "$SPLITPKG" -eq 0 ]; then - msg "$(gettext "Installing package ${pkgname} with pacman -U...")" + msg "$(gettext "Installing package %s with %s -U...")" "$pkgname" "$PACMAN" else - msg "$(gettext "Installing ${pkgbase} package group with pacman -U...")" + msg "$(gettext "Installing %s package group with %s -U...")" "$pkgbase" "$PACMAN" fi local pkglist @@ -1550,6 +1551,9 @@ if [ -r ~/.makepkg.conf ]; then source ~/.makepkg.conf fi +# set pacman command if not defined in config files +PACMAN=${PACMAN:-pacman} + # check if messages are to be printed using color unset ALL_OFF BOLD BLUE GREEN RED YELLOW if [ -t 2 -a ! "$USE_COLOR" = "n" -a "$(check_buildenv color)" = "y" ]; then @@ -1796,7 +1800,7 @@ if [ "$NODEPS" -eq 1 -o "$NOBUILD" -eq 1 -o "$REPKG" -eq 1 ]; then if [ "$NODEPS" -eq 1 ]; then warning "$(gettext "Skipping dependency checks.")" fi -elif [ $(type -p pacman) ]; then +elif [ $(type -p "$PACMAN") ]; then unset pkgdeps # Set by resolve_deps() and used by remove_deps() deperr=0 @@ -1811,7 +1815,7 @@ elif [ $(type -p pacman) ]; then exit 1 fi else - warning "$(gettext "pacman was not found in PATH; skipping dependency checks.")" + warning "$(gettext "%s was not found in PATH; skipping dependency checks.")" "$PACMAN" fi # ensure we have a sane umask set -- 1.6.5.2
Implements FS#13028. Signed-off-by: Cedric Staniewski <cedric@gmx.ca> --- Oops... forgot to adjust the documentation. -T support is not required anymore for the pacman wrappers. doc/makepkg.conf.5.txt | 5 +++++ scripts/makepkg.sh.in | 20 ++++++++++++-------- 2 files changed, 17 insertions(+), 8 deletions(-) diff --git a/doc/makepkg.conf.5.txt b/doc/makepkg.conf.5.txt index 704dccc..c23aaf9 100644 --- a/doc/makepkg.conf.5.txt +++ b/doc/makepkg.conf.5.txt @@ -178,6 +178,11 @@ Options *PKGEXT*, *SRCEXT*:: Do not touch these unless you know what you are doing. +*PACMAN*:: + The command which will be used to install and remove packages. Pacman's + -U, -S and -Rns operations must be supported by this command. If no + command is specified, makepkg will fall back to `pacman'. + See Also -------- diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in index 125b4e0..9463601 100644 --- a/scripts/makepkg.sh.in +++ b/scripts/makepkg.sh.in @@ -77,6 +77,7 @@ SPLITPKG=0 # when dealing with svn/cvs/etc PKGBUILDs. FORCE_VER="" +PACMAN= PACMAN_OPTS= ### SUBROUTINES ### @@ -331,9 +332,9 @@ download_file() { run_pacman() { local ret=0 if (( ! ASROOT )); then - sudo pacman $PACMAN_OPTS "$@" || ret=$? + sudo $PACMAN $PACMAN_OPTS "$@" || ret=$? else - pacman $PACMAN_OPTS "$@" || ret=$? + $PACMAN $PACMAN_OPTS "$@" || ret=$? fi return $ret } @@ -347,7 +348,7 @@ check_deps() { if [ $ret -eq 127 ]; then #unresolved deps echo "$pmout" elif [ $ret -ne 0 ]; then - error "$(gettext "Pacman returned a fatal error (%i): %s")" "$ret" "$pmout" + error "$(gettext "%s returned a fatal error (%i): %s")" "$PACMAN" "$ret" "$pmout" exit 1 fi } @@ -369,7 +370,7 @@ handle_deps() { msg "$(gettext "Installing missing dependencies...")" if ! run_pacman -S --asdeps $deplist; then - error "$(gettext "Pacman failed to install missing dependencies.")" + error "$(gettext "%s failed to install missing dependencies.")" "$PACMAN" exit 1 # TODO: error code fi fi @@ -1097,9 +1098,9 @@ install_package() { [ "$INSTALL" -eq 0 ] && return if [ "$SPLITPKG" -eq 0 ]; then - msg "$(gettext "Installing package ${pkgname} with pacman -U...")" + msg "$(gettext "Installing package %s with %s -U...")" "$pkgname" "$PACMAN" else - msg "$(gettext "Installing ${pkgbase} package group with pacman -U...")" + msg "$(gettext "Installing %s package group with %s -U...")" "$pkgbase" "$PACMAN" fi local pkglist @@ -1550,6 +1551,9 @@ if [ -r ~/.makepkg.conf ]; then source ~/.makepkg.conf fi +# set pacman command if not defined in config files +PACMAN=${PACMAN:-pacman} + # check if messages are to be printed using color unset ALL_OFF BOLD BLUE GREEN RED YELLOW if [ -t 2 -a ! "$USE_COLOR" = "n" -a "$(check_buildenv color)" = "y" ]; then @@ -1796,7 +1800,7 @@ if [ "$NODEPS" -eq 1 -o "$NOBUILD" -eq 1 -o "$REPKG" -eq 1 ]; then if [ "$NODEPS" -eq 1 ]; then warning "$(gettext "Skipping dependency checks.")" fi -elif [ $(type -p pacman) ]; then +elif [ $(type -p "$PACMAN") ]; then unset pkgdeps # Set by resolve_deps() and used by remove_deps() deperr=0 @@ -1811,7 +1815,7 @@ elif [ $(type -p pacman) ]; then exit 1 fi else - warning "$(gettext "pacman was not found in PATH; skipping dependency checks.")" + warning "$(gettext "%s was not found in PATH; skipping dependency checks.")" "$PACMAN" fi # ensure we have a sane umask set -- 1.6.5.2
On Fri, Nov 6, 2009 at 2:51 PM, Cedric Staniewski <cedric@gmx.ca> wrote:
Implements FS#13028.
Signed-off-by: Cedric Staniewski <cedric@gmx.ca> --- Oops... forgot to adjust the documentation. -T support is not required anymore for the pacman wrappers.
I think I'd rather see this as an environment variable- wouldn't that make more sense, as we wouldn't require it to be specified by the vast majority of people that just want to use the default? PACMAN=${PACMAN:-pacman} or whatever it is. -Dan
Dan McGee wrote:
On Fri, Nov 6, 2009 at 2:51 PM, Cedric Staniewski <cedric@gmx.ca> wrote:
Implements FS#13028.
Signed-off-by: Cedric Staniewski <cedric@gmx.ca> --- Oops... forgot to adjust the documentation. -T support is not required anymore for the pacman wrappers.
I think I'd rather see this as an environment variable- wouldn't that make more sense, as we wouldn't require it to be specified by the vast majority of people that just want to use the default?
PACMAN=${PACMAN:-pacman} or whatever it is.
-Dan
Sorry Dan, but I do not get your point. This patch makes it possible to specify a pacman command in one of the makepkg.conf files, but it is not mandatory because of the reason you mentioned. I already use your line in my patch: +# set pacman command if not defined in config files +PACMAN=${PACMAN:-pacman} + It is, however, not possible to provide a pacman command via environment variable, because I think it makes more sense to store it in a config file. So do you mean it should be possible to use an environment variable as well?
Cedric Staniewski wrote:
Dan McGee wrote:
On Fri, Nov 6, 2009 at 2:51 PM, Cedric Staniewski <cedric@gmx.ca> wrote:
Implements FS#13028.
Signed-off-by: Cedric Staniewski <cedric@gmx.ca> --- Oops... forgot to adjust the documentation. -T support is not required anymore for the pacman wrappers. I think I'd rather see this as an environment variable- wouldn't that make more sense, as we wouldn't require it to be specified by the vast majority of people that just want to use the default?
PACMAN=${PACMAN:-pacman} or whatever it is.
-Dan
Sorry Dan, but I do not get your point. This patch makes it possible to specify a pacman command in one of the makepkg.conf files, but it is not mandatory because of the reason you mentioned. I already use your line in my patch:
+# set pacman command if not defined in config files +PACMAN=${PACMAN:-pacman} +
It is, however, not possible to provide a pacman command via environment variable, because I think it makes more sense to store it in a config file. So do you mean it should be possible to use an environment variable as well?
I think Dan wants it to only be available through and environmental variable and not in the makepkg.conf. That should already be possible with your patch (as you point out, the line Dan suggested is already there). I have no real preference but am leaning towards it not being included in pacman.conf given the usage is expected to be low. If we are wrong and it becomes very popular to use some wrapper, then we can always add the config option later. Allan
On Tue, Nov 10, 2009 at 4:36 PM, Allan McRae <allan@archlinux.org> wrote:
Cedric Staniewski wrote:
Dan McGee wrote:
On Fri, Nov 6, 2009 at 2:51 PM, Cedric Staniewski <cedric@gmx.ca> wrote:
Implements FS#13028.
Signed-off-by: Cedric Staniewski <cedric@gmx.ca> --- Oops... forgot to adjust the documentation. -T support is not required anymore for the pacman wrappers.
I think I'd rather see this as an environment variable- wouldn't that make more sense, as we wouldn't require it to be specified by the vast majority of people that just want to use the default?
PACMAN=${PACMAN:-pacman} or whatever it is.
-Dan
Sorry Dan, but I do not get your point. This patch makes it possible to specify a pacman command in one of the makepkg.conf files, but it is not mandatory because of the reason you mentioned. I already use your line in my patch:
+# set pacman command if not defined in config files +PACMAN=${PACMAN:-pacman} +
It is, however, not possible to provide a pacman command via environment variable, because I think it makes more sense to store it in a config file. So do you mean it should be possible to use an environment variable as well?
I think Dan wants it to only be available through and environmental variable and not in the makepkg.conf. That should already be possible with your patch (as you point out, the line Dan suggested is already there).
I have no real preference but am leaning towards it not being included in pacman.conf given the usage is expected to be low. If we are wrong and it becomes very popular to use some wrapper, then we can always add the config option later.
My suggestion was as Allan says; only do it in the environment. Of course, since makepkg.conf is just sourced anyway, then to each his own, but I'd rather not advertise it there at all. Why? Things like $BROWSER, $EDITOR, etc. are pretty standard and accepted variables. There is no reason that on an Arch system, $PACMAN shouldn't join that list. This would address other things, such as people always wanting to have it run with a certain command line flag, or perhaps always use a pacman.static binary, or anything like that. Maybe I'm making orange juice out of bananas, but yeah. -Dan
Dan McGee wrote:
On Tue, Nov 10, 2009 at 4:36 PM, Allan McRae <allan@archlinux.org> wrote:
On Fri, Nov 6, 2009 at 2:51 PM, Cedric Staniewski <cedric@gmx.ca> wrote:
Implements FS#13028.
Signed-off-by: Cedric Staniewski <cedric@gmx.ca> --- Oops... forgot to adjust the documentation. -T support is not required anymore for the pacman wrappers. I think I'd rather see this as an environment variable- wouldn't that make more sense, as we wouldn't require it to be specified by the vast majority of people that just want to use the default?
PACMAN=${PACMAN:-pacman} or whatever it is.
-Dan Sorry Dan, but I do not get your point. This patch makes it possible to specify a pacman command in one of the makepkg.conf files, but it is not mandatory because of the reason you mentioned. I already use your line in my
Dan McGee wrote: patch:
+# set pacman command if not defined in config files +PACMAN=${PACMAN:-pacman} +
It is, however, not possible to provide a pacman command via environment variable, because I think it makes more sense to store it in a config file. So do you mean it should be possible to use an environment variable as well? I think Dan wants it to only be available through and environmental variable and not in the makepkg.conf. That should already be possible with your
Cedric Staniewski wrote: patch (as you point out, the line Dan suggested is already there).
I have no real preference but am leaning towards it not being included in pacman.conf given the usage is expected to be low. If we are wrong and it becomes very popular to use some wrapper, then we can always add the config option later.
My suggestion was as Allan says; only do it in the environment. Of course, since makepkg.conf is just sourced anyway, then to each his own, but I'd rather not advertise it there at all.
Why? Things like $BROWSER, $EDITOR, etc. are pretty standard and accepted variables. There is no reason that on an Arch system, $PACMAN shouldn't join that list. This would address other things, such as people always wanting to have it run with a certain command line flag, or perhaps always use a pacman.static binary, or anything like that. Maybe I'm making orange juice out of bananas, but yeah.
-Dan
I'm fine with your approach and it would definitely have some advantages, however I prefer documenting it somewhere. My worry is that we run into side-effects when someone has set this variable to something weird for some obscure reasons, but perhaps I am a bit too pessimistic here.
Signed-off-by: Cedric Staniewski <cedric@gmx.ca> --- same as [1], just rebased on master [1] http://mailman.archlinux.org/pipermail/pacman-dev/2009-November/010046.html scripts/makepkg.sh.in | 39 +++++++++++++++------------------------ 1 files changed, 15 insertions(+), 24 deletions(-) diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in index 3d776f0..3b7966e 100644 --- a/scripts/makepkg.sh.in +++ b/scripts/makepkg.sh.in @@ -331,10 +331,21 @@ download_file() { fi } +run_pacman() { + local ret=0 + if (( ! ASROOT )) && [[ $1 != "-T" ]]; then + sudo pacman $PACMAN_OPTS "$@" || ret=$? + else + pacman $PACMAN_OPTS "$@" || ret=$? + fi + return $ret +} + check_deps() { (( $# > 0 )) || return - pmout=$(pacman $PACMAN_OPTS -T "$@") + local ret=0 + pmout=$(run_pacman -T "$@") ret=$? if (( ret == 127 )); then #unresolved deps echo "$pmout" @@ -359,15 +370,8 @@ handle_deps() { if (( DEP_BIN )); then # install missing deps from binary packages (using pacman -S) msg "$(gettext "Installing missing dependencies...")" - local ret=0 - - if (( ! ASROOT )); then - sudo pacman $PACMAN_OPTS -S --asdeps $deplist || ret=$? - else - pacman $PACMAN_OPTS -S --asdeps $deplist || ret=$? - fi - if (( ret )); then + if ! run_pacman -S --asdeps $deplist; then error "$(gettext "Pacman failed to install missing dependencies.")" exit 1 # TODO: error code fi @@ -425,15 +429,9 @@ remove_deps() { done msg "Removing installed dependencies..." - local ret=0 - if (( ! ASROOT )); then - sudo pacman $PACMAN_OPTS -Rns $deplist || ret=$? - else - pacman $PACMAN_OPTS -Rns $deplist || ret=$? - fi # Fixes FS#10039 - exit cleanly as package has built successfully - if (( ret )); then + if ! run_pacman -Rns $deplist; then warning "$(gettext "Failed to remove installed dependencies.")" return 0 fi @@ -1116,14 +1114,7 @@ install_package() { fi done - local ret=0 - if (( ! ASROOT )); then - sudo pacman $PACMAN_OPTS -U ${pkglist} || ret=$? - else - pacman $PACMAN_OPTS -U ${pkglist} || ret=$? - fi - - if (( ret )); then + if ! run_pacman -U $pkglist; then warning "$(gettext "Failed to install built package(s).")" return 0 fi -- 1.6.5.3
If PACMAN environment variable is set, makepkg will try to use this command to check for installed dependencies and to install or remove packages. Otherwise, makepkg will fall back to pacman. Implements FS#13028. Signed-off-by: Cedric Staniewski <cedric@gmx.ca> --- scripts/makepkg.sh.in | 21 ++++++++++++--------- 1 files changed, 12 insertions(+), 9 deletions(-) diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in index 3b7966e..d05b608 100644 --- a/scripts/makepkg.sh.in +++ b/scripts/makepkg.sh.in @@ -333,10 +333,10 @@ download_file() { run_pacman() { local ret=0 - if (( ! ASROOT )) && [[ $1 != "-T" ]]; then - sudo pacman $PACMAN_OPTS "$@" || ret=$? + if (( ! ASROOT )) && [[ $1 != "-T" ]] && sudo -l $PACMAN &>/dev/null; then + sudo $PACMAN $PACMAN_OPTS "$@" || ret=$? else - pacman $PACMAN_OPTS "$@" || ret=$? + $PACMAN $PACMAN_OPTS "$@" || ret=$? fi return $ret } @@ -350,7 +350,7 @@ check_deps() { if (( ret == 127 )); then #unresolved deps echo "$pmout" elif (( ret )); then - error "$(gettext "Pacman returned a fatal error (%i): %s")" "$ret" "$pmout" + error "$(gettext "%s returned a fatal error (%i): %s")" "${PACMAN##*/}" "$ret" "$pmout" exit 1 fi } @@ -372,7 +372,7 @@ handle_deps() { msg "$(gettext "Installing missing dependencies...")" if ! run_pacman -S --asdeps $deplist; then - error "$(gettext "Pacman failed to install missing dependencies.")" + error "$(gettext "%s failed to install missing dependencies.")" "${PACMAN##*/}" exit 1 # TODO: error code fi fi @@ -1100,9 +1100,9 @@ install_package() { (( ! INSTALL )) && return if (( ! SPLITPKG )); then - msg "$(gettext "Installing package ${pkgname} with pacman -U...")" + msg "$(gettext "Installing package %s with %s -U...")" "$pkgname" "$PACMAN" else - msg "$(gettext "Installing ${pkgbase} package group with pacman -U...")" + msg "$(gettext "Installing %s package group with %s -U...")" "$pkgbase" "$PACMAN" fi local pkglist @@ -1557,6 +1557,9 @@ if [[ -r ~/.makepkg.conf ]]; then source ~/.makepkg.conf fi +# set pacman command if not already defined +PACMAN=${PACMAN:-pacman} + # check if messages are to be printed using color unset ALL_OFF BOLD BLUE GREEN RED YELLOW if [[ -t 2 && ! $USE_COLOR = "n" && $(check_buildenv color) = "y" ]]; then @@ -1810,7 +1813,7 @@ if (( NODEPS || NOBUILD || REPKG )); then if (( NODEPS )); then warning "$(gettext "Skipping dependency checks.")" fi -elif [ $(type -p pacman) ]; then +elif [ $(type -p "$PACMAN") ]; then unset pkgdeps # Set by resolve_deps() and used by remove_deps() deperr=0 @@ -1825,7 +1828,7 @@ elif [ $(type -p pacman) ]; then exit 1 fi else - warning "$(gettext "pacman was not found in PATH; skipping dependency checks.")" + warning "$(gettext "%s was not found in PATH; skipping dependency checks.")" "$PACMAN" fi # ensure we have a sane umask set -- 1.6.5.3
Cedric Staniewski wrote:
If PACMAN environment variable is set, makepkg will try to use this command to check for installed dependencies and to install or remove packages. Otherwise, makepkg will fall back to pacman.
Implements FS#13028.
Signed-off-by: Cedric Staniewski <cedric@gmx.ca>
Looks good. A couple of comments below. We also need to document the behaviour of the PACMAN environmental variable in the makepkg man page.
--- scripts/makepkg.sh.in | 21 ++++++++++++--------- 1 files changed, 12 insertions(+), 9 deletions(-)
diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in index 3b7966e..d05b608 100644 --- a/scripts/makepkg.sh.in +++ b/scripts/makepkg.sh.in @@ -333,10 +333,10 @@ download_file() {
run_pacman() { local ret=0 - if (( ! ASROOT )) && [[ $1 != "-T" ]]; then - sudo pacman $PACMAN_OPTS "$@" || ret=$? + if (( ! ASROOT )) && [[ $1 != "-T" ]] && sudo -l $PACMAN &>/dev/null; then
This change is behaviour in checking for sudo privileges needs to be a separate patch.
+ sudo $PACMAN $PACMAN_OPTS "$@" || ret=$? else - pacman $PACMAN_OPTS "$@" || ret=$? + $PACMAN $PACMAN_OPTS "$@" || ret=$? fi return $ret } @@ -350,7 +350,7 @@ check_deps() { if (( ret == 127 )); then #unresolved deps echo "$pmout" elif (( ret )); then - error "$(gettext "Pacman returned a fatal error (%i): %s")" "$ret" "$pmout" + error "$(gettext "%s returned a fatal error (%i): %s")" "${PACMAN##*/}" "$ret" "$pmout"
Should we also strip any flags provided in the PACMAN variable? e.g. I could set PACMAN="pacman -v" for verbose output. I guess some wrappers might have flags that would be useful to specify.
exit 1 fi } @@ -372,7 +372,7 @@ handle_deps() { msg "$(gettext "Installing missing dependencies...")"
if ! run_pacman -S --asdeps $deplist; then - error "$(gettext "Pacman failed to install missing dependencies.")" + error "$(gettext "%s failed to install missing dependencies.")" "${PACMAN##*/}" exit 1 # TODO: error code fi fi @@ -1100,9 +1100,9 @@ install_package() { (( ! INSTALL )) && return
if (( ! SPLITPKG )); then - msg "$(gettext "Installing package ${pkgname} with pacman -U...")" + msg "$(gettext "Installing package %s with %s -U...")" "$pkgname" "$PACMAN" else - msg "$(gettext "Installing ${pkgbase} package group with pacman -U...")" + msg "$(gettext "Installing %s package group with %s -U...")" "$pkgbase" "$PACMAN" fi
local pkglist @@ -1557,6 +1557,9 @@ if [[ -r ~/.makepkg.conf ]]; then source ~/.makepkg.conf fi
+# set pacman command if not already defined +PACMAN=${PACMAN:-pacman} + # check if messages are to be printed using color unset ALL_OFF BOLD BLUE GREEN RED YELLOW if [[ -t 2 && ! $USE_COLOR = "n" && $(check_buildenv color) = "y" ]]; then @@ -1810,7 +1813,7 @@ if (( NODEPS || NOBUILD || REPKG )); then if (( NODEPS )); then warning "$(gettext "Skipping dependency checks.")" fi -elif [ $(type -p pacman) ]; then +elif [ $(type -p "$PACMAN") ]; then unset pkgdeps # Set by resolve_deps() and used by remove_deps() deperr=0
@@ -1825,7 +1828,7 @@ elif [ $(type -p pacman) ]; then exit 1 fi else - warning "$(gettext "pacman was not found in PATH; skipping dependency checks.")" + warning "$(gettext "%s was not found in PATH; skipping dependency checks.")" "$PACMAN" fi
# ensure we have a sane umask set
On Thu, Nov 19, 2009 at 10:14 PM, Allan McRae <allan@archlinux.org> wrote:
@@ -350,7 +350,7 @@ check_deps() { if (( ret == 127 )); then #unresolved deps echo "$pmout" elif (( ret )); then - error "$(gettext "Pacman returned a fatal error (%i): %s")" "$ret" "$pmout" + error "$(gettext "%s returned a fatal error (%i): %s")" "${PACMAN##*/}" "$ret" "$pmout"
Should we also strip any flags provided in the PACMAN variable? e.g. I could set PACMAN="pacman -v" for verbose output. I guess some wrappers might have flags that would be useful to specify.
I would say no to this; if a person wants flags then we shouldn't mess with them. -Dan
Dan McGee wrote:
On Thu, Nov 19, 2009 at 10:14 PM, Allan McRae <allan@archlinux.org> wrote:
@@ -350,7 +350,7 @@ check_deps() { if (( ret == 127 )); then #unresolved deps echo "$pmout" elif (( ret )); then - error "$(gettext "Pacman returned a fatal error (%i): %s")" "$ret" "$pmout" + error "$(gettext "%s returned a fatal error (%i): %s")" "${PACMAN##*/}" "$ret" "$pmout" Should we also strip any flags provided in the PACMAN variable? e.g. I could set PACMAN="pacman -v" for verbose output. I guess some wrappers might have flags that would be useful to specify.
I would say no to this; if a person wants flags then we shouldn't mess with them.
Me too, thinking about it now.... The flag could be causing the return failure so it is important to keep it there.
On 12/01/2009 04:02 AM, Allan McRae wrote:
Dan McGee wrote:
On Thu, Nov 19, 2009 at 10:14 PM, Allan McRae <allan@archlinux.org> wrote:
@@ -350,7 +350,7 @@ check_deps() { if (( ret == 127 )); then #unresolved deps echo "$pmout" elif (( ret )); then - error "$(gettext "Pacman returned a fatal error (%i): %s")" "$ret" "$pmout" + error "$(gettext "%s returned a fatal error (%i): %s")" "${PACMAN##*/}" "$ret" "$pmout" Should we also strip any flags provided in the PACMAN variable? e.g. I could set PACMAN="pacman -v" for verbose output. I guess some wrappers might have flags that would be useful to specify.
I would say no to this; if a person wants flags then we shouldn't mess with them.
Me too, thinking about it now.... The flag could be causing the return failure so it is important to keep it there.
We should also keep the full path in this case. It was only meant as a cosmetic change anyway but it is not guaranteed that ${PACMAN##*/} will return the name of the pacman binary. $ PACMAN="pacman --logfile /dev/null" $ echo ${PACMAN##*/} null $
On 11/20/2009 05:14 AM, Allan McRae wrote:
Cedric Staniewski wrote:
If PACMAN environment variable is set, makepkg will try to use this command to check for installed dependencies and to install or remove packages. Otherwise, makepkg will fall back to pacman.
Implements FS#13028.
Signed-off-by: Cedric Staniewski <cedric@gmx.ca>
Looks good. A couple of comments below. We also need to document the behaviour of the PACMAN environmental variable in the makepkg man page.
First of all, thanks for your comments and sorry for the late response. I will send a new patch which addresses the remaining issues later on today.
--- scripts/makepkg.sh.in | 21 ++++++++++++--------- 1 files changed, 12 insertions(+), 9 deletions(-)
diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in index 3b7966e..d05b608 100644 --- a/scripts/makepkg.sh.in +++ b/scripts/makepkg.sh.in @@ -333,10 +333,10 @@ download_file() {
run_pacman() { local ret=0 - if (( ! ASROOT )) && [[ $1 != "-T" ]]; then - sudo pacman $PACMAN_OPTS "$@" || ret=$? + if (( ! ASROOT )) && [[ $1 != "-T" ]] && sudo -l $PACMAN &>/dev/null; then
This change is behaviour in checking for sudo privileges needs to be a separate patch.
Ok.
+ sudo $PACMAN $PACMAN_OPTS "$@" || ret=$? else - pacman $PACMAN_OPTS "$@" || ret=$? + $PACMAN $PACMAN_OPTS "$@" || ret=$? fi return $ret } @@ -350,7 +350,7 @@ check_deps() { if (( ret == 127 )); then #unresolved deps echo "$pmout" elif (( ret )); then - error "$(gettext "Pacman returned a fatal error (%i): %s")" "$ret" "$pmout" + error "$(gettext "%s returned a fatal error (%i): %s")" "${PACMAN##*/}" "$ret" "$pmout"
Should we also strip any flags provided in the PACMAN variable? e.g. I could set PACMAN="pacman -v" for verbose output. I guess some wrappers might have flags that would be useful to specify.
See http://mailman.archlinux.org/pipermail/pacman-dev/2009-December/010208.html
If PACMAN environment variable is set, makepkg will try to use this command to check for installed dependencies and to install or remove packages. Otherwise, makepkg will fall back to pacman. Implements FS#13028. Signed-off-by: Cedric Staniewski <cedric@gmx.ca> --- doc/makepkg.8.txt | 9 +++++++++ scripts/makepkg.sh.in | 19 +++++++++++-------- 2 files changed, 20 insertions(+), 8 deletions(-) diff --git a/doc/makepkg.8.txt b/doc/makepkg.8.txt index 18ee6ed..703c1b0 100644 --- a/doc/makepkg.8.txt +++ b/doc/makepkg.8.txt @@ -162,6 +162,15 @@ Options useful if you are redirecting makepkg output to file. +Environment Variables +--------------------- +*PACMAN*:: + The command which will be used to check for missing dependencies and to + install and remove packages. Pacman's -U, -T, -S and -Rns operations + must be supported by this command. If the variable is not set or + empty, makepkg will fall back to `pacman'. + + Additional Features ------------------- makepkg supports building development versions of packages without having to diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in index ceaa8a6..d5f032f 100644 --- a/scripts/makepkg.sh.in +++ b/scripts/makepkg.sh.in @@ -344,9 +344,9 @@ download_file() { run_pacman() { local ret=0 if (( ! ASROOT )) && [[ $1 != "-T" ]]; then - sudo pacman $PACMAN_OPTS "$@" || ret=$? + sudo $PACMAN $PACMAN_OPTS "$@" || ret=$? else - pacman $PACMAN_OPTS "$@" || ret=$? + $PACMAN $PACMAN_OPTS "$@" || ret=$? fi return $ret } @@ -360,7 +360,7 @@ check_deps() { if (( ret == 127 )); then #unresolved deps echo "$pmout" elif (( ret )); then - error "$(gettext "Pacman returned a fatal error (%i): %s")" "$ret" "$pmout" + error "$(gettext "'%s' returned a fatal error (%i): %s")" "$PACMAN" "$ret" "$pmout" exit 1 fi } @@ -382,7 +382,7 @@ handle_deps() { msg "$(gettext "Installing missing dependencies...")" if ! run_pacman -S --asdeps $deplist; then - error "$(gettext "Pacman failed to install missing dependencies.")" + error "$(gettext "'%s' failed to install missing dependencies.")" "$PACMAN" exit 1 # TODO: error code fi fi @@ -1119,9 +1119,9 @@ install_package() { (( ! INSTALL )) && return if (( ! SPLITPKG )); then - msg "$(gettext "Installing package ${pkgname} with pacman -U...")" + msg "$(gettext "Installing package %s with %s -U...")" "$pkgname" "$PACMAN" else - msg "$(gettext "Installing ${pkgbase} package group with pacman -U...")" + msg "$(gettext "Installing %s package group with %s -U...")" "$pkgbase" "$PACMAN" fi local pkglist @@ -1587,6 +1587,9 @@ if [[ -r ~/.makepkg.conf ]]; then source ~/.makepkg.conf fi +# set pacman command if not already defined +PACMAN=${PACMAN:-pacman} + # check if messages are to be printed using color unset ALL_OFF BOLD BLUE GREEN RED YELLOW if [[ -t 2 && ! $USE_COLOR = "n" && $(check_buildenv color) = "y" ]]; then @@ -1845,7 +1848,7 @@ if (( NODEPS || NOBUILD || REPKG )); then if (( NODEPS )); then warning "$(gettext "Skipping dependency checks.")" fi -elif [ $(type -p pacman) ]; then +elif [ $(type -p "${PACMAN%% *}") ]; then unset pkgdeps # Set by resolve_deps() and used by remove_deps() deperr=0 @@ -1860,7 +1863,7 @@ elif [ $(type -p pacman) ]; then exit 1 fi else - warning "$(gettext "pacman was not found in PATH; skipping dependency checks.")" + warning "$(gettext "%s was not found in PATH; skipping dependency checks.")" "${PACMAN%% *}" fi # ensure we have a sane umask set -- 1.6.5.3
This is particularly useful when using pacman wrappers which call sudo by themselves and therefore should not be run as root. Signed-off-by: Cedric Staniewski <cedric@gmx.ca> --- scripts/makepkg.sh.in | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in index d5f032f..6bee915 100644 --- a/scripts/makepkg.sh.in +++ b/scripts/makepkg.sh.in @@ -343,7 +343,7 @@ download_file() { run_pacman() { local ret=0 - if (( ! ASROOT )) && [[ $1 != "-T" ]]; then + if (( ! ASROOT )) && [[ $1 != "-T" ]] && sudo -l $PACMAN &>/dev/null; then sudo $PACMAN $PACMAN_OPTS "$@" || ret=$? else $PACMAN $PACMAN_OPTS "$@" || ret=$? -- 1.6.5.3
Signed-off-by: Cedric Staniewski <cedric@gmx.ca> --- doc/makepkg.8.txt | 15 +++++++++++++++ 1 files changed, 15 insertions(+), 0 deletions(-) diff --git a/doc/makepkg.8.txt b/doc/makepkg.8.txt index 703c1b0..ccb9a28 100644 --- a/doc/makepkg.8.txt +++ b/doc/makepkg.8.txt @@ -170,6 +170,21 @@ Environment Variables must be supported by this command. If the variable is not set or empty, makepkg will fall back to `pacman'. +**PKGDEST=**"/path/to/folder":: + If this value is not set, packages will by default be placed in the + current directory (location of the linkman:PKGBUILD[5]). Many people + like to keep all their packages in one place so this option allows + this behavior. A common location is ``/home/packages''. + This environment variable will override the corresponding value + defined in linkman:makepkg.conf[5]. + +**SRCDEST=**"/path/to/folder":: + If this value is not set, downloaded source files will only be stored + in the current directory. Many people like to keep all source files in + a central location for easy cleanup, so this path can be set here. + This environment variable will override the corresponding value + defined in linkman:makepkg.conf[5]. + Additional Features ------------------- -- 1.6.5.3
Cedric Staniewski wrote:
Signed-off-by: Cedric Staniewski <cedric@gmx.ca> --- doc/makepkg.8.txt | 15 +++++++++++++++ 1 files changed, 15 insertions(+), 0 deletions(-)
diff --git a/doc/makepkg.8.txt b/doc/makepkg.8.txt index 703c1b0..ccb9a28 100644 --- a/doc/makepkg.8.txt +++ b/doc/makepkg.8.txt @@ -170,6 +170,21 @@ Environment Variables must be supported by this command. If the variable is not set or empty, makepkg will fall back to `pacman'.
+**PKGDEST=**"/path/to/folder":: + If this value is not set, packages will by default be placed in the + current directory (location of the linkman:PKGBUILD[5]). Many people + like to keep all their packages in one place so this option allows + this behavior. A common location is ``/home/packages''. + This environment variable will override the corresponding value + defined in linkman:makepkg.conf[5].
I do not like the repetition of makepkg.conf.5 here. I'd prefer just **PKGDEST=**"/path/to/folder":: Overrides the corresponding value defined in linkman:makepkg.conf[5]. Or something quite simple like that. People can then look up makepkg.conf.5 to find out what the variable does.
+ +**SRCDEST=**"/path/to/folder":: + If this value is not set, downloaded source files will only be stored + in the current directory. Many people like to keep all source files in + a central location for easy cleanup, so this path can be set here. + This environment variable will override the corresponding value + defined in linkman:makepkg.conf[5]. +
Additional Features -------------------
On 12/03/2009 05:19 AM, Allan McRae wrote:
Cedric Staniewski wrote:
Signed-off-by: Cedric Staniewski <cedric@gmx.ca> --- doc/makepkg.8.txt | 15 +++++++++++++++ 1 files changed, 15 insertions(+), 0 deletions(-)
diff --git a/doc/makepkg.8.txt b/doc/makepkg.8.txt index 703c1b0..ccb9a28 100644 --- a/doc/makepkg.8.txt +++ b/doc/makepkg.8.txt @@ -170,6 +170,21 @@ Environment Variables must be supported by this command. If the variable is not set or empty, makepkg will fall back to `pacman'.
+**PKGDEST=**"/path/to/folder":: + If this value is not set, packages will by default be placed in the + current directory (location of the linkman:PKGBUILD[5]). Many people + like to keep all their packages in one place so this option allows + this behavior. A common location is ``/home/packages''. + This environment variable will override the corresponding value + defined in linkman:makepkg.conf[5].
I do not like the repetition of makepkg.conf.5 here. I'd prefer just **PKGDEST=**"/path/to/folder":: Overrides the corresponding value defined in linkman:makepkg.conf[5].
Or something quite simple like that. People can then look up makepkg.conf.5 to find out what the variable does.
I agree with you. How about **PKGDEST=**"/path/to/folder":: Folder where the resulting packages will be stored. Overrides the corresponding value defined in linkman:makepkg.conf[5]. **SRCDEST=**"/path/to/folder":: Folder where the downloaded sources will be stored. Overrides the corresponding value defined in linkman:makepkg.conf[5].
+ +**SRCDEST=**"/path/to/folder":: + If this value is not set, downloaded source files will only be stored + in the current directory. Many people like to keep all source files in + a central location for easy cleanup, so this path can be set here. + This environment variable will override the corresponding value + defined in linkman:makepkg.conf[5]. +
Additional Features -------------------
Cedric Staniewski wrote:
On 12/03/2009 05:19 AM, Allan McRae wrote:
Cedric Staniewski wrote:
Signed-off-by: Cedric Staniewski <cedric@gmx.ca> --- doc/makepkg.8.txt | 15 +++++++++++++++ 1 files changed, 15 insertions(+), 0 deletions(-)
diff --git a/doc/makepkg.8.txt b/doc/makepkg.8.txt index 703c1b0..ccb9a28 100644 --- a/doc/makepkg.8.txt +++ b/doc/makepkg.8.txt @@ -170,6 +170,21 @@ Environment Variables must be supported by this command. If the variable is not set or empty, makepkg will fall back to `pacman'.
+**PKGDEST=**"/path/to/folder":: + If this value is not set, packages will by default be placed in the + current directory (location of the linkman:PKGBUILD[5]). Many people + like to keep all their packages in one place so this option allows + this behavior. A common location is ``/home/packages''. + This environment variable will override the corresponding value + defined in linkman:makepkg.conf[5]. I do not like the repetition of makepkg.conf.5 here. I'd prefer just **PKGDEST=**"/path/to/folder":: Overrides the corresponding value defined in linkman:makepkg.conf[5].
Or something quite simple like that. People can then look up makepkg.conf.5 to find out what the variable does.
I agree with you. How about
**PKGDEST=**"/path/to/folder":: Folder where the resulting packages will be stored. Overrides the corresponding value defined in linkman:makepkg.conf[5].
**SRCDEST=**"/path/to/folder":: Folder where the downloaded sources will be stored. Overrides the corresponding value defined in linkman:makepkg.conf[5].
Looks good to me. I'll let Dan comment as he always picks holes in my documentation! :P
Signed-off-by: Cedric Staniewski <cedric@gmx.ca> --- doc/makepkg.8.txt | 8 ++++++++ 1 files changed, 8 insertions(+), 0 deletions(-) diff --git a/doc/makepkg.8.txt b/doc/makepkg.8.txt index 72206d8..63a2cf4 100644 --- a/doc/makepkg.8.txt +++ b/doc/makepkg.8.txt @@ -170,6 +170,14 @@ Environment Variables must be supported by this command. If the variable is not set or empty, makepkg will fall back to `pacman'. +**PKGDEST=**"/path/to/folder":: + Folder where the resulting packages will be stored. Overrides the + corresponding value defined in linkman:makepkg.conf[5]. + +**SRCDEST=**"/path/to/folder":: + Folder where the downloaded sources will be stored. Overrides the + corresponding value defined in linkman:makepkg.conf[5]. + Additional Features ------------------- -- 1.6.5.4
Cedric Staniewski wrote:
Signed-off-by: Cedric Staniewski <cedric@gmx.ca> ---
Good. Pushed to my working branch. Allan
On Thu, Dec 3, 2009 at 3:17 AM, Allan McRae <allan@archlinux.org> wrote:
Cedric Staniewski wrote:
On 12/03/2009 05:19 AM, Allan McRae wrote:
Cedric Staniewski wrote:
Signed-off-by: Cedric Staniewski <cedric@gmx.ca> --- doc/makepkg.8.txt | 15 +++++++++++++++ 1 files changed, 15 insertions(+), 0 deletions(-)
diff --git a/doc/makepkg.8.txt b/doc/makepkg.8.txt index 703c1b0..ccb9a28 100644 --- a/doc/makepkg.8.txt +++ b/doc/makepkg.8.txt @@ -170,6 +170,21 @@ Environment Variables must be supported by this command. If the variable is not set or empty, makepkg will fall back to `pacman'. +**PKGDEST=**"/path/to/folder":: + If this value is not set, packages will by default be placed in the + current directory (location of the linkman:PKGBUILD[5]). Many people + like to keep all their packages in one place so this option allows + this behavior. A common location is ``/home/packages''. + This environment variable will override the corresponding value + defined in linkman:makepkg.conf[5].
I do not like the repetition of makepkg.conf.5 here. I'd prefer just **PKGDEST=**"/path/to/folder":: Overrides the corresponding value defined in linkman:makepkg.conf[5].
Or something quite simple like that. People can then look up makepkg.conf.5 to find out what the variable does.
I agree with you. How about
**PKGDEST=**"/path/to/folder":: Folder where the resulting packages will be stored. Overrides the corresponding value defined in linkman:makepkg.conf[5].
**SRCDEST=**"/path/to/folder":: Folder where the downloaded sources will be stored. Overrides the corresponding value defined in linkman:makepkg.conf[5].
Looks good to me. I'll let Dan comment as he always picks holes in my documentation! :P
The hole I will pick is I don't believe this documentation is true. Why would we prefer an environment variable declaration over a sourced PKGDEST= set of the value? -Dan
On 12/14/2009 06:30 AM, Dan McGee wrote:
On Thu, Dec 3, 2009 at 3:17 AM, Allan McRae <allan@archlinux.org> wrote:
Cedric Staniewski wrote:
On 12/03/2009 05:19 AM, Allan McRae wrote:
Cedric Staniewski wrote:
Signed-off-by: Cedric Staniewski <cedric@gmx.ca> --- doc/makepkg.8.txt | 15 +++++++++++++++ 1 files changed, 15 insertions(+), 0 deletions(-)
diff --git a/doc/makepkg.8.txt b/doc/makepkg.8.txt index 703c1b0..ccb9a28 100644 --- a/doc/makepkg.8.txt +++ b/doc/makepkg.8.txt @@ -170,6 +170,21 @@ Environment Variables must be supported by this command. If the variable is not set or empty, makepkg will fall back to `pacman'. +**PKGDEST=**"/path/to/folder":: + If this value is not set, packages will by default be placed in the + current directory (location of the linkman:PKGBUILD[5]). Many people + like to keep all their packages in one place so this option allows + this behavior. A common location is ``/home/packages''. + This environment variable will override the corresponding value + defined in linkman:makepkg.conf[5].
I do not like the repetition of makepkg.conf.5 here. I'd prefer just **PKGDEST=**"/path/to/folder":: Overrides the corresponding value defined in linkman:makepkg.conf[5].
Or something quite simple like that. People can then look up makepkg.conf.5 to find out what the variable does.
I agree with you. How about
**PKGDEST=**"/path/to/folder":: Folder where the resulting packages will be stored. Overrides the corresponding value defined in linkman:makepkg.conf[5].
**SRCDEST=**"/path/to/folder":: Folder where the downloaded sources will be stored. Overrides the corresponding value defined in linkman:makepkg.conf[5].
Looks good to me. I'll let Dan comment as he always picks holes in my documentation! :P
The hole I will pick is I don't believe this documentation is true. Why would we prefer an environment variable declaration over a sourced PKGDEST= set of the value?
I do not know why support for these environment variables was originally added but the responsible commit is 1def746 [1]. [1] http://projects.archlinux.org/pacman.git/commit/?id=1def746ad5f1024c78db1935...
On Sun, Dec 13, 2009 at 11:54 PM, Cedric Staniewski <cedric@gmx.ca> wrote:
On 12/14/2009 06:30 AM, Dan McGee wrote:
On Thu, Dec 3, 2009 at 3:17 AM, Allan McRae <allan@archlinux.org> wrote:
Cedric Staniewski wrote:
On 12/03/2009 05:19 AM, Allan McRae wrote:
Cedric Staniewski wrote:
Signed-off-by: Cedric Staniewski <cedric@gmx.ca> --- doc/makepkg.8.txt | 15 +++++++++++++++ 1 files changed, 15 insertions(+), 0 deletions(-)
diff --git a/doc/makepkg.8.txt b/doc/makepkg.8.txt index 703c1b0..ccb9a28 100644 --- a/doc/makepkg.8.txt +++ b/doc/makepkg.8.txt @@ -170,6 +170,21 @@ Environment Variables must be supported by this command. If the variable is not set or empty, makepkg will fall back to `pacman'. +**PKGDEST=**"/path/to/folder":: + If this value is not set, packages will by default be placed in the + current directory (location of the linkman:PKGBUILD[5]). Many people + like to keep all their packages in one place so this option allows + this behavior. A common location is ``/home/packages''. + This environment variable will override the corresponding value + defined in linkman:makepkg.conf[5].
I do not like the repetition of makepkg.conf.5 here. I'd prefer just **PKGDEST=**"/path/to/folder":: Overrides the corresponding value defined in linkman:makepkg.conf[5].
Or something quite simple like that. People can then look up makepkg.conf.5 to find out what the variable does.
I agree with you. How about
**PKGDEST=**"/path/to/folder":: Folder where the resulting packages will be stored. Overrides the corresponding value defined in linkman:makepkg.conf[5].
**SRCDEST=**"/path/to/folder":: Folder where the downloaded sources will be stored. Overrides the corresponding value defined in linkman:makepkg.conf[5].
Looks good to me. I'll let Dan comment as he always picks holes in my documentation! :P
The hole I will pick is I don't believe this documentation is true. Why would we prefer an environment variable declaration over a sourced PKGDEST= set of the value?
I do not know why support for these environment variables was originally added but the responsible commit is 1def746 [1].
[1] http://projects.archlinux.org/pacman.git/commit/?id=1def746ad5f1024c78db1935...
/me eats his words That was probably done for good reason. It is a whole lot easier for mass build tools and chroot build tools to set environment variables rather than rewriting stock config files or those pulled from the host system. With that said, the patch is probably reasonable (I'll proofread it) and I'll get it in there on the next big patch crunch I go through. -Dan
Cedric Staniewski wrote:
If PACMAN environment variable is set, makepkg will try to use this command to check for installed dependencies and to install or remove packages. Otherwise, makepkg will fall back to pacman.
Implements FS#13028.
Signed-off-by: Cedric Staniewski <cedric@gmx.ca> --- doc/makepkg.8.txt | 9 +++++++++ scripts/makepkg.sh.in | 19 +++++++++++-------- 2 files changed, 20 insertions(+), 8 deletions(-)
diff --git a/doc/makepkg.8.txt b/doc/makepkg.8.txt index 18ee6ed..703c1b0 100644 --- a/doc/makepkg.8.txt +++ b/doc/makepkg.8.txt @@ -162,6 +162,15 @@ Options useful if you are redirecting makepkg output to file.
+Environment Variables
Useful Environment Variables? The lead in sounds better, but maybe not quite right. Not sure here...
+--------------------- +*PACMAN*:: + The command which will be used to check for missing dependencies and to + install and remove packages. Pacman's -U, -T, -S and -Rns operations + must be supported by this command. If the variable is not set or + empty, makepkg will fall back to `pacman'.
The command _that_ will... The rest of the description sounds fine. Otherwise, the patch is good.
On Thu, Dec 3, 2009 at 6:09 AM, Allan McRae <allan@archlinux.org> wrote:
Cedric Staniewski wrote:
If PACMAN environment variable is set, makepkg will try to use this command to check for installed dependencies and to install or remove packages. Otherwise, makepkg will fall back to pacman.
Implements FS#13028.
Signed-off-by: Cedric Staniewski <cedric@gmx.ca> --- doc/makepkg.8.txt | 9 +++++++++ scripts/makepkg.sh.in | 19 +++++++++++-------- 2 files changed, 20 insertions(+), 8 deletions(-)
diff --git a/doc/makepkg.8.txt b/doc/makepkg.8.txt index 18ee6ed..703c1b0 100644 --- a/doc/makepkg.8.txt +++ b/doc/makepkg.8.txt @@ -162,6 +162,15 @@ Options useful if you are redirecting makepkg output to file. +Environment Variables
Useful Environment Variables? The lead in sounds better, but maybe not quite right. Not sure here...
"Environment Variables" is a standard manpage header, usually found near the bottom. I didn't look where this patch actually put it in the manpage, but see examples like less, grep, etc.
+--------------------- +*PACMAN*:: + The command which will be used to check for missing dependencies and to + install and remove packages. Pacman's -U, -T, -S and -Rns operations + must be supported by this command. If the variable is not set or + empty, makepkg will fall back to `pacman'.
The command _that_ will... The rest of the description sounds fine.
Otherwise, the patch is good.
Dan McGee wrote:
On Thu, Dec 3, 2009 at 6:09 AM, Allan McRae <allan@archlinux.org> wrote:
Cedric Staniewski wrote:
If PACMAN environment variable is set, makepkg will try to use this command to check for installed dependencies and to install or remove packages. Otherwise, makepkg will fall back to pacman.
Implements FS#13028.
Signed-off-by: Cedric Staniewski <cedric@gmx.ca> --- doc/makepkg.8.txt | 9 +++++++++ scripts/makepkg.sh.in | 19 +++++++++++-------- 2 files changed, 20 insertions(+), 8 deletions(-)
diff --git a/doc/makepkg.8.txt b/doc/makepkg.8.txt index 18ee6ed..703c1b0 100644 --- a/doc/makepkg.8.txt +++ b/doc/makepkg.8.txt @@ -162,6 +162,15 @@ Options useful if you are redirecting makepkg output to file. +Environment Variables Useful Environment Variables? The lead in sounds better, but maybe not quite right. Not sure here...
"Environment Variables" is a standard manpage header, usually found near the bottom. I didn't look where this patch actually put it in the manpage, but see examples like less, grep, etc.
OK, fine. It is place after OPTIONS and before ADDITIONAL FEATURES and CONFIGURATION. Seems a reasonable place.
+--------------------- +*PACMAN*:: + The command which will be used to check for missing dependencies and to + install and remove packages. Pacman's -U, -T, -S and -Rns operations + must be supported by this command. If the variable is not set or + empty, makepkg will fall back to `pacman'. The command _that_ will... The rest of the description sounds fine. I will change the "which" to "that" and pull to my working branch.
Allan
Cedric Staniewski wrote:
Signed-off-by: Cedric Staniewski <cedric@gmx.ca> ---
Pushed to my working branch. Allan
On Fri, Nov 6, 2009 at 2:20 PM, Cedric Staniewski <cedric@gmx.ca> wrote:
Signed-off-by: Cedric Staniewski <cedric@gmx.ca> ---
Seems pretty reasonable to me; Allan, is this OK? -Dan
Dan McGee wrote:
On Fri, Nov 6, 2009 at 2:20 PM, Cedric Staniewski <cedric@gmx.ca> wrote:
Signed-off-by: Cedric Staniewski <cedric@gmx.ca> ---
Seems pretty reasonable to me; Allan, is this OK?
I have been thinking about this and its companion patch. I like the refactoring of the pacman call into the function but dislike not replacing the "pacman -T" call with it. If there is a config option for setting the "pacman" binary, and I have program that replaces pacman (e.g. the one based on the python alpm wrapper should work), then I should not need pacman on my system at all. So I prefer the original version where the "pacman -T" call was replaced too. Allan
Allan McRae wrote:
Dan McGee wrote:
On Fri, Nov 6, 2009 at 2:20 PM, Cedric Staniewski <cedric@gmx.ca> wrote:
Signed-off-by: Cedric Staniewski <cedric@gmx.ca> ---
Seems pretty reasonable to me; Allan, is this OK?
I have been thinking about this and its companion patch. I like the refactoring of the pacman call into the function but dislike not replacing the "pacman -T" call with it.
If there is a config option for setting the "pacman" binary, and I have program that replaces pacman (e.g. the one based on the python alpm wrapper should work), then I should not need pacman on my system at all.
So I prefer the original version where the "pacman -T" call was replaced too.
And leave it to the pacman wrapper authors to fix their programs? Sounds good. :) I also prefer the original patch, mainly because it seems 'cleaner' to me, but being able to replace pacman completely on a system is a valid reason, too.
On Thu, Nov 12, 2009 at 2:08 PM, Cedric Staniewski <cedric@gmx.ca> wrote:
I have been thinking about this and its companion patch. I like the refactoring of the pacman call into the function but dislike not replacing the "pacman -T" call with it.
If there is a config option for setting the "pacman" binary, and I have program that replaces pacman (e.g. the one based on the python alpm wrapper should work), then I should not need pacman on my system at all.
So I prefer the original version where the "pacman -T" call was replaced too.
And leave it to the pacman wrapper authors to fix their programs? Sounds good. :) I also prefer the original patch, mainly because it seems 'cleaner' to me, but being able to replace pacman completely on a system is a valid reason, too.
Well, I am still not convinced. Why would any wrapper have to care about pacman -T ? This is a hidden / undocumented / internal argument just for the usage of makepkg. In the best case, a wrapper will just forward it correctly. In the worst case, it will break it.
On Thu, Nov 12, 2009 at 7:48 AM, Xavier <shiningxc@gmail.com> wrote:
On Thu, Nov 12, 2009 at 2:08 PM, Cedric Staniewski <cedric@gmx.ca> wrote:
I have been thinking about this and its companion patch. I like the refactoring of the pacman call into the function but dislike not replacing the "pacman -T" call with it.
If there is a config option for setting the "pacman" binary, and I have program that replaces pacman (e.g. the one based on the python alpm wrapper should work), then I should not need pacman on my system at all.
So I prefer the original version where the "pacman -T" call was replaced too.
And leave it to the pacman wrapper authors to fix their programs? Sounds good. :) I also prefer the original patch, mainly because it seems 'cleaner' to me, but being able to replace pacman completely on a system is a valid reason, too.
Well, I am still not convinced. Why would any wrapper have to care about pacman -T ? This is a hidden / undocumented / internal argument just for the usage of makepkg.
Doesn't look undocumented to me: -T, --deptest Check dependencies; this is useful in scripts such as makepkg to check installed packages. This operation will check each dependency specified and return a list of those which are not currently satisfied on the system. This operation accepts no other options. Example usage: pacman -T qt "bash>=3.2".
In the best case, a wrapper will just forward it correctly. In the worst case, it will break it.
On Thu, Nov 12, 2009 at 2:51 PM, Dan McGee <dpmcgee@gmail.com> wrote:
Doesn't look undocumented to me: -T, --deptest Check dependencies; this is useful in scripts such as makepkg to check installed packages. This operation will check each dependency specified and return a list of those which are not currently satisfied on the system. This operation accepts no other options. Example usage: pacman -T qt "bash>=3.2".
Ahah ok. Well I am still not sure what a wrapper could add to that functionality, besides breaking it. But if everyone is against me, then I will shut up :) It is not a big deal.
Xavier wrote:
On Thu, Nov 12, 2009 at 2:51 PM, Dan McGee <dpmcgee@gmail.com> wrote:
Doesn't look undocumented to me: -T, --deptest Check dependencies; this is useful in scripts such as makepkg to check installed packages. This operation will check each dependency specified and return a list of those which are not currently satisfied on the system. This operation accepts no other options. Example usage: pacman -T qt "bash>=3.2".
Ahah ok. Well I am still not sure what a wrapper could add to that functionality, besides breaking it. But if everyone is against me, then I will shut up :) It is not a big deal.
It would be possible to get rid of -T and depend on vercmp for the installed dependency check. I am not sure if this is any better though (probably rather worse).
Xavier wrote:
On Thu, Nov 12, 2009 at 2:08 PM, Cedric Staniewski <cedric@gmx.ca> wrote:
I have been thinking about this and its companion patch. I like the refactoring of the pacman call into the function but dislike not replacing the "pacman -T" call with it.
If there is a config option for setting the "pacman" binary, and I have program that replaces pacman (e.g. the one based on the python alpm wrapper should work), then I should not need pacman on my system at all.
So I prefer the original version where the "pacman -T" call was replaced too.
And leave it to the pacman wrapper authors to fix their programs? Sounds good. :) I also prefer the original patch, mainly because it seems 'cleaner' to me, but being able to replace pacman completely on a system is a valid reason, too.
Well, I am still not convinced. Why would any wrapper have to care about pacman -T ? This is a hidden / undocumented / internal argument just for the usage of makepkg.
In the best case, a wrapper will just forward it correctly. In the worst case, it will break it.
Since pacman 3.3 it is not that hidden anymore[1]. [1] http://projects.archlinux.org/pacman.git/commit/?id=9af9c0f328094228fa363d84...
participants (4)
-
Allan McRae
-
Cedric Staniewski
-
Dan McGee
-
Xavier