[pacman-dev] [PATCH] Allow makepkg to use busybox find
From: Jeremy Huntwork <jhuntwork@lightcubesolutions.com> This is an updated version to the previous patch which had a misplaced -o option and a misformatted line. Allow makepkg to work correctly when used with find from busybox. The switches -empty, -samefile and -lname are not available. It is easy to work around -empty with rmdir, and -lname with readlink. However, -samefile functionality requires tracking and storing inodes to ensure hard links are re-created correctly. Signed-off-by: Jeremy Huntwork <jhuntwork@lightcubesolutions.com> --- scripts/makepkg.sh.in | 53 ++++++++++++++++++++++-------------------------- 1 files changed, 24 insertions(+), 29 deletions(-) diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in index d36dbd6..acf9c99 100644 --- a/scripts/makepkg.sh.in +++ b/scripts/makepkg.sh.in @@ -1057,37 +1057,32 @@ tidy_install() { if check_option "zipman" "y" && [[ -n ${MAN_DIRS[*]} ]]; then msg2 "$(gettext "Compressing man and info pages...")" - local manpage ext file link hardlinks hl - find ${MAN_DIRS[@]} -type f 2>/dev/null | - while read manpage ; do - ext="${manpage##*.}" - file="${manpage##*/}" - if [[ $ext != gz && $ext != bz2 ]]; then - # update symlinks to this manpage - find ${MAN_DIRS[@]} -lname "$file" 2>/dev/null | - while read link ; do + local manpages inode files link + # the '|| true' part keeps the script from bailing on the EOF returned + # by read at the end of the find output + IFS=$'\n' read -rd '' -a files < \ + <(find ${MAN_DIRS[@]} \! -name "*.bz2" \! -name "*.gz" \ + -type f 2>/dev/null || true) || true + for file in ${files[@]} ; do + # Track inodes so hard links can be identified and removed + inode=`stat -c %i ${file}` + if [ -z ${manpages[$inode]} ] ; then + manpages[$inode]="$file" + gzip -9 "$file" + else + rm -f "$file" + ln "${manpages[$inode]}.gz" "${file}.gz" + chmod 644 "${file}.gz" + fi + # update any symlinks to this manpage + file="${file##*/}" + find ${MAN_DIRS[@]} -type l 2>/dev/null | + while read link ; do + if [ "${file}" = "`readlink ${link}`" ] ; then rm -f "$link" "${link}.gz" ln -s -- "${file}.gz" "${link}.gz" - done - - # check file still exists (potentially already compressed due to hardlink) - if [[ -f ${manpage} ]]; then - # find hard links and remove them - # the '|| true' part keeps the script from bailing on the EOF returned - # by read at the end of the find output - IFS=$'\n' read -rd '' -a hardlinks < \ - <(find ${MAN_DIRS[@]} \! -name "$file" -samefile "$manpage" \ - 2>/dev/null || true) || true - rm -f "${hardlinks[@]}" - # 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 done fi @@ -1116,7 +1111,7 @@ tidy_install() { if check_option "emptydirs" "n"; then msg2 "$(gettext "Removing empty directories...")" - find . -depth -type d -empty -delete + find . -depth -mindepth 1 -type d -exec rmdir --ignore-fail-on-non-empty '{}' + fi if check_option "upx" "y"; then -- 1.7.2.2
On Thu, May 03, 2012 at 03:29:08AM -0400, jhuntwork@lightcubesolutions.com wrote:
From: Jeremy Huntwork <jhuntwork@lightcubesolutions.com>
This is an updated version to the previous patch which had a misplaced -o option and a misformatted line.
Allow makepkg to work correctly when used with find from busybox. The switches -empty, -samefile and -lname are not available. It is easy to work around -empty with rmdir, and -lname with readlink. However, -samefile functionality requires tracking and storing inodes to ensure hard links are re-created correctly.
Signed-off-by: Jeremy Huntwork <jhuntwork@lightcubesolutions.com> ---
I'm hard pressed to believe that this is the only place we choke on busybox's "coreutils". We have other, more complete and more sane userspaces which we need to support, and this patch does not honor that. Comments inline. d
scripts/makepkg.sh.in | 53 ++++++++++++++++++++++-------------------------- 1 files changed, 24 insertions(+), 29 deletions(-)
diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in index d36dbd6..acf9c99 100644 --- a/scripts/makepkg.sh.in +++ b/scripts/makepkg.sh.in @@ -1057,37 +1057,32 @@ tidy_install() {
if check_option "zipman" "y" && [[ -n ${MAN_DIRS[*]} ]]; then msg2 "$(gettext "Compressing man and info pages...")" - local manpage ext file link hardlinks hl - find ${MAN_DIRS[@]} -type f 2>/dev/null | - while read manpage ; do - ext="${manpage##*.}" - file="${manpage##*/}" - if [[ $ext != gz && $ext != bz2 ]]; then - # update symlinks to this manpage - find ${MAN_DIRS[@]} -lname "$file" 2>/dev/null | - while read link ; do + local manpages inode files link + # the '|| true' part keeps the script from bailing on the EOF returned + # by read at the end of the find output + IFS=$'\n' read -rd '' -a files < \ + <(find ${MAN_DIRS[@]} \! -name "*.bz2" \! -name "*.gz" \ + -type f 2>/dev/null || true) || true
This ||true crap isn't wanted since dca10b062. I realize you're just copying it, but it needs to go away if you're touching this line.
+ for file in ${files[@]} ; do
Quoting.
+ # Track inodes so hard links can be identified and removed + inode=`stat -c %i ${file}`
Style -- we use $() over `` everywhere else in the codebase. You're breaking compat here, as not every stat implementation supports the -c flag for format. Additionally, it'd probably be better to do this all in one step, rather than stat every single file individually (the forks will add up fast). while read -rd ' ' inode; do read file files[inode]=$file done < <(find ... -exec @STATINODE@ {} +) Where find actually has all the right predicates and @STATINODE@ is something portable (BSDs, OSX, Cygwin) to grab the inode and the filename.
+ if [ -z ${manpages[$inode]} ] ; then
Style -- We don't use POSIX tests anywhere else in makepkg.
+ manpages[$inode]="$file" + gzip -9 "$file" + else + rm -f "$file" + ln "${manpages[$inode]}.gz" "${file}.gz" + chmod 644 "${file}.gz" + fi + # update any symlinks to this manpage + file="${file##*/}" + find ${MAN_DIRS[@]} -type l 2>/dev/null | + while read link ; do + if [ "${file}" = "`readlink ${link}`" ] ; then
Style fail again... And please use -ef rather than explicitly resolving the symlink (which could be a symlink to another symlink).
rm -f "$link" "${link}.gz" ln -s -- "${file}.gz" "${link}.gz" - done - - # check file still exists (potentially already compressed due to hardlink) - if [[ -f ${manpage} ]]; then - # find hard links and remove them - # the '|| true' part keeps the script from bailing on the EOF returned - # by read at the end of the find output - IFS=$'\n' read -rd '' -a hardlinks < \ - <(find ${MAN_DIRS[@]} \! -name "$file" -samefile "$manpage" \ - 2>/dev/null || true) || true - rm -f "${hardlinks[@]}" - # 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 done fi
@@ -1116,7 +1111,7 @@ tidy_install() {
if check_option "emptydirs" "n"; then msg2 "$(gettext "Removing empty directories...")" - find . -depth -type d -empty -delete + find . -depth -mindepth 1 -type d -exec rmdir --ignore-fail-on-non-empty '{}' +
There's no way --ignore-fail-on-non-empty is anything but a ridiculous GNU option that busybox picked up.
fi
if check_option "upx" "y"; then -- 1.7.2.2
On 5/3/12 6:16 AM, Dave Reisner wrote:
I'm hard pressed to believe that this is the only place we choke on busybox's "coreutils". We have other, more complete and more sane userspaces which we need to support, and this patch does not honor that.
There may very well be other places that choke, but I didn't claim this patch fixed all issues. :) I only stated that it fixed something very specific: makepkg now works with find from busybox. I've not yet encountered other issues, but if and when I do, I'll be certain to identify and send along a patch, provided I'm still using pacman. Thanks for the style hints, I'll fix those up.
There's no way --ignore-fail-on-non-empty is anything but a ridiculous GNU option that busybox picked up.
I was unaware you were actually aiming to be portable beyond GNU given the reliance on bash. But you're right, the solutions offered should if anything be more portable, not less :). I'll find a better fix for the empty dirs and the inode discovery. Thanks, JH
On 5/3/12 6:16 AM, Dave Reisner wrote:
if check_option "emptydirs" "n"; then msg2 "$(gettext "Removing empty directories...")" - find . -depth -type d -empty -delete + find . -depth -mindepth 1 -type d -exec rmdir --ignore-fail-on-non-empty '{}' +
There's no way --ignore-fail-on-non-empty is anything but a ridiculous GNU option that busybox picked up.
I have a solution for the compression of man pages that I believe incorporates all the suggestions you gave and works well. It's certainly an improvement of the last version. I'll submit that shortly. For replacing the -empty param, the most elegant thing I've found is: find . -mindepth 1 -depth -type d -exec rmdir "{}" + 2>/dev/null || true Anything else is a lot more code and more cumbersome. If the || true in this instance is unacceptable, then I will probably just submit the changes for the compression section and maintain this modification as a private patch or sed command for myself. Please advise. Thanks, JH
On 04/05/12 09:12, Jeremy Huntwork wrote:
On 5/3/12 6:16 AM, Dave Reisner wrote:
if check_option "emptydirs" "n"; then msg2 "$(gettext "Removing empty directories...")" - find . -depth -type d -empty -delete + find . -depth -mindepth 1 -type d -exec rmdir --ignore-fail-on-non-empty '{}' +
There's no way --ignore-fail-on-non-empty is anything but a ridiculous GNU option that busybox picked up.
I have a solution for the compression of man pages that I believe incorporates all the suggestions you gave and works well. It's certainly an improvement of the last version. I'll submit that shortly.
For replacing the -empty param, the most elegant thing I've found is:
find . -mindepth 1 -depth -type d -exec rmdir "{}" + 2>/dev/null || true
Anything else is a lot more code and more cumbersome. If the || true in this instance is unacceptable, then I will probably just submit the changes for the compression section and maintain this modification as a private patch or sed command for myself.
Why do you need "|| true"?
On 5/3/12 7:34 PM, Allan McRae wrote:
On 04/05/12 09:12, Jeremy Huntwork wrote:
find . -mindepth 1 -depth -type d -exec rmdir "{}" + 2>/dev/null || true
Anything else is a lot more code and more cumbersome. If the || true in this instance is unacceptable, then I will probably just submit the changes for the compression section and maintain this modification as a private patch or sed command for myself.
Why do you need "|| true"?
Because of #!/bin/bash -e find may also discover non-empty directories for which rmdir will fail (we actually want it to fail, obviously). But that exit status would also stop execution of the code. JH
On Thu, May 03, 2012 at 07:40:03PM -0400, Jeremy Huntwork wrote:
On 5/3/12 7:34 PM, Allan McRae wrote:
On 04/05/12 09:12, Jeremy Huntwork wrote:
find . -mindepth 1 -depth -type d -exec rmdir "{}" + 2>/dev/null || true
Anything else is a lot more code and more cumbersome. If the || true in this instance is unacceptable, then I will probably just submit the changes for the compression section and maintain this modification as a private patch or sed command for myself.
Why do you need "|| true"?
Because of #!/bin/bash -e
find may also discover non-empty directories for which rmdir will fail (we actually want it to fail, obviously). But that exit status would also stop execution of the code.
JH
As I mentioned earlier: "This ||true crap isn't wanted since dca10b062". I'll just leave the commit here... http://projects.archlinux.org/pacman.git/commit/?id=dca10b062 d
On 04/05/12 09:40, Jeremy Huntwork wrote:
On 5/3/12 7:34 PM, Allan McRae wrote:
On 04/05/12 09:12, Jeremy Huntwork wrote:
find . -mindepth 1 -depth -type d -exec rmdir "{}" + 2>/dev/null || true
Anything else is a lot more code and more cumbersome. If the || true in this instance is unacceptable, then I will probably just submit the changes for the compression section and maintain this modification as a private patch or sed command for myself.
Why do you need "|| true"?
Because of #!/bin/bash -e
find may also discover non-empty directories for which rmdir will fail (we actually want it to fail, obviously). But that exit status would also stop execution of the code.
Please work from the master branch where "-e" does not exist anymore... Allan
On 5/3/12 7:44 PM, Allan McRae wrote:
Please work from the master branch where "-e" does not exist anymore...
Ah, thanks - I was working on master but didn't notice that change, nor realize what Dave meant regarding it being 'not wanted'. I'll submit the full patch shortly. Thanks, JH
participants (4)
-
Allan McRae
-
Dave Reisner
-
Jeremy Huntwork
-
jhuntwork＠lightcubesolutions.com