Re: [PATCH] contrib/workdir: add a simple script to create a working directory

From: Junio C Hamano <junkio@cox.net>
Date: Mar 27, 2007, 05:59 UTC
Message-ID: <7vabxzrzur.fsf@assigned-by-dhcp.cox.net>
In-Reply-To: <20070327003033.4226.8413.julian@quantumfyre.co.uk>

Julian Phillips <julian@quantumfyre.co.uk> writes:

Show 7 quoted lines

> diff --git a/contrib/workdir/git-new-workdir b/contrib/workdir/git-new-workdir
> new file mode 100755
> index 0000000..5bfd87e
> --- /dev/null
> +++ b/contrib/workdir/git-new-workdir
> @@ -0,0 +1,55 @@
> +#!/bin/bash

I do not see anything bash specific you need to do in your script.

Show 10 quoted lines

> +
> +function usage () {
> +	echo "usage:" $1;
> +	exit 127;
> +}
> +
> +function die () {
> +	echo $1;
> +	exit 128;
> +}

Do not add noiseword "function" in our shell scripts, please. This is the only thing POSIX says "produces unspecified results" I found in your script, so if you lose them you shouldn't have to say "#!/bin/bash".

Show 5 quoted lines

> +test $# -eq 3 || usage "$0 <original> <new_workdir> <branch>";
> +
> +orig_git=$1;
> +new_workdir=$2;
> +branch=$3;

Perhaps default branch to whatever original's HEAD points at?

Show 9 quoted lines

> +
> +# want to make sure that what is pointed to has a .git directory ...
> +test -d ${orig_git}/.git || die "${original_git} is not a git repository!";
> +
> +# don't link to a workdir, link to the original repo the workdir is linked to
> +if test -L ${orig_git}/.git/config
> +then
> +	orig_git=$(dirname $(dirname $(readlink -f gm/.git/config)));
> +fi

"gm"? I think it is not worth doing this, as readlink is not all that portable. Just see if it is a symlink and error out.

Dq all pathname values you get from the user, like "$orig_git". They may have SP in them.

Do you need all those braces around shell variable names?

> +# vim: tabstop=8
> +# vim: noexpandtab

Lose these two lines, please.