[pacman-dev] [PATCH] Compress hard linked man pages

[Dan McGee]

On Dec 4, 2007 6:18 PM, Dan McGee <dpmcgee at gmail.com> wrote:
> On Dec 4, 2007 6:06 PM, Aaron Griffin <aaronmgriffin at gmail.com> wrote:
> > On Dec 4, 2007 5:54 PM, Allan McRae <mcrae_allan at hotmail.com> wrote:
> > > Aaron Griffin wrote:
> > > > Besides what Roman mentioned, I noticed you used the backtick inline
> > > > execution syntax - we like to frown on that around here. Could you
> > > > please use $() instead?
> > > >
> > > Fixed.  I didn't know about that syntax - how is it better?
> >
> > Well, first off, backtick expansions is officially deprecated if I
> > remember correctly, but at the rate these standards move it will be
> > some time before it dies. In addition to hat, $() is much cleaner when
> > it comes to parsing and nesting. It's easier to match a pair of
> > characters than two of the same characters. Take this for example:
> >
> >    `foo -c `bar -x``
> >    $(foo -c $(bar -x))
> >
> > The first one might not even parse right in the first place.
> >
> > > New patch attached.
> >
> > Danke! It looks good to me. We don't have a testsuite for makepkg like
> > we do for pacman, so do you happen to know any packages, offhand, that
> > suffer from the hardlink issue, so that I can test this?
>
> I'm making a few changes to the patch locally, and I'll send it here
> if it works right.
>
> -Dan
>

I've pasted the new patch below, as well as a sample PKGBUILD. Some
comments inlined where I changed things.

diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in
index 953bda2..0ef0e52 100644
--- a/scripts/makepkg.sh.in
+++ b/scripts/makepkg.sh.in
@@ -722,18 +722,34 @@ tidy_install() {


        msg2 "$(gettext "Compressing man pages...")"
-       local manpage ext file link
-       find {usr{,/local},opt/*}/man -type f 2>/dev/null | while read
manpage ; do
-               ext="${manpage##*.}"
-               file="${manpage##*/}"
-               if [ "$ext" != "gz" -a "$ext" != "bz2" ]; then
-                       # update symlinks to this manpage
-                       find {usr{,/local},opt/*}/man -lname "$file"
2>/dev/null | while read link ; do
-                               rm -f "$link"
-                               ln -sf "${file}.gz" "${link}.gz"
-                       done
-                       # compress the original
-                       gzip -9 "$manpage"
+       local manpage mandirs ext file link hardlinks hl
+       mandirs="usr/man usr/local/man usr/share/man opt/*/man"
I added usr/share/man to the list as it is supposed to be the standard
location for manpages (even though we move them above). BTW- why do we
move them, Aaron? This uncovered another bug which I will explain
below.

+       find ${mandirs} -type f 2>/dev/null | while read manpage ; do
+               # check file still exists (potentially compressed with
hard link)
+               if [ -f ${manpage} ]; then
+                       ext="${manpage##*.}"
+                       file="${manpage##*/}"
+                       if [ "$ext" != "gz" -a "$ext" != "bz2" ]; then
+                               # update symlinks to this manpage
+                               find ${mandirs} -lname "$file"
2>/dev/null | while read link ; do
+                                       rm -f "$link"
+                                       ln -sf "${file}.gz" "${link}.gz"
+                               done
+                               # find hard links and remove them
+                               #   the '|| true' part keeps the
script from bailing if find returned an
+                               #   error, such as when one of the man
directories doesn't exist
+                               hardlinks="$(find ${mandirs} \! -name
"$file" -samefile "$manpage" 2>/dev/null)" || true
I didn't like having to parse ls output (and start another process)
just to get an inode number, so I changed it to use the -samefile test
of find. When doing this, I realized that because we execute our
makepkg script with the -e option, returning an error code causes
hardlinks to get set to nothing, and thus we don't do any file removal
or creation. Thus the '|| true' and my comment above, which is similar
to a construct used during the getopts call (Andrew, I believe you did
that).

+                               for hl in ${hardlinks}; do
+                                       rm -f "${hl}";
+                               done
No need for the if statements, as a loop on nothing does nothing wrong.

+                               # compress the original
+                               gzip -9 "$manpage"
+                               # recreate hard links removed earlier
+                               for hl in ${hardlinks}; do
+                                       ln "${manpage}.gz" "${hl}.gz"
+                                       chmod 644 ${hl}.gz
+                               done
+                       fi
                fi
        done


$ cat PKGBUILD
pkgname=hardlink_test
pkgver=0.1
pkgrel=1
arch=('i686' 'x86_64')

build()
{
        cd "$pkgdir"

        for dir in usr usr/local usr/share opt/test; do
                mkdir -pv $dir/man/man1

                echo "test manpage compression" > $dir/man/man1/foo.1
                ln -v $dir/man/man1/foo.1 $dir/man/man1/bar.1
                ln -v $dir/man/man1/foo.1 $dir/man/man1/xyz.1
        done
}