Re: [PATCH v5 00/15] config: make git_config_set die on failure
- From
Patrick Steinhardt <ps@pks.im>
- Date
- Feb 17, 2016, 10:35 UTC
- Message-ID
- <20160217103548.GC743@pks-xps.fritz.box>
- In-Reply-To
- <CAPig+cQav4AS6-RwUKGA2fmWxRL-2e2o72oYkk83emqz3z6q3w@mail.gmail.com>
On Tue, Feb 16, 2016 at 12:15:35PM -0500, Eric Sunshine wrote:
Show 33 quoted lines
> On Tue, Feb 16, 2016 at 7:56 AM, Patrick Steinhardt <ps@pks.im> wrote:
> > [...]
> > Junio pointed out that it might not always be the most sensible
> > thing to die when install_branch_config fails. After thinking
> > about it I changed the behavior of the function to print an error
> > and advise message and return an error code. The error code is
> > then only used by the `git branch --set-upstream-to=` command to
> > abort early, as its main intent will usually be to set the
> > tracking information. The other callers (related to git-clone and
> > git-push) simply ignore the returned value while the messages are
> > still printed.
> >
> > I think it does make sense to not abort clones and pushes when
> > the function fails. Setting the upstream information is only a
> > small part of these commands and especially when cloning a large
> > repository it is harmful to die as this would delete everything
> > that has just been cloned. The user can still fix up the remote
> > tracking branch afterwards.
> >
> > Interdiff between v4 and v5:
> >
> > diff --git a/branch.c b/branch.c
> > @@ -49,7 +49,13 @@ static int should_setup_rebase(const char *origin)
> > +static const char tracking_advice[] =
> > +N_("\n"
> > +"After fixing the error cause you may try to fix up\n"
> > +"the remote tracking information by invoking\n"
> > +"\"git branch --set-upstream-to=\".");
>
> Do you have enough information at the point this message is emitted to
> make it even more useful for the user by showing the actual argument
> to --set-upstream-to= that the user would need to invoke once the
> issue is resolved?Actually, yes. The message is invoked by `install_branch_config` which naturally has to know what upstream we want to set to. Will improve the message in the next revision, thanks.
Patrick