summaryrefslogtreecommitdiff
path: root/builtin/commit-graph.c
Commit message (Collapse)AuthorAgeFilesLines
* object-store-ll.h: split this header out of object-store.hElijah Newren2023-05-161-1/+1
| | | | | | | | | | | | | | | | | The vast majority of files including object-store.h did not need dir.h nor khash.h. Split the header into two files, and let most just depend upon object-store-ll.h, while letting the two callers that need it depend on the full object-store.h. After this patch: $ git grep -h include..object-store | sort | uniq -c 2 #include "object-store.h" 129 #include "object-store-ll.h" Diff best viewed with `--color-moved`. Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* builtin.h: remove unneccessary includesElijah Newren2023-05-161-0/+1
| | | | | | | | | This also made it clear that a few .c files under builtin/ were depending upon some headers but had forgotten to #include them. Add the missing direct includes while at it. Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* treewide: be explicit about dependence on trace.h & trace2.hElijah Newren2023-04-111-0/+1
| | | | | | | | | | | Dozens of files made use of trace and trace2 functions, without explicitly including trace.h or trace2.h. This made it more difficult to find which files could remove a dependence on cache.h. Make C files explicitly include trace.h or trace2.h if they are using them. Signed-off-by: Elijah Newren <newren@gmail.com> Acked-by: Calvin Wan <calvinwan@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* environment.h: move declarations for environment.c functions from cache.hElijah Newren2023-03-211-0/+1
| | | | | Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* treewide: be explicit about dependence on gettext.hElijah Newren2023-03-211-0/+1
| | | | | | | | | | | | | | Dozens of files made use of gettext functions, without explicitly including gettext.h. This made it more difficult to find which files could remove a dependence on cache.h. Make C files explicitly include gettext.h if they are using it. However, while compat/fsmonitor/fsm-ipc-darwin.c should also gain an include of gettext.h, it was left out to avoid conflicting with an in-flight topic. Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* replace-object.h: move read_replace_refs declaration from cache.h to hereElijah Newren2023-02-231-0/+1
| | | | | | | | Adjust several files to be more explicit about their dependency on replace-objects to accommodate this change. Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* cache.h: remove dependence on hex.h; make other files include it explicitlyElijah Newren2023-02-231-0/+1
| | | | | Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* commit-graph: fix a parse_options_concat() leakÆvar Arnfjörð Bjarmason2023-02-061-2/+2
| | | | | | | | | | | | When the parse_options_concat() was added to this file in 84e4484f128 (commit-graph: use parse_options_concat(), 2021-08-23) we wouldn't free() it if we returned early in these cases. Since "result" is 0 by default we can "goto cleanup" in both cases, and only need to set "result" if write_commit_graph_reachable() fails. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* commit-graph: use free_commit_graph() instead of UNLEAK()Ævar Arnfjörð Bjarmason2023-02-061-2/+4
| | | | | | | | | | In 0bfb48e6723 (builtin/commit-graph.c: UNLEAK variables, 2018-10-03) this was made to UNLEAK(), but we can just as easily invoke the free_commit_graph() function added in c3756d5b7fc (commit-graph: add free_commit_graph, 2018-07-11) instead. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* Merge branch 'ab/doc-synopsis-and-cmd-usage'Junio C Hamano2022-10-281-5/+5
|\ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The short-help text shown by "git cmd -h" and the synopsis text shown at the beginning of "git help cmd" have been made more consistent. * ab/doc-synopsis-and-cmd-usage: (34 commits) tests: assert consistent whitespace in -h output tests: start asserting that *.txt SYNOPSIS matches -h output doc txt & -h consistency: make "worktree" consistent worktree: define subcommand -h in terms of command -h reflog doc: list real subcommands up-front doc txt & -h consistency: make "commit" consistent doc txt & -h consistency: make "diff-tree" consistent doc txt & -h consistency: use "[<label>...]" for "zero or more" doc txt & -h consistency: make "annotate" consistent doc txt & -h consistency: make "stash" consistent doc txt & -h consistency: add missing options doc txt & -h consistency: use "git foo" form, not "git-foo" doc txt & -h consistency: make "bundle" consistent doc txt & -h consistency: make "read-tree" consistent doc txt & -h consistency: make "rerere" consistent doc txt & -h consistency: add missing options and labels doc txt & -h consistency: make output order consistent doc txt & -h consistency: add or fix optional "--" syntax doc txt & -h consistency: fix mismatching labels doc SYNOPSIS & -h: use "-" to separate words in labels, not "_" ...
| * doc txt & -h consistency: fix mismatching labelsÆvar Arnfjörð Bjarmason2022-10-131-2/+2
| | | | | | | | | | | | | | | | | | | | | | | | Fix various inconsistencies between command SYNOPSIS and the corresponding -h output where our translatable labels didn't match up. In some cases we need to adjust the prose that follows the SYNOPSIS accordingly, as it refers back to the changed label. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * doc txt & -h consistency: correct padding around "[]()"Ævar Arnfjörð Bjarmason2022-10-131-1/+1
| | | | | | | | | | | | | | | | | | | | The whitespace padding of alternatives should be of the form "[-f | --force]" not "[-f|--force]". Likewise we should not have padding before the first option, so "(--all | <pack-filename>...)" is correct, not "( --all | <pack-filename>... )". Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * doc txt & -h consistency: word-wrapÆvar Arnfjörð Bjarmason2022-10-131-4/+4
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Change the documentation and -h output for those built-in commands where both the -h output and *.txt were lacking in word-wrapping. There are many more built-ins that could use this treatment, this change is narrowed to those where this whitespace change is needed to make the -h and *.txt consistent in the end. In the case of "Documentation/git-hash-object.txt" and "builtin/hash-object.c" this is not a "doc txt & -h consistency" change, as we're changing both versions, doing so here makes a subsequent change smaller. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | Merge branch 'ab/unused-annotation'Junio C Hamano2022-09-141-1/+1
|\ \ | | | | | | | | | | | | | | | | | | | | | | | | Undoes 'jk/unused-annotation' topic and redoes it to work around Coccinelle rules misfiring false positives in unrelated codepaths. * ab/unused-annotation: git-compat-util.h: use "deprecated" for UNUSED variables git-compat-util.h: use "UNUSED", not "UNUSED(var)"
| * | git-compat-util.h: use "UNUSED", not "UNUSED(var)"Ævar Arnfjörð Bjarmason2022-09-011-1/+1
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | As reported in [1] the "UNUSED(var)" macro introduced in 2174b8c75de (Merge branch 'jk/unused-annotation' into next, 2022-08-24) breaks coccinelle's parsing of our sources in files where it occurs. Let's instead partially go with the approach suggested in [2] of making this not take an argument. As noted in [1] "coccinelle" will ignore such tokens in argument lists that it doesn't know about, and it's less of a surprise to syntax highlighters. This undoes the "help us notice when a parameter marked as unused is actually use" part of 9b240347543 (git-compat-util: add UNUSED macro, 2022-08-19), a subsequent commit will further tweak the macro to implement a replacement for that functionality. 1. https://lore.kernel.org/git/220825.86ilmg4mil.gmgdl@evledraar.gmail.com/ 2. https://lore.kernel.org/git/220819.868rnk54ju.gmgdl@evledraar.gmail.com/ Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | | Merge branch 'jk/unused-annotation'Junio C Hamano2022-09-141-1/+1
|\ \ \ | |/ / | | / | |/ |/| | | | | | | | | | | | | | | | | | | | | | | | | | | | | Annotate function parameters that are not used (but cannot be removed for structural reasons), to prepare us to later compile with -Wunused warning turned on. * jk/unused-annotation: is_path_owned_by_current_uid(): mark "report" parameter as unused run-command: mark unused async callback parameters mark unused read_tree_recursive() callback parameters hashmap: mark unused callback parameters config: mark unused callback parameters streaming: mark unused virtual method parameters transport: mark bundle transport_options as unused refs: mark unused virtual method parameters refs: mark unused reflog callback parameters refs: mark unused each_ref_fn parameters git-compat-util: add UNUSED macro
| * config: mark unused callback parametersJeff King2022-08-191-1/+1
| | | | | | | | | | | | | | | | | | | | | | | | | | | | The callback passed to git_config() must conform to a particular interface. But most callbacks don't actually look at the extra "void *data" parameter. Let's mark the unused parameters to make -Wunused-parameter happy. Note there's one unusual case here in get_remote_default() where we actually ignore the "value" parameter. That's because it's only checking whether the option is found at all, and not parsing its value. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | pass subcommand "prefix" arguments to parse_options()Jeff King2022-08-251-2/+2
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Recent commits such as bf0a6b65fc (builtin/multi-pack-index.c: let parse-options parse subcommands, 2022-08-19) converted a few functions to match our usual argc/argv/prefix conventions, but the prefix argument remains unused. However, there is a good use for it: they should pass it to their own parse_options() functions, where it may be used to adjust the value of any filename options. In all but one of these functions, there's no behavior change, since they don't use OPT_FILENAME. But this is an actual fix for one option, which you can see by modifying the test suite like so: diff --git a/t/t5326-multi-pack-bitmaps.sh b/t/t5326-multi-pack-bitmaps.sh index 4fe57414c1..d0974d4371 100755 --- a/t/t5326-multi-pack-bitmaps.sh +++ b/t/t5326-multi-pack-bitmaps.sh @@ -186,7 +186,11 @@ test_expect_success 'writing a bitmap with --refs-snapshot' ' # Then again, but with a refs snapshot which only sees # refs/tags/one. - git multi-pack-index write --bitmap --refs-snapshot=snapshot && + ( + mkdir subdir && + cd subdir && + git multi-pack-index write --bitmap --refs-snapshot=../snapshot + ) && test_path_is_file $midx && test_path_is_file $midx-$(midx_checksum $objdir).bitmap && I'd emphasize that this wasn't broken by bf0a6b65fc; it has been broken all along, because the sub-function never got to see the prefix. It is that commit which is actually enabling us to fix it (and which also brought attention to the problem because it triggers -Wunused-parameter!) The other functions changed here don't use OPT_FILENAME at all. In their cases this isn't fixing anything visible, but it's following the usual pattern and future-proofing them against somebody adding new options and being surprised. I didn't include a test for the one visible case above. We don't generally test routine parse-options behavior for individual options. The challenge here was finding the problem, and now that this has been done, it's not likely to regress. Likewise, we could apply the patch above to cover it "for free" but it makes reading the rest of the test unnecessarily complicated. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | builtin/commit-graph.c: let parse-options parse subcommandsSZEDER Gábor2022-08-191-17/+13
|/ | | | | | | | | | | | | | | | '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 <szeder.dev@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* commit-graph: fix memory leak in misused string_list APIÆvar Arnfjörð Bjarmason2022-03-041-3/+3
| | | | | | | | | | | | | | | | | | | | | | When this code was migrated to the string_list API in d88b14b3fd6 (commit-graph: use string-list API for input, 2018-06-27) it was made to use used both STRING_LIST_INIT_NODUP and a strbuf_detach() pattern. Those should not be used together if string_list_clear() is expected to free the memory, instead we need to either use STRING_LIST_INIT_DUP with a string_list_append_nodup(), or a STRING_LIST_INIT_NODUP and manually fiddle with the "strdup_strings" member before calling string_list_clear(). Let's do the former. Since "strdup_strings = 1" is set now other code might be broken by relying on "pack_indexes" not to duplicate it strings, but that doesn't happen. When we pass this down to write_commit_graph() that code uses the "struct string_list" without modifying it. Let's add a "const" to the variable to have the compiler enforce that assumption. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* Merge branch 'ab/ignore-replace-while-working-on-commit-graph'Junio C Hamano2021-11-011-1/+1
|\ | | | | | | | | | | | | | | | | | | | | Teach "git commit-graph" command not to allow using replace objects at all, as we do not use the commit-graph at runtime when we see object replacement. * ab/ignore-replace-while-working-on-commit-graph: commit-graph: don't consider "replace" objects with "verify" commit-graph tests: fix another graph_git_two_modes() helper commit-graph tests: fix error-hiding graph_git_two_modes() helper
| * commit-graph: don't consider "replace" objects with "verify"Ævar Arnfjörð Bjarmason2021-10-151-1/+1
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Extend the code added in d6538246d3d (commit-graph: not compatible with replace objects, 2018-08-20) which ignored replace objects in the "write" command to ignore it in the "verify" command too. We can just move this assignment to the cmd_commit_graph(), it dispatches to "write" and "verify", and we're unlikely to ever get a sub-command that would like to consider replace refs. This will make tests added in eddc1f556cd (mktag tests: test update-ref and reachable fsck, 2021-06-17) pass in combination with the "GIT_TEST_COMMIT_GRAPH" mode added in 859fdc0c3cf (commit-graph: define GIT_TEST_COMMIT_GRAPH, 2018-08-29), except that mode is currently broken (but is being fixed concurrently). See the discussion starting at [1]. 1. https://lore.kernel.org/git/87wnmihswp.fsf@evledraar.gmail.com/ Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | Merge branch 'ab/parse-options-cleanup'Junio C Hamano2021-10-251-2/+2
|\ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Random changes to parse-options implementation. * ab/parse-options-cleanup: parse-options: change OPT_{SHORT,UNSET} to an enum parse-options tests: test optname() output parse-options.[ch]: make opt{bug,name}() "static" commit-graph: stop using optname() parse-options.c: move optname() earlier in the file parse-options.h: make the "flags" in "struct option" an enum parse-options.c: use exhaustive "case" arms for "enum parse_opt_result" parse-options.[ch]: consistently use "enum parse_opt_result" parse-options.[ch]: consistently use "enum parse_opt_flags" parse-options.h: move PARSE_OPT_SHELL_EVAL between enums
| * | commit-graph: stop using optname()Ævar Arnfjörð Bjarmason2021-10-081-2/+2
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Stop using optname() in builtin/commit-graph.c to emit an error with the --max-new-filters option. This changes code added in 809e0327f57 (builtin/commit-graph.c: introduce '--max-new-filters=<n>', 2020-09-18). See 9440b831ad5 (parse-options: replace opterror() with optname(), 2018-11-10) for why using optname() like this is considered bad, i.e. it's assembling human-readable output piecemeal, and the "option `X'" at the start can't be translated. It didn't matter in this case, but this code was also buggy in its use of "opt->flags" to optname(), that function expects flags, but not *those* flags. Let's pass "max-new-filters" to the new error because the option name isn't translatable, and because we can re-use a translation added in f7e68a08780 (parse-options: check empty value in OPT_INTEGER and OPT_ABBREV, 2019-05-29). Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | | Merge branch 'tb/commit-graph-usage-fix'Junio C Hamano2021-10-061-2/+4
|\ \ \ | |/ / |/| | | | | | | | | | | | | | | | | | | | Regression in "git commit-graph" command line parsing has been corrected. * tb/commit-graph-usage-fix: builtin/multi-pack-index.c: disable top-level --[no-]progress builtin/commit-graph.c: don't accept common --[no-]progress
| * | builtin/commit-graph.c: don't accept common --[no-]progressTaylor Blau2021-09-201-2/+4
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | In 84e4484f12 (commit-graph: use parse_options_concat(), 2021-08-23) we unified common options of commit-graph's subcommands into a single "common_opts" array. But 84e4484f12 introduced a behavior change which is to accept the "--[no-]progress" option before any sub-commands, e.g., git commit-graph --progress write ... Prior to that commit, the above would error out with "unknown option". There are two issues with this behavior change. First is that the top-level --[no-]progress is not always respected. This is because isatty(2) is performed in the sub-commands, which unconditionally overwrites any --[no-]progress that was given at the top-level. But the second issue is that the existing sub-commands of commit-graph only happen to both have a sensible interpretation of what `--progress` or `--no-progress` means. If we ever added a sub-command which didn't have a notion of progress, we would be forced to ignore the top-level `--[no-]progress` altogether. Since we haven't released a version of Git that supports --[no-]progress as a top-level option for `git commit-graph`, let's remove it. Suggested-by: SZEDER Gábor <szeder.dev@gmail.com> Signed-off-by: Taylor Blau <me@ttaylorr.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | | Merge branch 'tb/multi-pack-bitmaps'Junio C Hamano2021-09-201-22/+0
|\ \ \ | |/ / |/| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The reachability bitmap file used to be generated only for a single pack, but now we've learned to generate bitmaps for history that span across multiple packfiles. * tb/multi-pack-bitmaps: (29 commits) pack-bitmap: drop bitmap_index argument from try_partial_reuse() pack-bitmap: drop repository argument from prepare_midx_bitmap_git() p5326: perf tests for MIDX bitmaps p5310: extract full and partial bitmap tests midx: respect 'GIT_TEST_MULTI_PACK_INDEX_WRITE_BITMAP' t7700: update to work with MIDX bitmap test knob t5319: don't write MIDX bitmaps in t5319 t5310: disable GIT_TEST_MULTI_PACK_INDEX_WRITE_BITMAP t0410: disable GIT_TEST_MULTI_PACK_INDEX_WRITE_BITMAP t5326: test multi-pack bitmap behavior t/helper/test-read-midx.c: add --checksum mode t5310: move some tests to lib-bitmap.sh pack-bitmap: write multi-pack bitmaps pack-bitmap: read multi-pack bitmaps pack-bitmap.c: avoid redundant calls to try_partial_reuse pack-bitmap.c: introduce 'bitmap_is_preferred_refname()' pack-bitmap.c: introduce 'nth_bitmap_object_oid()' pack-bitmap.c: introduce 'bitmap_num_objects()' midx: avoid opening multiple MIDXs when writing midx: close linked MIDXs, avoid leaking memory ...
| * | midx: avoid opening multiple MIDXs when writingTaylor Blau2021-09-011-22/+0
| |/ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Opening multiple instance of the same MIDX can lead to problems like two separate packed_git structures which represent the same pack being added to the repository's object store. The above scenario can happen because prepare_midx_pack() checks if `m->packs[pack_int_id]` is NULL in order to determine if a pack has been opened and installed in the repository before. But a caller can construct two copies of the same MIDX by calling get_multi_pack_index() and load_multi_pack_index() since the former manipulates the object store directly but the latter is a lower-level routine which allocates a new MIDX for each call. So if prepare_midx_pack() is called on multiple MIDXs with the same pack_int_id, then that pack will be installed twice in the object store's packed_git pointer. This can lead to problems in, for e.g., the pack-bitmap code, which does something like the following (in pack-bitmap.c:open_pack_bitmap()): struct bitmap_index *bitmap_git = ...; for (p = get_all_packs(r); p; p = p->next) { if (open_pack_bitmap_1(bitmap_git, p) == 0) ret = 0; } which is a problem if two copies of the same pack exist in the packed_git list because pack-bitmap.c:open_pack_bitmap_1() contains a conditional like the following: if (bitmap_git->pack || bitmap_git->midx) { /* ignore extra bitmap file; we can only handle one */ warning("ignoring extra bitmap file: %s", packfile->pack_name); close(fd); return -1; } Avoid this scenario by not letting write_midx_internal() open a MIDX that isn't also pointed at by the object store. So long as this is the case, other routines should prefer to open MIDXs with get_multi_pack_index() or reprepare_packed_git() instead of creating instances on their own. Because get_multi_pack_index() returns `r->object_store->multi_pack_index` if it is non-NULL, we'll only have one instance of a MIDX open at one time, avoiding these problems. To encourage this, drop the `struct multi_pack_index *` parameter from `write_midx_internal()`, and rely instead on the `object_dir` to find (or initialize) the correct MIDX instance. Likewise, replace the call to `close_midx()` with `close_object_store()`, since we're about to replace the MIDX with a new one and should invalidate the object store's memory of any MIDX that might have existed beforehand. Note that this now forbids passing object directories that don't belong to alternate repositories over `--object-dir`, since before we would have happily opened a MIDX in any directory, but now restrict ourselves to only those reachable by `r->objects->multi_pack_index` (and alternate MIDXs that we can see by walking the `next` pointer). As far as I can tell, supporting arbitrary directories with `--object-dir` was a historical accident, since even the documentation says `<alt>` when referring to the value passed to this option. Signed-off-by: Taylor Blau <me@ttaylorr.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | commit-graph: show "unexpected subcommand" errorÆvar Arnfjörð Bjarmason2021-08-301-0/+1
| | | | | | | | | | | | | | | | | | | | | | | | | | | | Bring the "commit-graph" command in line with the error output and general pattern in cmd_multi_pack_index(). Let's test for that output, and also cover the same potential bug as was fixed in the multi-pack-index command in 88617d11f9d (multi-pack-index: fix potential segfault without sub-command, 2021-07-19). Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Reviewed-by: Taylor Blau <me@ttaylorr.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | commit-graph: show usage on "commit-graph [write|verify] garbage"Ævar Arnfjörð Bjarmason2021-08-301-0/+4
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Change the parse_options() invocation in the commit-graph code to error on unknown leftover argv elements, in addition to the existing and implicit erroring via parse_options() on unknown options. We'd already error in cmd_commit_graph() on e.g.: git commit-graph unknown verify git commit-graph --unknown verify But here we're calling parse_options() twice more for the "write" and "verify" subcommands. We did not do the same checking for leftover argv elements there. As a result we'd silently accept garbage in these subcommands, let's not do that. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Reviewed-by: Taylor Blau <me@ttaylorr.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | commit-graph: early exit to "usage" on !argcÆvar Arnfjörð Bjarmason2021-08-301-6/+7
| | | | | | | | | | | | | | | | | | | | Rather than guarding all of the !argc with an additional "if" arm let's do an early goto to "usage". This also makes it clear that "save_commit_buffer" is not needed in this case. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Reviewed-by: Taylor Blau <me@ttaylorr.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | commit-graph: use parse_options_concat()Ævar Arnfjörð Bjarmason2021-08-301-16/+21
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Make use of the parse_options_concat() so we don't need to copy/paste common options like --object-dir. This is inspired by a similar change to "checkout" in 2087182272 (checkout: split options[] array in three pieces, 2019-03-29), and the same pattern in the multi-pack-index command, see 60ca94769ce (builtin/multi-pack-index.c: split sub-commands, 2021-03-30). A minor behavior change here is that now we're going to list both --object-dir and --progress first, before we'd list --progress along with other options. Co-authored-by: Taylor Blau <me@ttaylorr.com> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Reviewed-by: Taylor Blau <me@ttaylorr.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | commit-graph: remove redundant handling of -hÆvar Arnfjörð Bjarmason2021-08-301-4/+0
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | If we don't handle the -h option here like most parse_options() users we'll fall through and it'll do the right thing for us. I think this code added in 4ce58ee38d (commit-graph: create git-commit-graph builtin, 2018-04-02) was always redundant, parse_options() did this at the time, and the commit-graph code never used PARSE_OPT_NO_INTERNAL_HELP. We don't need a test for this, it's tested by the t0012-help.sh test added in d691551192a (t0012: test "-h" with builtins, 2017-05-30). Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Reviewed-by: Taylor Blau <me@ttaylorr.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | commit-graph: define common usage with a macroÆvar Arnfjörð Bjarmason2021-08-301-14/+17
|/ | | | | | | | | | | | | | | Share the usage message between these three variables by using a macro. Before this new options needed to copy/paste the usage information, see e.g. 809e0327f5 (builtin/commit-graph.c: introduce '--max-new-filters=<n>', 2020-09-18). See b25b727494f (builtin/multi-pack-index.c: define common usage with a macro, 2021-03-30) for another use of this pattern (but on-list this one came first). Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Reviewed-by: Taylor Blau <me@ttaylorr.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* builtin/*: update usage formatZheNing Hu2021-01-061-3/+3
| | | | | | | | | | According to the guidelines in parse-options.h, we should not end in a full stop or start with a capital letter. Fix old error and usage messages to match this expectation. Signed-off-by: ZheNing Hu <adlternative@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* assert PARSE_OPT_NONEG in parse-options callbacksJeff King2020-09-301-0/+2
| | | | | | | | | | | | | | | | | In the spirit of 517fe807d6 (assert NOARG/NONEG behavior of parse-options callbacks, 2018-11-05), let's cover some parse-options callbacks which expect to be used with PARSE_OPT_NONEG but don't explicitly assert that this is the case. These callbacks are all used correctly in the current code, but this will help document their expectations and future-proof the code. As a bonus, it also silences -Wunused-parameters (these were added since the initial sweep of 517fe807d6, and we can't yet turn on -Wunused-parameters to remind people because it has too many existing false positives). Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* commit-graph: introduce 'commitGraph.maxNewFilters'Taylor Blau2020-09-181-0/+14
| | | | | | | | | | Introduce a configuration variable to specify a default value for the recently-introduce '--max-new-filters' option of 'git commit-graph write'. Helped-by: Junio C Hamano <gitster@pobox.com> Signed-off-by: Taylor Blau <me@ttaylorr.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* builtin/commit-graph.c: introduce '--max-new-filters=<n>'Taylor Blau2020-09-181-2/+25
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Introduce a command-line flag to specify the maximum number of new Bloom filters that a 'git commit-graph write' is willing to compute from scratch. Prior to this patch, a commit-graph write with '--changed-paths' would compute Bloom filters for all selected commits which haven't already been computed (i.e., by a previous commit-graph write with '--split' such that a roll-up or replacement is performed). This behavior can cause prohibitively-long commit-graph writes for a variety of reasons: * There may be lots of filters whose diffs take a long time to generate (for example, they have close to the maximum number of changes, diffing itself takes a long time, etc). * Old-style commit-graphs (which encode filters with too many entries as not having been computed at all) cause us to waste time recomputing filters that appear to have not been computed only to discover that they are too-large. This can make the upper-bound of the time it takes for 'git commit-graph write --changed-paths' to be rather unpredictable. To make this command behave more predictably, introduce '--max-new-filters=<n>' to allow computing at most '<n>' Bloom filters from scratch. This lets "computing" already-known filters proceed quickly, while bounding the number of slow tasks that Git is willing to do. Helped-by: Junio C Hamano <gitster@pobox.com> Signed-off-by: Taylor Blau <me@ttaylorr.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* commit-graph: rename 'split_commit_graph_opts'Taylor Blau2020-09-171-10/+10
| | | | | | | | | | | | | | In the subsequent commit, additional options will be added to the commit-graph API which have nothing to do with splitting. Rename the 'split_commit_graph_opts' structure to the more-generic 'commit_graph_opts' to encompass both. Likewise, rename the 'flags' member to instead be 'split_flags' to clarify that it only has to do with the behavior implied by '--split'. Suggested-by: Derrick Stolee <dstolee@microsoft.com> Signed-off-by: Taylor Blau <me@ttaylorr.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* commit-graph: pass a 'struct repository *' in more placesTaylor Blau2020-09-091-1/+1
| | | | | | | | | | | | In a future commit, some commit-graph internals will want access to 'r->settings', but we only have the 'struct object_directory *' corresponding to that repository. Add an additional parameter to pass the repository around in more places. Signed-off-by: Taylor Blau <me@ttaylorr.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* Merge branch 'ds/commit-graph-bloom-updates' into masterJunio C Hamano2020-07-301-1/+4
|\ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Updates to the changed-paths bloom filter. * ds/commit-graph-bloom-updates: commit-graph: check all leading directories in changed path Bloom filters revision: empty pathspecs should not use Bloom filters revision.c: fix whitespace commit-graph: check chunk sizes after writing commit-graph: simplify chunk writes into loop commit-graph: unify the signatures of all write_graph_chunk_*() functions commit-graph: persist existence of changed-paths bloom: fix logic in get_bloom_filter() commit-graph: change test to die on parse, not load commit-graph: place bloom_settings in context
| * commit-graph: persist existence of changed-pathsDerrick Stolee2020-07-011-1/+4
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The changed-path Bloom filters were released in v2.27.0, but have a significant drawback. A user can opt-in to writing the changed-path filters using the "--changed-paths" option to "git commit-graph write" but the next write will drop the filters unless that option is specified. This becomes even more important when considering the interaction with gc.writeCommitGraph (on by default) or fetch.writeCommitGraph (part of features.experimental). These config options trigger commit-graph writes that the user did not signal, and hence there is no --changed-paths option available. Allow a user that opts-in to the changed-path filters to persist the property of "my commit-graph has changed-path filters" automatically. A user can drop filters using the --no-changed-paths option. In the process, we need to be extremely careful to match the Bloom filter settings as specified by the commit-graph. This will allow future versions of Git to customize these settings, and the version with this change will persist those settings as commit-graphs are rewritten on top. Use the trace2 API to signal the settings used during the write, and check that output in a test after manually adjusting the correct bytes in the commit-graph file. Signed-off-by: Derrick Stolee <dstolee@microsoft.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | Merge branch 'tb/commit-graph-no-check-oids' into masterJunio C Hamano2020-07-151-3/+1
|\ \ | | | | | | | | | | | | | | | | | | | | | Fix to the code to produce progress bar, which is new in the upcoming release. * tb/commit-graph-no-check-oids: commit-graph: fix "Collecting commits from input" progress line
| * | commit-graph: fix "Collecting commits from input" progress lineSZEDER Gábor2020-07-151-3/+1
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | To display a progress line while reading commits from standard input and looking them up, 5b6653e523 (builtin/commit-graph.c: dereference tags in builtin, 2020-05-13) should have added a pair of start_delayed_progress() and stop_progress() calls around the loop reading stdin. Alas, the stop_progress() call ended up at the wrong place, after write_commit_graph(), which does all the commit-graph computation and writing, and has several progress lines of its own. Consequently, that new Collecting commits from input: 1234 progress line is overwritten by the first progress line shown by write_commit_graph(), and its final "done" line is shown last, after everything is finished: $ { sleep 3 ; git rev-list -3 HEAD ; sleep 1 ; } | ~/src/git/git commit-graph write --stdin-commits Expanding reachable commits in commit graph: 873402, done. Writing out commit graph in 4 passes: 100% (3493608/3493608), done. Collecting commits from input: 3, done. Furthermore, that stop_progress() call was added after the 'cleanup' label, where that loop reading stdin jumps in case of an error. In case of invalid input this then results in the "done" line shown after the error message: $ { sleep 3 ; git rev-list -3 HEAD ; echo junk ; } | ~/src/git/git commit-graph write --stdin-commits error: unexpected non-hex object ID: junk Collecting commits from input: 3, done. Move that stop_progress() call to the right place. While at it, drop the unnecessary 'if (progress)' condition protecting the stop_progress() call, because that function is prepared to handle a NULL progress struct. Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com> Reviewed-by: Derrick Stolee <stolee@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | | object: drop parsed_object_pool->commit_countAbhishek Kumar2020-06-171-1/+1
|/ / | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 14ba97f8 (alloc: allow arbitrary repositories for alloc functions, 2018-05-15) introduced parsed_object_pool->commit_count to keep count of commits per repository and was used to assign commit->index. However, commit-slab code requires commit->index values to be unique and a global count would be correct, rather than a per-repo count. Let's introduce a static counter variable, `parsed_commits_count` to keep track of parsed commits so far. As commit_count has no use anymore, let's also drop it from the struct. Signed-off-by: Abhishek Kumar <abhishekkumar8222@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | commit-graph: drop COMMIT_GRAPH_WRITE_CHECK_OIDS flagTaylor Blau2020-05-181-7/+8
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Since 7c5c9b9c57 (commit-graph: error out on invalid commit oids in 'write --stdin-commits', 2019-08-05), the commit-graph builtin dies on receiving non-commit OIDs as input to '--stdin-commits'. This behavior can be cumbersome to work around in, say, the case of piping 'git for-each-ref' to 'git commit-graph write --stdin-commits' if the caller does not want to cull out non-commits themselves. In this situation, it would be ideal if 'git commit-graph write' wrote the graph containing the inputs that did pertain to commits, and silently ignored the remainder of the input. Some options have been proposed to the effect of '--[no-]check-oids' which would allow callers to have the commit-graph builtin do just that. After some discussion, it is difficult to imagine a caller who wouldn't want to pass '--no-check-oids', suggesting that we should get rid of the behavior of complaining about non-commit inputs altogether. If callers do wish to retain this behavior, they can easily work around this change by doing the following: git for-each-ref --format='%(objectname) %(objecttype) %(*objecttype)' | awk ' !/commit/ { print "not-a-commit:"$1 } /commit/ { print $1 } ' | git commit-graph write --stdin-commits To make it so that valid OIDs that refer to non-existent objects are indeed an error after loosening the error handling, perform an extra lookup to make sure that object indeed exists before sending it to the commit-graph internals. Helped-by: Jeff King <peff@peff.net> Signed-off-by: Taylor Blau <me@ttaylorr.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | builtin/commit-graph.c: dereference tags in builtinTaylor Blau2020-05-181-3/+21
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | When given a list of commits, the commit-graph machinery calls 'lookup_commit_reference_gently()' on each element in the set and treats the resulting set of OIDs as the base over which to close for reachability. In an earlier collection of commits, the 'git commit-graph write --reachable' case made the inner-most call to 'lookup_commit_reference_gently()' by peeling references before they were passed over to the commit-graph internals. Do the analog for 'git commit-graph write --stdin-commits' by calling 'lookup_commit_reference_gently()' outside of the commit-graph machinery, making the inner-most call a noop. Since this may incur additional processing time, surround 'read_one_commit' with a progress meter to provide output to the caller. Signed-off-by: Taylor Blau <me@ttaylorr.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | builtin/commit-graph.c: extract 'read_one_commit()'Taylor Blau2020-05-181-29/+29
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | With either '--stdin-commits' or '--stdin-packs', the commit-graph builtin will read line-delimited input, and interpret it either as a series of commit OIDs, or pack names. In a subsequent commit, we will begin handling '--stdin-commits' differently by processing each line as it comes in, instead of in one shot at the end. To make adequate room for this additional logic, split the '--stdin-commits' case from '--stdin-packs' by only storing the input when '--stdin-packs' is given. In the case of '--stdin-commits', feed each line to a new 'read_one_commit' helper, which (for now) will merely call 'parse_oid_hex'. Signed-off-by: Taylor Blau <me@ttaylorr.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | Merge branch 'gs/commit-graph-path-filter'Junio C Hamano2020-05-011-2/+8
|\ \ | |/ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Introduce an extension to the commit-graph to make it efficient to check for the paths that were modified at each commit using Bloom filters. * gs/commit-graph-path-filter: bloom: ignore renames when computing changed paths commit-graph: add GIT_TEST_COMMIT_GRAPH_CHANGED_PATHS test flag t4216: add end to end tests for git log with Bloom filters revision.c: add trace2 stats around Bloom filter usage revision.c: use Bloom filters to speed up path based revision walks commit-graph: add --changed-paths option to write subcommand commit-graph: reuse existing Bloom filters during write commit-graph: write Bloom filters to commit graph file commit-graph: examine commits by generation number commit-graph: examine changed-path objects in pack order commit-graph: compute Bloom filters for changed paths diff: halt tree-diff early after max_changes bloom.c: core Bloom filter implementation for changed paths. bloom.c: introduce core Bloom filter constructs bloom.c: add the murmur3 hash implementation commit-graph: define and use MAX_NUM_CHUNKS
| * commit-graph: add GIT_TEST_COMMIT_GRAPH_CHANGED_PATHS test flagGarima Singh2020-04-061-1/+2
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Add GIT_TEST_COMMIT_GRAPH_CHANGED_PATHS test flag to the test setup suite in order to toggle writing Bloom filters when running any of the git tests. If set to true, we will compute and write Bloom filters every time a test calls `git commit-graph write`, as if the `--changed-paths` option was passed in. The test suite passes when GIT_TEST_COMMIT_GRAPH and GIT_TEST_COMMIT_GRAPH_CHANGED_PATHS are enabled. Helped-by: Derrick Stolee <dstolee@microsoft.com> Signed-off-by: Garima Singh <garima.singh@microsoft.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
View Lorry Controller Status