Re: How can git pull be up-to-date and git push fail?

Jeff King [off-list ref] writes:

On Thu, Apr 05, 2007 at 02:18:58PM -0700, Junio C Hamano wrote:

quoted

IIRC "git push" without explicit refspecs push the matching
refs, but I am a bit under the weather and feverish, so don't
take my word literally but look at git-push manual page please.

Ah, yes you're right. It really doesn't make sense to push
refs/remotes/* in most cases, since they're just tracking branches, and
if the destination _does_ have them, then it is unlikely to be in sync
with you (leading to Bill's problem).  OTOH, you might want to be able
to push them explicitly if you are doing a strict mirror of a
repository.

The patch below turns off refs/remotes/ sending for "git-push" and
"git-push --all", but still allows "git-push origin
remotes/origin/master". I'm not sure about the semantics; maybe --all
should imply even remotes?

The behaviour of git-send-pack that pushes all the matching refs
made sense when there was no separate-remotes layout (and there
was no "git push" wrapper).  Back then, the expected use was
really:

 * For integrators who own public repositories, send-pack to
   their public repositories without refspecs updated what they
   already decided to publish in refs/heads and refs/tags.

   Updating refs/tags hierarchy means moving tags which should
   have done with caution (since it changes the general public's
   view of what the version v2.6.12 really is), but no one can
   update tag without first forcing with "git tag -f" in one's
   own repository to begin with, it was not a problem in
   practice.

 * For people who used pull/push with a central repository as
   the CVS-style synchronization point, the central repository,
   being the mother of all participant repositories, was
   supposed to lack the 'origin' branch, and participant
   repositories had 'origin' that tracked 'master' of the
   central repository.  So matching push never needed to worry
   about 'origin', and 'master' of participant was pushed to
   'master' of the central repository, undergoing the usual
   fast-forward checks and all.

   Even here, we had a few trouble reports from people who
   primed their central repository by cloning from somewhere
   (and cloning without --bare) and left the 'origin' branch
   around.  We might have added instruction to our documentation
   that tells people to remove 'origin' branch from their
   central repositories to avoid confusion (but I am too lazy to
   look).

 * For anybody who has more than one private repositories
   (e.g. mothership and notebook) and uses push between them
   when switching repositories to work in, new development might
   have been on a new branch, in which case it needed an
   explicit refspec to send the new branch or tag.  But
   "matching refs" rule was useful for heads most of the time,
   as long as the two repositories were reasonably in sync
   (e.g. after supper you sit on your couch working on notebook,
   and you finish the day by pushing matching refs to your
   mothership to prepare for tomorrow, which you would start on
   your mothership machine).

In all these cases, what we meant by "matching refs" rule is to
update matching branch heads.  Even pushing matching tags is
debatable, as consciously replacing a tag that was already
pushed out with a new one is already a special case, and I would
suspect that replacing a public tag should be done via an
explicit "git send-pack $url tag v2.6.12", even though the
"matching refs" rule would silently update it.

People who built git-send-pack did not use separate remotes
layout (which has not been even invented yet), nor StGIT, so
refs/remotes, refs/bases nor refs/patches were not even on the
radar.  Speaking of StGIT, I do not think it makes sense to sync
only .git/refs/{bases,patches} without sending .git/patches.

Does this seem like a sane direction to take? It just seems silly to be
pushing refs/remotes, which 99% of the time should be a purely local
thing.

So I'd suspect "git push" (not limited to git-send-pack) without
explicit refspecs should only do "matching branch heads" these
days, to keep the original spirit but yet to adjust to today's
reality.  In other words, instead of treating remotes/ specially
(in a negative sense), like your patch does, I think we should
do only "matching refs in heads/" hierarchy.

On Fri, Apr 06, 2007 at 02:44:02PM -0700, Junio C Hamano wrote:

was no "git push" wrapper).  Back then, the expected use was
really:

Thanks for this for the historical information.

So I'd suspect "git push" (not limited to git-send-pack) without
explicit refspecs should only do "matching branch heads" these
days, to keep the original spirit but yet to adjust to today's
reality.  In other words, instead of treating remotes/ specially
(in a negative sense), like your patch does, I think we should
do only "matching refs in heads/" hierarchy.

Agreed. That was actually my initial feeling, but it meant changing the
semantics of all of the other parts of the refs/* hierarchy with respect
to publishing, which I was a bit nervous about.

A patch is attached below for comment; it's the same as my previous
patch, except:
  - matching only refs/heads/
  - rename local_ref to local_head for consistency (one of the functions
    was already get_local_heads, but the other's didn't match)
  - avoid calling strlen if we're just going to return immediately
  - add a test to simulate Bill's problem
  - update documentation for git-push

But it looks like I have broken pushing of existing tags using --tags,
so I think I might have to put the ref-culling in a different spot. Let
me look in that.

BTW, my gut feeling is that send-pack is plumbing, and git-push is
porcelain, and therefore the decisions about what to push should be made
at the push layer, and send-pack should only push what it is explicitly
told to push. But changing the defaults of send-pack at this point is
probably a bad idea.

---
 Documentation/git-push.txt      |    4 ++--
 Documentation/git-send-pack.txt |    4 ++--
 send-pack.c                     |   15 +++++++++------
 t/t5400-send-pack.sh            |   18 ++++++++++++++++++
 4 files changed, 31 insertions(+), 10 deletions(-)

diff --git a/Documentation/git-push.txt b/Documentation/git-push.txt
index f8cc2b5..0b21306 100644
--- a/Documentation/git-push.txt
+++ b/Documentation/git-push.txt

@@ -46,7 +46,7 @@ even if it does not result in a fast forward update.
 Note: If no explicit refspec is found, (that is neither
 on the command line nor in any Push line of the
 corresponding remotes file---see below), then all the
-refs that exist both on the local side and on the remote
+heads that exist both on the local side and on the remote
 side are updated.
 +
 `tag <tag>` means the same as `refs/tags/<tag>:refs/tags/<tag>`.

@@ -60,7 +60,7 @@ the remote repository.
 
 \--all::
 	Instead of naming each ref to push, specifies that all
-	refs be pushed.
+	refs under `$GIT_DIR/refs/heads/` be pushed.
 
 \--tags::
 	All refs under `$GIT_DIR/refs/tags` are pushed, in

diff --git a/Documentation/git-send-pack.txt b/Documentation/git-send-pack.txt
index 205bfd2..3271e88 100644
--- a/Documentation/git-send-pack.txt
+++ b/Documentation/git-send-pack.txt

@@ -32,7 +32,7 @@ OPTIONS
 
 \--all::
 	Instead of explicitly specifying which refs to update,
-	update all refs that locally exist.
+	update all heads that locally exist.
 
 \--force::
 	Usually, the command refuses to update a remote ref that

@@ -70,7 +70,7 @@ With '--all' flag, all refs that exist locally are transferred to
 the remote side.  You cannot specify any '<ref>' if you use
 this flag.
 
-Without '--all' and without any '<ref>', the refs that exist
+Without '--all' and without any '<ref>', the heads that exist
 both on the local side and on the remote side are updated.
 
 When one or more '<ref>' are specified explicitly, it can be either a

diff --git a/send-pack.c b/send-pack.c
index d5b5162..b2b6886 100644
--- a/send-pack.c
+++ b/send-pack.c

@@ -124,13 +124,16 @@ static int ref_newer(const unsigned char *new_sha1,
 	return found;
 }
 
-static struct ref *local_refs, **local_tail;
+static struct ref *local_heads, **local_tail;
 static struct ref *remote_refs, **remote_tail;
 
-static int one_local_ref(const char *refname, const unsigned char *sha1, int flag, void *cb_data)
+static int one_local_head(const char *refname, const unsigned char *sha1, int flag, void *cb_data)
 {
 	struct ref *ref;
-	int len = strlen(refname) + 1;
+	int len;
+	if (prefixcmp(refname, "refs/heads/"))
+		return 0;
+	len = strlen(refname) + 1;
 	ref = xcalloc(1, sizeof(*ref) + len);
 	hashcpy(ref->new_sha1, sha1);
 	memcpy(ref->name, refname, len);

@@ -141,8 +144,8 @@ static int one_local_ref(const char *refname, const unsigned char *sha1, int fla
 
 static void get_local_heads(void)
 {
-	local_tail = &local_refs;
-	for_each_ref(one_local_ref, NULL);
+	local_tail = &local_heads;
+	for_each_ref(one_local_head, NULL);
 }
 
 static int receive_status(int in)

@@ -198,7 +201,7 @@ static int send_pack(int in, int out, int nr_refspec, char **refspec)
 	/* match them up */
 	if (!remote_tail)
 		remote_tail = &remote_refs;
-	if (match_refs(local_refs, remote_refs, &remote_tail,
+	if (match_refs(local_heads, remote_refs, &remote_tail,
 		       nr_refspec, refspec, send_all))
 		return -1;
 

diff --git a/t/t5400-send-pack.sh b/t/t5400-send-pack.sh
index 477b267..ca4485a 100755
--- a/t/t5400-send-pack.sh
+++ b/t/t5400-send-pack.sh

@@ -113,4 +113,22 @@ test_expect_success \
 	! git diff .git/refs/heads/master victim/.git/refs/heads/master
 '
 
+test_expect_success \
+	'pushing does not include non-head refs' '
+	mkdir parent && cd parent &&
+	  git-init && touch file && git-add file && git-commit -m add &&
+	cd .. &&
+	git-clone parent child1 &&
+	git-clone parent child2 &&
+	cd parent &&
+	  echo parent >file && git-commit -a -m change &&
+	cd .. &&
+	cd child1 && git-pull && cd .. &&
+	cd child2 &&
+	  touch >file2 git-add file2 && git-add file2 && git-commit -m add2 &&
+	  git-pull ../child1 &&
+	  git-push ../child1 2>err &&
+	  ! grep ^error err
+'
+
 test_done