[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