git/t/t5318-commit-graph.sh, branch v2.42.2

commit-graph: avoid repeated mixed generation number warnings

2023-08-21T23:16:23Z

When validating that a commit-graph has either all zero, or all non-zero generation numbers, we emit a warning on both the rising and falling edge of transitioning between the two. So if we are unfortunate enough to see a commit-graph which has a repeating sequence of zero, then non-zero generation numbers, we'll generate many warnings that contain more or less the same information. Avoid this by keeping track of a single example for a commit with zero- and non-zero generation, and emit a single warning at the end of verification if both are non-NULL. Co-authored-by: Jeff King Signed-off-by: Jeff King Signed-off-by: Taylor Blau Signed-off-by: Junio C Hamano

t/t5318-commit-graph.sh: test generation zero transitions during fsck

2023-08-21T23:07:34Z

The second test called "detect incorrect generation number" asserts that we correctly warn during an fsck when we see a non-zero generation number after seeing a zero beforehand. The other transition (going from non-zero to zero) was previously untested. Test both directions, and rename the existing test to make clear which direction it is exercising. Signed-off-by: Taylor Blau Signed-off-by: Junio C Hamano

commit-graph: introduce `commit_graph_generation_from_graph()`

2023-08-21T23:07:33Z

In 2ee11f7261 (commit-graph: return generation from memory, 2023-03-20), the `commit_graph_generation()` function stopped returning zeros when asked to locate the generation number of a given commit. This was done at the time to prepare for a later change which set generation values in memory, meaning that we could no longer rely on `graph_pos` alone to tell us whether or not to trust the generation number returned by this function. In 2ee11f7261, it was noted that this change only impacted very old commit-graphs, which were written with all commits having generation number 0. Indeed, zero is not a valid generation number, so we should never expect to see that value outside of the aforementioned case. The test fallout in 2ee11f7261 indicated that we were no longer able to fsck a specific old case of commit-graph corruption, where we see a non-zero generation number after having seen a generation number of 0 earlier. Introduce a variant of `commit_graph_generation()` which behaves like that function did prior to 2ee11f7261, known as `commit_graph_generation_from_graph()`. Then use this function in the context of `verify_one_commit_graph()`, where we only want to trust the values from the graph. Signed-off-by: Taylor Blau Signed-off-by: Junio C Hamano

t5318: avoid top-level directory changes

2023-07-24T21:35:22Z

Avoid changing the current working directory from outside of a sub-shell during the tests in t5318. Each test has mostly straightforward changes, either: - Removing the top-level `cd "$TRASH_DIRECTORY/full"`, which is unnecessary after ensuring that other tests don't change their working directory outside of a sub-shell. - Changing any Git invocations which want to be in a sub-directory by either (a) adding a "-C $DIR" argument, or (b) moving the whole test into a sub-shell. While we're here, remove any explicit "git config core.commitGraph true" invocations which were designed to enable use of the commit-graph. These are unnecessary following 31b1de6a09b (commit-graph: turn on commit-graph by default, 2019-08-13). Signed-off-by: Taylor Blau Signed-off-by: Junio C Hamano

fsck: suppress commit-graph output with `--no-progress`

2023-07-10T17:02:37Z

Since e0fd51e1d7 (fsck: verify commit-graph, 2018-06-27), `fsck` runs `git commit-graph verify` to check the integrity of any commit-graph(s). Originally, the `git commit-graph verify` step would always print to stdout/stderr, regardless of whether or not `fsck` was invoked with `--[no-]progress` or not. But in 7371612255 (commit-graph: add --[no-]progress to write and verify, 2019-08-26), the commit-graph machinery learned the `--[no-]progress` option, though `fsck` was not updated to pass this new flag (or not). This led to seeing output from running `git fsck`, even with `--no-progress` on repositories that have a commit-graph: $ git.compile fsck --connectivity-only --no-progress --no-dangling Verifying commits in commit graph: 100% (4356/4356), done. Verifying commits in commit graph: 100% (131912/131912), done. Ensure that `fsck` passes `--[no-]progress` as appropriate when calling `git commit-graph verify`. Signed-off-by: Taylor Blau Acked-by: Derrick Stolee Signed-off-by: Junio C Hamano

commit-graph: return generation from memory

2023-03-20T19:17:33Z

The commit_graph_generation() method used to report a value of GENERATION_NUMBER_INFINITY if the commit_graph_data_slab had an instance for the given commit but the graph_pos indicated the commit was not in the commit-graph file. However, an upcoming change will introduce the ability to set generation values in-memory without writing the commit-graph file. Thus, we can no longer trust 'graph_pos' to indicate whether or not the generation member can be trusted. Instead, trust the 'generation' member if the commit has a value in the slab _and_ the 'generation' member is non-zero. Otherwise, treat it as GENERATION_NUMBER_INFINITY. This only makes a difference for a very old case for the commit-graph: the very first Git release to write commit-graph files wrote zeroes in the topological level positions. If we are parsing a commit-graph with all zeroes, those commits will now appear to have GENERATION_NUMBER_INFINITY (as if they were not parsed from the commit-graph). I attempted several variations to work around the need for providing an uninitialized 'generation' member, but this was the best one I found. It does require a change to a verification test in t5318 because it reports a different error than the one about non-zero generation numbers. Signed-off-by: Derrick Stolee Signed-off-by: Junio C Hamano

Merge branch 'sg/parse-options-subcommand'

2022-09-01T20:40:18Z

Introduce the "subcommand" mode to parse-options API and update the command line parser of Git commands with subcommands. * sg/parse-options-subcommand: (23 commits) remote: run "remote rm" argv through parse_options() maintenance: add parse-options boilerplate for subcommands pass subcommand "prefix" arguments to parse_options() builtin/worktree.c: let parse-options parse subcommands builtin/stash.c: let parse-options parse subcommands builtin/sparse-checkout.c: let parse-options parse subcommands builtin/remote.c: let parse-options parse subcommands builtin/reflog.c: let parse-options parse subcommands builtin/notes.c: let parse-options parse subcommands builtin/multi-pack-index.c: let parse-options parse subcommands builtin/hook.c: let parse-options parse subcommands builtin/gc.c: let parse-options parse 'git maintenance's subcommands builtin/commit-graph.c: let parse-options parse subcommands builtin/bundle.c: let parse-options parse subcommands parse-options: add support for parsing subcommands parse-options: drop leading space from '--git-completion-helper' output parse-options: clarify the limitations of PARSE_OPT_NODASH parse-options: PARSE_OPT_KEEP_UNKNOWN only applies to --options api-parse-options.txt: fix description of OPT_CMDMODE t0040-parse-options: test parse_options() with various 'parse_opt_flags' ...

builtin/commit-graph.c: let parse-options parse subcommands

2022-08-19T18:13:15Z

'git commit-graph' parses its subcommands with an if-else if statement. parse-options has just learned to parse subcommands, so let's use that facility instead, with the benefits of shorter code, handling missing or unknown subcommands, and listing subcommands for Bash completion. Note that the functions implementing each subcommand only accept the 'argc' and '**argv' parameters, so add a (unused) '*prefix' parameter to make them match the type expected by parse-options, and thus avoid casting function pointers. Signed-off-by: SZEDER Gábor Signed-off-by: Junio C Hamano

Merge branch 'tb/commit-graph-genv2-upgrade-fix'

2022-08-03T20:36:08Z

There was a bug in the codepath to upgrade generation information in commit-graph from v1 to v2 format, which has been corrected. * tb/commit-graph-genv2-upgrade-fix: commit-graph: fix corrupt upgrade from generation v1 to v2 commit-graph: introduce `repo_find_commit_pos_in_graph()` t5318: demonstrate commit-graph generation v2 corruption

commit-graph: fix corrupt upgrade from generation v1 to v2

2022-07-15T23:51:39Z

The previous commit demonstrates a bug where a commit-graph using generation v2 could enter a state where one of the GDA2 values has its most-significant bit set (indicating that its value should be read from the extended offset table in the GDO2 chunk) without having a GDO2 chunk to read from. This results in the following error message being displayed to the caller: fatal: commit-graph requires overflow generation data but has none This bug arises in the following scenario: - We decide to write a commit-graph using generation number v2, and decide (correctly) that no GDO2 chunk is necessary (e.g., because all of the commiter date offsets are no larger than 2^31-1). - The v2 generation numbers are stored in the `->generation` member of the commit slab holding `struct commit_graph_data`'s. - Later on, `load_commit_graph_info()` is called, overwriting the v2 generation data in the aforementioned slab with any existing v1 generation data. Then, when the commit-graph code goes to write the GDA2 chunk via `write_graph_chunk_generation_data()`, we use the overwritten generation v1 data in a place where we expect to use a v2 generation number: offset = commit_graph_data_at(c)->generation - c->date; ...because `commit_graph_data_at(c)->generation` used to hold the v2 generation data, but it was overwritten to contain the v1 generation number via `load_commit_graph_info()`. If the `offset` computation above overflows the v2 generation number max, then `write_graph_chunk_generation_data()` will update its count of large offsets and write the marker accordingly: if (offset > GENERATION_NUMBER_V2_OFFSET_MAX) { offset = CORRECTED_COMMIT_DATE_OFFSET_OVERFLOW | num_generation_data_overflows; num_generation_data_overflows++; } and reads will look for the GDO2 chunk containing the overflowing v2 generation number, *after* the commit-graph code decided that no such chunk was necessary. The main problem is that the slab containing `struct commit_graph_data` has a dual purpose. It is used to hold data that we are about to write to disk while generating a commit-graph, as well as hold data that was read from an existing commit-graph. When the two mix, namely when the result of reading the commit-graph has a side-effect that mixes poorly with an in-progress commit-graph write, we end up with corrupt data. A complete fix might be to introduce a new slab that is used exclusively for writing, and gate access between the two slabs based on context provided by the caller (e.g., whether this computation is part of a "read" or "write" operation). But a more minimal fix addresses the only known path which overwrites the slab data, which is `compute_bloom_filters()` -> `get_or_compute_bloom_filter()` -> `load_commit_graph_info()` -> `fill_commit_graph_info()` by avoiding the last call which clobbers the data altogether. This path only needs to learn the graph position of a given commit so that it can be used in `load_bloom_filter_from_graph()`. By replacing the last steps of the above with one that records the graph position into a temporary variable which is then used to load the existing Bloom data, we eliminate the clobbering, removing the corruption. Signed-off-by: Taylor Blau Signed-off-by: Junio C Hamano