Hi Peter, thanks for your work on this!
* Peter Rosin wrote on Wed, Aug 04, 2010 at 12:14:28PM CEST: > Den 2010-08-01 20:06 skrev Ralf Wildenhues: > >Yes, in principle: when them bugs are fixed. Would you be willing to > >rewrite the script so it > > > >- has the blurb header similar to 'compile', > >- half-way decent option handling (doesn't barf on "shift" if given too > > few options, diagnoses unknown options, treats contents of ARFLAGS as > > options not members, etc.) > >- prints error messages on stderr and exits nonzero then, > > I have all of the above fixed, with the (minor IMHO) limitation that > only archive members can be specified in @files. Also, I have not > fixed handling of ARFLAGS, since I don't understand what you mean. > The archiver has no knowledge of ARFLAGS, it just sees a bunch of > options, but I'm sure you know that so you must mean something > different... Yes, I meant that it should have sane option handling. The things that users pass in ARFLAGS end up as command line options to this script. > >I wonder whether we can and should call it 'archive', or whether that > >would clash with user file names in case they don't use > >AC_CONFIG_AUX_DIR. OTOH, a script named 'ar' may not be installable > >anywhere due to the ar binary already there. Thoughts? > > Let's use the prefix am, and append the short form ar -> amar. Because > who can resist a little bit of love in the source trees? FWIW, I came up > with no relevant hits for that in codesearch. I'm really not that excited about the name: It doesn't make it too clear what the tool does (unlike am-ar maybe) and we don't use the 'am' prefix anywhere else: depcomp, compile, mkinstalldirs, gendocs, gnupload. Why should that be different here? What's wrong with 'archive', is it taken already? If you care about it wrapping MS lib only for now: it could wrap other junk as well in the future, no? Now if you insist, then maybe we can just find another compromise name. Please set $me to the name and use that throughout, so at least a change is easily done. ;-) I found a few nits, but nothing substantial, so if you can format your next iteration as git patch, and given testing, it can just go in. > -h | --h*) > cat <<\EOF > Usage: amar [--help] [--version] PROGRAM ACTION ARCHIVE [MEMBER...] > > Members can be specified on separate lines in a file named with @<file>. I think s/can/may/ sounds better because that is not mandatory. Please use @FILE not @<file>; the capitalization is already meant to denote metasyntactic variables (as is done by info pages). > EOF > # strip leading dash in $action > case $action in > -*) action="${action#-}" ;; > esac The case statement is unnecessary, you can use just action=${action#-} > while test -n "$action" > do > case $action in > d*) > delete=yes > action="${action#d}" > ;; > x*) > extract=yes > action="${action#x}" > ;; > t*) > list=yes > action="${action#t}" > ;; > r*) > replace=yes > action="${action#r}" > ;; > c*) > create=yes > action="${action#c}" > ;; > u*) > # TODO: don't ignore update modifier > action="${action#u}" > ;; You can factor out all the action=... lines to after the case statement as action=${action#?} > ?*) > echo "amar: unknown action specified" 1>&2 > exit 1 > ;; > esac > done > if test -n "$delete"; then > if test ! -f "$orig_archive"; then Can it be a symbolic link or another non-regular file type? Won't $AR detect a non-existing file? > echo "amar: archive not found" 1>&2 > exit 1 > fi > for member > do > case $1 in > @*) > # When interpreting the content of the @file, do NOT > # use func_file_conv, since the user would need to > # supply preconverted file names to binutils ar, at > # least for MinGW. > cat "$...@}" | while read file > do > $AR -NOLOGO -REMOVE:"$file" "$archive" > done You can omit the cat and pipe and just done < "$...@}" OTOH, it is common (at least in newer GCC and binutils) to allow @file contents to be just whitespace-separated, including single or double quoting. That could be done with something like atfile_contents=`cat "$...@}"` eval set x "$atfile_contents" shift which you might want to factor out into a function to reuse below. I have no idea whether native w32 tools do something similar, though. Shouldn't the script stop and fail if one of the $AR commands fails? > ;; > *) > func_file_conv "$1" > $AR -NOLOGO -REMOVE:"$file" "$archive" > ;; > esac > done > > elif test -n "$extract"; then > if test ! -f "$orig_archive"; then > echo "amar: archive not found" 1>&2 > exit 1 Repeated code, could be in an 'error' function. > fi > if test $# -gt 0; then > for member > do > case $1 in > @*) > cat "$...@}" | while read file > do > $AR -NOLOGO -EXTRACT:"$file" "$archive" > done > ;; > *) > func_file_conv "$1" > $AR -NOLOGO -EXTRACT:"$file" "$archive" > ;; > esac > done > else > $AR -NOLOGO -LIST "$archive" | while read member > do > func_file_conv "$member" > $AR -NOLOGO -EXTRACT:"$file" "$archive" Is this file conversion necessary, or even right? I mean, is it really that '$AR -list' prints names that '$AR -extract:' cannot eat? Or am I missing something? Does lib have no way to just extract the whole archive? > done > fi > > elif test -n "$replace"; then > if test ! -f "$orig_archive"; then > if test -z "$create"; then > echo "amar: creating $orig_archive" > fi > orig_archive= > else > orig_archive=$archive > fi > > for member > do > case $1 in > @*) > func_file_conv "$...@}" > set x "$@" "@$file" > shift > ;; > *) > func_file_conv "$1" > set x "$@" "$file" > shift The two shifts can be factored outside the case. > ;; > esac > shift > done > > if test -n "$orig_archive"; then > $AR -NOLOGO -OUT:"$archive" "$orig_archive" "$@" > else > $AR -NOLOGO -OUT:"$archive" "$@" > fi > > elif test -n "$list"; then > if test ! -f "$orig_archive"; then > echo "amar: archive not found" 1>&2 > exit 1 > fi > $AR -NOLOGO -LIST "$archive" > fi Thanks, Ralf