Re: [PATCH] git-new-workdir: Don't fail if the target directory is empty

[Paul Smith <paul@mad-scientist.net>]

On Thu, 2014-11-20 at 09:13 -0800, Junio C Hamano wrote:
> Paul Smith <paul@mad-scientist.net> writes:
> > +# don't recreate a workdir over an existing directory, unless it's empty
> > +if test -d "$new_workdir"
> >  then
> > -	die "destination directory '$new_workdir' already exists."
> > +	if test $(ls -a1 "$new_workdir"/. | wc -l) -ne 2
> 
> You used to quote this as
> 
> 	if test $(ls -a1 "$new_workdir/." | wc -l) -ne 2
> 
> and I think that made a lot more sense than the new quoting.

Just personal script coding style.  I tend to quote just variables, in
case I want to add globbing or whatever later.  I've run into people who
think that "$foo/bar" needs quotes because it concatenates two strings,
but you don't need to quote $foo by itself; quoting just the variable
helps avoid that impression.

But this doesn't matter much to me.

> Case arms come at the same indentation level as "case/esac".

OK.

> Why these changes?  I thought you are making sure $cleandir is
> absolute so that you do not have to do this and can just "cd" into
> the new working tree, the same way the user would do once it is set
> up.

I made cleandir absolute mainly because you asked me to :-).  I didn't
realize that the implication behind that request was that I'd put back
the original "cd" as well.

I personally distrust and avoid raw cd in shell scripts.  It's
action-at-a-distance and can cause all sorts of problems, sometimes
disastrous ones.  If I need to change directories I localize it, e.g.:

  (cd "$somedir" && do some command)

so it's clear that it's in effect only for that command.

I understand that in this particular situation the "cd" is right before
the end so this doesn't hurt (as the script stands today).  If you
prefer the "cd" explicitly I'll add it back.  You're the one that needs
to be happy with it :-).

What if I move the cd down to right before the checkout command, and use
$new_workdir in the copy of HEAD?  At least it won't be in effect until
after the workdir is completely set up.  I would feel better about it
then :-).

> > -# now setup the workdir
> > -cd "$new_workdir"
> >  # copy the HEAD from the original repository as a default branch
> > -cp "$git_dir/HEAD" .git/HEAD
> > -# checkout the branch (either the same as HEAD from the original repository, or
> > -# the one that was asked for)
> > -git checkout -f $branch