On Thu, Dec 29, 2011 at 5:40 PM, Junio C Hamano <gitster@xxxxxxxxx> wrote:
> 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.
Yes, so I think a simple s/checkout/clone/ should fix it.
[...]
>> 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 it was accepted practice to clone a local repo with rsync.
This method continues to work well even with submodules before
git-files became the norm. But now it breaks because of the absolute
paths.
Similarly, clones on network mounts and portable drives where absolute
paths may change from time to time or machine to machine will also
break now but worked before.
So, who said you were NOT allowed to move the superproject directory
directory in the first place? It seems natural that you should be
able to do so, especially since the submodules are all contained
within the superproject path.
>> 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
Revisionism nit: the real danger here is that you lose local commits.
> "<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>".
This doesn't explain why one path is absolute and one is relative.
But I don't suppose this is the place for historical documentation
anyway.
> 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.
I had to read this three times before I understood it. There are some
minor grammatical nits in it, but also the use of nearness and use of
"path" and "codepath" to mean two unrelated things was misleading me.
Here's my attempt to clean it up:
However, the current code is not structured that way. The code to
deal with newly cloned submodules is different from the code to
checkout a workdir for existing submodules. The "newly cloned
submodule" code uses "git clone --separate-git-dir" to create
"<path>" and "<path>/.git". The "existing submodules" code
simply creates the "<path>/.git" internally, using a relative path.
This makes the resulting submodule working tree at "<path>" different
depending on which code is used. An example of such differences
is that the "newly cloned submodule" code prepares "<path>/.git"
with an absolute path, while the "existing submodules" code
prepares the same file using 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.
You may be right about this one. I still think the addition of a
--relative-path option to 'git-checkout --separate-work-dir' could be
useful and also easier to maintain/describe.
> 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>?
Ooh, yes it does. Maybe that should be fixed in this case too.
Because submodule cloning with a separate work-dir is a special case
of git-files and work-dirs because we know that each is relative
(subordinate) to the superproject path. Therefore, I think in this
special-case version of the "separate work-dir" scenario, we should
use super-project-relative paths for both cases.
How do we codify this so this functionality is reliably retained by
future developers? I think moving the code into someplace more
explicit would help, but I haven't looked too deeply at the code.
> 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"
> }
Doesn't this avoid creating core.worktree in the first place? I'm ok
with that because I assume it's never used in the submodule scenario,
but I also suspect that assumption could be wrong. Any concerns?
Phil
--
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