Re: [PATCH v2] submodule: Allow tracking of the newest revision of a branch in a submodule

From: Johannes Sixt <j.sixt@viscovery.net>
Date: Dec 11, 2008, 14:24 UTC
Message-ID: <494122B5.5060409@viscovery.net>
In-Reply-To: <1229001361-9301-1-git-send-email-git@fabian-franz.de>

Fabian Franz schrieb:

Show 6 quoted lines

> Submodules currently only allow tracking a specific revision
> and each update in a submodule leads to a new commit in the
> master repository. However some users may want to always track
> the newest revision of a specific (named) tag or branch or HEAD.
> For example the user might want to track a staging branch in all
> submodules.

Personally, I don't particularly like this feature (but then, nobody forces me to use it ;) In which situation do you need this?

By tieing a project commit to a particular submodule commit the committer gives the guarantee: "I've tested this with this module version, and it works; all is ok." With this new feature, this guarantee vanishes, because the committer has no control over which version of the module will ultimately be used; it could be newer or it could be older.

I've reviewed the patch just from a shell code writer's point of view.

> +	[ -n "$track" ] && echo "160000 0000000000000000000000000000000000000000\t$path" | git update-index --index-info

We tend to use "test" instead of "[ ]". You cannot rely on that echo or the shell translates "\t"; use printf.

	test "$track" && printf '160000
0000000000000000000000000000000000000000\t%s\n' "$path" | git update-index
--index-info

(The line-wrapping is from my MUA; sorry.)

Show 5 quoted lines

> @@ -327,10 +339,12 @@ cmd_update()
>  			say "Maybe you want to use 'update --init'?"
>  			continue
>  		fi
> +		track=$(git config submodule."$name".track)

You don't need $track *here*, do you?

Show 5 quoted lines

>  		if ! test -d "$path"/.git -o -f "$path"/.git
>  		then
>  			module_clone "$path" "$url" || exit
> +
>  			subsha1=

And this extra blank line is an accident, isn't it?

> +				[ -z "$track" ] && track="HEAD"

Instead of this you can use a shell trick (but I don't know if it's portable):
				: "${track:=HEAD}"

And I think you can even spare the quotes.

> +				# if the local branch does not yet exist, create it
> +				( unset GIT_DIR; cd "$path"; git-show-ref --heads --tags -q "$track" || git branch --track "$track" "origin/$track" )

Ugh! A *branch* named "HEAD"?? I think you should reconsider this decision.

> +			if [ "$pull" = "1" ]

			if test "$pull"

> +			then
> +				# Now pull new updates from origin
> +				( unset GIT_DIR; cd "$path"; git-pull )

Wow! Creating new commits on the fly while doing a "git submodule update"! Error check is missing here.

Show 9 quoted lines

> @@ -596,7 +627,10 @@ cmd_status()
>  		set_name_rev "$path" "$sha1"
>  		if git diff-files --quiet -- "$path"
>  		then
> -			say " $sha1 $path$revname"
> +			track=$(git config submodule."$name".track)
> +			tracking=
> +			[ -n "$track" ] && tracking=" (tracking $track)"
> +			say " $sha1 $path$revname$tracking"

The last three lines can be shortened to this:

	say " $sha1 $path$revname${track:+ (tracking "$track")}"

-- Hannes