On Thu, Oct 17, 2024 at 02:46:45PM -0400, Taylor Blau wrote: > > Rather not as config file is parsed only once: > > > > https://github.com/git/git/blob/15030f9556f545b167b1879b877a5d780252dc16/upload-pack.c#L1368 > > I am not sure I follow... upload_pack_config() is invoked on every > configuration line that we see. So the first time when we read > "allowAnySHA1InWant = true", we take the first path through that > function you linked. The second time, we take the else branch, and > correspondingly unset the bits from ALLOW_ANY_SHA1. > > So today that is doing the right thing (it will end with the same bits > set that we started with). But under the proposed patch that behavior > would change, and the lower two bits would still be set. Reading Piotr's response I wondered if upload-pack might be using the configset API instead of a config callback. But no, it does look like a config callback. But it does hint at a possible path if we wanted to process these independently: we could read each of the config options separately, resolving them in the usual last-one-wins way, and then turning the result into a bitfield. Something like this, outside of the callback (totally untested): int v; unsigned bits = 0; if (!git_config_get_bool("uploadpack.allowtipsha1inwant", &v) && v) bits |= ALLOW_TIP_SHA1; if (!git_config_get_bool("uploadpack.allowreachablesha1inwant", &v) && v) bits |= ALLOW_REACHABLE_SHA1; if (!git_config_get_bool("uploadpack.allowanysha1inwant", &v) && v) bits |= ALLOW_ANY_SHA1; That makes the config flags independent. But the features themselves are not really independent. If you do: [uploadpack] allowAnySHA1InWant = true allowTipSHA1InWant = false it is still going to allow the "tip" flag, because it's a subset of the "any" flag. And you can't escape that. So I still think we're better off to just document (and of course in the long run all of these become less and less interesting as they are v0-only options). -Peff
