Date: Fri, 4 Feb 2011 10:22:21 +0000 From: Alexander Best <arundel@freebsd.org> To: Eygene Ryabinkin <rea@freebsd.org> Cc: freebsd-hackers@freebsd.org Subject: Re: insufficient flag handling in tools/install.sh Message-ID: <20110204102221.GA41464@freebsd.org> In-Reply-To: <tO0VRfG8IHdubUqZgtUh/v4OTF0@QsmfhJNucgI88DfvPJdT1/nyboE> References: <20110203194306.GA55376@freebsd.org> <tO0VRfG8IHdubUqZgtUh/v4OTF0@QsmfhJNucgI88DfvPJdT1/nyboE>
next in thread | previous in thread | raw e-mail | index | archive | help
--EVF5PPMfhYS0aIcm
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
On Fri Feb 4 11, Eygene Ryabinkin wrote:
> Alexander, good day.
hi eygene. thanks a lot for your patch. i made two changes:
1) also catch the -v option for install(1).
2) install(1) accepts multiple directories as arguments, when he -d flag has
been specified. so i don't think it's necessary to iterate through all the
directories passed to install.sh and invoke install(1) with only one
argument at a time.
could you take a look at the attached patch?
thanks.
alex
>
> Thu, Feb 03, 2011 at 07:43:06PM +0000, Alexander Best wrote:
> > it seems the -d flag breaks the semantics of tools/install.sh entirely and
> > using the script in such a case simply passes all args over to install(1).
> >
> > simply adding the -d flag to the first switch statement won't work, since we
> > need to tell install(1) that it should only expect a single directory as
> > argument. so the -d flag needs to be passed over to install(1), while options
> > such as -o X, -g X, etc. need to be stripped away.
>
> The attached patch should fix this:
> {{{
> $ sh install.sh.orig -d -m 700 `pwd`/a-test
> $ ls -ld a-test
> drwx------ 2 rea rea 512 4 Feb 10:03 a-test
> $ sh install.sh -d -m 700 `pwd`/a-test
> $ ls -ld a-test
> drwxr-xr-x 2 rea rea 512 4 Feb 10:03 a-test
> }}}
>
> It also adds some proper quoting for the remaining arguments (plain $*
> vs quoted "$@") to the install.sh (this is a pathological case, but it
> is better to fix this too, while we're on topic):
> {{{
> $ rm -rf 1; mkdir 1; cd 1; sh -x ../install.sh.orig -d -m 512 "this is a test"; ls -l; cd ..
> + [ 4 -gt 0 ]
> + break
> + exec install -p -d -m 512 this is a test
> total 8
> dr-x--x-w- 2 rea rea 512 Feb 4 10:10 a
> dr-x--x-w- 2 rea rea 512 Feb 4 10:10 is
> dr-x--x-w- 2 rea rea 512 Feb 4 10:10 test
> dr-x--x-w- 2 rea rea 512 Feb 4 10:10 this
>
> $ rm -rf 1; mkdir 1; cd 1; sh -x ../install.sh -d -m 512 "this is a test"; ls -l; cd ..
> + dirmode=''
> + [ 4 -gt 0 ]
> + dirmode=YES
> + shift
> + [ 3 -gt 0 ]
> + shift
> + shift
> + [ 1 -gt 0 ]
> + break
> + [ 1 -eq 0 ]
> + [ -z YES ]
> + install -d 'this is a test'
> total 2
> drwxr-xr-x 2 rea rea 512 Feb 4 10:10 this is a test
> }}}
> --
> Eygene Ryabinkin ,,,^..^,,,
> [ Life's unfair - but root password helps! | codelabs.ru ]
> [ 82FE 06BC D497 C0DE 49EC 4FF0 16AF 9EAE 8152 ECFB | freebsd.org ]
--
a13x
--EVF5PPMfhYS0aIcm
Content-Type: text/plain; charset=us-ascii
Content-Disposition: attachment; filename="install.sh.diff"
Index: tools/install.sh
===================================================================
--- tools/install.sh (revision 218217)
+++ tools/install.sh (working copy)
@@ -29,14 +29,25 @@
# $FreeBSD$
# parse install's options and ignore them completely.
+dirmode=""
while [ $# -gt 0 ]; do
case $1 in
- -[bCcMpSs]) shift;;
+ -d) dirmode="YES"; shift;;
+ -[bCcMpSsv]) shift;;
-[Bfgmo]) shift; shift;;
-[Bfgmo]*) shift;;
*) break;
esac
done
+if [ "$#" -eq 0 ]; then
+ echo "Nothing to do: no files/dirs specified" >&2
+ exit 1
+fi
+
# the remaining arguments are assumed to be files/dirs only.
-exec install -p $*
+if [ -z "$dirmode" ]; then
+ exec install -p "$@"
+else
+ exec install -d "$@"
+fi
--EVF5PPMfhYS0aIcm--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20110204102221.GA41464>