git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH v2] branch: do not fail a no-op --edit-desc

From
Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Date
Sep 30, 2022, 17:58 UTC
Message-ID
<220930.861qrsvj12.gmgdl@evledraar.gmail.com>
In-Reply-To
<xmqq7d1klrua.fsf@gitster.g>
On Fri, Sep 30 2022, Junio C Hamano wrote:
Show 14 quoted lines
> Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:
>
>>> The end result is OK in that the configuration variable does not
>>> exist in the resulting repository, but we should do better, by using
>>> git_config_set_gently() and ignoring only the specific error that is
>>> returned when removing a missing configuration variable.
>> ...
>> I was curious to follow up on your suggestion in your 3rd paragraph, so
>> I tried implementing this in the config API, results below, if you're
>> interested in it assume my SOB.
>
> Did I make any suggestion?  I am assuming that what I left in the
> quote above is the paragraph you are referring to, and that is not a
> suggestion but a description of what the patch did, so I am puzzled.

I think I just misread "by using git_config_set_gently() and ignoring only the specific error that is returned when removing a missing configuration variable." as "an alternative of this might do this via the config API"...

Show 8 quoted lines
>> But, having done that I discovered that your code here has a bug,
>> admittedly a pretty obscure one. The CONFIG_NOTHING_SET flag on "set"
>> doesn't mean "nothing to set, because it's there already", it means
>> "either <that>, or the key is multi-value and I'm bailing out".
>
> Ah, OK, so in short, _gently() is still unusable to use for that.  I
> guess it means that the approach taken by v1 would be a better
> solution, then.

As you noted it's got a TOCTOU instead, so we might wipe away good config entirely.

I think between the two I'd go with v2, bugs and all, it's pretty unlikely that someone has two "description" entries.

Previous: Junio C HamanoNext: Junio C Hamano
Message 11 of 13 in “branch: do not fail a no-op --edit-desc”
  1. branch: do not fail a no-op --edit-descJunio C Hamano, Sep 28, 2022
  2. Rubén JustoSep 28, 2022
  3. Junio C HamanoSep 28, 2022
  4. Rubén JustoSep 29, 2022
  5. Junio C HamanoSep 29, 2022
  6. Rubén JustoSep 30, 2022
  7. Rubén JustoOct 1, 2022
  8. branch: do not fail a no-op --edit-descJunio C Hamano, Sep 30, 2022
  9. Ævar Arnfjörð BjarmasonSep 30, 2022
  10. Junio C HamanoSep 30, 2022
  11. Ævar Arnfjörð BjarmasonSep 30, 2022
  12. Junio C HamanoSep 30, 2022
  13. branch: do not fail a no-op --edit-descJunio C Hamano, Sep 30, 2022

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.