Re: New auxiliary archive script

[Ralf Wildenhues]

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 "address@hidden" | while read file
>         do
>           $AR -NOLOGO -REMOVE:"$file" "$archive"
>         done

You can omit the cat and pipe and just
          done < "address@hidden"

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 "address@hidden"`
  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 "address@hidden" | 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 "address@hidden"
>       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