Re: [PATCH v5 00/15] config: make git_config_set die on failure

On Tue, Feb 16, 2016 at 12:15:35PM -0500, Eric Sunshine wrote:

On Tue, Feb 16, 2016 at 7:56 AM, Patrick Steinhardt [off-list ref] wrote:

quoted

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