git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
To: Jason Frey <jfrey@redhat.com>
Cc: git@vger.kernel.org, Stefan Beller <sbeller@google.com>
Subject: Re: Bug: duplicate sections in .git/config after remote removal
Date: Tue, 27 Mar 2018 23:49:52 +0200	[thread overview]
Message-ID: <87r2o5w5mn.fsf@evledraar.gmail.com> (raw)
In-Reply-To: <CAP6Vx84GRRxgMZF5P6tb6F4rJ8ozxx-d0o_LsNe=kEYVRkBTKQ@mail.gmail.com>


On Tue, Mar 27 2018, Jason Frey wrote:

> While the impact of this bug is minimal, and git itself is not
> affected, it can affect external tools that want to read the
> .git/config file, expecting unique section names.
>
> To reproduce:
>
> Given the following example .git/config file (I am leaving out the
> [core] section for brevity):
>
>     [remote "origin"]
>         url = git@github.com:Fryguy/example.git
>         fetch = +refs/heads/*:refs/remotes/origin/*
>     [branch "master"]
>         remote = origin
>         merge = refs/heads/master
>
> Running `git remote rm origin` will result in the following contents:
>
>     [branch "master"]
>
> Running `git remote add origin git@github.com:Fryguy/example.git` will
> result in the following contents:
>
>     [branch "master"]
>     [remote "origin"]
>         url = git@github.com:Fryguy/example.git
>         fetch = +refs/heads/*:refs/remotes/origin/*
>
> And finally, running `git fetch origin; git branch -u origin/master`
> will result in the following contents:
>
>     [branch "master"]
>     [remote "origin"]
>         url = git@github.com:Fryguy/example.git
>         fetch = +refs/heads/*:refs/remotes/origin/*
>     [branch "master"]
>         remote = origin
>         merge = refs/heads/master
>
> at which point you can see the duplicate sections (even though one is
> empty).  Also note that if you do the steps again, you will be left
> with 3 sections, 2 of which are empty.  This process can be repeated
> over and over.

This can be annoying and result in some very verbose config files when
we automatically edit them, e.g.:

    (rm -v /tmp/test.ini; for i in {1..3}; do git config -f /tmp/test.ini foo.bar 0 && git config -f /tmp/test.ini --unset foo.bar; done; cat /tmp/test.ini)
    removed '/tmp/test.ini'
    [foo]
    [foo]
    [foo]

But it's not so clear that it should be called a bug, yes we could be a
bit smarter and not add obvious crap like the example above (duplicate
sections at the end), but it gets less obvious in more complex cases,
see my c8b2cec09e ("branch: add test for -m renaming multiple config
sections", 2017-06-18) for one such example.

Git has a config format that's hybrid human/machine editable. Consider a
case like:

    [gc]
    ;; Here's all the gc config we set up to avoid the great outage of 2015
    autoDetach = false
    ;; Our aliases
    [alias]
    st = status

Now, if I run `git config gc.auto 0` is it better if we end up with:

    [gc]
    ;; Here's all the gc config we set up to avoid the great outage of 2015
    autoDetach = false
    auto = 0
    ;; Our aliases
    [alias]
    st = status

Or something that makes it more clear that a machine added something at
the end:

    [gc]
    ;; Here's all the gc config we set up to avoid the great outage of 2015
    autoDetach = false
    ;; Our aliases
    [alias]
    st = status
    [gc]
    auto = 0

Most importantly though, regardless of what we decide to do when we
machine-edit the file, it's also human-editable, and being able to
repeat sections is part of our config format that you're simply going to
have to deal with.

The external tool (presumably some generic *.ini parser) you're trying
to point at git's config is broken for that purpose if it doesn't handle
duplicate sections. You're probably better off trying to parse `git
config --list --null` than trying to make it work.

I don't think we'd ever want to get rid of this feature, it's *very*
useful. Both for config via the include macro, and for people to
manually paste some config they want to try out to the end of their
config, without having to manually edit it to incorporate it into their
already existing sections.

  parent reply	other threads:[~2018-03-27 21:49 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-03-27 20:41 Bug: duplicate sections in .git/config after remote removal Jason Frey
2018-03-27 21:45 ` Stefan Beller
2018-03-28 16:34   ` Johannes Schindelin
2018-03-27 21:49 ` Ævar Arnfjörð Bjarmason [this message]
2018-03-28  7:54   ` Philip Oakley
2018-03-28  7:54   ` Philip Oakley
2018-03-28  7:55   ` Philip Oakley
2018-03-28  8:06   ` Philip Oakley

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87r2o5w5mn.fsf@evledraar.gmail.com \
    --to=avarab@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=jfrey@redhat.com \
    --cc=sbeller@google.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).