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

Junio C Hamano [off-list ref] writes:

Paul Smith [off-list ref] writes:

No need to resend only to correct the above, even though there may
be comments on the patch itself from me or others that may make you
want to reroll this patch, in which case I'd like to see these nits
gone.

I'll fix in the next iteration.

quoted

-# don't recreate a workdir over an existing repository
-if test -e "$new_workdir"
+# make sure the links use full paths
+git_dir=$(cd "$git_dir"; pwd)

With this change, the comment gets much harder to understand.  "What
links?"  would be the reaction from those who are reading the patch.

I just moved this line; I don't think it had much more context
beforehand, but I'll definitely rewrite the comment to be more clear.

quoted

+	if test $(ls -a1 "$new_workdir/." | wc -l) -ne 2

I wonder if this check is portable for all platforms we care about,
but that is OK, as it should be so for the ones I think of and care
about ;-)

Do you mean "." and ".." representing an empty directory?  That will
work on any system where /bin/sh works, for sure.

Or do you mean using "ls" and "wc"?  I can easily avoid this.

Recall that new-workdir itself only works on systems that support
symlinks; this limits its portability.  For example it doesn't work on
Windows (unfortunately).  I spent the better part of a day a few months
ago playing with the various "symlink-ish" capabilities of Windows NTFS
and couldn't find any reliable way to make this work (although I have
virtually no Windows fu).

The script chdirs around; did you turn $new_workdir into an absolute
path already, or given a relative $new_workdir this is attempting to
remove a hierarchy that is different from what you created?

Good point, I'll fix this up

quoted

+}
+siglist="0 1 2 15"
+trap cleanup $siglist
 
-# create the workdir
-mkdir -p "$new_workdir/.git" || die "unable to create \"$new_workdir\"!"
+# create embedded directories
+for x in logs
+do
+	mkdir -p "$new_workdir/.git/$x" || failed
+done
 
 # create the links to the original repo.  explicitly exclude index, HEAD and
 # logs/HEAD from the list since they are purely related to the current
working
 # directory, and should not be shared.
 for x in config refs logs/refs objects info hooks packed-refs remotes
rr-cache svn
 do
-	case $x in
-	*/*)
-		mkdir -p "$(dirname "$new_workdir/.git/$x")"
-		;;
-	esac

What's this removal about?  If $new_workdir/.git/logs does not
exist, would "ln -s $there/logs/refs $new_workdir/.git/logs/refs"
succeed without first creating $new_workdir/.git/logs directory?

I split the creation of the directories from the symlinks: see the new
loop above.  This allows us to avoid the icky dirname stuff.

New iteration will follow shortly.

Cheers!