Re: [PATCH 06/10] Change incorrect "remote branch" to "remote tracking branch" in C code

Jonathan Nieder [off-list ref] writes:

Matthieu Moy wrote:

quoted

--- a/branch.h
+++ b/branch.h

@@ -22,7 +22,7 @@ void create_branch(const char *head, const char *name, const char *start_name,
 void remove_branch_state(void);

 /*
- * Configure local branch "local" to merge remote branch "remote"
+ * Configure local branch "local" to merge remote-tracking branch "remote"
  * taken from origin "origin".

	Configure local branch "local" as downstream to branch "remote"
	from remote "origin".  Used by git branch --set-upstream.

I buy this one.

quoted

--- a/builtin/fetch.c
+++ b/builtin/fetch.c

@@ -359,7 +359,7 @@ static int store_updated_refs(const char *raw_url, const char *remote_name,
 			what = rm->name + 10;
 		}
 		else if (!prefixcmp(rm->name, "refs/remotes/")) {
-			kind = "remote branch";
+			kind = "remote-tracking branch";
 			what = rm->name + 13;

I didn't follow before --- what concrete effect does this have?

Almost none ;-).

From reading the code and testing, it affects

$ git fetch
$ cat FETCH_HEAD
...
21d039[...]  not-for-merge  remote-tracking branch 'origin/branch' of /tmp/cloned
...

when ran in a repository obtained with "git clone --mirror" (i.e. when
you fetch remote-tracking branches from the remote repository).

and

$ git fetch origin refs/remotes/origin/some-branch
From /tmp/cloned
 * remote-tracking branch origin/oaue -> FETCH_HEAD

Are scripts affected?

I don't think any script would parse the last part of FETCH_HEAD,
which seems really to be meant as a comment, nor the precise output of
git-fetch-in-a-mirror-with-refspec-given-explicitely.

But anyway, that's in a place where bare mortals wouldn't go, so I
don't mind much is anyone wants to revert this particular hunk. That's
not going to affect the learning curve of newcommers.

Is fmt-merge-msg affected?

No (that's why it's a separate patch. In case you missed my earlier
message: yes, the testsuite passes between the two).

Maybe the commit message should explain the reasoning, something like:

 The non-deleted branch is somehow remote-tracking since "fetch" feeds
 it, and the code detecting this case is:

I've tried a shorter version :

  In the process, we change "non-remote branch" to "branch outside
  the refs/remotes/ hierarchy" to avoid the ugly "non-remote-tracking
  branch". The new formulation actually corresponds to how the code
  detects this case (i.e. prefixcmp(refname, "refs/remotes")).

--
Matthieu Moy
http://www-verimag.imag.fr/~moy/

Matthieu Moy wrote:

I don't think any script would parse the last part of FETCH_HEAD,
which seems really to be meant as a comment

Yes, believable.

Jonathan Nieder [off-list ref] writes:

quoted

Is fmt-merge-msg affected?

No (that's why it's a separate patch. In case you missed my earlier
message: yes, the testsuite passes between the two).

I suspect the testsuite passing is only because the fmt-merge-msg
tests don't try "git fetch . remotes/foo/bar".

In modern times, "git merge" bypasses fmt-merge-msg with its own
code (as you probably noticed :)).

  In the process, we change "non-remote branch" to "branch outside
  the refs/remotes/ hierarchy" to avoid the ugly "non-remote-tracking
  branch". The new formulation actually corresponds to how the code
  detects this case (i.e. prefixcmp(refname, "refs/remotes")).

Sounds good.  Thanks.