Junio C Hamano <gitster@xxxxxxxxx> writes: > Paul Smith <paul@xxxxxxxxxxxxxxxxx> 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. >> -# 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. >> + 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 >> +} >> +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! -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html