Hi Ævar, On Thu, 19 Nov 2020, Ævar Arnfjörð Bjarmason wrote: > On Thu, Nov 19 2020, Johannes Schindelin wrote: > > > On Wed, 18 Nov 2020, Ævar Arnfjörð Bjarmason wrote: > > > >> On Wed, Nov 18 2020, Johannes Schindelin via GitGitGadget wrote: > >> > >> > From: Johannes Schindelin <johannes.schindelin@xxxxxx> > >> > > >> > Therefore, the actual branch name does not matter at all. We might > >> > just as well avoid racially-charged names here. > >> > >> It seems to me the actual name matters a lot, and it must whatever > >> the default branch name is. > > > > Nope. Not at all. Because what we're exercising is the code paths when > > we _don't_ have a branch name to work with. > > > > In the non-Git case, this is trivial to see. There is not even a > > repository! How can there be a branch? > > > > In the early config case, it is too early to access the refs. I meant > > to reference (but forgot) the commit 85fe0e800ca (config: work around > > bug with includeif:onbranch and early config, 2019-07-31) because that > > commit's commit message describes the catch-22 that is the reason why > > the early config cannot see the current branch name (if any). > > > > I should probably have thought of referencing 22932d9169f (config: > > stop checking whether the_repository is NULL, 2019-08-06) for the > > second test case, too. > > > > So again, these two test cases do _not_ exercise the code path where > > another config file is included. To the contrary, they try to prevent > > a regression where `onbranch` would segfault in one case, and BUG in > > the other (in both cases because the now-fixed code used to try to > > look at the current branch name _anyway_). > > > >> I.e. what the test is doing is producing intentionally broken config, > >> and asserting that we don't read it at an early stage. > >> > >> Therefore if we regressed and started doing that the test wouldn't > >> catch it, because the default branch name is "master", or "main" > >> if/when that refs.c change lands, neither of which is "topic". > > > > No, if we regressed, the code would start to throw a BUG, or a > > segfault, respectively. > > > > We never expect these two test cases to look at any branch name at > > all. > > Thanks. I mis(understood|read) it. I guess the commit message could use an update. Ciao, Dscho
