[PATCH 1/3] push: indicate partialness of error message

[PATCH 1/3] push: indicate partialness of error message

[Jeff King]

The existing message indicates that an error occured during
push, but it is unclear whether _any_ refs were actually
pushed (even though the status table above shows which were
pushed successfully and which were not, the message "failed
to push" implies a total failure). By indicating that "some
refs" failed, we hopefully indicate to the user that the
table above contains the details.

We could also put in an explicit "see above for details"
message, but it seemed to clutter the output quite a bit
(both on a line of its own, or at the end of the error line,
which inevitably wraps).

This could also be made more fancy if the transport
mechanism passed back more details on how many refs
succeeded and failed:

  error: failed to push %d out of %d refs to '%s'

Signed-off-by: Jeff King <peff@peff.net>
---
This patch series is meant to address some user issues encountered in
the recent thread "git push [rejected] question". It's entirely
message and documentation updates.

 builtin-push.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/builtin-push.c b/builtin-push.c
index c8cb63e..9f727c0 100644
--- a/builtin-push.c
+++ b/builtin-push.c
@@ -90,7 +90,7 @@ static int do_push(const char *repo, int flags)
 		if (!err)
 			continue;
 
-		error("failed to push to '%s'", remote->url[i]);
+		error("failed to push some refs to '%s'", remote->url[i]);
 		errs++;
 	}
 	return !!errs;
-- 
1.5.4.1.143.ge7e51-dirty

Re: [PATCH 1/3] push: indicate partialness of error message

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> This patch series is meant to address some user issues encountered in
> the recent thread "git push [rejected] question". It's entirely
> message and documentation updates.

Thanks; I think all of them are fine 'maint' material.

Distinguishing between [rejected] and [stale] would belong in
1.5.5 if it is really needed.  Together with the "git checkout
notices forks" enhancement on Daniel's git-checkout rewritten in
C, I think it would solve the issue in the "push [rejected]
question".

Re: [PATCH 1/3] push: indicate partialness of error message

[Jeff King]

On Tue, Feb 19, 2008 at 01:34:40PM -0800, Junio C Hamano wrote:

> Distinguishing between [rejected] and [stale] would belong in
> 1.5.5 if it is really needed.  Together with the "git checkout
> notices forks" enhancement on Daniel's git-checkout rewritten in
> C, I think it would solve the issue in the "push [rejected]
> question".

I am still a little uncomfortable with the rejected/stale distinction,
because the semantics aren't clear.

Let's say we figure out which is which in send-pack. Do we:

  - simply change the "rejected" text to "stale, and leave as-is? I
    think that is safe, but I also think it isn't a significant
    improvement for workflows that leave lots of stale branches around
    (they clutter the push output).
  - omit stale listings when -v is not given?
    - this is dangerous with the patch I posted, because "git push; #
      oops, I forgot I amended; git push -f" will push stale branches
      that weren't even mentioned in the first case.
    - instead, should we require some extra magic to force stale
      branches to be pushed? Forcing such a push is almost never a good
      idea, whereas forked branches are not too uncommon.
    - instead, should we disallow "-f" without an explicit refspec (or
      --all, or --mirror, etc) I can't think of a workflow where you
      want to force _many_ branches at once, except the special case of
      mirroring.
    - we could also combine the two: don't respect -f on stale pushes,
      but do respect pushing "+stale"

Thoughts?

-Peff

Re: [PATCH 1/3] push: indicate partialness of error message

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> On Tue, Feb 19, 2008 at 01:34:40PM -0800, Junio C Hamano wrote:
>
>> Distinguishing between [rejected] and [stale] would belong in
>> 1.5.5 if it is really needed.  Together with the "git checkout
>> notices forks" enhancement on Daniel's git-checkout rewritten in
>> C, I think it would solve the issue in the "push [rejected]
>> question".
>
> I am still a little uncomfortable with the rejected/stale distinction,
> because the semantics aren't clear.
>
> Let's say we figure out which is which in send-pack. Do we:
>
>   - simply change the "rejected" text to "stale, and leave as-is? I
>     think that is safe, but I also think it isn't a significant
>     improvement for workflows that leave lots of stale branches around
>     (they clutter the push output).
>   - omit stale listings when -v is not given?
>     - this is dangerous with the patch I posted, because "git push; #
>       oops, I forgot I amended; git push -f" will push stale branches
>       that weren't even mentioned in the first case.
>     - instead, should we require some extra magic to force stale
>       branches to be pushed? Forcing such a push is almost never a good
>       idea, whereas forked branches are not too uncommon.
>     - instead, should we disallow "-f" without an explicit refspec (or
>       --all, or --mirror, etc) I can't think of a workflow where you
>       want to force _many_ branches at once, except the special case of
>       mirroring.
>     - we could also combine the two: don't respect -f on stale pushes,
>       but do respect pushing "+stale"
>
> Thoughts?

I think the "push -f is unsafe and forbidden unless you are
using --all, --mirror or being explicit" is extremely sensible,
with or without the "omit [stale] unless -v".  You can even
throw in --matching to the mix.

By the way, I am not opposed to adding --matching to push.  Just
don't make anything but --matching the default, nor introduce
configuration, in the first round.  We might want to do that
later when we have a solid migration path.  I just still do not
think it is necessary to make a backward incompatible changes.

When your workflow is to work against a shared more-or-less
central repository with many branches, I think it is reasonable
to want be able to push out the work in your current branch and
nothing else after you are done with a change you made in your
branch and are convinced that it is in a good enough shape to be
shared with others,

When the --matching behaviour was made the default at the very
beginning, the prevalent git workflow was not about a shared
repository, but to work with a set of _owned_ public
distribution points.  Also the model strongly discouraged the
developer to be unorganized.  You use "matching" push only when
your public branches in your private working area are all good
and ready.  You can still work on private topic branches to
build on top of what are "public" all you want, but until you
are ready, you do not start merging your half-baked ideas you
have on your topic to the "matching" branches.  Because the
repositories you would be pushing into are owned by you, there
is no "staleness" issue with it, as long as you worked that way.
The branches are either ready or unchanged, as long as you are
organized well enough.

But with a central shared repository, even if you are well
organized and your changes to your "matching" branches are
ready, your other branches can now go "stale".  It is a new
problem "sharedness" introduces.

So in that sense, not showing individual [stale] branches by
default would be a sensible solution to that new problem.  The
solution does not break existing users.  We may even want to
squelch down [failed] somewhat, because the users may have WIP
on their matching topics that have diverged but are not ready to
be pushed out.  So perhaps saying "N stale branches and M
diverged branches did not get pushed out" at the end might be a
good idea, even when run without -v option.

HOWEVER.

It is only half a solution.  It still requires you to be well
organized for you to use "matching" semantics.

For example, if you did this:

	$ git checkout -b origin/ticket-99 ticket-99
        $ hack hack half baked hack well that does not work blech
        $ git commit -a -m 'WIP: I give up for now'

        $ git checkout -b origin/ticket-47 ticket-47
        $ hack hack this is easy and I am done
	$ test suite runs well
        $ git commit -a -m 'Fixed ticket#47 issue'

you cannot use "matching" push without pushing out your
half-baked attempt to fix issue #99 when you are done with
issue#47.  Instead you would have done this if you are
organized and really wanted to use "matching":

	$ git checkout -b origin/ticket-99 ticket-99
        $ hack hack half baked hack well that does not work blech
        $ git commit -a -m 'WIP: I give up for now'
	$ git branch -m wip-ticket-99

        $ git checkout -b origin/ticket-47 ticket-47
        $ hack hack this is easy and I am done
	$ test suite runs well
        $ git commit -a -m 'Fixed ticket#47 issue'
        $ some other cleanups, perhaps rebase -i to make it presentable
	$ test suite runs well
        $ git push

But I tend to think it is easier to train your finger to say
"git push origin ticket-47" (or "HEAD").  New user questions
would also become easier and unambiguous to deal with, as
mentioned in another message in that "[rejected]" thread.

By the way, we might want to add

	$ git checkout -B origin/ticket-74

that pretends the user said "-b origin/ticket-74 ticket-74" by
stripping the "remote/$nick" part of the dwimmed starting point
ref to come up with the local refname.