Re: [PATCH] Submodules always use a relative path to gitdir

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Antony Male <antony.male@xxxxxxxxx> writes:

> Git submoule clone uses git clone --separate-git-dir to checkout a
> submodule's git repository into <supermodule>/.git/modules,...

This is misleading. The <superproject>/.git/modules/<name> is the location
of the $GIT_DIR for the submodule <name>, not the location of its checkout
at <superproject>/<path> that is outside <superproject>/.git/modules/
hierarchy.

> folder does not already exist. git clone --separate-git-dir was
> designed for a scenario where the git repository stays in one location
> and the working copy can be moved.

Are you sure about this "clone's design"? It sounds like a revisionist
history.

Saying something like "it would be nicer if it also let us use in this new
different scenario" in the proposed commit log message is perfectly fine,
but my understanding is that the --separate-git-dir option and "gitdir: "
support were designed to allow having the $GIT_DIR in a different place
from the working tree that has ".git" in it, nothing more, nothing less. I
do not think we meant to support moving either directory after they are
set up. If you want to move either, you would need to (and you can, like
your patch does) tweak "gitdir:" to adjust.

By-the-way-Nit. We do not use any folders. s/folder/directory/.

> In the submodules scenario, neither the git repository nor the working
> copy will be moved relative to each other. However, the supermodule may
> be moved,...

Again, who said that you are allowed to move the superproject directory in
the first place? I would understand that it might be nicer if it could be
moved but I haven't thought this through thoroughly yet---there may be
other side effects from doing so, other than the relativeness of "gitdir".

> Previously, if git submodule clone was called when the submodule's git
> repository already existed in <supermodule>/.git/modules, it would
> simply re-create the submodule's .git file, using a relative path.

... "to point at the existing <superproject>/.git/modules/<name>".

Overall, I think I can agree with the goal, but the tone of the proposed
commit log message rubs the reader in a wrong way to see clearly what this
patch is proposing to do and where its merit lies. It is probably not a
big deal, and perhaps it may be just the order of explanation.

I would probably explain the goal like this if I were doing this patch,
without triggering any need for revisionist history bias.

    Recent versions of "git submodule" maintain the submodule <name> at
    <path> in the superproject using a "separate git-dir" mechanism. The
    repository data for the submodule is stored in ".git/modules/<name>/"
    directory of the superproject, and its working tree is created at
    "<path>/" directory, with "<path>/.git" file pointing at the
    ".git/modules/<name>/" directory.

    This is so that we can check out an older version of the superproject
    that does not yet have the submodule <name> anywhere without losing
    (and later having to re-clone) the submodule repository. Removing
    "<path>" won't lose ".git/modules/<name>", and a different branch that
    has the submodule at different location in the superproject, say
    "<path2>", can create "<path2>/" and ".git" in it to point at the same
    ".git/modules/<name>".

    When instantiating such a submodule, if ".git/modules/<name>/" does
    not exist in the superproject, the submodule repository needs to be
    cloned there first. Then we only need to create "<path>" directory,
    point ".git/modules/<name>/" in the superproject with "<path>/.git",
    and check out the working tree.

    However, the current code is not structured that way. The codepath to
    deal with newly cloned submodules uses "git clone --separate-git-dir"
    and creates "<path>" and "<path>/.git". This can make the resulting
    submodule working tree at "<path>" different from the codepath for
    existing submodules. An example of such differences is that this
    codepath prepares "<path>/.git" with an absolute path, while the
    normal codepath uses a relative path.

When explained this way, the remedy is quite clear, and the change is more
forward-looking, isn't it?  If we later start doing more in the codepath
to deal with existing submodules, your patch may break without having
extra code to cover the "newly cloned" case, too.

I further wonder if we can get away without using separate-git-dir option
in this codepath, though. IOW using

        git clone $quiet -bare ${reference:+"$reference"} "$url" "$gitdir"

might be a better solution.

For example (this relates to the point I mumbled "haven't thought this
through thoroughly yet"), doesn't the newly cloned repository have
core.worktree that points at the working tree that records the <path>,
which would become meaningless when a commit in the superproject that
binds the submodule at different path <path2>?

 git-submodule.sh |   21 ++++++++-------------
 1 files changed, 8 insertions(+), 13 deletions(-)

diff --git a/git-submodule.sh b/git-submodule.sh
index 3adab93..9a23e9d 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -156,21 +156,16 @@ module_clone()
 		;;
 	esac
 
-	if test -d "$gitdir"
+	if ! test -d "$gitdir"
 	then
-		mkdir -p "$path"
-		echo "gitdir: $rel_gitdir" >"$path/.git"
-		rm -f "$gitdir/index"
-	else
-		mkdir -p "$gitdir_base"
-		if test -n "$reference"
-		then
-			git-clone $quiet "$reference" -n "$url" "$path" --separate-git-dir "$gitdir"
-		else
-			git-clone $quiet -n "$url" "$path" --separate-git-dir "$gitdir"
-		fi ||
-		die "$(eval_gettext "Clone of '\$url' into submodule path '\$path' failed")"
+		git clone $quiet -n ${reference:+"$reference"} \
+			--separate-git-dir "$gitdir" "$url" "$path" ||
+		die "$(eval_gettext "Clone of '\$url' for submodule '\$name' failed")
 	fi
+
+	mkdir -p "$path"
+	echo "gitdir: $rel_gitdir" >"$path/.git"
+	rm -f "$gitdir/index"
 }
 
 #

--
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


[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]