Thanks for the thorough review! I really appreciate it :) On Mon, Sep 13, 2021 at 03:29:25PM -0400, Taylor Blau wrote: > Small nit; "the_repository->settings()" should be spelled as > "the_repository->settings", since "settings" is not a function. Oh that's a good catch. Thanks! > It may be worth noting that this was totally fine before > core.commitGraph's default changed to true. That happened in 31b1de6a09 > (commit-graph: turn on commit-graph by default, 2019-08-13), which is > the first time this sort of regression would have appeared. Sounds good, I'll note that down. > Nit; I recommend replacing the `-c` style configuration with > `test_config`, which modifies `$GIT_DIR/config` but only for the > duration of the sub-shell. Sounds good. This works great :) > > + cd "$TRASH_DIRECTORY/full" && > > + git fsck && > > + corrupt_graph_and_verify $GRAPH_BYTE_FOOTER "\00" \ > > + "incorrect checksum" && > > + git config --unset core.commitGraph && > > But I'm not aware of a way to temporarily unset a configuration variable > for the duration of a test, so here I would probably write: > > test_must_fail git -c core.commitGraph= fsck > > which Git interprets as "pretend this variable is unset in-core". >From my testing, I suspect that git does not pretend the variable is unset, but rather, it pretends that the variable is set to the empty string. It seems that git behaves as if the variable is set to "false". This is called out in Documentation/git.txt: Including the equals but with an empty value (like `git -c foo.bar= ...`) sets `foo.bar` to the empty string which `git config --type=bool` will convert to `false`. If the variable really is set to false, how might we proceed here? Shall we stick with test_when_finished?
