summaryrefslogtreecommitdiff
path: root/builtin
AgeCommit message (Collapse)AuthorFilesLines
2021-09-14midx.c: respect 'pack.writeBitmapHashcache' when writing bitmapsLibravatar Taylor Blau1-0/+21
In the previous commit, the bitmap writing code learned to propagate values from an existing hash-cache extension into the bitmap that it is writing. Now that that functionality exists, let's expose it by teaching the 'git multi-pack-index' builtin to respect the `pack.writeBitmapHashCache` option so that the hash-cache may be written at all. Two minor points worth noting here: - The 'git multi-pack-index write' sub-command didn't previously read any configuration (instead this is handled in the base command). A separate handler is added here to respect this write-specific config option. - I briefly considered adding a 'bitmap_flags' field to the static options struct, but decided against it since it would require plumbing through a new parameter to the write_midx_file() function. Instead, a new MIDX-specific flag is added, which is translated to the corresponding bitmap one. Signed-off-by: Taylor Blau <me@ttaylorr.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-09-01midx: respect 'GIT_TEST_MULTI_PACK_INDEX_WRITE_BITMAP'Libravatar Taylor Blau1-2/+10
Introduce a new 'GIT_TEST_MULTI_PACK_INDEX_WRITE_BITMAP' environment variable to also write a multi-pack bitmap when 'GIT_TEST_MULTI_PACK_INDEX' is set. Signed-off-by: Taylor Blau <me@ttaylorr.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-09-01pack-bitmap: write multi-pack bitmapsLibravatar Taylor Blau1-0/+2
Write multi-pack bitmaps in the format described by Documentation/technical/bitmap-format.txt, inferring their presence with the absence of '--bitmap'. To write a multi-pack bitmap, this patch attempts to reuse as much of the existing machinery from pack-objects as possible. Specifically, the MIDX code prepares a packing_data struct that pretends as if a single packfile has been generated containing all of the objects contained within the MIDX. Signed-off-by: Taylor Blau <me@ttaylorr.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-09-01pack-bitmap: read multi-pack bitmapsLibravatar Taylor Blau1-0/+5
This prepares the code in pack-bitmap to interpret the new multi-pack bitmaps described in Documentation/technical/bitmap-format.txt, which mostly involves converting bit positions to accommodate looking them up in a MIDX. Note that there are currently no writers who write multi-pack bitmaps, and that this will be implemented in the subsequent commit. Note also that get_midx_checksum() and get_midx_filename() are made non-static so they can be called from pack-bitmap.c. Signed-off-by: Taylor Blau <me@ttaylorr.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-09-01midx: avoid opening multiple MIDXs when writingLibravatar Taylor Blau1-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>
2021-08-24pack-bitmap-write.c: gracefully fail to write non-closed bitmapsLibravatar Taylor Blau1-1/+2
The set of objects covered by a bitmap must be closed under reachability, since it must be the case that there is a valid bit position assigned for every possible reachable object (otherwise the bitmaps would be incomplete). Pack bitmaps are never written from 'git repack' unless repacking all-into-one, and so we never write non-closed bitmaps (except in the case of partial clones where we aren't guaranteed to have all objects). But multi-pack bitmaps change this, since it isn't known whether the set of objects in the MIDX is closed under reachability until walking them. Plumb through a bit that is set when a reachable object isn't found. As soon as a reachable object isn't found in the set of objects to include in the bitmap, bitmap_writer_build() knows that the set is not closed, and so it now fails gracefully. A test is added in t0410 to trigger a bitmap write without full reachability closure by removing local copies of some reachable objects from a promisor remote. Signed-off-by: Taylor Blau <me@ttaylorr.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-08-04Merge branch 'rs/use-fspathhash'Libravatar Junio C Hamano1-8/+2
Code simplification. * rs/use-fspathhash: use fspathhash() everywhere
2021-08-04Merge branch 'pb/merge-autostash-more'Libravatar Junio C Hamano1-1/+3
The local changes stashed by "git merge --autostash" were lost when the merge failed in certain ways, which has been corrected. * pb/merge-autostash-more: merge: apply autostash if merge strategy fails merge: apply autostash if fast-forward fails Documentation: define 'MERGE_AUTOSTASH' merge: add missing word "strategy" to a message
2021-08-04Merge branch 'ds/commit-and-checkout-with-sparse-index'Libravatar Junio C Hamano2-5/+6
"git checkout" and "git commit" learn to work without unnecessarily expanding sparse indexes. * ds/commit-and-checkout-with-sparse-index: unpack-trees: resolve sparse-directory/file conflicts t1092: document bad 'git checkout' behavior checkout: stop expanding sparse indexes sparse-index: recompute cache-tree commit: integrate with sparse-index p2000: compress repo names p2000: add 'git checkout -' test and decrease depth
2021-08-04Merge branch 'ah/plugleaks'Libravatar Junio C Hamano5-14/+17
Leak plugging. * ah/plugleaks: reset: clear_unpack_trees_porcelain to plug leak builtin/rebase: fix options.strategy memory lifecycle builtin/merge: free found_ref when done builtin/mv: free or UNLEAK multiple pointers at end of cmd_mv convert: release strbuf to avoid leak read-cache: call diff_setup_done to avoid leak ref-filter: also free head for ATOM_HEAD to avoid leak diffcore-rename: move old_dir/new_dir definition to plug leak builtin/for-each-repo: remove unnecessary argv copy to plug leak builtin/submodule--helper: release unused strbuf to avoid leak environment: move strbuf into block to plug leak fmt-merge-msg: free newly allocated temporary strings when done
2021-08-04Merge branch 'ar/submodule-add'Libravatar Junio C Hamano1-113/+304
Rewrite of "git submodule" in C continues. * ar/submodule-add: submodule: drop unused sm_name parameter from show_fetch_remotes() submodule--helper: introduce add-clone subcommand submodule--helper: refactor module_clone() submodule: prefix die messages with 'fatal' t7400: test failure to add submodule in tracked path
2021-07-30use fspathhash() everywhereLibravatar René Scharfe1-8/+2
cf2dc1c238 (speed up alt_odb_usable() with many alternates, 2021-07-07) introduced the function fspathhash() for calculating path hashes while respecting the configuration option core.ignorecase. Call it instead of open-coding it; the resulting code is shorter and less repetitive. Signed-off-by: René Scharfe <l.s.r@web.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-07-28Merge branch 'tb/reverse-midx'Libravatar Junio C Hamano1-1/+1
The code that gives an error message in "git multi-pack-index" when no subcommand is given tried to print a NULL pointer as a strong, which has been corrected. * tb/reverse-midx: multi-pack-index: fix potential segfault without sub-command
2021-07-28Merge branch 'ds/status-with-sparse-index'Libravatar Junio C Hamano1-0/+3
"git status" codepath learned to work with sparsely populated index without hydrating it fully. * ds/status-with-sparse-index: t1092: document bad sparse-checkout behavior fsmonitor: integrate with sparse index wt-status: expand added sparse directory entries status: use sparse-index throughout status: skip sparse-checkout percentage with sparse-index diff-lib: handle index diffs with sparse dirs dir.c: accept a directory as part of cone-mode patterns unpack-trees: unpack sparse directory entries unpack-trees: rename unpack_nondirectories() unpack-trees: compare sparse directories correctly unpack-trees: preserve cache_bottom t1092: add tests for status/add and sparse files t1092: expand repository data shape t1092: replace incorrect 'echo' with 'cat' sparse-index: include EXTENDED flag when expanding sparse-index: skip indexes with unmerged entries
2021-07-28Merge branch 'dl/diff-merge-base'Libravatar Junio C Hamano1-2/+2
"git diff --merge-base" documentation has been updated. * dl/diff-merge-base: git-diff: fix missing --merge-base docs
2021-07-28Merge branch 'ab/attribute-format'Libravatar Junio C Hamano4-6/+6
Many "printf"-like helper functions we have have been annotated with __attribute__() to catch placeholder/parameter mismatches. * ab/attribute-format: advice.h: add missing __attribute__((format)) & fix usage *.h: add a few missing __attribute__((format)) *.c static functions: add missing __attribute__((format)) sequencer.c: move static function to avoid forward decl *.c static functions: don't forward-declare __attribute__
2021-07-28Merge branch 'jk/log-decorate-optim'Libravatar Junio C Hamano1-0/+3
Optimize "git log" for cases where we wasted cycles to load ref decoration data that may not be needed. * jk/log-decorate-optim: load_ref_decorations(): fix decoration with tags add_ref_decoration(): rename s/type/deco_type/ load_ref_decorations(): avoid parsing non-tag objects object.h: add lookup_object_by_type() function object.h: expand docstring for lookup_unknown_object() log: avoid loading decorations for userformats that don't need it pretty.h: update and expand docstring for userformat_find_requirements()
2021-07-28Merge branch 'sm/worktree-add-lock'Libravatar Junio C Hamano1-5/+16
"git worktree add --lock" learned to record why the worktree is locked with a custom message. * sm/worktree-add-lock: worktree: teach `add` to accept --reason <string> with --lock worktree: mark lock strings with `_()` for translation t2400: clean up '"add" worktree with lock' test
2021-07-28Merge branch 'hj/commit-allow-empty-message'Libravatar Junio C Hamano1-11/+18
"git commit --allow-empty-message" won't abort the operation upon an empty message, but the hint shown in the editor said otherwise. * hj/commit-allow-empty-message: commit: remove irrelavent prompt on `--allow-empty-message` commit: reorganise commit hint strings
2021-07-26builtin/rebase: fix options.strategy memory lifecycleLibravatar Andrzej Hunt1-1/+2
- cmd_rebase populates rebase_options.strategy with newly allocated strings, hence we need to free those strings at the end of cmd_rebase to avoid a leak. - In some cases: get_replay_opts() is called, which prepares replay_opts using data from rebase_options. We used to simply copy the pointer from rebase_options.strategy, however that would now result in a double-free because sequencer_remove_state() is eventually used to free replay_opts.strategy. To avoid this we xstrdup() strategy when adding it to replay_opts. The original leak happens because we always populate rebase_options.strategy, but we don't always enter the path that calls get_replay_opts() and later sequencer_remove_state() - in other words we'd always allocate a new string into rebase_options.strategy but only sometimes did we free it. We now make sure that rebase_options and replay_opts both own their own copies of strategy, and each copy is free'd independently. This was first seen when running t0021 with LSAN, but t2012 helped catch the fact that we can't just free(options.strategy) at the end of cmd_rebase (as that can cause a double-free). LSAN output from t0021: LSAN output from t0021: Direct leak of 4 byte(s) in 1 object(s) allocated from: #0 0x486804 in strdup ../projects/compiler-rt/lib/asan/asan_interceptors.cpp:452:3 #1 0xa71eb8 in xstrdup wrapper.c:29:14 #2 0x61b1cc in cmd_rebase builtin/rebase.c:1779:22 #3 0x4ce83e in run_builtin git.c:475:11 #4 0x4ccafe in handle_builtin git.c:729:3 #5 0x4cb01c in run_argv git.c:818:4 #6 0x4cb01c in cmd_main git.c:949:19 #7 0x6b3fad in main common-main.c:52:11 #8 0x7f267b512349 in __libc_start_main (/lib64/libc.so.6+0x24349) SUMMARY: AddressSanitizer: 4 byte(s) leaked in 1 allocation(s). Signed-off-by: Andrzej Hunt <andrzej@ahunt.org> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-07-26builtin/merge: free found_ref when doneLibravatar Andrzej Hunt1-1/+2
merge_name() calls dwim_ref(), which allocates a new string into found_ref. Therefore add a free() to avoid leaking found_ref. LSAN output from t0021: Direct leak of 16 byte(s) in 1 object(s) allocated from: #0 0x486804 in strdup ../projects/compiler-rt/lib/asan/asan_interceptors.cpp:452:3 #1 0xa8beb8 in xstrdup wrapper.c:29:14 #2 0x954054 in expand_ref refs.c:671:12 #3 0x953cb6 in repo_dwim_ref refs.c:644:22 #4 0x5d3759 in dwim_ref refs.h:162:9 #5 0x5d3759 in merge_name builtin/merge.c:517:6 #6 0x5d3759 in collect_parents builtin/merge.c:1214:5 #7 0x5cf60d in cmd_merge builtin/merge.c:1458:16 #8 0x4ce83e in run_builtin git.c:475:11 #9 0x4ccafe in handle_builtin git.c:729:3 #10 0x4cb01c in run_argv git.c:818:4 #11 0x4cb01c in cmd_main git.c:949:19 #12 0x6bdbfd in main common-main.c:52:11 #13 0x7f0430502349 in __libc_start_main (/lib64/libc.so.6+0x24349) SUMMARY: AddressSanitizer: 16 byte(s) leaked in 1 allocation(s). Signed-off-by: Andrzej Hunt <andrzej@ahunt.org> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-07-26builtin/mv: free or UNLEAK multiple pointers at end of cmd_mvLibravatar Andrzej Hunt1-0/+5
These leaks all happen at the end of cmd_mv, hence don't matter in any way. But we still fix the easy ones and squash the rest to get us closer to being able to run tests without leaks. LSAN output from t0050: Direct leak of 384 byte(s) in 1 object(s) allocated from: #0 0x49ab49 in realloc ../projects/compiler-rt/lib/asan/asan_malloc_linux.cpp:164:3 #1 0xa8c015 in xrealloc wrapper.c:126:8 #2 0xa0a7e1 in add_entry string-list.c:44:2 #3 0xa0a7e1 in string_list_insert string-list.c:58:14 #4 0x5dac03 in cmd_mv builtin/mv.c:248:4 #5 0x4ce83e in run_builtin git.c:475:11 #6 0x4ccafe in handle_builtin git.c:729:3 #7 0x4cb01c in run_argv git.c:818:4 #8 0x4cb01c in cmd_main git.c:949:19 #9 0x6bd9ad in main common-main.c:52:11 #10 0x7fbfeffc4349 in __libc_start_main (/lib64/libc.so.6+0x24349) Direct leak of 16 byte(s) in 1 object(s) allocated from: #0 0x49a82d in malloc ../projects/compiler-rt/lib/asan/asan_malloc_linux.cpp:145:3 #1 0xa8bd09 in do_xmalloc wrapper.c:41:8 #2 0x5dbc34 in internal_prefix_pathspec builtin/mv.c:32:2 #3 0x5da575 in cmd_mv builtin/mv.c:158:14 #4 0x4ce83e in run_builtin git.c:475:11 #5 0x4ccafe in handle_builtin git.c:729:3 #6 0x4cb01c in run_argv git.c:818:4 #7 0x4cb01c in cmd_main git.c:949:19 #8 0x6bd9ad in main common-main.c:52:11 #9 0x7fbfeffc4349 in __libc_start_main (/lib64/libc.so.6+0x24349) Direct leak of 16 byte(s) in 1 object(s) allocated from: #0 0x49a82d in malloc ../projects/compiler-rt/lib/asan/asan_malloc_linux.cpp:145:3 #1 0xa8bd09 in do_xmalloc wrapper.c:41:8 #2 0x5dbc34 in internal_prefix_pathspec builtin/mv.c:32:2 #3 0x5da4e4 in cmd_mv builtin/mv.c:148:11 #4 0x4ce83e in run_builtin git.c:475:11 #5 0x4ccafe in handle_builtin git.c:729:3 #6 0x4cb01c in run_argv git.c:818:4 #7 0x4cb01c in cmd_main git.c:949:19 #8 0x6bd9ad in main common-main.c:52:11 #9 0x7fbfeffc4349 in __libc_start_main (/lib64/libc.so.6+0x24349) Direct leak of 8 byte(s) in 1 object(s) allocated from: #0 0x49a9a2 in calloc ../projects/compiler-rt/lib/asan/asan_malloc_linux.cpp:154:3 #1 0xa8c119 in xcalloc wrapper.c:140:8 #2 0x5da585 in cmd_mv builtin/mv.c:159:22 #3 0x4ce83e in run_builtin git.c:475:11 #4 0x4ccafe in handle_builtin git.c:729:3 #5 0x4cb01c in run_argv git.c:818:4 #6 0x4cb01c in cmd_main git.c:949:19 #7 0x6bd9ad in main common-main.c:52:11 #8 0x7fbfeffc4349 in __libc_start_main (/lib64/libc.so.6+0x24349) Direct leak of 4 byte(s) in 1 object(s) allocated from: #0 0x49a9a2 in calloc ../projects/compiler-rt/lib/asan/asan_malloc_linux.cpp:154:3 #1 0xa8c119 in xcalloc wrapper.c:140:8 #2 0x5da4f8 in cmd_mv builtin/mv.c:149:10 #3 0x4ce83e in run_builtin git.c:475:11 #4 0x4ccafe in handle_builtin git.c:729:3 #5 0x4cb01c in run_argv git.c:818:4 #6 0x4cb01c in cmd_main git.c:949:19 #7 0x6bd9ad in main common-main.c:52:11 #8 0x7fbfeffc4349 in __libc_start_main (/lib64/libc.so.6+0x24349) Indirect leak of 65 byte(s) in 1 object(s) allocated from: #0 0x49ab49 in realloc ../projects/compiler-rt/lib/asan/asan_malloc_linux.cpp:164:3 #1 0xa8c015 in xrealloc wrapper.c:126:8 #2 0xa00226 in strbuf_grow strbuf.c:98:2 #3 0xa00226 in strbuf_vaddf strbuf.c:394:3 #4 0xa065c7 in xstrvfmt strbuf.c:981:2 #5 0xa065c7 in xstrfmt strbuf.c:991:8 #6 0x9e7ce7 in prefix_path_gently setup.c:115:15 #7 0x9e7fa6 in prefix_path setup.c:128:12 #8 0x5dbdbf in internal_prefix_pathspec builtin/mv.c:55:23 #9 0x5da575 in cmd_mv builtin/mv.c:158:14 #10 0x4ce83e in run_builtin git.c:475:11 #11 0x4ccafe in handle_builtin git.c:729:3 #12 0x4cb01c in run_argv git.c:818:4 #13 0x4cb01c in cmd_main git.c:949:19 #14 0x6bd9ad in main common-main.c:52:11 #15 0x7fbfeffc4349 in __libc_start_main (/lib64/libc.so.6+0x24349) Indirect leak of 65 byte(s) in 1 object(s) allocated from: #0 0x49ab49 in realloc ../projects/compiler-rt/lib/asan/asan_malloc_linux.cpp:164:3 #1 0xa8c015 in xrealloc wrapper.c:126:8 #2 0xa00226 in strbuf_grow strbuf.c:98:2 #3 0xa00226 in strbuf_vaddf strbuf.c:394:3 #4 0xa065c7 in xstrvfmt strbuf.c:981:2 #5 0xa065c7 in xstrfmt strbuf.c:991:8 #6 0x9e7ce7 in prefix_path_gently setup.c:115:15 #7 0x9e7fa6 in prefix_path setup.c:128:12 #8 0x5dbdbf in internal_prefix_pathspec builtin/mv.c:55:23 #9 0x5da4e4 in cmd_mv builtin/mv.c:148:11 #10 0x4ce83e in run_builtin git.c:475:11 #11 0x4ccafe in handle_builtin git.c:729:3 #12 0x4cb01c in run_argv git.c:818:4 #13 0x4cb01c in cmd_main git.c:949:19 #14 0x6bd9ad in main common-main.c:52:11 #15 0x7fbfeffc4349 in __libc_start_main (/lib64/libc.so.6+0x24349) SUMMARY: AddressSanitizer: 558 byte(s) leaked in 7 allocation(s). Signed-off-by: Andrzej Hunt <andrzej@ahunt.org> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-07-26builtin/for-each-repo: remove unnecessary argv copy to plug leakLibravatar Andrzej Hunt1-10/+4
cmd_for_each_repo() copies argv into args (a strvec), which is later passed into run_command_on_repo(), which in turn copies that strvec onto the end of child.args. The initial copy is unnecessary (we never modify args). We therefore choose to just pass argv directly into run_command_on_repo(), which lets us avoid the copy and fixes the leak. LSAN output from t0068: Direct leak of 192 byte(s) in 1 object(s) allocated from: #0 0x7f63bd4ab8b0 in realloc (/usr/lib64/libasan.so.4+0xdc8b0) #1 0x98d7e6 in xrealloc wrapper.c:126 #2 0x916914 in strvec_push_nodup strvec.c:19 #3 0x916a6e in strvec_push strvec.c:26 #4 0x4be4eb in cmd_for_each_repo builtin/for-each-repo.c:49 #5 0x410dcd in run_builtin git.c:475 #6 0x410dcd in handle_builtin git.c:729 #7 0x414087 in run_argv git.c:818 #8 0x414087 in cmd_main git.c:949 #9 0x40e9ec in main common-main.c:52 #10 0x7f63bc9fa349 in __libc_start_main (/lib64/libc.so.6+0x24349) Indirect leak of 22 byte(s) in 2 object(s) allocated from: #0 0x7f63bd445e30 in __interceptor_strdup (/usr/lib64/libasan.so.4+0x76e30) #1 0x98d698 in xstrdup wrapper.c:29 #2 0x916a63 in strvec_push strvec.c:26 #3 0x4be4eb in cmd_for_each_repo builtin/for-each-repo.c:49 #4 0x410dcd in run_builtin git.c:475 #5 0x410dcd in handle_builtin git.c:729 #6 0x414087 in run_argv git.c:818 #7 0x414087 in cmd_main git.c:949 #8 0x40e9ec in main common-main.c:52 #9 0x7f63bc9fa349 in __libc_start_main (/lib64/libc.so.6+0x24349) See also discussion about the original implementation below - this code appears to have evolved from a callback explaining the double-strvec-copy pattern, but there's no strong reason to keep that now: https://lore.kernel.org/git/68bbeca5-314b-08ee-ef36-040e3f3814e9@gmail.com/ Signed-off-by: Andrzej Hunt <andrzej@ahunt.org> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-07-26builtin/submodule--helper: release unused strbuf to avoid leakLibravatar Andrzej Hunt1-2/+4
relative_url() populates sb. In the normal return path, its buffer is detached using strbuf_detach(). However the early return path does nothing with sb, which means that sb's memory is leaked - therefore we add a release to avoid this leak. The reset is also only necessary for the normal return path, hence we move it down to after the early-return to avoid unnecessary work. LSAN output from t0060: Direct leak of 121 byte(s) in 1 object(s) allocated from: #0 0x7f31246f28b0 in realloc (/usr/lib64/libasan.so.4+0xdc8b0) #1 0x98d7d6 in xrealloc wrapper.c:126 #2 0x909a60 in strbuf_grow strbuf.c:98 #3 0x90bf00 in strbuf_vaddf strbuf.c:401 #4 0x90c321 in strbuf_addf strbuf.c:335 #5 0x5cb78d in relative_url builtin/submodule--helper.c:182 #6 0x5cbe46 in resolve_relative_url_test builtin/submodule--helper.c:248 #7 0x410dcd in run_builtin git.c:475 #8 0x410dcd in handle_builtin git.c:729 #9 0x414087 in run_argv git.c:818 #10 0x414087 in cmd_main git.c:949 #11 0x40e9ec in main common-main.c:52 #12 0x7f3123c41349 in __libc_start_main (/lib64/libc.so.6+0x24349) SUMMARY: AddressSanitizer: 121 byte(s) leaked in 1 allocation(s). Signed-off-by: Andrzej Hunt <andrzej@ahunt.org> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-07-26submodule: drop unused sm_name parameter from show_fetch_remotes()Libravatar Jeff King1-3/+2
This parameter has not been used since the function was introduced in 8c8195e9c3 (submodule--helper: introduce add-clone subcommand, 2021-07-10). Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-07-23merge: apply autostash if merge strategy failsLibravatar Philippe Blain1-0/+1
Since 'git merge' learned '--autostash' in a03b55530a (merge: teach --autostash option, 2020-04-07), 'cmd_merge', once it is determined that we have to create a merge commit, calls 'create_autostash' if '--autostash' is given. As explained in a03b55530a, and made more abvious by the tests added in that commit, the autostash is then applied if the merge succeeds, either directly or by committing (after conflict resolution or if '--no-commit' was given), or if the merge is aborted with 'git merge --abort'. In some other cases, like the user calling 'git reset --merge' or 'git merge --quit', the autostash is not applied, but saved in the stash list. However, there exists a scenario that creates an autostash but does not apply nor save it to the stash list: if the chosen merge strategy completely fails to handle the merge, i.e. 'try_merge_strategy' returns 2. Apply the autostash in that case also. An easy way to test that is to try to merge more than two commits but explicitely ask for the 'recursive' merge strategy. Signed-off-by: Philippe Blain <levraiphilippeblain@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-07-23merge: apply autostash if fast-forward failsLibravatar Philippe Blain1-0/+1
Since 'git merge' learned '--autostash' in a03b55530a (merge: teach --autostash option, 2020-04-07), 'cmd_merge', in the fast-forward case, calls 'create_autostash' before calling 'checkout_fast_forward' if '--autostash' is given. However, if 'checkout_fast_forward' fails, the autostash is not applied to the working tree, nor saved in the stash list, since the code simply calls 'goto done'. Be more helpful to the user by applying the autostash in that case. An easy way to test a failing fast-forward is when we are merging a branch that has a tracked file that conflicts with an untracked file in the working tree. Signed-off-by: Philippe Blain <levraiphilippeblain@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-07-23merge: add missing word "strategy" to a messageLibravatar Philippe Blain1-1/+1
The variable 'best_strategy' holds the name of the merge strategy that resulted in fewer conflicts, if several strategies were tried. When that's the case but the best strategy was not the first one tried, we inform the user which strategy was the "best" one before recreating the merge and leaving the conflicted files in the tree. This informational message is missing the word "strategy", so it shows something like: Using the recursive to prepare resolving by hand. Fix that. Signed-off-by: Philippe Blain <levraiphilippeblain@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-07-22Merge branch 'bc/rev-list-without-commit-line'Libravatar Junio C Hamano1-9/+24
"git rev-list" learns to omit the "commit <object-name>" header lines from the output with the `--no-commit-header` option. * bc/rev-list-without-commit-line: rev-list: add option for --pretty=format without header
2021-07-19multi-pack-index: fix potential segfault without sub-commandLibravatar Taylor Blau1-1/+1
Since cd57bc41bb (builtin/multi-pack-index.c: display usage on unrecognized command, 2021-03-30) we have used a "usage" label to avoid having two separate callers of usage_with_options (one when no arguments are given, and another for unrecognized sub-commands). But the first caller has been broken since cd57bc41bb, since it will happily jump to usage without arguments, and then pass argv[0] to the "unrecognized subcommand" error. Many compilers will save us from a segfault here, but the end result is ugly, since it mentions an unrecognized subcommand when we didn't even pass one, and (on GCC) includes "(null)" in its output. Move the "usage" label down past the error about unrecognized subcommands so that it is only triggered when it should be. While we're at it, bulk up our test coverage in this area, too. Signed-off-by: Taylor Blau <me@ttaylorr.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-07-16Merge branch 'ab/struct-init'Libravatar Junio C Hamano6-14/+7
Code cleanup around struct_type_init() functions. * ab/struct-init: string-list.h users: change to use *_{nodup,dup}() string-list.[ch]: add a string_list_init_{nodup,dup}() dir.[ch]: replace dir_init() with DIR_INIT *.c *_init(): define in terms of corresponding *_INIT macro *.h: move some *_INIT to designated initializers
2021-07-16Merge branch 'ar/help-micro-cleanup'Libravatar Junio C Hamano1-9/+8
Tiny code clean-up. * ar/help-micro-cleanup: help: convert git_cmd to page in one place
2021-07-16Merge branch 'ar/submodule-helper-include-cleanup'Libravatar Junio C Hamano1-1/+0
Code clean-up. * ar/submodule-helper-include-cleanup: submodule--helper: remove redundant include
2021-07-16Merge branch 'ab/bundle-updates'Libravatar Junio C Hamano1-27/+47
Code clean-up and leak plugging in "git bundle". * ab/bundle-updates: bundle: remove "ref_list" in favor of string-list.c API bundle.c: use a temporary variable for OIDs and names bundle cmd: stop leaking memory from parse_options_cmd_bundle()
2021-07-16Merge branch 'ab/show-branch-tests'Libravatar Junio C Hamano1-3/+6
Fill test gaps. * ab/show-branch-tests: show-branch tests: add missing tests show-branch: don't <COLOR></RESET> for space characters show-branch tests: modernize test code show-branch tests: rename the one "show-branch" test file
2021-07-16Merge branch 'ab/fetch-negotiate-segv-fix'Libravatar Junio C Hamano1-0/+3
Code recently added to support common ancestry negotiation during "git push" did not sanity check its arguments carefully enough. * ab/fetch-negotiate-segv-fix: fetch: fix segfault in --negotiate-only without --negotiation-tip=* fetch: document the --negotiate-only option send-pack.c: move "no refs in common" abort earlier
2021-07-15worktree: teach `add` to accept --reason <string> with --lockLibravatar Stephen Manz1-5/+16
The default reason stored in the lock file, "added with --lock", is unlikely to be what the user would have given in a separate `git worktree lock` command. Allowing `--reason` to be specified along with `--lock` when adding a working tree gives the user control over the reason for locking without needing a second command. Signed-off-by: Stephen Manz <smanz@alum.mit.edu> Reviewed-by: Eric Sunshine <sunshine@sunshineco.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-07-14checkout: stop expanding sparse indexesLibravatar Derrick Stolee1-5/+3
Previous changes did the necessary improvements to unpack-trees.c and diff-lib.c in order to modify a sparse index based on its comparision with a tree. The only remaining work is to remove some ensure_full_index() calls and add tests that verify that the index is not expanded in our interesting cases. Include 'switch' and 'restore' in these tests, as they share a base implementation with 'checkout'. Here are the relevant performance results from p2000-sparse-operations.sh: Test HEAD~1 HEAD -------------------------------------------------------------------------------- 2000.18: git checkout -f - (full-v3) 0.49(0.43+0.03) 0.47(0.39+0.05) -4.1% 2000.19: git checkout -f - (full-v4) 0.45(0.37+0.06) 0.42(0.37+0.05) -6.7% 2000.20: git checkout -f - (sparse-v3) 0.76(0.71+0.07) 0.04(0.03+0.04) -94.7% 2000.21: git checkout -f - (sparse-v4) 0.75(0.72+0.04) 0.05(0.06+0.04) -93.3% It is important to compare the full index case to the sparse index case, as the previous results for the sparse index were inflated by the index expansion. For index v4, this is an 88% improvement. On an internal repository with over two million paths at HEAD and a sparse-checkout definition containing ~60,000 of those paths, 'git checkout' went from 3.5s to 297ms with this change. The theoretical optimum where only those ~60,000 paths exist was 275ms, so the extra sparse directory entries contribute a 22ms overhead. Signed-off-by: Derrick Stolee <dstolee@microsoft.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-07-14commit: integrate with sparse-indexLibravatar Derrick Stolee1-0/+3
Update 'git commit' to allow using the sparse-index in memory without expanding to a full one. The only place that had an ensure_full_index() call was in cache_tree_update(). The recursive algorithm for update_one() was already updated in 2de37c536 (cache-tree: integrate with sparse directory entries, 2021-03-03) to handle sparse directory entries in the index. Most of this change involves testing different command-line options that allow specifying which on-disk changes should be included in the commit. This includes no options (only take currently-staged changes), -a (take all tracked changes), and --include (take a list of specific changes). To simplify testing that these options do not expand the index, update the test that previously verified that 'git status' does not expand the index with a helper method, ensure_not_expanded(). This allows 'git commit' to operate much faster when the sparse-checkout cone is much smaller than the full list of files at HEAD. Here are the relevant lines from p2000-sparse-operations.sh: Test HEAD~1 HEAD ---------------------------------------------------------------------------------- 2000.14: git commit -a -m A (full-v3) 0.35(0.26+0.06) 0.36(0.28+0.07) +2.9% 2000.15: git commit -a -m A (full-v4) 0.32(0.26+0.05) 0.34(0.28+0.06) +6.3% 2000.16: git commit -a -m A (sparse-v3) 0.63(0.59+0.06) 0.04(0.05+0.05) -93.7% 2000.17: git commit -a -m A (sparse-v4) 0.64(0.59+0.08) 0.04(0.04+0.04) -93.8% It is important to compare the full-index case to the sparse-index case, so the improvement for index version v4 is actually an 88% improvement in this synthetic example. In a real repository with over two million files at HEAD and 60,000 files in the sparse-checkout definition, the time for 'git commit -a' went from 2.61 seconds to 134ms. I compared this to the result if the index only contained the paths in the sparse-checkout definition and found the theoretical optimum to be 120ms, so the out-of-cone paths only add a 12% overhead. Signed-off-by: Derrick Stolee <dstolee@microsoft.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-07-14status: use sparse-index throughoutLibravatar Derrick Stolee1-0/+3
By testing 'git -c core.fsmonitor= status -uno', we can check for the simplest index operations that can be made sparse-aware. The necessary implementation details are already integrated with sparse-checkout, so modify command_requires_full_index to be zero for cmd_status(). In refresh_index(), we loop through the index entries to refresh their stat() information. However, sparse directories have no stat() information to populate. Ignore these entries. This allows 'git status' to no longer expand a sparse index to a full one. This is further tested by dropping the "-uno" option and adding an untracked file into the worktree. The performance test p2000-sparse-checkout-operations.sh demonstrates these improvements: Test HEAD~1 HEAD ----------------------------------------------------------------------------- 2000.2: git status (full-index-v3) 0.31(0.30+0.05) 0.31(0.29+0.06) +0.0% 2000.3: git status (full-index-v4) 0.31(0.29+0.07) 0.34(0.30+0.08) +9.7% 2000.4: git status (sparse-index-v3) 2.35(2.28+0.10) 0.04(0.04+0.05) -98.3% 2000.5: git status (sparse-index-v4) 2.35(2.24+0.15) 0.05(0.04+0.06) -97.9% Note that since HEAD~1 was expanding the sparse index by parsing trees, it was artificially slower than the full index case. Thus, the 98% improvement is misleading, and instead we should celebrate the 0.34s to 0.05s improvement of 85%. This is more indicative of the peformance gains we are expecting by using a sparse index. Note: we are dropping the assignment of core.fsmonitor here. This is not necessary for the test script as we are not altering the config any other way. Correct integration with FS Monitor will be validated in later changes. Reviewed-by: Elijah Newren <newren@gmail.com> Signed-off-by: Derrick Stolee <dstolee@microsoft.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-07-14worktree: mark lock strings with `_()` for translationLibravatar Stephen Manz1-2/+2
- default lock string, "added with --lock" - temporary lock string, "initializing" Signed-off-by: Stephen Manz <smanz@alum.mit.edu> Reviewed-by: Eric Sunshine <sunshine@sunshineco.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-07-13Merge branch 'ab/pickaxe-pcre2'Libravatar Junio C Hamano2-7/+2
Rewrite the backend for "diff -G/-S" to use pcre2 engine when available. * ab/pickaxe-pcre2: (22 commits) xdiff-interface: replace discard_hunk_line() with a flag xdiff users: use designated initializers for out_line pickaxe -G: don't special-case create/delete pickaxe -G: terminate early on matching lines xdiff-interface: allow early return from xdiff_emit_line_fn xdiff-interface: prepare for allowing early return pickaxe -S: slightly optimize contains() pickaxe: rename variables in has_changes() for brevity pickaxe -S: support content with NULs under --pickaxe-regex pickaxe: assert that we must have a needle under -G or -S pickaxe: refactor function selection in diffcore-pickaxe() perf: add performance test for pickaxe pickaxe/style: consolidate declarations and assignments diff.h: move pickaxe fields together again pickaxe: die when --find-object and --pickaxe-all are combined pickaxe: die when -G and --pickaxe-regex are combined pickaxe tests: add missing test for --no-pickaxe-regex being an error pickaxe tests: test for -G, -S and --find-object incompatibility pickaxe tests: add test for "log -S" not being a regex pickaxe tests: add test for diffgrep_consume() internals ...
2021-07-13Merge branch 'fc/push-simple-updates-cleanup'Libravatar Junio C Hamano1-59/+34
Some more code and doc clarification around "git push". * fc/push-simple-updates-cleanup: push: don't get a full remote object push: only check same_remote when needed push: remove trivial function push: remove redundant check push: factor out the typical case push: get rid of all the setup_push_* functions push: trivial simplifications push: make setup_push_* return the dst push: only get the branch when needed push: factor out null branch check push: split switch cases push: return immediately in trivial switch case push: create new get_upstream_ref() helper
2021-07-13Merge branch 'fc/push-simple-updates'Libravatar Junio C Hamano1-16/+32
Some code and doc clarification around "git push". * fc/push-simple-updates: doc: push: explain default=simple correctly push: remove unused code in setup_push_upstream() push: simplify setup_push_simple() push: reorganize setup_push_simple() push: copy code to setup_push_simple() push: hedge code of default=simple push: rename !triangular to same_remote
2021-07-13Merge branch 'zh/cat-file-batch-fix'Libravatar Junio C Hamano1-6/+4
"git cat-file --batch-all-objects"" misbehaved when "--batch" is in use and did not ask for certain object traits. * zh/cat-file-batch-fix: cat-file: merge two block into one cat-file: handle trivial --batch format with --batch-all-objects
2021-07-13*.c static functions: add missing __attribute__((format))Libravatar Ævar Arnfjörð Bjarmason2-0/+3
Add missing __attribute__((format)) function attributes to various "static" functions that take printf arguments. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-07-12git-diff: fix missing --merge-base docsLibravatar Denton Liu1-2/+2
When `git diff --merge-base` was introduced at around Git 2.30, the documentation included a few errors. In the example given for `git diff --cached --merge-base`, the `--cached` flag was omitted for the `--merge-base` example. Add the missing flag. In the `git diff <commit>` case, we failed to mention that `--merge-base` is an available option. Give the usage of `--merge-base` as an option there. Finally, there are two errors in the usage of `git diff`. Firstly, we do not mention `--merge-base` in the `git diff --cached` case. Mention it so that it's consistent with the documentation. Secondly, we put the `[--merge-base]` in between `<commit>` and `[<commit>...]`. Move the `[--merge-base]` so that it's beside `[<options>]` which is a more logical grouping. Signed-off-by: Denton Liu <liu.denton@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-07-12*.c static functions: don't forward-declare __attribute__Libravatar Ævar Arnfjörð Bjarmason2-6/+3
9cf6d3357aa (Add git-index-pack utility, 2005-10-12) and 466dbc42f58 (receive-pack: Send internal errors over side-band #2, 2010-02-10) we added these static functions and forward-declared their __attribute__((printf)). I think this may have been to work around some compiler limitation at the time, but in any case we have a lot of code that uses the briefer way of declaring these that I'm using here, so if we had any such issues with compilers we'd have seen them already. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-07-12submodule--helper: introduce add-clone subcommandLibravatar Atharva Raykar1-0/+177
Let's add a new "add-clone" subcommand to `git submodule--helper` with the goal of converting part of the shell code in git-submodule.sh related to `git submodule add` into C code. This new subcommand clones the repository that is to be added, and checks out to the appropriate branch. This is meant to be a faithful conversion that leaves the behaviour of 'cmd_add()' script unchanged. Signed-off-by: Atharva Raykar <raykar.ath@gmail.com> Mentored-by: Christian Couder <christian.couder@gmail.com> Mentored-by: Shourya Shukla <periperidip@gmail.com> Based-on-patch-by: Shourya Shukla <periperidip@gmail.com> Based-on-patch-by: Prathamesh Chavan <pc44800@gmail.com> Helped-by: Đoàn Trần Công Danh <congdanhqx@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-07-12submodule--helper: refactor module_clone()Libravatar Atharva Raykar1-113/+128
Separate out the core logic of module_clone() from the flag parsing---this way we can call the equivalent of the `submodule--helper clone` subcommand directly within C, without needing to push arguments in a strvec. Signed-off-by: Atharva Raykar <raykar.ath@gmail.com> Mentored-by: Christian Couder <christian.couder@gmail.com> Mentored-by: Shourya Shukla <periperidip@gmail.com> Suggested-by: Junio C Hamano <gitster@pobox.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>