Giulio Eulisse <Giulio.Eulisse@xxxxxxx> writes: > There was a thread a while ago aboyt having --upload-pack support for > git-submodule. > > Given that there was no followup (as far as I can tell) and I needed > pretty much > the same functionality I ported Jason's patch to work on top of 1.6.4.2. Thanks. Can you point at the original patch with a usable commit log message, in the gmane archive (i.e. http://thread.gmane.org/...) if possible? I do not think we have that patch queued anywhere even in 'pu'. Given that it looks like a new feature, I do not think it would be appropriate for any of the future 1.6.4.X series, but if it is useful we may want to have it in the upcoming 1.6.5 release. > Comments? See below. > diff --git a/Documentation/git-submodule.txt b/Documentation/git- > submodule.txt The patch is linewrapped and will not be applicable. But I'll comment on the contents to save a round-trip. > diff --git a/Documentation/gitmodules.txt b/Documentation/gitmodules.txt > index 5daf750..bf982a6 100644 > --- a/Documentation/gitmodules.txt > +++ b/Documentation/gitmodules.txt > @@ -30,6 +30,14 @@ submodule.<name>.path:: > submodule.<name>.url:: > Defines an url from where the submodule repository can be cloned. > > +submodule.<name>.receivepack:: > + The default program to execute on the remote side when pushing. See > + option \--receive-pack of linkgit:git-push[1]. > + > +submodule.<name>.uploadpack:: > + The default program to execute on the remote side when fetching. See > + option \--upload-pack of linkgit:git-fetch-pack[1]. This placement of description in the documentation and variables in the namespace quite sane, as these are properties of the remote site, and they belong together with submodule.<name>.url. > @@ -53,12 +60,16 @@ Consider the following .gitmodules file: > > [submodule "libbar"] > path = include/bar > - url = git://bar.com/git/lib.git > + url = ssh://bar.com/~/git/lib.git > + uploadpack = /home/you/bin/git-upload-pack-wrapper > + receivepack = /home/you/bin/git-receive-pack-wrapper > ... > +For `libbar`, packs are retrieved and stored via the upload and receive > +wrappers, respectively. Using a custom wrapper in this example feels very misleading. The option is primarily meant as a workaround for a broken or hard-to-modify sshd settings that does not allow you to include the directory you installed upload-pack/receive-pack to the PATH environment when the ssh session is not interactive. > @@ -97,13 +99,30 @@ module_clone() > test -e "$path" && > die "A file already exist at path '$path'" > > + uploadpackCmd="" > + > + if test "$uploadpack" > + then > + uploadpackCmd="--upload-pack $uploadpack" Can the value of uploadpack contain a shell IFS? This is a rhetorical question---read on. > + fi > + > if test -n "$reference" > then > - git-clone "$reference" -n "$url" "$path" > + git-clone $uploadpackCmd "$reference" -n "$url" "$path" Without using uploadpackCmd and risking to trash IFS characters in the variable, you can do something like: git clone ${uploadpack+--upload-pack "$uploadpack"} ... I would prefer the code not to set variable "uploadpack" when nothing is specified, instead of setting it to an empty string like this patch does, but if you are going to use an empty string as a signal that no uploadpack is specified, then you would need a colon between 'k'and '+' in the above. > @@ -738,6 +801,18 @@ cmd_sync() > remote=$(get_default_remote) > say "Synchronizing submodule url for '$name'" > git config remote."$remote".url "$url" > + uploadpack=$(git config -f .gitmodules submodule."$name".uploadpack) > + receivepack=$(git config -f .gitmodules > submodule."$name".receivepack) > + if test "$uploadpack" > + then > + git config submodule."$name".uploadpack "$uploadpack" || > + echo " Warn: Failed to set uploadpack for > $url' in submodule path '$name'." > + fi > + if test "$receivepack" > + then > + git config submodule."$name".receivepack "$receivepack" || > + echo " Warn: Failed to set receivepack > for '$url' in submodule path '$name'." > + fi > ) I do not agree with this part, nor what the "cmd_init" does. Having URL/uploadpack/receivepack 3-tuple in the tracked .gitmodules is sensible, as that is how the project expresses its recommendations to people who clone the toplevel project. However, after the top-level project is cloned and the submodules are populated in the work tree of the person who cloned, I think these values should be propagated to $path/.git/config, i.e. the configuration file of the submodule checkout. Inside cmd_update(), there is this code snippet: if test -z "$nofetch" then (unset GIT_DIR; cd "$path" && git-fetch) || die "Unable to fetch in submodule path '$path'" fi And the patch does not touch it. For this git-fetch to honour the custom uploadpack your user configured, remote.origin.uploadpack variable in the configuration file of the submodule checkout needs to be updated, because this fetch will not (and should not) look at the configuration file of the superproject. You _could_ also copy them to submodule.$name.$var of the top-level project if you really wanted to, but I do not think doing so serves any useful purpose. -- 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