threads / discuss / 10917

! [rejected] master -> master (non-fast forward)

Subject: ! [rejected] master -> master (non-fast forward)

## tl;dr

19 messages between Nov 18, 2007 and Nov 21, 2007.

replies: 18people: 6as markdown or json

Jon Smirl· Nov 18, 2007, 15:12 UTC · lore

What's causing this? I'm using stgit on the master branch. I'm fixing it each time on the remote server by deleting the ref to master.

jonsmirl@terra:~/ds$ git push digispeaker
To ssh://jonsmirl1@git.digispeaker.com/~/projects/digispeaker-kernel.git
 ! [rejected]        master -> master (non-fast forward)
error: failed to push to
'ssh://jonsmirl1@git.digispeaker.com/~/projects/digispeaker-kernel.git'
jonsmirl@terra:~/ds$
On the server I have:
[core]
        repositoryformatversion = 0
        filemode = true
        bare = true
        logallrefupdates = true

bare was set false, I just flipped it to true. The server repo was originally created via a clone from kernel.org and then renamed to be a bare repo. Why do we need a 'bare' attribute?

-- 
Jon Smirl
jonsmirl@gmail.com
Junio C Hamano· Nov 18, 2007, 18:10 UTC · re: Jon Smirl · lore

Re: ! [rejected] master -> master (non-fast forward)

"Jon Smirl" <jonsmirl@gmail.com> writes:
Show 9 quoted lines
> What's causing this? I'm using stgit on the master branch.
> I'm fixing it each time on the remote server by deleting the ref to master.
>
> jonsmirl@terra:~/ds$ git push digispeaker
> To ssh://jonsmirl1@git.digispeaker.com/~/projects/digispeaker-kernel.git
>  ! [rejected]        master -> master (non-fast forward)
> error: failed to push to
> 'ssh://jonsmirl1@git.digispeaker.com/~/projects/digispeaker-kernel.git'
> jonsmirl@terra:~/ds$
A "non-fast forward" means that you had this history earlier:
         o---o---A
        /
    ---o
pushed "A" to the remote's 'master', then built this history:
         o---o---A
        /
    ---o---o---o---o---A'

by rewinding and rebuilding, and the tip of the branch now points at A', which you tried to push to the remote.

Which often causes troubles for people who are fetching from the branch you are pushing into, and forbidden by default as a safety measure.

As long as the people who fetch from the branch knows that you will regularly rewinding the tip of the branch, there is no confusion, and you can "force" a non-fast forward update. There are two independent safety mechanisms:

 - the sending end safety can be overriden by "git push --force"
   and/or by using a refspec prefixed with a '+');
 - the receiving end safety can be overriden by the config
   variable receive.denynonfastworwards of the repository you
   are pushing into.

The latter defaults to "unsafe", but if the safety is activated in the repository, forcing from the sending side will not deactivate it. IOW, both ends need to agree to allow the unsafe behaviour.

Show 10 quoted lines
> On the server I have:
> [core]
>         repositoryformatversion = 0
>         filemode = true
>         bare = true
>         logallrefupdates = true
>
> bare was set false, I just flipped it to true. The server repo was
> originally created via a clone from kernel.org and then renamed to be
> a bare repo. Why do we need a 'bare' attribute?

There are a few things that change behaviour depending on the bareness of the repository. One example is that "git fetch" that updates to the current branch (i.e. what HEAD points at) by having it as the RHS of a refspec is accepted only for bare repositories (for non-bare repositories such a fetch would make the HEAD and the work tree go out-of-sync).

Jon Smirl· Nov 18, 2007, 18:42 UTC · re: Junio C Hamano · lore

Re: ! [rejected] master -> master (non-fast forward)

On 11/18/07, Junio C Hamano <gitster@pobox.com> wrote:
Show 26 quoted lines
> "Jon Smirl" <jonsmirl@gmail.com> writes:
>
> > What's causing this? I'm using stgit on the master branch.
> > I'm fixing it each time on the remote server by deleting the ref to master.
> >
> > jonsmirl@terra:~/ds$ git push digispeaker
> > To ssh://jonsmirl1@git.digispeaker.com/~/projects/digispeaker-kernel.git
> >  ! [rejected]        master -> master (non-fast forward)
> > error: failed to push to
> > 'ssh://jonsmirl1@git.digispeaker.com/~/projects/digispeaker-kernel.git'
> > jonsmirl@terra:~/ds$
>
> A "non-fast forward" means that you had this history earlier:
>
>          o---o---A
>         /
>     ---o
>
> pushed "A" to the remote's 'master', then built this history:
>
>          o---o---A
>         /
>     ---o---o---o---o---A'
>
> by rewinding and rebuilding, and the tip of the branch now
> points at A', which you tried to push to the remote.

stgit must be doing this when I rebase. It pops all of it's patches, moves to head of linus/master and then rebases them.

I'm the only person with write access to the repo.
> Which often causes troubles for people who are fetching from the
> branch you are pushing into, and forbidden by default as a
> safety measure.

What is the right way to share repositories using stgit? I have a set of patches which I am working on for kernel inclusion. I have them applied as a stgit stack on top of linus/master. I need to share this patch stack with other developers. These developers may want to change one of my patches. Right now they are emailing me deltas and I apply them to the appropriate stgit patch. I have seventeen patches in my stack currently.

I am starting to see why several people have made comments about integrating stgit into git. When other developers clone my repo the stgit setup doesn't come with it. It would be great it we could use git to push the stgit patches around and work on them.

Show 35 quoted lines
> As long as the people who fetch from the branch knows that you
> will regularly rewinding the tip of the branch, there is no
> confusion, and you can "force" a non-fast forward update.  There
> are two independent safety mechanisms:
>
>  - the sending end safety can be overriden by "git push --force"
>    and/or by using a refspec prefixed with a '+');
>
>  - the receiving end safety can be overriden by the config
>    variable receive.denynonfastworwards of the repository you
>    are pushing into.
>
> The latter defaults to "unsafe", but if the safety is activated
> in the repository, forcing from the sending side will not
> deactivate it.  IOW, both ends need to agree to allow the unsafe
> behaviour.
>
> > On the server I have:
> > [core]
> >         repositoryformatversion = 0
> >         filemode = true
> >         bare = true
> >         logallrefupdates = true
> >
> > bare was set false, I just flipped it to true. The server repo was
> > originally created via a clone from kernel.org and then renamed to be
> > a bare repo. Why do we need a 'bare' attribute?
>
> There are a few things that change behaviour depending on the
> bareness of the repository.  One example is that "git fetch"
> that updates to the current branch (i.e. what HEAD points at) by
> having it as the RHS of a refspec is accepted only for bare
> repositories (for non-bare repositories such a fetch would make
> the HEAD and the work tree go out-of-sync).
>
-- 
Jon Smirl
jonsmirl@gmail.com
Catalin Marinas· Nov 19, 2007, 17:47 UTC · re: Jon Smirl · lore

Re: ! [rejected] master -> master (non-fast forward)

"Jon Smirl" <jonsmirl@gmail.com> wrote:
> On 11/18/07, Junio C Hamano <gitster@pobox.com> wrote:
>> "Jon Smirl" <jonsmirl@gmail.com> writes:
>>
>> > What's causing this? I'm using stgit on the master branch.
[...]
Show 11 quoted lines
>> pushed "A" to the remote's 'master', then built this history:
>>
>>          o---o---A
>>         /
>>     ---o---o---o---o---A'
>>
>> by rewinding and rebuilding, and the tip of the branch now
>> points at A', which you tried to push to the remote.
>
> stgit must be doing this when I rebase. It pops all of it's patches,
> moves to head of linus/master and then rebases them.
[...]
Show 7 quoted lines
> What is the right way to share repositories using stgit? I have a set
> of patches which I am working on for kernel inclusion. I have them
> applied as a stgit stack on top of linus/master. I need to share this
> patch stack with other developers. These developers may want to change
> one of my patches. Right now they are emailing me deltas and I apply
> them to the appropriate stgit patch. I have seventeen patches in my
> stack currently.

StGIT is meant for keeping a clean history but with the big disadvantage that this history is volatile.

A solution is for the other developers to use StGIT or just git-rebase so that they always have the same base (volatile) history and keep their patches on top of yours.

A 2nd approach is to use topic branches rather than StGIT patches but you might lose some flexibility.

Yet another approach (which I used) is to keep a public branch (can be maintained by StGIT) where the history doesn't change and a devel volatile branch. When I modify some patches and they are ready for publishing, switch to the public branch and cherry-pick them (stg pick) from the devel branch. Because this is done with a three-way merge in StGIT, you will only get the new changes rather than the full patch. You need to change the patch message (as it is that of the original patch) to describe the specific changes and run 'stg refresh && stg commit' to store it into the immutable history (well, there is an 'uncommit' command as well if something goes wrong).

-- 
Catalin
Jon Smirl· Nov 19, 2007, 22:54 UTC · re: Catalin Marinas · lore

Re: ! [rejected] master -> master (non-fast forward)

On 11/19/07, Catalin Marinas <catalin.marinas@arm.com> wrote:
Show 46 quoted lines
> "Jon Smirl" <jonsmirl@gmail.com> wrote:
> > On 11/18/07, Junio C Hamano <gitster@pobox.com> wrote:
> >> "Jon Smirl" <jonsmirl@gmail.com> writes:
> >>
> >> > What's causing this? I'm using stgit on the master branch.
> [...]
> >> pushed "A" to the remote's 'master', then built this history:
> >>
> >>          o---o---A
> >>         /
> >>     ---o---o---o---o---A'
> >>
> >> by rewinding and rebuilding, and the tip of the branch now
> >> points at A', which you tried to push to the remote.
> >
> > stgit must be doing this when I rebase. It pops all of it's patches,
> > moves to head of linus/master and then rebases them.
> [...]
> > What is the right way to share repositories using stgit? I have a set
> > of patches which I am working on for kernel inclusion. I have them
> > applied as a stgit stack on top of linus/master. I need to share this
> > patch stack with other developers. These developers may want to change
> > one of my patches. Right now they are emailing me deltas and I apply
> > them to the appropriate stgit patch. I have seventeen patches in my
> > stack currently.
>
> StGIT is meant for keeping a clean history but with the big
> disadvantage that this history is volatile.
>
> A solution is for the other developers to use StGIT or just git-rebase
> so that they always have the same base (volatile) history and keep
> their patches on top of yours.
>
> A 2nd approach is to use topic branches rather than StGIT patches but
> you might lose some flexibility.
>
> Yet another approach (which I used) is to keep a public branch (can be
> maintained by StGIT) where the history doesn't change and a devel
> volatile branch. When I modify some patches and they are ready for
> publishing, switch to the public branch and cherry-pick them (stg
> pick) from the devel branch. Because this is done with a three-way
> merge in StGIT, you will only get the new changes rather than the full
> patch. You need to change the patch message (as it is that of the
> original patch) to describe the specific changes and run 'stg refresh
> && stg commit' to store it into the immutable history (well, there is
> an 'uncommit' command as well if something goes wrong).

Is a StGit where we can transparently share patches under version control in the works?

Something like this: clone repo with stgit normal stg commands work with no setup change a patch stg push the repo

I stg pull and the patch is updated. I also get rebased forward if the base repo has been updated

-- 
Jon Smirl
jonsmirl@gmail.com
Daniel Barkalow· Nov 20, 2007, 00:03 UTC · re: Jon Smirl · lore

Re: ! [rejected] master -> master (non-fast forward)

On Mon, 19 Nov 2007, Jon Smirl wrote:
Show 60 quoted lines
> On 11/19/07, Catalin Marinas <catalin.marinas@arm.com> wrote:
> > "Jon Smirl" <jonsmirl@gmail.com> wrote:
> > > On 11/18/07, Junio C Hamano <gitster@pobox.com> wrote:
> > >> "Jon Smirl" <jonsmirl@gmail.com> writes:
> > >>
> > >> > What's causing this? I'm using stgit on the master branch.
> > [...]
> > >> pushed "A" to the remote's 'master', then built this history:
> > >>
> > >>          o---o---A
> > >>         /
> > >>     ---o---o---o---o---A'
> > >>
> > >> by rewinding and rebuilding, and the tip of the branch now
> > >> points at A', which you tried to push to the remote.
> > >
> > > stgit must be doing this when I rebase. It pops all of it's patches,
> > > moves to head of linus/master and then rebases them.
> > [...]
> > > What is the right way to share repositories using stgit? I have a set
> > > of patches which I am working on for kernel inclusion. I have them
> > > applied as a stgit stack on top of linus/master. I need to share this
> > > patch stack with other developers. These developers may want to change
> > > one of my patches. Right now they are emailing me deltas and I apply
> > > them to the appropriate stgit patch. I have seventeen patches in my
> > > stack currently.
> >
> > StGIT is meant for keeping a clean history but with the big
> > disadvantage that this history is volatile.
> >
> > A solution is for the other developers to use StGIT or just git-rebase
> > so that they always have the same base (volatile) history and keep
> > their patches on top of yours.
> >
> > A 2nd approach is to use topic branches rather than StGIT patches but
> > you might lose some flexibility.
> >
> > Yet another approach (which I used) is to keep a public branch (can be
> > maintained by StGIT) where the history doesn't change and a devel
> > volatile branch. When I modify some patches and they are ready for
> > publishing, switch to the public branch and cherry-pick them (stg
> > pick) from the devel branch. Because this is done with a three-way
> > merge in StGIT, you will only get the new changes rather than the full
> > patch. You need to change the patch message (as it is that of the
> > original patch) to describe the specific changes and run 'stg refresh
> > && stg commit' to store it into the immutable history (well, there is
> > an 'uncommit' command as well if something goes wrong).
> 
> 
> Is a StGit where we can transparently share patches under version
> control in the works?
> 
> Something like this:
> clone repo with stgit
> normal stg commands work with no setup
> change a patch
> stg push the repo
> 
> I stg pull and the patch is updated.
> I also get rebased forward if the base repo has been updated

I've been thinking vaguely about support for essentially version controlling a quilt series, with the fundamental idea being that the history of the branch you're working on is a sequence of states of the series, with magic for having both the series specification and the final state in each commit.

Note that this is a different concept from stgit (and, I think, guilt), which uses the git engine as the magic behind the quilt-like operation, meaning that the history of the commit at the end of the series goes back through the patches in the series, not back through the changes to the series.

I've got a bunch of ideas on the subject, but I don't really have the quilt experience to know how to make this useful to people who want to do this kind of thing. My dream, from the perspective of a user of the results of somebody else's use of this feature, would be being able to bisect -mm to determine first that -mm stopped working when Andrew updated a particular tree, and then bisect within that tree (in each case generating the test tree with the complete -mm series, but with that tree's patch series being from the test point).

	-Daniel
*This .sig left intentionally blank*
Alex Riesen· Nov 18, 2007, 18:29 UTC · re: Jon Smirl · lore

Re: ! [rejected] master -> master (non-fast forward)

Jon Smirl, Sun, Nov 18, 2007 16:12:10 +0100:
Show 7 quoted lines
> What's causing this? I'm using stgit on the master branch.
> I'm fixing it each time on the remote server by deleting the ref to master.
> 
> jonsmirl@terra:~/ds$ git push digispeaker
> To ssh://jonsmirl1@git.digispeaker.com/~/projects/digispeaker-kernel.git
>  ! [rejected]        master -> master (non-fast forward)
> error: failed to push to

the branch you are pushing does not fast-forward to the one on digispeaker. IOW, the master on digispeaker has changed since you made changes on your local master.

Jeff King· Nov 20, 2007, 04:16 UTC · re: Alex Riesen · lore

Re: ! [rejected] master -> master (non-fast forward)

On Sun, Nov 18, 2007 at 07:29:01PM +0100, Alex Riesen wrote:
Show 8 quoted lines
> > jonsmirl@terra:~/ds$ git push digispeaker
> > To ssh://jonsmirl1@git.digispeaker.com/~/projects/digispeaker-kernel.git
> >  ! [rejected]        master -> master (non-fast forward)
> > error: failed to push to
> 
> the branch you are pushing does not fast-forward to the one on
> digispeaker. IOW, the master on digispeaker has changed since you made
> changes on your local master.

Alex, are you going to rework your "consolidate nonff errors and print a more extended message" patch on top of what's in next (I think it should be much simpler now).

-Peff
Alex Riesen· Nov 20, 2007, 06:50 UTC · re: Jeff King · lore

Re: ! [rejected] master -> master (non-fast forward)

Jeff King, Tue, Nov 20, 2007 05:16:20 +0100:
Show 14 quoted lines
> On Sun, Nov 18, 2007 at 07:29:01PM +0100, Alex Riesen wrote:
> 
> > > jonsmirl@terra:~/ds$ git push digispeaker
> > > To ssh://jonsmirl1@git.digispeaker.com/~/projects/digispeaker-kernel.git
> > >  ! [rejected]        master -> master (non-fast forward)
> > > error: failed to push to
> > 
> > the branch you are pushing does not fast-forward to the one on
> > digispeaker. IOW, the master on digispeaker has changed since you made
> > changes on your local master.
> 
> Alex, are you going to rework your "consolidate nonff errors and print a
> more extended message" patch on top of what's in next (I think it should
> be much simpler now).

Do you mean that "Beautify the output of send-pack a bit", Message-ID: <20071112221140.GD2918@steel.home>?

If so, I'm confused. Didn't you already do that in your recent "send-pack:" patch series? It looked to me as if you did (except for the hint regarding non-ff branches to be updated everything is there).

Jeff King· Nov 20, 2007, 11:13 UTC · re: Alex Riesen · lore

Re: ! [rejected] master -> master (non-fast forward)

On Tue, Nov 20, 2007 at 07:50:21AM +0100, Alex Riesen wrote:
Show 10 quoted lines
> > Alex, are you going to rework your "consolidate nonff errors and print a
> > more extended message" patch on top of what's in next (I think it should
> > be much simpler now).
> 
> Do you mean that "Beautify the output of send-pack a bit", Message-ID:
> <20071112221140.GD2918@steel.home>?
> 
> If so, I'm confused. Didn't you already do that in your recent
> "send-pack:" patch series? It looked to me as if you did (except for
> the hint regarding non-ff branches to be updated everything is there).

That is what I meant. In the current code, the errors aren't consolidated at the end, and as you mentioned, we don't print the extra message. Two patch series follows (but see comments on each patch).

-Peff
Jeff King· Nov 20, 2007, 11:18 UTC · re: Jeff King · lore

[PATCH 1/2] send-pack: cluster ref status reporting

Instead of intermingling success and failure, we now print:
  1. all uptodate refs (if args.verbose is enabled)
  2. successfully pushed refs
  3. failed refs

with the assumption that the user is most likely to see the ones at the end, and therefore we order them from "least interesting" to "most interesting."

Signed-off-by: Jeff King <peff@peff.net>
---
I am somewhat ambivalent on this patch. In most cases, it won't make a
difference, since the output is small enough. For large pushes, it might
be better to have errors at the end. OTOH, you do still get "error:
failed to push to ..." at the end, and you can scroll up and see the
errors if you like. And it's not like preserving the original ref output
order was all that interesting.
 builtin-send-pack.c |   93 +++++++++++++++++++++++++++++----------------------
 1 files changed, 53 insertions(+), 40 deletions(-)
diff --git a/builtin-send-pack.c b/builtin-send-pack.c
index 3ce218f..f1cdb97 100644
--- a/builtin-send-pack.c
+++ b/builtin-send-pack.c
@@ -298,52 +298,65 @@ static void print_ok_ref_status(struct ref *ref)
 	}
 }
 
+static int print_one_push_status(struct ref *ref, const char *dest, int count)
+{
+	if (!count)
+		fprintf(stderr, "To %s\n", dest);
+
+	switch(ref->status) {
+	case REF_STATUS_NONE:
+		print_ref_status('X', "[no match]", ref, NULL, NULL);
+		break;
+	case REF_STATUS_REJECT_NODELETE:
+		print_ref_status('!', "[rejected]", ref, NULL,
+				"remote does not support deleting refs");
+		break;
+	case REF_STATUS_UPTODATE:
+		print_ref_status('=', "[up to date]", ref,
+				ref->peer_ref, NULL);
+		break;
+	case REF_STATUS_REJECT_NONFASTFORWARD:
+		print_ref_status('!', "[rejected]", ref, ref->peer_ref,
+				"non-fast forward");
+		break;
+	case REF_STATUS_REMOTE_REJECT:
+		print_ref_status('!', "[remote rejected]", ref,
+				ref->deletion ? ref->peer_ref : NULL,
+				ref->remote_status);
+		break;
+	case REF_STATUS_EXPECTING_REPORT:
+		print_ref_status('!', "[remote failure]", ref,
+				ref->deletion ? ref->peer_ref : NULL,
+				"remote failed to report status");
+		break;
+	case REF_STATUS_OK:
+		print_ok_ref_status(ref);
+		break;
+	}
+
+	return 1;
+}
+
 static void print_push_status(const char *dest, struct ref *refs)
 {
 	struct ref *ref;
-	int shown_dest = 0;
+	int n = 0;
 
-	for (ref = refs; ref; ref = ref->next) {
-		if (!ref->status)
-			continue;
-		if (ref->status == REF_STATUS_UPTODATE && !args.verbose)
-			continue;
+	if (args.verbose) {
+		for (ref = refs; ref; ref = ref->next)
+			if (ref->status == REF_STATUS_UPTODATE)
+				n += print_one_push_status(ref, dest, n);
+	}
 
-		if (!shown_dest) {
-			fprintf(stderr, "To %s\n", dest);
-			shown_dest = 1;
-		}
+	for (ref = refs; ref; ref = ref->next)
+		if (ref->status == REF_STATUS_OK)
+			n += print_one_push_status(ref, dest, n);
 
-		switch(ref->status) {
-		case REF_STATUS_NONE:
-			print_ref_status('X', "[no match]", ref, NULL, NULL);
-			break;
-		case REF_STATUS_REJECT_NODELETE:
-			print_ref_status('!', "[rejected]", ref, NULL,
-					"remote does not support deleting refs");
-			break;
-		case REF_STATUS_UPTODATE:
-			print_ref_status('=', "[up to date]", ref,
-					ref->peer_ref, NULL);
-			break;
-		case REF_STATUS_REJECT_NONFASTFORWARD:
-			print_ref_status('!', "[rejected]", ref, ref->peer_ref,
-					"non-fast forward");
-			break;
-		case REF_STATUS_REMOTE_REJECT:
-			print_ref_status('!', "[remote rejected]", ref,
-					ref->deletion ? ref->peer_ref : NULL,
-					ref->remote_status);
-			break;
-		case REF_STATUS_EXPECTING_REPORT:
-			print_ref_status('!', "[remote failure]", ref,
-					ref->deletion ? ref->peer_ref : NULL,
-					"remote failed to report status");
-			break;
-		case REF_STATUS_OK:
-			print_ok_ref_status(ref);
-			break;
-		}
+	for (ref = refs; ref; ref = ref->next) {
+		if (ref->status != REF_STATUS_NONE &&
+		    ref->status != REF_STATUS_UPTODATE &&
+		    ref->status != REF_STATUS_OK)
+			n += print_one_push_status(ref, dest, n);
 	}
 }
 
-- 
1.5.3.6.1784.gd1b1d-dirty
Alex Riesen· Nov 20, 2007, 18:22 UTC · re: Jeff King · lore

Re: [PATCH 1/2] send-pack: cluster ref status reporting

Jeff King, Tue, Nov 20, 2007 12:18:01 +0100:
Show 5 quoted lines
> Instead of intermingling success and failure, we now print:
> 
>   1. all uptodate refs (if args.verbose is enabled)
>   2. successfully pushed refs
>   3. failed refs

I vote for this. The non-ff refs clustered together was the real reason behind my beautification patch.

Show 6 quoted lines
> I am somewhat ambivalent on this patch. In most cases, it won't make a
> difference, since the output is small enough. For large pushes, it might
> be better to have errors at the end. OTOH, you do still get "error:
> failed to push to ..." at the end, and you can scroll up and see the
> errors if you like. And it's not like preserving the original ref output
> order was all that interesting.

It is just simplier to distinguish visually: non-ff's are formatted significantly different. You don't even have to read what is written to immediately notice if there are problems.

Junio C Hamano· Nov 21, 2007, 07:24 UTC · re: Jeff King · lore

Re: [PATCH 1/2] send-pack: cluster ref status reporting

Jeff King <peff@peff.net> writes:
Show 13 quoted lines
> +static int print_one_push_status(struct ref *ref, const char *dest, int count)
> +{
> ...
> +	case REF_STATUS_REMOTE_REJECT:
> +		print_ref_status('!', "[remote rejected]", ref,
> +				ref->deletion ? ref->peer_ref : NULL,
> +				ref->remote_status);
> +		break;
> +	case REF_STATUS_EXPECTING_REPORT:
> +		print_ref_status('!', "[remote failure]", ref,
> +				ref->deletion ? ref->peer_ref : NULL,
> +				"remote failed to report status");
> +		break;
Eh,... in ref->deletion mode, the peer_ref is...
Jeff King· Nov 21, 2007, 07:33 UTC · re: Junio C Hamano · lore

Re: [PATCH 1/2] send-pack: cluster ref status reporting

On Tue, Nov 20, 2007 at 11:24:55PM -0800, Junio C Hamano wrote:
Show 12 quoted lines
> > +	case REF_STATUS_REMOTE_REJECT:
> > +		print_ref_status('!', "[remote rejected]", ref,
> > +				ref->deletion ? ref->peer_ref : NULL,
> > +				ref->remote_status);
> > +		break;
> > +	case REF_STATUS_EXPECTING_REPORT:
> > +		print_ref_status('!', "[remote failure]", ref,
> > +				ref->deletion ? ref->peer_ref : NULL,
> > +				"remote failed to report status");
> > +		break;
> 
> Eh,... in ref->deletion mode, the peer_ref is...

Gah, sorry. This crept in because I based it on the previous, broken version of the other patch series which had the same problem (and obviously this chunk is just a pure code move + reindent).

But pretend like it was competently prepared and give your comments on the idea. ;)

-Peff
Junio C Hamano· Nov 21, 2007, 07:36 UTC · re: Jeff King · lore

Re: [PATCH 1/2] send-pack: cluster ref status reporting

Jeff King <peff@peff.net> writes:
Show 6 quoted lines
> Gah, sorry. This crept in because I based it on the previous, broken
> version of the other patch series which had the same problem (and
> obviously this chunk is just a pure code move + reindent).
>
> But pretend like it was competently prepared and give your comments on
> the idea. ;)

Heh, at least I've privately queued it at the tip of jk/send-pack for review but not merged it to 'next' yet, after fixing it up (the last round I also fixed up so that was partly why I noticed -- the patch did not apply).

The idea feels sound, and code under cursory look was fine.
Jeff King· Nov 21, 2007, 07:39 UTC · re: Junio C Hamano · lore

Re: [PATCH 1/2] send-pack: cluster ref status reporting

On Tue, Nov 20, 2007 at 11:36:05PM -0800, Junio C Hamano wrote:
> Heh, at least I've privately queued it at the tip of
> jk/send-pack for review but not merged it to 'next' yet, after
> fixing it up (the last round I also fixed up so that was partly
> why I noticed -- the patch did not apply).

OK, good. I should have noticed because I also ran across the same patch failure when I did a rebase earlier today, but I didn't consider that the mistake had slipped out of my repo. :) Sorry for the confusion.

> The idea feels sound, and code under cursory look was fine.

I think it is probably worth applying then. It is slightly more useful to the user, and I haven't been able to see any downside.

-Peff
Jeff King· Nov 21, 2007, 07:37 UTC · re: Jeff King · lore

Re: [PATCH 1/2] send-pack: cluster ref status reporting

On Wed, Nov 21, 2007 at 02:33:32AM -0500, Jeff King wrote:
Show 9 quoted lines
> > > +	case REF_STATUS_REMOTE_REJECT:
> > > +		print_ref_status('!', "[remote rejected]", ref,
> > > +				ref->deletion ? ref->peer_ref : NULL,
> > > +				ref->remote_status);
> > > +		break;
> 
> Gah, sorry. This crept in because I based it on the previous, broken
> version of the other patch series which had the same problem (and
> obviously this chunk is just a pure code move + reindent).

You will also find that it doesn't apply cleanly to 'next', since it attempts to remove the bogus version of the lines (while you correctly fixed them up when you applied to 'next').

Below is a fixed version of the patch for convenience.

-- >8 -- send-pack: cluster ref status reporting

Instead of intermingling success and failure, we now print:
  1. all uptodate refs (if args.verbose is enabled)
  2. successfully pushed refs
  3. failed refs
with the assumption that the user is most likely to see the
ones at the end, and therefore we order them from "least
interesting" to "most interesting."
---
 builtin-send-pack.c |   93 +++++++++++++++++++++++++++++----------------------
 1 files changed, 53 insertions(+), 40 deletions(-)
diff --git a/builtin-send-pack.c b/builtin-send-pack.c
index 3aab89c..25ae1fe 100644
--- a/builtin-send-pack.c
+++ b/builtin-send-pack.c
@@ -298,52 +298,65 @@ static void print_ok_ref_status(struct ref *ref)
 	}
 }
 
+static int print_one_push_status(struct ref *ref, const char *dest, int count)
+{
+	if (!count)
+		fprintf(stderr, "To %s\n", dest);
+
+	switch(ref->status) {
+	case REF_STATUS_NONE:
+		print_ref_status('X', "[no match]", ref, NULL, NULL);
+		break;
+	case REF_STATUS_REJECT_NODELETE:
+		print_ref_status('!', "[rejected]", ref, NULL,
+				"remote does not support deleting refs");
+		break;
+	case REF_STATUS_UPTODATE:
+		print_ref_status('=', "[up to date]", ref,
+				ref->peer_ref, NULL);
+		break;
+	case REF_STATUS_REJECT_NONFASTFORWARD:
+		print_ref_status('!', "[rejected]", ref, ref->peer_ref,
+				"non-fast forward");
+		break;
+	case REF_STATUS_REMOTE_REJECT:
+		print_ref_status('!', "[remote rejected]", ref,
+				ref->deletion ? NULL : ref->peer_ref,
+				ref->remote_status);
+		break;
+	case REF_STATUS_EXPECTING_REPORT:
+		print_ref_status('!', "[remote failure]", ref,
+				ref->deletion ? NULL : ref->peer_ref,
+				"remote failed to report status");
+		break;
+	case REF_STATUS_OK:
+		print_ok_ref_status(ref);
+		break;
+	}
+
+	return 1;
+}
+
 static void print_push_status(const char *dest, struct ref *refs)
 {
 	struct ref *ref;
-	int shown_dest = 0;
+	int n = 0;
 
-	for (ref = refs; ref; ref = ref->next) {
-		if (!ref->status)
-			continue;
-		if (ref->status == REF_STATUS_UPTODATE && !args.verbose)
-			continue;
+	if (args.verbose) {
+		for (ref = refs; ref; ref = ref->next)
+			if (ref->status == REF_STATUS_UPTODATE)
+				n += print_one_push_status(ref, dest, n);
+	}
 
-		if (!shown_dest) {
-			fprintf(stderr, "To %s\n", dest);
-			shown_dest = 1;
-		}
+	for (ref = refs; ref; ref = ref->next)
+		if (ref->status == REF_STATUS_OK)
+			n += print_one_push_status(ref, dest, n);
 
-		switch(ref->status) {
-		case REF_STATUS_NONE:
-			print_ref_status('X', "[no match]", ref, NULL, NULL);
-			break;
-		case REF_STATUS_REJECT_NODELETE:
-			print_ref_status('!', "[rejected]", ref, NULL,
-					"remote does not support deleting refs");
-			break;
-		case REF_STATUS_UPTODATE:
-			print_ref_status('=', "[up to date]", ref,
-					ref->peer_ref, NULL);
-			break;
-		case REF_STATUS_REJECT_NONFASTFORWARD:
-			print_ref_status('!', "[rejected]", ref, ref->peer_ref,
-					"non-fast forward");
-			break;
-		case REF_STATUS_REMOTE_REJECT:
-			print_ref_status('!', "[remote rejected]", ref,
-					ref->deletion ? NULL : ref->peer_ref,
-					ref->remote_status);
-			break;
-		case REF_STATUS_EXPECTING_REPORT:
-			print_ref_status('!', "[remote failure]", ref,
-					ref->deletion ? NULL : ref->peer_ref,
-					"remote failed to report status");
-			break;
-		case REF_STATUS_OK:
-			print_ok_ref_status(ref);
-			break;
-		}
+	for (ref = refs; ref; ref = ref->next) {
+		if (ref->status != REF_STATUS_NONE &&
+		    ref->status != REF_STATUS_UPTODATE &&
+		    ref->status != REF_STATUS_OK)
+			n += print_one_push_status(ref, dest, n);
 	}
 }
 
-- 
1.5.3.6.1786.g2e199
Jeff King· Nov 20, 2007, 11:21 UTC · re: Jeff King · lore

[PATCH 2/2] send-pack: print "maybe you need to pull" hint

If there were refs rejected for not being fastforwards, then we used to print a "maybe you are not up-to-date" hint. This was lost in the recent terse-output patches.

Signed-off-by: Jeff King <peff@peff.net>
---
I am slightly negative on this patch, just because I always found that
particular warning a bit ugly (and the new output is so nice and
compact). But for new users, perhaps the extra hint is helpful?
If we do want the warning, then other options include:
  - listing each non-fast forward, as Alex's original patch did; this
    seems kind of pointless given that they are clustered at the bottom
    already
  - possibly cluster non-fast forwards differently from other errors in
    the output
  - tweak the text
 builtin-send-pack.c |    9 +++++++++
 1 files changed, 9 insertions(+), 0 deletions(-)
diff --git a/builtin-send-pack.c b/builtin-send-pack.c
index f1cdb97..4bfa847 100644
--- a/builtin-send-pack.c
+++ b/builtin-send-pack.c
@@ -341,6 +341,7 @@ static void print_push_status(const char *dest, struct ref *refs)
 {
 	struct ref *ref;
 	int n = 0;
+	int nonff = 0;
 
 	if (args.verbose) {
 		for (ref = refs; ref; ref = ref->next)
@@ -357,7 +358,15 @@ static void print_push_status(const char *dest, struct ref *refs)
 		    ref->status != REF_STATUS_UPTODATE &&
 		    ref->status != REF_STATUS_OK)
 			n += print_one_push_status(ref, dest, n);
+		if (ref->status == REF_STATUS_REJECT_NONFASTFORWARD)
+			nonff++;
 	}
+
+	if (nonff)
+		error("%d remote refs were not ancestors of their "
+		      "corresponding local ref\n"
+		      "Maybe you are not up-to-date and need to pull first?",
+		      nonff);
 }
 
 static int refs_pushed(struct ref *ref)
-- 
1.5.3.6.1784.gd1b1d-dirty
Alex Riesen· Nov 20, 2007, 18:24 UTC · re: Jeff King · lore

Re: [PATCH 2/2] send-pack: print "maybe you need to pull" hint

Jeff King, Tue, Nov 20, 2007 12:21:46 +0100:
Show 9 quoted lines
> If there were refs rejected for not being fastforwards, then
> we used to print a "maybe you are not up-to-date" hint. This
> was lost in the recent terse-output patches.
> 
> Signed-off-by: Jeff King <peff@peff.net>
> ---
> I am slightly negative on this patch, just because I always found that
> particular warning a bit ugly (and the new output is so nice and
> compact). But for new users, perhaps the extra hint is helpful?

Dunno. It's kind of dumb hint and it takes lots of space. I was even thinking about hint(const char *,...), which can be configured to compile out in Makefile.

← back to recent threads