[pacman-dev] [PATCH 2/2] Use sysconfdir, localstatedir, BASH instead of hard-coded values

Nezmer git at nezmer.info
Mon Oct 11 18:16:07 EDT 2010


On Mon, Oct 11, 2010 at 04:35:46PM -0500, Dan McGee wrote:
> 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.
> 

Should I rename README to README.in ? Or split the *.in files out in
another variable and append it to EXTRA_DIST ?

> > +
> > +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.
> 

I'll look into it.

> > +
> > +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.
> 

Will do.

> > +
> > +$(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...

What 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?
> 

Files in contrib don't have similar permissions (some 755, some 644).
'>' If I understand correctly gets permissions from umask. So I think
what I did is the most efficient way to reserve permissions
automatically.

contrib files are packaged manually, right? So 'make install' wouldn't
take care of it!

> > +
> > +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?
> 

I'm not sure what you mean. /etc,/var are hardcoded in *.txt files.
Are those autoreplaced by asciidoc?

> >  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