Re: [PATCH 5/7] bundle-uri: parse bundle list in config format

From: Junio C Hamano <gitster@pobox.com>
Date: Aug 22, 2022, 19:25 UTC
Message-ID: <xmqqilmkcc7i.fsf@gitster.g>
In-Reply-To: <1d1bd9c710327b4d705cfede017771da7fb6ec52.1661181174.git.gitgitgadget@gmail.com>

"Derrick Stolee via GitGitGadget" <gitgitgadget@gmail.com> writes:

Show 6 quoted lines

> To allow for the incremental fetch case, teach Git to understand a
> bundle list that could be advertised at an independent bundle URI. Such
> a bundle list is likely to be inspected by human readers, even if only
> by the bundle provider creating the list. For this reason, we can take
> our expected "key=value" pairs and instead format them using Git config
> format.

"can" does not explain why it is a good idea. "As a sequence of key=value pairs is a lot more dense and harder to read than the configuration file format, let's declare that it is the format we use in a file that holds a bundle-list" would be.

I do not personally buy it, though. As I hinted in an earlier step, some trait we associate with our configuration fioe format, like the "last one wins" semantics, are undesirable ones, so even if we reuse the appearance of the text, the semantics would have to become different (including "syntax errors lead to die()" mentioned elsewhere in the proposed log message).

Show 6 quoted lines

> Update 'test-tool bundle-uri' to take this config file format as input.
> It uses a filename instead of stdin because there is no existing way to
> parse a FILE pointer in the config machinery. Using
> git_config_from_mem() is overly complicated and more likely to introduce
> bugs than this simpler version. I would rather have a slightly confusing
> test helper than complicated product code.

All the troubles described above seem to come from the initial mistake to try reusing the configuration file parser or reusing the configuration file format, at least to me.