threads / patch / 20801

patchgit-submodule and --upload-pack

Subject: [PATCH] git-submodule and --upload-pack

## tl;dr

3 messages between Aug 31, 2009 and Sep 2, 2009. Diffs are folded; open one to read it.

replies: 2people: 2as markdown or json

Giulio Eulisse· Aug 31, 2009, 13:52 UTC · lore
Ciao,

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.

Comments?

Ciao, Giulio --

Show changes to 3 files +108 −9

Documentation/git-submodule.txt, Documentation/gitmodules.txt, git-submodule.sh

diff --git a/Documentation/git-submodule.txt b/Documentation/git- 
submodule.txt
index 683ba1a..50aa930 100644
--- a/Documentation/git-submodule.txt
+++ b/Documentation/git-submodule.txt
@@ -9,8 +9,8 @@ git-submodule - Initialize, update or inspect submodules
  SYNOPSIS
  --------
  [verse]
-'git submodule' [--quiet] add [-b branch]
-	      [--reference <repository>] [--] <repository> <path>
+'git submodule' [--quiet] add [-b branch]
+              [-u <git-upload-pack>] [--receive-pack <git-receive- 
pack>] [--reference <repository>] [--] <repository> <path>
  'git submodule' [--quiet] status [--cached] [--] [<path>...]
  'git submodule' [--quiet] init [--] [<path>...]
  'git submodule' [--quiet] update [--init] [-N|--no-fetch] [--rebase]
@@ -164,6 +164,19 @@ OPTIONS
  --branch::
  	Branch of repository to add as submodule.

+--upload-pack <upload-pack>::
+-u <upload-pack>::
+	When given, and the repository to clone from is accessed
+	via ssh, this specifies a non-default path for the
+	'git-upload-pack' program on the remote end.  See
+	linkgit:git-fetch-pack[1].
+
+--receive-pack <receive-pack>::
+	When given, and the repository to clone from is accessed
+	via ssh, this specifies a non-default path for the
+	'git-receive-pack' program on the remote end.  See
+	linkgit:git-push[1].
+
  --cached::
  	This option is only valid for status and summary commands.  These
  	commands typically use the commit found in the submodule HEAD, but
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].
+
  submodule.<name>.update::
  	Defines what to do when the submodule is updated by the superproject.
  	If 'checkout' (the default), the new commit specified in the
@@ -41,7 +49,6 @@ submodule.<name>.update::
  	This config option is overridden if 'git submodule update' is given
  	the '--merge' or '--rebase' options.

-
  EXAMPLES
  --------

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


  This defines two submodules, `libfoo` and `libbar`. These are  
expected to
  be checked out in the paths 'include/foo' and 'include/bar', and for  
both
  submodules an url is specified which can be used for cloning the  
submodules.
+For `libbar`, packs are retrieved and stored via the upload and receive
+wrappers, respectively.

  SEE ALSO
  --------
diff --git a/git-submodule.sh b/git-submodule.sh
index ebed711..25edd59 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -82,6 +82,8 @@ module_clone()
  	path=$1
  	url=$2
  	reference="$3"
+        uploadpack=$4
+        receivepack=$5

  	# If there already is a directory at the submodule path,
  	# expect it to be empty (since that is the default checkout
@@ -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"
+        fi
+
  	if test -n "$reference"
  	then
-		git-clone "$reference" -n "$url" "$path"
+		git-clone $uploadpackCmd "$reference" -n "$url" "$path"
  	else
-		git-clone -n "$url" "$path"
+		git-clone $uploadpackCmd -n "$url" "$path"
  	fi ||
  	die "Clone of '$url' into submodule path '$path' failed"
+	if test "$uploadpack"
+	then
+	    git config -f "${path}/.git/config" remote.origin.uploadpack  
"$uploadpack" ||
+	    echo "  Warn: Failed to set uploadpack for '$url' in submodule  
path '$path'."
+	fi
+	if test "$receivepack"
+	then
+	    git config -f "${path}/.git/config" remote.origin.receivepack  
"$receivepack" ||
+	    echo "  Warn: Failed to set receivepack for '$url' in submodule  
path '$path'."
+	fi
  }

  #
@@ -136,6 +155,16 @@ cmd_add()
  			reference="$1"
  			shift
  			;;
+		-u|--upload-pack)
+			case "$2" in '') usage ;; esac
+			uploadpack=$2
+			shift
+			;;
+		--receive-pack)
+			case "$2" in '') usage ;; esac
+			receivepack=$2
+			shift
+			;;
  		--)
  			shift
  			break
@@ -206,9 +235,17 @@ cmd_add()
  			;;
  		esac
  		git config submodule."$path".url "$url"
+		if test "$uploadpack"
+			then
+		    git config submodule."$path".uploadpack "$uploadpack"
+		fi
+		if test "$receivepack"
+		then
+		    git config submodule."$path".receivepack "$receivepack"
+		fi
  	else

-		module_clone "$path" "$realrepo" "$reference" || exit
+		module_clone "$path" "$realrepo" "$reference" "$uploadpack"  
"$receivepack" || exit
  		(
  			unset GIT_DIR
  			cd "$path" &&
@@ -224,7 +261,19 @@ cmd_add()
  	die "Failed to add submodule '$path'"

  	git config -f .gitmodules submodule."$path".path "$path" &&
-	git config -f .gitmodules submodule."$path".url "$repo" &&
+	git config -f .gitmodules submodule."$path".url "$repo" ||
+	die "Failed to register submodule '$path'"
+
+	if test "$uploadpack"
+        then
+	    git config -f .gitmodules submodule."$path".uploadpack  
"$uploadpack" ||
+	    die "Failed to register submodule '$path'"
+	fi
+	if test "$receivepack"
+        then
+	    git config -f .gitmodules submodule."$path".receivepack  
"$receivepack" ||
+	    die "Failed to register submodule '$path'"
+	fi
  	git add .gitmodules ||
  	die "Failed to register submodule '$path'"
  }
@@ -299,6 +348,19 @@ cmd_init()
  		git config submodule."$name".url "$url" ||
  		die "Failed to register url for submodule path '$path'"

+		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 '$path'."
+		fi
+		if test "$receivepack"
+		then
+		    git config submodule."$name".receivepack "$receivepack" ||
+		    echo "  Warn: Failed to set receivepack for '$url' in submodule  
path '$path'."
+		fi
+
  		upd="$(git config -f .gitmodules submodule."$name".update)"
  		test -z "$upd" ||
  		git config submodule."$name".update "$upd" ||
@@ -384,7 +446,8 @@ cmd_update()

  		if ! test -d "$path"/.git -o -f "$path"/.git
  		then
-			module_clone "$path" "$url" "$reference"|| exit
+			module_clone "$path" "$url" "$reference" "$(git config  
submodule."$name".uploadpack)" \
+                            "$(git config  
submodule."$name".receivepack)" || exit
  			subsha1=
  		else
  			subsha1=$(unset GIT_DIR; cd "$path" &&
@@ -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
  		)
  		fi
  	done
Junio C Hamano· Aug 31, 2009, 23:51 UTC · re: Giulio Eulisse · lore

Re: [PATCH] git-submodule and --upload-pack

Giulio Eulisse <Giulio.Eulisse@cern.ch> writes:
Show 6 quoted lines
> 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.

Show 15 quoted lines
> 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.

Show 11 quoted lines
> @@ -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.

Show 9 quoted lines
> @@ -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.

Show 6 quoted lines
> +        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.

Show 20 quoted lines
> @@ -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.

Giulio Eulisse· Sep 2, 2009, 12:41 UTC · re: Junio C Hamano · lore

Re: [PATCH] git-submodule and --upload-pack

Show 14 quoted lines
>> 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'.

Ciao, sorry for the mangled patch... Looks like Mail.app did something weird...

http://article.gmane.org/gmane.comp.version-control.git/104188/match=git+submodule+upload+pack
> 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.
[...]
Ok. I will try to follow your suggestion and post an updated patch.

Ciao, Giulio

← back to recent threads