[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: New auxiliary archive script
From: |
Ralf Wildenhues |
Subject: |
Re: New auxiliary archive script |
Date: |
Wed, 4 Aug 2010 22:18:32 +0200 |
User-agent: |
Mutt/1.5.20 (2010-04-22) |
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
- Re: pr-msvc-support merge, Ralf Wildenhues, 2010/08/01
- Re: New auxiliary archive script, Peter Rosin, 2010/08/05
- Re: New auxiliary archive script, Ralf Wildenhues, 2010/08/06
- Re: New auxiliary archive script, Peter Rosin, 2010/08/06
- cond5.test spurious failure (was: New auxiliary archive script), Stefano Lattarini, 2010/08/06
- Re: cond5.test spurious failure, Peter Rosin, 2010/08/06
- Re: cond5.test spurious failure, Stefano Lattarini, 2010/08/06