[pacman-dev] [PATCH 2/2] Use sysconfdir, localstatedir, BASH instead of hard-coded values
Dan McGee
dpmcgee at gmail.com
Mon Oct 11 17:35:46 EDT 2010
On Mon, Oct 11, 2010 at 4:20 PM, Nezmer <git at nezmer.info> wrote:
> Signed-off-by: Nezmer <git at nezmer.info>
> ---
> contrib/Makefile.am | 18 ++++++++++++++++--
> contrib/bacman.in | 16 ++++++++--------
> contrib/pacdiff.in | 2 +-
> contrib/pacscripts.in | 8 ++++----
> contrib/pactree.in | 10 +++++-----
> contrib/wget-xdelta.sh.in | 10 +++++-----
> contrib/zsh_completion.in | 18 +++++++++---------
> doc/Makefile.am | 12 ++++++++++++
> doc/makepkg.8.txt | 2 +-
> doc/pacman.8.txt | 4 ++--
> doc/pacman.conf.5.txt | 12 ++++++------
> scripts/Makefile.am | 3 +++
> scripts/makepkg.sh.in | 2 +-
> scripts/pacman-optimize.sh.in | 2 +-
> scripts/pkgdelta.sh.in | 2 +-
> scripts/rankmirrors.sh.in | 4 ++--
> scripts/repo-add.sh.in | 2 +-
> 17 files changed, 78 insertions(+), 49 deletions(-)
>
> diff --git a/contrib/Makefile.am b/contrib/Makefile.am
> index c6243b1..15dc227 100644
> --- a/contrib/Makefile.am
> +++ b/contrib/Makefile.am
> @@ -9,7 +9,21 @@ EXTRA_DIST = \
> pactree \
> vimprojects \
> wget-xdelta.sh \
> - zsh_completion \
> - README
> + zsh_completion
So we aren't going to distribute the README anymore? I don't see where
this is included anymore.
> +
> +BASH := $(shell sh -c 'which bash|| echo /bin/bash')
I think this task belongs in configure.ac rather than in the
makefiles. It would then allow overrriding rather than auto-detection.
See the AC_CHECK_PROGS([PYTHON]) line already in there, you can pretty
much do the exact same thing.
> +
> +edit = sed \
> + -e 's|@sysconfdir[@]|$(sysconfdir)|g' \
> + -e 's|@BASH[@]|$(BASH)|g' \
> + -e 's|@localstatedir[@]|$(localstatedir)|g'
I know this is nitpicky, but it would be good to keep these in a
similar order to the one in scripts/Makefile.am, and keep the
directory substitutions together.
> +
> +$(EXTRA_DIST): Makefile
> + @rm -f $@ $@.tmp
> + @cp -a $@.in $@.tmp # To reserve permissions in tmp files
Comments are better if they don't have typos...
> + $(edit) $@.in >$@.tmp
> + @mv $@.tmp $@
I'm not sure which way of doing this is better (vs. the existing
logic), other than I know I cribbed the logic in scripts/Makefile.am
from existing scripts. What is the idea of preserving permissions
rather than explicit permission setting?
> +
> +all-am: $(EXTRA_DIST)
>
> # vim:set ts=2 sw=2 noet:
> diff --git a/contrib/bacman.in b/contrib/bacman.in
> index 6dd7839..2a93f8b 100755
> --- a/contrib/bacman.in
> +++ b/contrib/bacman.in
> @@ -1,4 +1,4 @@
> -#!/bin/bash
> +#!@BASH@
> #
> # bacman: recreate a package from a running system
> # This script rebuilds an already installed package using metadata
> @@ -67,20 +67,20 @@ fi
> #
> # Setting environmental variables
> #
> -if [ ! -r /etc/pacman.conf ]; then
> - echo "ERROR: unable to read /etc/pacman.conf"
> +if [ ! -r @sysconfdir@/pacman.conf ]; then
> + echo "ERROR: unable to read @sysconfdir@/pacman.conf"
> exit 1
> fi
>
> -eval $(awk '/DBPath/ {print $1$2$3}' /etc/pacman.conf)
> -pac_db="${DBPath:-/var/lib/pacman/}/local"
> +eval $(awk '/DBPath/ {print $1$2$3}' @sysconfdir@/pacman.conf)
> +pac_db="${DBPath:- at localstatedir@/lib/pacman/}/local"
>
> -if [ ! -r /etc/makepkg.conf ]; then
> - echo "ERROR: unable to read /etc/makepkg.conf"
> +if [ ! -r @sysconfdir@/makepkg.conf ]; then
> + echo "ERROR: unable to read @sysconfdir@/makepkg.conf"
> exit 1
> fi
>
> -source "/etc/makepkg.conf"
> +source "@sysconfdir@/makepkg.conf"
> if [ -r ~/.makepkg.conf ]; then
> source ~/.makepkg.conf
> fi
> diff --git a/contrib/pacdiff.in b/contrib/pacdiff.in
> index 3f26f38..716333a 100755
> --- a/contrib/pacdiff.in
> +++ b/contrib/pacdiff.in
> @@ -1,4 +1,4 @@
> -#!/bin/bash
> +#!@BASH@
> # pacdiff : a simple pacnew/pacorig/pacsave updater
> #
> # Copyright (c) 2007 Aaron Griffin <aaronmgriffin at gmail.com>
> diff --git a/contrib/pacscripts.in b/contrib/pacscripts.in
> index 101fb15..123c79d 100755
> --- a/contrib/pacscripts.in
> +++ b/contrib/pacscripts.in
> @@ -1,4 +1,4 @@
> -#!/bin/bash
> +#!@BASH@
> #
> # pacscripts : tries to print out the {pre,post}_{install,remove,upgrade}
> # scripts of a given package
> @@ -27,7 +27,7 @@ set -o errexit
> progname=$(basename $0)
> progver="0.4"
>
> -conf="/etc/pacman.conf"
> +conf="@sysconfdir@/pacman.conf"
>
> if [ ! -r "$conf" ]; then
> echo "ERROR: unable to read $conf"
> @@ -36,8 +36,8 @@ fi
>
> eval $(awk '/DBPath/ {print $1$2$3}' "$conf")
> eval $(awk '/CacheDir/ {print $1$2$3}' "$conf")
> -pac_db="${DBPath:-/var/lib/pacman}/local"
> -pac_cache="${CacheDir:-/var/cache/pacman/pkg}"
> +pac_db="${DBPath:- at localstatedir@/lib/pacman}/local"
> +pac_cache="${CacheDir:- at localstatedir@/cache/pacman/pkg}"
>
> error() {
> local mesg=$1; shift
> diff --git a/contrib/pactree.in b/contrib/pactree.in
> index 6051724..29c6af6 100755
> --- a/contrib/pactree.in
> +++ b/contrib/pactree.in
> @@ -1,4 +1,4 @@
> -#!/bin/bash
> +#!@BASH@
> # pactree : a simple dependency tree viewer
> #
> # Copyright (C) 2008 Carlo "carlocci" Bersani <carlocci at gmail.com>
> @@ -272,14 +272,14 @@ if [ $graphviz -eq 1 ]; then
> fi
> fi
>
> -if [ ! -r /etc/pacman.conf ]; then
> - echo "ERROR: unable to read /etc/pacman.conf"
> +if [ ! -r @sysconfdir@/pacman.conf ]; then
> + echo "ERROR: unable to read @sysconfdir@/pacman.conf"
> exit 1
> else
> - eval $(awk '/DBPath/ {print $1$2$3}' /etc/pacman.conf)
> + eval $(awk '/DBPath/ {print $1$2$3}' @sysconfdir@/pacman.conf)
> fi
>
> -pac_db="${DBPath:-/var/lib/pacman}/local"
> +pac_db="${DBPath:- at localstatedir@/lib/pacman}/local"
>
> if [ ! -d "$pac_db" ] ; then
> echo "ERROR: pacman database directory ${pac_db} not found"
> diff --git a/contrib/wget-xdelta.sh.in b/contrib/wget-xdelta.sh.in
> index 4656f4d..caf0171 100755
> --- a/contrib/wget-xdelta.sh.in
> +++ b/contrib/wget-xdelta.sh.in
> @@ -1,7 +1,7 @@
> -#!/bin/bash
> +#!@BASH@
>
> -if [ -r "/etc/makepkg.conf" ]; then
> - source /etc/makepkg.conf
> +if [ -r "@sysconfdir@/makepkg.conf" ]; then
> + source @sysconfdir@/makepkg.conf
> else
> echo "wget-xdelta: Unable to find makepkg.conf"
> exit 1
> @@ -30,11 +30,11 @@ new_version=$(echo $pkg_data | cut -d ' ' -f 2)
> base_url=${file_url%/*}
>
> # Look for the last version
> -for file in $(ls -r /var/cache/pacman/pkg/${pkgname}-*-*{,-$CARCH}$PKGEXT 2>/dev/null); do
> +for file in $(ls -r @localstatedir@/cache/pacman/pkg/${pkgname}-*-*{,-$CARCH}$PKGEXT 2>/dev/null); do
> [[ "$file" =~ "$CARCH" ]] && arch="-$CARCH" || arch=""
> check_version=$(echo $file | \
> sed "s|^.*/${pkgname}-\([[:alnum:]_\.]*-[[:alnum:]_\.]*\)${arch}$PKGEXT$|\1|" | \
> - grep -v "^/var/cache/pacman/pkg")
> + grep -v "^@localstatedir@/cache/pacman/pkg")
>
> [ "$check_version" = "" ] && continue
>
> diff --git a/contrib/zsh_completion.in b/contrib/zsh_completion.in
> index f69fe63..1691913 100644
> --- a/contrib/zsh_completion.in
> +++ b/contrib/zsh_completion.in
> @@ -222,20 +222,20 @@ _pacman_completions_all_groups() {
> _pacman_completions_all_packages() {
> local -a cmd packages repositories packages_long
>
> - repositories=(${(o)${${${(M)${(f)"$(</etc/pacman.conf)"}:#\[*}/\[/}/\]/}:#options})
> + repositories=(${(o)${${${(M)${(f)"$(<@sysconfdir@/pacman.conf)"}:#\[*}/\[/}/\]/}:#options})
> typeset -U repositories
> - packages_long=(/var/lib/pacman/sync/${^repositories}/*(/))
> - packages=(${(o)${${packages_long/\/var\/lib\/pacman\/sync\//}#*/}%-*-*} )
> + packages_long=(@localstatedir@/lib/pacman/sync/${^repositories}/*(/))
> + packages=(${(o)${${packages_long#@localstatedir@/lib/pacman/sync/}#*/}%-*-*} )
> typeset -U packages
> _wanted packages expl "packages" compadd - "${(@)packages}"
> if [[ $PREFIX != */* ]] ; then
> - repositories=(${(o)${${${(M)${(f)"$(</etc/pacman.conf)"}:#\[*}/\[/}/\]/}:#options})
> + repositories=(${(o)${${${(M)${(f)"$(<@sysconfdir@/pacman.conf)"}:#\[*}/\[/}/\]/}:#options})
> typeset -U repositories
> _wanted repo_packages expl "repository/package" compadd -S "/" $repositories
> else
> compset -P '*/'
> - packages_long=(/var/lib/pacman/sync/$IPREFIX*(/))
> - packages=(${(o)${${packages_long/\/var\/lib\/pacman\/sync\//}#*/}%-*-*} )
> + packages_long=(@localstatedir@/lib/pacman/sync/$IPREFIX*(/))
> + packages=(${(o)${${packages_long#@localstatedir@/lib/pacman/sync/}#*/}%-*-*} )
> typeset -U packages
> _wanted repo_packages expl "repository/package" compadd ${(@)packages}
> fi
> @@ -253,15 +253,15 @@ _pacman_completions_installed_groups() {
> # provides completions for installed packages
> _pacman_completions_installed_packages() {
> local -a cmd packages packages_long
> - packages_long=(/var/lib/pacman/local/*(/))
> - packages=( ${${packages_long/\/var\/lib\/pacman\/local\//}%-*-*} )
> + packages_long=(@localstatedir@/lib/pacman/local/*(/))
> + packages=( ${${packages_long#@localstatedir@/lib/pacman/local/}%-*-*} )
> compadd "$@" -a packages
> }
>
> # provides completions for repository names
> _pacman_completions_repositories() {
> local -a cmd repositories
> - repositories=(${(o)${${${(M)${(f)"$(</etc/pacman.conf)"}:#\[*}/\[/}/\]/}:#options})
> + repositories=(${(o)${${${(M)${(f)"$(<@sysconfdir@/pacman.conf)"}:#\[*}/\[/}/\]/}:#options})
> # Uniq the array
> typeset -U repositories
> compadd "$@" -a repositories
> diff --git a/doc/Makefile.am b/doc/Makefile.am
> index 2e656f6..461528f 100644
> --- a/doc/Makefile.am
> +++ b/doc/Makefile.am
> @@ -66,6 +66,10 @@ else
> REAL_PACKAGE_VERSION = $(PACKAGE_VERSION)
> endif
>
> +edit = sed \
> + -e 's|@sysconfdir[@]|$(sysconfdir)|g' \
> + -e 's|@localstatedir[@]|$(localstatedir)|g'
> +
> man_MANS =
> dist_man_MANS = $(ASCIIDOC_MANS) repo-remove.8
>
> @@ -107,10 +111,18 @@ A2X_OPTS = \
> # These rules are due to the includes and files of the asciidoc text
> $(ASCIIDOC_MANS): asciidoc.conf footer.txt
> a2x $(A2X_OPTS) --asciidoc-opts="$(ASCIIDOC_OPTS)" $@.txt
> + @rm -f $@.tmp
> + @cp -a $@ $@.tmp # To reserve permissions in tmp files
> + $(edit) $@ >$@.tmp
> + @mv $@.tmp $@
>
> %.html: %.txt
> asciidoc $(ASCIIDOC_OPTS) $*.txt
> dos2unix $@
> + @rm -f $@.tmp
> + @cp -a $@ $@.tmp # To reserve permissions in tmp files
> + $(edit) $@ >$@.tmp
> + @mv $@.tmp $@
Hmm, I don't know at all what you are doing here. We already do
sysconfdir substitution using ASCIIDOC_OPTS; surely we can just fix
the docs to use that and add localstatedir as well?
> HACKING.html: ../HACKING
> asciidoc $(ASCIIDOC_OPTS) -o $@ ../HACKING
> diff --git a/doc/makepkg.8.txt b/doc/makepkg.8.txt
> index a2fdb3f..4251f38 100644
> --- a/doc/makepkg.8.txt
> +++ b/doc/makepkg.8.txt
> @@ -53,7 +53,7 @@ Options
> in linkman:makepkg.conf[5].
>
> *--config* <`/path/to/config`>::
> - Use an alternate config file instead of the `/etc/makepkg.conf` default;
> + Use an alternate config file instead of the `@sysconfdir@/makepkg.conf` default;
>
> *-d, \--nodeps*::
> Do not perform any dependency checks. This will let you override and
> diff --git a/doc/pacman.8.txt b/doc/pacman.8.txt
> index de1f51f..98922ba 100644
> --- a/doc/pacman.8.txt
> +++ b/doc/pacman.8.txt
> @@ -118,7 +118,7 @@ Options
>
> *-b, \--dbpath* <'path'>::
> Specify an alternative database location (a typical default is
> - ``/var/lib/pacman''). This should not be used unless you know what you are
> + ``@localstatedir@/lib/pacman''). This should not be used unless you know what you are
> doing. *NOTE*: if specified, this is an absolute path and the root path is
> not automatically prepended.
>
> @@ -151,7 +151,7 @@ Options
>
> *\--cachedir* <'dir'>::
> Specify an alternative package cache location (a typical default is
> - ``/var/cache/pacman/pkg''). Multiple cache directories can be specified,
> + ``@localstatedir@/cache/pacman/pkg''). Multiple cache directories can be specified,
> and they are tried in the order they are passed to pacman. *NOTE*: this
> is an absolute path, the root path is not automatically prepended.
>
> diff --git a/doc/pacman.conf.5.txt b/doc/pacman.conf.5.txt
> index 0e8426a..1bd58e8 100644
> --- a/doc/pacman.conf.5.txt
> +++ b/doc/pacman.conf.5.txt
> @@ -35,7 +35,7 @@ NoUpgrade = etc/passwd etc/group etc/shadow
> NoUpgrade = etc/fstab
>
> [core]
> -Include = /etc/pacman.d/core
> +Include = @sysconfdir@/pacman.d/core
>
> [custom]
> Server = file:///home/pkgs
> @@ -57,13 +57,13 @@ Options
>
> *DBPath =* path/to/db/dir::
> Overrides the default location of the toplevel database directory. A
> - typical default is ``/var/lib/pacman/''. Most users will not need to set
> + typical default is ``@localstatedir@/lib/pacman/''. Most users will not need to set
> this option. *NOTE*: if specified, this is an absolute path and the root
> path is not automatically prepended.
>
> *CacheDir =* path/to/cache/dir::
> Overrides the default location of the package cache directory. A typical
> - default is ``/var/cache/pacman/pkg/''. Multiple cache directories can be
> + default is ``@localstatedir@/cache/pacman/pkg/''. Multiple cache directories can be
> specified, and they are tried in the order they are listed in the config
> file. If a file is not found in any cache directory, it will be downloaded
> to the first cache directory with write access. *NOTE*: this is an absolute
> @@ -72,7 +72,7 @@ Options
>
> *LogFile =* '/path/to/file'::
> Overrides the default location of the pacman log file. A typical default
> - is ``/var/log/pacman.log''. This is an absolute path and the root directory
> + is ``@localstatedir@/log/pacman.log''. This is an absolute path and the root directory
> is not prepended.
>
> *HoldPkg =* package ...::
> @@ -147,7 +147,7 @@ Options
>
> *UseSyslog*::
> Log action messages through syslog(). This will insert log entries into
> - ``/var/log/messages'' or equivalent.
> + ``@localstatedir@/log/messages'' or equivalent.
>
> *ShowSize*::
> Display the size of individual packages for '\--sync' and '\--query' modes.
> @@ -180,7 +180,7 @@ contain a file that lists the servers for that repository.
> # use this repository first
> Server = ftp://ftp.archlinux.org/core/os/arch
> # next use servers as defined in the mirrorlist below
> -Include = /etc/pacman.d/mirrorlist
> +Include = @sysconfdir@/pacman.d/mirrorlist
> --------
>
> During parsing, pacman will define the `$repo` variable to the name of the
> diff --git a/scripts/Makefile.am b/scripts/Makefile.am
> index 31e8fb5..5967ab1 100644
> --- a/scripts/Makefile.am
> +++ b/scripts/Makefile.am
> @@ -29,12 +29,15 @@ else
> REAL_PACKAGE_VERSION = $(PACKAGE_VERSION)
> endif
>
> +BASH := $(shell sh -c 'which bash|| echo /bin/bash')
> +
> #### Taken from the autoconf scripts Makefile.am ####
> edit = sed \
> -e 's|@localedir[@]|$(localedir)|g' \
> -e 's|@sysconfdir[@]|$(sysconfdir)|g' \
> -e 's|@localstatedir[@]|$(localstatedir)|g' \
> -e 's|@prefix[@]|$(prefix)|g' \
> + -e 's|@BASH[@]|$(BASH)|g' \
> -e 's|@PACKAGE_VERSION[@]|$(REAL_PACKAGE_VERSION)|g' \
> -e 's|@PACKAGE_BUGREPORT[@]|$(PACKAGE_BUGREPORT)|g' \
> -e 's|@PACKAGE_NAME[@]|$(PACKAGE_NAME)|g' \
> diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in
> index 3a7a4d1..baabf26 100644
> --- a/scripts/makepkg.sh.in
> +++ b/scripts/makepkg.sh.in
> @@ -1,4 +1,4 @@
> -#!/bin/bash -e
> +#!@BASH@ -e
> #
> # makepkg - make packages compatible for use with pacman
> # @configure_input@
> diff --git a/scripts/pacman-optimize.sh.in b/scripts/pacman-optimize.sh.in
> index 78b2345..eb6691c 100644
> --- a/scripts/pacman-optimize.sh.in
> +++ b/scripts/pacman-optimize.sh.in
> @@ -1,4 +1,4 @@
> -#!/bin/bash
> +#!@BASH@
> #
> # pacman-optimize
> # @configure_input@
> diff --git a/scripts/pkgdelta.sh.in b/scripts/pkgdelta.sh.in
> index 1550fa1..d45678d 100644
> --- a/scripts/pkgdelta.sh.in
> +++ b/scripts/pkgdelta.sh.in
> @@ -1,4 +1,4 @@
> -#!/bin/bash
> +#!@BASH@
> #
> # pkgdelta - create delta files for use with pacman and repo-add
> # @configure_input@
> diff --git a/scripts/rankmirrors.sh.in b/scripts/rankmirrors.sh.in
> index 0cd4c08..3851b77 100644
> --- a/scripts/rankmirrors.sh.in
> +++ b/scripts/rankmirrors.sh.in
> @@ -1,4 +1,4 @@
> -#!/bin/bash
> +#!@BASH@
> #
> # rankmirrors - read a list of mirrors from a file and rank them by speed
> # @configure_input@
> @@ -25,7 +25,7 @@ usage() {
> echo "Usage: rankmirrors [options] MIRRORFILE | URL"
> echo
> echo "Ranks pacman mirrors by their connection and opening speed. Pacman mirror"
> - echo "files are located in /etc/pacman.d/. It can also rank one mirror if the URL is"
> + echo "files are located in @sysconfdir@/pacman.d/. It can also rank one mirror if the URL is"
> echo "provided."
> echo
> echo "Options:"
> diff --git a/scripts/repo-add.sh.in b/scripts/repo-add.sh.in
> index 8ef940d..4338011 100644
> --- a/scripts/repo-add.sh.in
> +++ b/scripts/repo-add.sh.in
> @@ -1,4 +1,4 @@
> -#!/bin/bash
> +#!@BASH@
> #
> # repo-add - add a package to a given repo database file
> # repo-remove - remove a package entry from a given repo database file
> --
I think if these points are addressed, this is a really good step
forward for codebase portability, so thank you for submitting this.
-Dan
More information about the pacman-dev
mailing list