Re: [PATCH] Teach remote machinery about remotes.default config variable

Mark Levedahl [off-list ref] writes:

Basically, I think an important (but not complete) test of the design
is that

   git clone -o frotz git://frotz.foo.bar/myproject.git
   cd myproject
   git submodule init
   git submodule update

work, with origin = frotz throughout the submodules, and with the
whole project correctly checked out even if the entire project was
rehosted onto a different server.

I like that.  This is a very good argument, especially because
it clarifies very well that the issue is not about "'submodule
init' misbehaves" but "fetch/pull/merge does not play well with
clone -o".

The only remaining (minor) doubt I have (not in the sense that
"I object to it!", but in the sense that "I wish there could be
a better alternative, but I do not think of one offhand") is
polluting the core.* namespace with this configuration variable.

Looking at Documentation/config.txt, I realize that we already
have made a mistake of allowing core.gitproxy, but other than
that single mistake, everything in core.* is still about things
that apply to the use of git even when the repository does not
talk with any other repository.  If we deprecate and rename away
that one mistake, we can again make core.* to mean things that
are _really_ core, but using core.origin for "the default remote
is not called 'origin' but 'frotz' here" is a step backwards
from that ideal.

But that's a minor naming issue.

Junio C Hamano wrote:

Mark Levedahl [off-list ref] writes:

  

quoted

Basically, I think an important (but not complete) test of the design
is that

   git clone -o frotz git://frotz.foo.bar/myproject.git
   cd myproject
   git submodule init
   git submodule update

work, with origin = frotz throughout the submodules, and with the
whole project correctly checked out even if the entire project was
rehosted onto a different server.
    

I like that.  This is a very good argument, especially because
it clarifies very well that the issue is not about "'submodule
init' misbehaves" but "fetch/pull/merge does not play well with
clone -o"

Carrying the above forward...  Assume I have a checked out project as 
above, then in top-level master project I do:

git remote add zoo git://zoo.tar.fu hisfork.git
git fetch zoo
git checkout --track -b fork zoo/fork
git submodule update

I claim the submodule machinery *should* now follow master's default 
remote, which is "zoo", for the current branch. In addition, the 
submodule machinery should define remote "zoo" in each submodule  where 
it does not already exist, using the same logic using in the original 
init/update phase. This should only apply to modules defined using 
relative urls.

Basically, this formalizes the notion that:

* submodules defined using relative urls are "owned" by the master 
project and will exist anywhere the master does.
* submodules defined using absolute urls are incorporated into the 
project but are separately managed. (While some improved mechanism to 
automate their management from top-level may be proposed, it is not 
obvious to me nor addressed here.)

The subsequent patch modifies git-submodule to implement this logic, and 
applies on top of my previous series.

(Note: I cannot find my latest series in the git-archives on gmane nor 
on marc.info, both have only part, and I am suspicious that something 
went wrong in my sending via gmail, so I am resending the series here, 
now five patches long. Please excuse if this is redundant).

Mark

This introduces a new configuration variable, core.origin, that
defines the name of the default remote to be used. Traditionally, this
is "origin", and could be overridden for a given branch. This change
introduces a way to redefine the default as desired and have that honored
regardless of the currently checked out head (e.g., core.origin is
used when on a detached head or any other non-tracking branch).

Signed-off-by: Mark Levedahl <redacted>
---
 Documentation/config.txt |    6 ++++++
 git-parse-remote.sh      |    5 +++--
 remote.c                 |   11 ++++++++++-
 3 files changed, 19 insertions(+), 3 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index df091d1..b7241cf 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt

@@ -291,6 +291,12 @@ core.editor::
 	`GIT_EDITOR` environment, `core.editor`, `VISUAL` and
 	`EDITOR` environment variables and then finally `vi`.
 
+core.origin::
+	The name of the remote used by default for fetch / pull. If unset,
+	origin is assumed. This value is used whenever the current branch
+	has no corresponding branch.<name>.remote, such as when working on
+	a detached head.
+
 core.pager::
 	The command that git will use to paginate output.  Can be overridden
 	with the `GIT_PAGER` environment variable.

diff --git a/git-parse-remote.sh b/git-parse-remote.sh
index 695a409..c7ac7c7 100755
--- a/git-parse-remote.sh
+++ b/git-parse-remote.sh

@@ -56,8 +56,9 @@ get_remote_url () {
 
 get_default_remote () {
 	curr_branch=$(git symbolic-ref -q HEAD | sed -e 's|^refs/heads/||')
-	origin=$(git config --get "branch.$curr_branch.remote")
-	echo ${origin:-origin}
+	git config --get "branch.$curr_branch.remote" ||
+	git config --get "core.origin" ||
+	echo origin
 }
 
 get_remote_default_refs_for_push () {

diff --git a/remote.c b/remote.c
index 0e00680..302d499 100644
--- a/remote.c
+++ b/remote.c

@@ -10,6 +10,7 @@ static int allocated_branches;
 
 static struct branch *current_branch;
 static const char *default_remote_name;
+static const char *core_origin;
 
 #define BUF_SIZE (2048)
 static char buffer[BUF_SIZE];

@@ -233,6 +234,11 @@ static int handle_config(const char *key, const char *value)
 			add_merge(branch, xstrdup(value));
 		return 0;
 	}
+	if (!strcmp(key, "core.origin")) {
+		if (value)
+			core_origin = xstrdup(value);
+		return 0;
+	}
 	if (prefixcmp(key,  "remote."))
 		return 0;
 	name = key + 7;

@@ -291,7 +297,6 @@ static void read_config(void)
 	int flag;
 	if (default_remote_name) // did this already
 		return;
-	default_remote_name = xstrdup("origin");
 	current_branch = NULL;
 	head_ref = resolve_ref("HEAD", sha1, 0, &flag);
 	if (head_ref && (flag & REF_ISSYMREF) &&

@@ -300,6 +305,10 @@ static void read_config(void)
 			make_branch(head_ref + strlen("refs/heads/"), 0);
 	}
 	git_config(handle_config);
+	if (!default_remote_name) {
+		default_remote_name = core_origin ?
+		core_origin : xstrdup("origin");
+	}
 }
 
 struct refspec *parse_ref_spec(int nr_refspec, const char **refspec)

-- 
1.5.4.rc3.14.gc50f

This records the users choice of default remote name (by default "origin")
as given by the -o option.

Signed-off-by: Mark Levedahl <redacted>
---
 Documentation/git-clone.txt |    3 ++-
 git-clone.sh                |    1 +
 2 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/Documentation/git-clone.txt b/Documentation/git-clone.txt
index fdccbd4..6c15fa4 100644
--- a/Documentation/git-clone.txt
+++ b/Documentation/git-clone.txt

@@ -95,7 +95,8 @@ OPTIONS
 --origin <name>::
 -o <name>::
 	Instead of using the remote name 'origin' to keep track
-	of the upstream repository, use <name> instead.
+	of the upstream repository, use <name> instead. The name
+	is recorded in the core.origin config variable.
 
 --upload-pack <upload-pack>::
 -u <upload-pack>::

diff --git a/git-clone.sh b/git-clone.sh
index b4e858c..7208d68 100755
--- a/git-clone.sh
+++ b/git-clone.sh

@@ -242,6 +242,7 @@ fi &&
 export GIT_DIR &&
 GIT_CONFIG="$GIT_DIR/config" git-init $quiet ${template+"$template"} || usage
 
+git config core.origin $origin
 if test -n "$bare"
 then
 	GIT_CONFIG="$GIT_DIR/config" git config core.bare true

-- 
1.5.4.rc3.14.gc50f

Signed-off-by: Mark Levedahl <redacted>
---
 git-remote.perl |    5 +++++
 1 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/git-remote.perl b/git-remote.perl
index d13e4c1..75d2371 100755
--- a/git-remote.perl
+++ b/git-remote.perl

@@ -328,6 +328,11 @@ sub rm_remote {
 
 	$git->command('config', '--remove-section', "remote.$name");
 
+	my $defremote = $git->config("core.origin");
+	if (defined $defremote && $defremote eq $name) {
+	       $git->command("config", "--unset", "core.origin");
+	}
+
 	eval {
 	    my @trackers = $git->command('config', '--get-regexp',
 			'branch.*.remote', $name);

-- 
1.5.4.rc3.14.gc50f

Modules that are defined using relative urls to the master project are
assumed to be completely owned by the project. When running
"submodule update" from the top level, it is reasonable that the entire
project exists at the current master's remote. Using the
branch.$name.remote machinery, this remote can be different for each
branch and can be different than the current defaults in each submodule.

This teaches submodule to:

1) Possibly define the current master's remote in each submodule, using
the same relative url used by submodule init.
2) Fetch each submodule's updates from the master's remote.

Submodules defined using absolute urls (not relative to the parent) are
not touched by this logic. These modules are assumed to be independent
of the master project so submodule can do no better than to fetch from
their currently defined default remotes as already done.


Signed-off-by: Mark Levedahl <redacted>
---
 git-submodule.sh |   20 +++++++++++++++++++-
 1 files changed, 19 insertions(+), 1 deletions(-)

diff --git a/git-submodule.sh b/git-submodule.sh
index 42be4b9..5b4b16f 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh

@@ -209,11 +209,14 @@ modules_init()
 
 #
 # Update each submodule path to correct revision, using clone and checkout as needed
+# For owned submodules (defined using relative url), we use master project's remote
+# and define that in each submodule if not already there
 #
 # $@ = requested paths (default to all)
 #
 modules_update()
 {
+	master_remote=$(get_default_remote)
 	git ls-files --stage -- "$@" | grep -e '^160000 ' |
 	while read mode sha1 stage path
 	do

@@ -240,9 +243,24 @@ modules_update()
 			die "Unable to find current revision in submodule path '$path'"
 		fi
 
+		baseurl="$(GIT_CONFIG=.gitmodules git config submodule."$name".url)"
+		case "$baseurl" in
+		./*|../*)
+			fetch_remote=$master_remote
+			(unset GIT_DIR ; cd "$path" && git config remote."$fetch_remote".url > nul) ||
+			(
+				absurl="$(resolve_relative_url $baseurl)"
+				unset GIT_DIR; cd "$path" && git remote add "$master_remote" "$absurl"
+			) || die "Unable to define remote '$fetch_remote' in submodule path '$path'"
+			;;
+		*)
+			fetch_remote=
+			;;
+		esac
+
 		if test "$subsha1" != "$sha1"
 		then
-			(unset GIT_DIR; cd "$path" && git-fetch &&
+			(unset GIT_DIR; cd "$path" && git-fetch "$fetch_remote" &&
 				git-checkout -q "$sha1") ||
 			die "Unable to checkout '$sha1' in submodule path '$path'"
 

-- 
1.5.4.rc3.14.gc50f

For submodules defined relative to their parent, it is likely that the
parent's defined default remote is correct for the child as well. This
allows use of remote names other than "origin", important as managed
submodules are typically checked out on a detached head and therefore
submodule-update invokes git-fetch using the default remote. Without this
change, submodules effectively had to have a default remote of "origin."

Signed-off-by: Mark Levedahl <redacted>
---
 Documentation/git-submodule.txt |    8 +++++---
 git-submodule.sh                |   19 +++++++++++++------
 2 files changed, 18 insertions(+), 9 deletions(-)

diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt
index cffc6d4..440e234 100644
--- a/Documentation/git-submodule.txt
+++ b/Documentation/git-submodule.txt

@@ -36,9 +36,11 @@ status::
 
 init::
 	Initialize the submodules, i.e. register in .git/config each submodule
-	name and url found in .gitmodules. The key used in .git/config is
-	`submodule.$name.url`. This command does not alter existing information
-	in .git/config.
+	name and url found in .gitmodules, along with the default remote origin.
+	For submodules using a relative url, the default remote is inherited
+	from the parent project, for absolute urls the default "origin" is used.
+	The key used in .git/config is submodule.$name.url`. This command does
+	not alter existing information in .git/config.
 
 update::
 	Update the registered submodules, i.e. clone missing submodules and

diff --git a/git-submodule.sh b/git-submodule.sh
index ad9fe62..42be4b9 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh

@@ -7,6 +7,7 @@
 USAGE='[--quiet] [--cached] [add <repo> [-b branch]|status|init|update] [--] [<path>...]'
 OPTIONS_SPEC=
 . git-sh-setup
+. git-parse-remote
 require_work_tree
 
 add=

@@ -43,9 +44,7 @@ get_repo_base() {
 # Resolve relative url by appending to parent's url
 resolve_relative_url ()
 {
-	branch="$(git symbolic-ref HEAD 2>/dev/null)"
-	remote="$(git config branch.${branch#refs/heads/}.remote)"
-	remote="${remote:-origin}"
+	remote="$(get_default_remote)"
 	remoteurl="$(git config remote.$remote.url)" ||
 		die "remote ($remote) does not have a url in .git/config"
 	url="$1"

@@ -95,6 +94,7 @@ module_clone()
 {
 	path=$1
 	url=$2
+	origin=${3:-origin}
 
 	# If there already is a directory at the submodule path,
 	# expect it to be empty (since that is the default checkout

@@ -110,7 +110,7 @@ module_clone()
 	test -e "$path" &&
 	die "A file already exist at path '$path'"
 
-	git-clone -n "$url" "$path" ||
+	git-clone -n -o "$origin" "$url" "$path" ||
 	die "Clone of '$url' into submodule path '$path' failed"
 }
 

@@ -130,9 +130,11 @@ module_add()
 		usage
 	fi
 
+	origin=origin
 	case "$repo" in
 	./*|../*)
 		# dereference source url relative to parent's url
+		origin=$(get_default_remote)
 		realrepo="$(resolve_relative_url $repo)" ;;
 	*)
 		# Turn the source into an absolute path if

@@ -157,7 +159,7 @@ module_add()
 	git ls-files --error-unmatch "$path" > /dev/null 2>&1 &&
 	die "'$path' already exists in the index"
 
-	module_clone "$path" "$realrepo" || exit
+	module_clone "$path" "$realrepo" "$origin" || exit
 	(unset GIT_DIR; cd "$path" && git checkout -q ${branch:+-b "$branch" "origin/$branch"}) ||
 	die "Unable to checkout submodule '$path'"
 	git add "$path" ||

@@ -189,12 +191,15 @@ modules_init()
 		die "No url found for submodule path '$path' in .gitmodules"
 
 		# Possibly a url relative to parent
+		origin=origin
 		case "$url" in
 		./*|../*)
 			url="$(resolve_relative_url "$url")"
+			origin=$(get_default_remote)
 			;;
 		esac
 
+		git config submodule."$name".origin "$origin" &&
 		git config submodule."$name".url "$url" ||
 		die "Failed to register url for submodule path '$path'"
 

@@ -222,10 +227,12 @@ modules_update()
 			say "Submodule path '$path' not initialized"
 			continue
 		fi
+		origin=$(git config submodule."$name".origin)
+		origin=${origin:-origin}
 
 		if ! test -d "$path"/.git
 		then
-			module_clone "$path" "$url" || exit
+			module_clone "$path" "$url" "$origin" || exit
 			subsha1=
 		else
 			subsha1=$(unset GIT_DIR; cd "$path" &&

-- 
1.5.4.rc3.14.gc50f

Hi,

On Sat, 12 Jan 2008, Junio C Hamano wrote:

Mark Levedahl [off-list ref] writes:

quoted

Basically, I think an important (but not complete) test of the design
is that

   git clone -o frotz git://frotz.foo.bar/myproject.git
   cd myproject
   git submodule init
   git submodule update

work, with origin = frotz throughout the submodules, and with the
whole project correctly checked out even if the entire project was
rehosted onto a different server.

I like that.  This is a very good argument, especially because it 
clarifies very well that the issue is not about "'submodule init' 
misbehaves" but "fetch/pull/merge does not play well with clone -o".

FWIW I disagree.

I never understood why people want to complicate things by being able to 
name default _keys_ differently.  Why not letting "origin" being the 
default being pulled from, and be done with it?

Besides, I _really_ do not understand why we have such a discussion in rc 
phase.  There are _many_ more interesting discussions now that _also_ do 
not belong into a freeze phase.

Ciao,
Dscho

On Sun, Jan 13, 2008 at 11:27:08AM -0500, Mark Levedahl wrote:

I'm not sure I see the use case that this helps. Presumably you are
doing one of (assuming your core.origin is 'foo'):

  - delete 'foo', and then proceed with usual git commands. In this
    case, your core.origin has reverted to 'origin', but what is the
    chance that you actually have such a remote (since you presumably
    cloned with -o foo)?

  - delete 'foo', then re-add 'foo'. I would expect this to be
    equivalent to editing the config, but as a side effect, your
    core.origin has mysteriously changed.

  - delete 'foo', then re-add 'bar' with the intent of making it your
    new origin. This doesn't help at all, since there's nothing
    automatically setting core.origin to 'bar', so you might as well
    leave it as the bogus 'foo' rather than the bogus 'origin'. And to
    help this use case, something like a "-d" flag to git-remote to set
    the new origin as the default might make sense. I.e.,

      git remote rm foo
      git remote add -d bar git://bar/project.git

    Alternatively, when adding a remote, if it is the _only_ remote (or
    perhaps if the current core.origin doesn't exist), we could set
    core.origin which would automagically cover the latter two cases.
    Although it feels a little too DWIM.

-Peff

Jeff King wrote:

On Sun, Jan 13, 2008 at 11:27:08AM -0500, Mark Levedahl wrote:

  

quoted

--- a/git-remote.perl
+++ b/git-remote.perl

@@ -328,6 +328,11 @@ sub rm_remote {
 
 	$git->command('config', '--remove-section', "remote.$name");
 
+	my $defremote = $git->config("core.origin");
+	if (defined $defremote && $defremote eq $name) {
+	       $git->command("config", "--unset", "core.origin");
+	}
+
    

I'm not sure I see the use case that this helps.
  

Just being thorough: the man page claims that "git remote rm foo" 
removes all mention of remote foo.

    Alternatively, when adding a remote, if it is the _only_ remote (or
    perhaps if the current core.origin doesn't exist), we could set
    core.origin which would automagically cover the latter two cases.
    Although it feels a little too DWIM.

-Peff
  

I suspect anything done in this case is going to suffer from DWIM-itis 
in some conditions. I can't offer a better argument than the one above.

Mark

On Tue, Jan 15, 2008 at 12:02:45AM -0500, Mark Levedahl wrote:

quoted

I'm not sure I see the use case that this helps.
  

Just being thorough: the man page claims that "git remote rm foo" removes 
all mention of remote foo.

I was going to respond "by that rationale, 'git remote rm' should be
removing branch.*.remote keys that point to the removed remote". But
looking at the code, it already does that. So your change actually keeps
things consistent.

Not the choice I would have made, but I guess it just goes to show that
I use "vi" instead of "git remote". Consider my objection withdrawn.

-Peff