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