[pacman-dev] [PATCH] makepkg : fix a lot of breakages caused by Allan

[Dan McGee]

I sent this here so I could comment on it. Overall seems fine.

On Wed, Jan 7, 2009 at 8:31 PM, Dan McGee <dan at archlinux.org> wrote:
> From: Xavier Chantry <shiningxc at gmail.com>
>
> This patch started as a simple typo fix (pugre instead of purge in two
> places), as well as a fix of a test which was using PURGE_TARGETS instead of
> $PURGE_TARGETS.
>
> It evolved in a slight handling change of the OPTIONS which have a variable
> affecting their behavior (strip STRIP_DIRS, docs DOC_DIRS, zipman MAN_DIRS
> and purge PURGE_TARGETS), as well as a clarification in makepkg.conf. Now
> when a variable is undefined or empty, the corresponding option will have no
> effect. It looked weird to have a fallback when a option is defined but
> empty, it seems more natural to not have any fallbacks.
I think this is valid reasoning. Does anyone object?

> Also re-enable docs by default. It seems arbitrary to delete files from
> packages by default, and it would be more vanilla and distro agnostic to
> keep them. docs was also the only negated option.
Also agreed. We will probably want to ship a distro-specific
makepkg.conf with Arch, but that is a different story.

> Signed-off-by: Xavier Chantry <shiningxc at gmail.com>
> ---
>  doc/makepkg.conf.5.txt |    5 +++--
>  etc/makepkg.conf.in    |   20 ++++++++++----------
>  scripts/makepkg.sh.in  |   20 +++++---------------
>  3 files changed, 18 insertions(+), 27 deletions(-)
>
> diff --git a/doc/makepkg.conf.5.txt b/doc/makepkg.conf.5.txt
> index 044c69e..f699f50 100644
> --- a/doc/makepkg.conf.5.txt
> +++ b/doc/makepkg.conf.5.txt
> @@ -128,10 +128,11 @@ Options
>                Leave empty directories in packages.
>
>        *zipman*;;
> -               Compress manual (man and info) pages with gzip.
> +               Compress manual (man and info) pages with gzip. The directories
> +               affected are specified by the `MAN_DIRS` variable.
>
>        *purge*;;
> -               Remove files specified by the `PUGRE_TARGETS` variable from the
> +               Remove files specified by the `PURGE_TARGETS` variable from the
>                package.
Good.

>  **INTEGRITY_CHECK=(**check1 ...**)**::
> diff --git a/etc/makepkg.conf.in b/etc/makepkg.conf.in
> index 675e5f9..d811867 100644
> --- a/etc/makepkg.conf.in
> +++ b/etc/makepkg.conf.in
> @@ -59,27 +59,27 @@ BUILDENV=(fakeroot !distcc color !ccache !xdelta)
>  #   These are default values for the options=() settings
>  #########################################################################
>  #
> -# Default: OPTIONS=(strip !docs libtool emptydirs zipman purge)
> +# Default: OPTIONS=(strip docs libtool emptydirs zipman purge)
>  #  A negated option will do the opposite of the comments below.
>  #
> -#-- strip:     Strip symbols from binaries/libraries
> -#-- docs:      Save doc and info directories
> +#-- strip:     Strip symbols from binaries/libraries in STRIP_DIRS
> +#-- docs:      Save doc directories specified by DOC_DIRS
>  #-- libtool:   Leave libtool (.la) files in packages
>  #-- emptydirs: Leave empty directories in packages
> -#-- zipman:    Compress manual (man and info) pages with gzip
> -#-- purge:     Remove files sepecified below from package
> +#-- zipman:    Compress manual (man and info) pages in MAN_DIRS with gzip
> +#-- purge:     Remove files specified by PURGE_TARGETS
>  #
> -OPTIONS=(strip !docs libtool emptydirs zipman purge)
> +OPTIONS=(strip docs libtool emptydirs zipman purge)
>
>  #-- File integrity checks to use. Valid: md5, sha1, sha256, sha384, sha512
>  INTEGRITY_CHECK=(md5)
> -#-- Manual (man and info) directories to compress (if option set correctly above)
> +#-- Manual (man and info) directories to compress (if zipman is specified)
>  MAN_DIRS=({usr{,/local}{,/share},opt/*}/{man,info})
> -#-- Doc directories to remove (if option set correctly above)
> +#-- Doc directories to remove (if !docs is specified)
>  DOC_DIRS=(usr/{,share/}{doc,gtk-doc} opt/*/{doc,gtk-doc})
> -#-- Directories to be searched for the strip option (if option set correctly above)
> +#-- Directories to be searched for the strip option (if strip is specified)
>  STRIP_DIRS=(bin lib sbin usr/{bin,lib,sbin,local/{bin,lib,sbin}} opt/*/{bin,lib,sbin})
> -#-- Files to be removed from all packages
> +#-- Files to be removed from all packages (if purge is specified)
>  PURGE_TARGETS=(usr/{,share}/info/dir .packlist *.pod)
>
>  #########################################################################
> diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in
> index 3e0781f..cb52b80 100644
> --- a/scripts/makepkg.sh.in
> +++ b/scripts/makepkg.sh.in
> @@ -712,14 +712,13 @@ tidy_install() {
>        cd "$pkgdir"
>        msg "$(gettext "Tidying install...")"
>
> -       if [ "$(check_option docs)" = "n" ]; then
> +       if [ "$(check_option docs)" = "n" -a -n "${DOC_DIRS[*]}" ]; then
>                msg2 "$(gettext "Removing doc files...")"
> -               #fix flyspray bug #5021
>                rm -rf ${DOC_DIRS[@]}
>        fi
>
> -       if [ "$(check_option purge)" = "y" -a -n "PURGE_TARGETS" ]; then
> -               msg2 "$(gettext "Removing pugre targets...")"
> +       if [ "$(check_option purge)" = "y" -a -n "${PURGE_TARGETS[*]}" ]; then
> +               msg2 "$(gettext "Removing files in %s...")" "PURGE_TARGETS"
It's not necessarily always "Removing files in", is it? e.g. *.pod

>                local pt
>                for pt in "${PURGE_TARGETS[@]}"; do
>                        if [ "${pt}" == "${pt//\/}" ]; then
> @@ -730,13 +729,9 @@ tidy_install() {
>                done
>        fi
>
> -       if [ "$(check_option zipman)" = "y" ]; then
> +       if [ "$(check_option zipman)" = "y" -a -n "${MAN_DIRS[*]}" ]; then
>                msg2 "$(gettext "Compressing man and info pages...")"
>                local manpage ext file link hardlinks hl
> -               if [ -z "${MAN_DIRS[*]}" ]; then
> -                       # fall back to default value
> -                       MAN_DIRS=({usr{,/local}{,/share},opt/*}/{man,info})
> -               fi
>                find ${MAN_DIRS[@]} -type f 2>/dev/null |
>                while read manpage ; do
>                        # check file still exists (potentially compressed with hard link)
> @@ -769,14 +764,9 @@ tidy_install() {
>                done
>        fi
>
> -       if [ "$(check_option strip)" = "y" ]; then
> +       if [ "$(check_option strip)" = "y" -a -n "${STRIP_DIRS[*]}" ]; then
>                msg2 "$(gettext "Stripping debugging symbols from binaries and libraries...")"
>                local binary
> -               if [ -z "${STRIP_DIRS[*]}" ]; then
> -                       # fall back to default value
> -                       STRIP_DIRS=(bin lib sbin usr/{bin,lib,sbin,local/{bin,lib,sbin}}
> -                       opt/*/{bin,lib,sbin})
> -               fi
>                find ${STRIP_DIRS[@]} -type f 2>/dev/null | while read binary ; do
>                        case "$(file -biz "$binary")" in
>                                *application/x-sharedlib*)  # Libraries (.so)
> --
> 1.6.1