On Wed, Jan 24, 2024 at 9:52 AM Patrick Steinhardt <ps@xxxxxx> wrote:
>
> On Tue, Jan 23, 2024 at 05:15:48PM +0100, Christian Couder wrote:
> > On Wed, Jan 10, 2024 at 10:01 AM Patrick Steinhardt <ps@xxxxxx> wrote:
> > >
> > > The t1300 test suite exercises the git-config(1) tool. To do so we
> > > overwrite ".git/config" to contain custom contents.
> >
> > Here "we" means "tests in t1300" I guess.
> >
> > > While this is easy
> > > enough to do, it may create problems when using a non-default repository
> > > format because we also overwrite the repository format version as well
> > > as any potential extensions.
> >
> > But I am not sure that "we" in the above sentence is also "tests in
> > t1300". I think overwriting the repo format version and potential
> > extensions is done by other tests, right? Anyway it would be nice to
> > clarify this.
> >
> > > With the upcoming "reftable" ref backend
> > > the result is that we may try to access refs via the "files" backend
> > > even though the repository has been initialized with the "reftable"
> > > backend.
> >
> > Not sure here also what "we" is. When could refs be accessed via the
> > "files" backend even though the repo was initialized with the
> > "reftable" backend?
>
> Yeah, I've rephrased all of these to sey "the tests" or something
> similar.
>
> > Does this mean that some of the tests in t1300 try to access refs via
> > the "files" backend while we may want to run all the tests using the
> > reftable backend?
>
> Exactly. We overwrite the ".git/config", which contains the "refStorage"
> extension that tells us to use the "reftable" backend. So the extension
> is gone, and thus Git would fall back to use the "files" backend
> instead, which will fail.
Let's take a look at this test:
test_expect_success 'check split_cmdline return' "
git config alias.split-cmdline-fix 'echo \"' &&
test_must_fail git split-cmdline-fix &&
echo foo > foo &&
git add foo &&
git commit -m 'initial commit' &&
git config branch.main.mergeoptions 'echo \"' &&
test_must_fail git merge main
"
I don't really see how it overwrites anything. When putting some debug
commands before and after that test, it looks like the config file
contains the following before that test:
---
[section "sub=section"]
val1 = foo=bar
val2 = foo\nbar
val3 = \n\n
val4 =
val5
[section]
val = foo \t bar
---
and the following after that test:
---
[section "sub=section"]
val1 = foo=bar
val2 = foo\nbar
val3 = \n\n
val4 =
val5
[section]
val = foo \t bar
[alias]
split-cmdline-fix = echo \"
[branch "main"]
mergeoptions = echo \"
---
So it doesn't look like it overwrites anything. To me it just adds
stuff at the end of the file.
> > > Refactor tests which access the refdb to be more robust by using their
> > > own separate repositories, which allows us to be more careful and not
> > > discard required extensions.
> >
> > Not sure what exactly is discarding extensions. Also robust is not
> > very clear. It would be better to give at least an example of how
> > things could fail.
>
> Hm. I don't really know how to phrase this better. The preceding
> paragraph already explains why we're discarding the extension and what
> the consequence is. I added a sentence saying ", which will cause
> failures when trying to access any refs."
To me the preceding paragraph said that we are overwriting the config
file, but I just don't see how for example the above test overwrites
anything. So maybe I am missing something obvious, or maybe you don't
quite mean "overwrite", but I don't see how the extension would be
discarded by the test which only seems to add stuff.
