git branch --editdescription fatal error

Hello,

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. I am not too familiar with the inner workings of git, but if you accept pull requests, I'm happy to give it a shot if you can give me directions as to how to submit one to you.

While I have you, I would also like to make a feature request for branch descriptions to (optionally) be included in the output of git branch <noargs> (or something like git branch --descriptions). This is particularly helpful where branch names match JIRA tickets numbers or are simply so old I don't remember what they are but don't want to delete them just in case.

Thank you for maintaining this absolutely wonderful program, and have a happy New Year!

Best,
Jake
     
 Jake Lambert
 Full Stack Developer
 
 
 The Company On A Hill
 www.onahill.co
    

On Fri, Jan 6, 2017 at 9:25 AM, Jake Lambert <jake@onahill.co> wrote:

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)

We rather do email based workflow, see https://github.com/git/git/blob/master/Documentation/SubmittingPatches (We use github only as a host, you could also obtain it from one of https://git-blame.blogspot.com/p/git-public-repositories.html)

Apparently the branch descriptions are only used for the request-pull subcommand, https://github.com/git/git/commit/c0168147831fce00975949213eef3471b7a2b76b and the merge message https://github.com/git/git/commit/898eacd8ada2d012f977948350ed60845e238037

If you want to work on both of these issues, have a look at edit_branch_description as well as print_ref_list in builtin/branch.c

Happy new year! Stefan

2017-01-06 18:55 GMT+01:00 Stefan Beller <sbeller@google.com>:

Have you actually tried to reproduce this issue? I'm on current next and can reproduce the problem.

It seems Git is not happy with unsetting non-existent configs. And at least the following piece of code seems to be there since 10bea152a (Add functions git_config_set() and git_config_set_multivar(), 2005-11-17).

config.c:2212 /* if nothing to unset, or too many matches, error out */ if ((store.seen == 0 && value == NULL) ||

I wonder why it's an error to remove something that doesn't exist except the case we 'know' it exists because of further actions. I don't think this is a thing, though. Just for the record: `git config --unset branch.$branch.description` does return with code "5" here as well.

I think removing things that don't exist should behave the same like removing things that do exist, with a success.

Ralf

On Fri, Jan 6, 2017 at 11:31 AM, Ralf Thielow <ralf.thielow@gmail.com> wrote:

eh, I was on $random_version that I currently have installed (with messed up submodule code, but otherwise close to master).

this gives hope that a bisect between master..next will give us a culprit?

I am not sure. Consider the tool "rm"

    $ rm no_exist
    rm: cannot remove ‘no_exist’: No such file or directory
    $ echo $?
    1

You have to use the --force.

For this specific use case I agree we should then set the force flag to let the the removal of the config option succeed no matter if it existed before.

Stefan

2017-01-06 20:43 GMT+01:00 Stefan Beller <sbeller@google.com>:

Hm. I can confirm it appears on current master (e05806da9).

Can you think of a case where this flag should be set to false?

Ralf

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

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