Re: git branch --editdescription fatal error

From: Jeff King <peff@peff.net>
Date: Jan 6, 2017, 20:10 UTC
Message-ID: <20170106201001.pgxo4rl3d36veezs@sigill.intra.peff.net>
In-Reply-To: <CAGZ79kZOfHWP_pQGN1QcmR71Ft6ib0aPwNKX80YMT7KcK0_Stg@mail.gmail.com>

On Fri, Jan 06, 2017 at 11:43:52AM -0800, Stefan Beller wrote:

Show 12 quoted lines

> >>> When executing "git branch <branch> --edit-description" on a branch with no description set, I get "fatal: could not unset 'branch.<branch>.description". It would seem that the unsetting piece should occur only after checking if it was set in the first place.
> >>
> >> That seems strange. Is it possible that your config is not writable?
> >> (.git/config, ~/gitconfig, you'd need to find out where the <branch>
> >> is configured already via git config --global/--system/--local --list)
> >>
> >
> > Have you actually tried to reproduce this issue? I'm on current next
> > and can reproduce the problem.
> 
> eh, I was on $random_version that I currently have installed
> (with messed up submodule code, but otherwise close to master).

Hmm. I can reproduce, but only in one situation: when the new description is empty. In which case we try to delete the variable. In other words:

  [this breaks; the file remains empty and we try to delete the
   nonexistent config]
  $ EDITOR=true git branch --edit-description master
  fatal: could not unset 'branch.master.description'

  [this works; we actually set the variable]
  $ EDITOR='echo foo >' git branch --edit-description master
  $ git config branch.master.description
  foo

  [and now the unsetting works; note we have to truncate here, since
   the file will be prepopulated with "foo" from the existing desc]
  $ EDITOR='>' git branch --edit-description master
  $ git config branch.master.description
  [no output]

The history of this behavior is a bit funny.

In old versions of git, we would return a failing exit code of "1" from git-branch, with no message.

Then in bd25f89014 (branch: die on config error when editing branch description, 2016-02-22), we actually started returning "0"! This was because the config code did not propagate errors from its helper functions in all cases.

That was fixed by 9c14bb08a4 (git_config_set_multivar_in_file: all non-zero returns are errors, 2016-04-09), giving the behavior we see today.

So between v2.7.3 and v2.8.3, we did return 0, but I think that was a bug (we also returned 0 for a lot of other bogus cases, too).

I could see either behavior as reasonable, but I think the right solution would be for the branch code from bd25f89014 to use the "gently" function set the variable, and then decide which cases should be silently ignored, and which propagated as errors.

-Peff