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

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

[Jon Smirl]

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

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

[Junio C Hamano]

"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.

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.

> 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).

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

[Jon Smirl]

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.
> > 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.

> 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

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

[Catalin Marinas]

"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).

-- 
Catalin

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

[Jon Smirl]

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

-- 
Jon Smirl
jonsmirl@gmail.com

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

[Daniel Barkalow]

On Mon, 19 Nov 2007, Jon Smirl wrote:

> 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*

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

[Alex Riesen]

Jon Smirl, Sun, Nov 18, 2007 16:12:10 +0100:
> 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.

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

[Jeff King]

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).

-Peff

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

[Alex Riesen]

Jeff King, Tue, Nov 20, 2007 05:16:20 +0100:
> 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).

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

[Jeff King]

On Tue, Nov 20, 2007 at 07:50:21AM +0100, Alex Riesen wrote:

> > 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

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

[Jeff King]

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

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

[Alex Riesen]

Jeff King, Tue, Nov 20, 2007 12:18:01 +0100:
> 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.

> 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.

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

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> +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...

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

[Jeff King]

On Tue, Nov 20, 2007 at 11:24:55PM -0800, Junio C Hamano wrote:

> > +	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

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

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> 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.

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

[Jeff King]

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

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

[Jeff King]

On Wed, Nov 21, 2007 at 02:33:32AM -0500, Jeff King wrote:

> > > +	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

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

[Jeff King]

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

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

[Alex Riesen]

Jeff King, Tue, Nov 20, 2007 12:21:46 +0100:
> 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.