Bunch of tests are marked as "passing leak check".
* ab/mark-leak-free-tests-more:
merge: add missing strbuf_release()
ls-files: add missing string_list_clear()
ls-files: fix a trivial dir_clear() leak
tests: fix test-oid-array leak, test in SANITIZE=leak
tests: fix a memory leak in test-oidtree.c
tests: fix a memory leak in test-parse-options.c
tests: fix a memory leak in test-prio-queue.c
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
This function was added in 4981fe750b (pkt-line: share
buffer/descriptor reading implementation, 2013-02-23), but in
01f9ec64c8 (Use packet_reader instead of packet_read_line,
2018-12-29) the code that was using it was removed.
Since it's being removed we can in turn remove the "src" and "src_len"
arguments to packet_read(), all the remaining users just passed a
NULL/NULL pair to it.
That function is only a thin wrapper for packet_read_with_status()
which still needs those arguments, but for the thin packet_read()
convenience wrapper we can do away with it for now.
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
"git fsck" has been taught to report mismatch between expected and
actual types of an object better.
* ab/fsck-unexpected-type:
fsck: report invalid object type-path combinations
fsck: don't hard die on invalid object types
object-file.c: stop dying in parse_loose_header()
object-file.c: return ULHR_TOO_LONG on "header too long"
object-file.c: use "enum" return type for unpack_loose_header()
object-file.c: simplify unpack_loose_short_header()
object-file.c: make parse_loose_header_extended() public
object-file.c: return -1, not "status" from unpack_loose_header()
object-file.c: don't set "typep" when returning non-zero
cat-file tests: test for current --allow-unknown-type behavior
cat-file tests: add corrupt loose object test
cat-file tests: test for missing/bogus object with -t, -s and -p
cat-file tests: move bogus_* variable declarations earlier
fsck tests: test for garbage appended to a loose object
fsck tests: test current hash/type mismatch behavior
fsck tests: refactor one test to use a sub-repo
fsck tests: add test for fsck-ing an unknown type
"git clone" from a repository whose HEAD is unborn into a bare
repository didn't follow the branch name the other side used, which
is corrected.
* jk/clone-unborn-head-in-bare:
clone: handle unborn branch in bare repos
"git stash", where the tentative change involves changing a
directory to a file (or vice versa), was confused, which has been
corrected.
* en/stash-df-fix:
stash: restore untracked files AFTER restoring tracked files
stash: avoid feeding directories to update-index
t3903: document a pair of directory/file bugs
When "git am --abort" fails to abort correctly, it still exited
with exit status of 0, which has been corrected.
* en/am-abort-fix:
am: fix incorrect exit status on am fail to abort
t4151: add a few am --abort tests
git-am.txt: clarify --abort behavior
"git update-ref --stdin" failed to flush its output as needed,
which potentially led the conversation to a deadlock.
* ps/update-ref-batch-flush:
t1400: avoid SIGPIPE race condition on fifo
update-ref: fix streaming of status updates
The order in which various files that make up a single (conceptual)
packfile has been reevaluated and straightened up. This matters in
correctness, as an incomplete set of files must not be shown to a
running Git.
* tb/pack-finalize-ordering:
pack-objects: rename .idx files into place after .bitmap files
pack-write: split up finish_tmp_packfile() function
builtin/index-pack.c: move `.idx` files into place last
index-pack: refactor renaming in final()
builtin/repack.c: move `.idx` files into place last
pack-write.c: rename `.idx` files after `*.rev`
pack-write: refactor renaming in finish_tmp_packfile()
bulk-checkin.c: store checksum directly
pack.h: line-wrap the definition of finish_tmp_packfile()
The output from "git fast-export", when its anonymization feature
is in use, showed an annotated tag incorrectly.
* tk/fast-export-anonymized-tag-fix:
fast-export: fix anonymized tag using original length
"git branch -D <branch>" used to refuse to remove a broken branch
ref that points at a missing commit, which has been corrected.
* rs/branch-allow-deleting-dangling:
branch: allow deleting dangling branches with --force
The delayed checkout code path in "git checkout" etc. were chatty
even when --quiet and/or --no-progress options were given.
* mt/quiet-with-delayed-checkout:
checkout: make delayed checkout respect --quiet and --no-progress
"git commit --fixup" now works with "--edit" again, after it was
broken in v2.32.
* jk/commit-edit-fixup-fix:
commit: restore --edit when combined with --fixup
"git pull" had various corner cases that were not well thought out
around its --rebase backend, e.g. "git pull --ff-only" did not stop
but went ahead and rebased when the history on other side is not a
descendant of our history. The series tries to fix them up.
* en/pull-conflicting-options:
pull: fix handling of multiple heads
pull: update docs & code for option compatibility with rebasing
pull: abort by default when fast-forwarding is not possible
pull: make --rebase and --no-rebase override pull.ff=only
pull: since --ff-only overrides, handle it first
pull: abort if --ff-only is given and fast-forwarding is impossible
t7601: add tests of interactions with multiple merge heads and config
t7601: test interaction of merge/rebase/fast-forward flags and options
Bugfix for common ancestor negotiation recently introduced in "git
push" codepath.
* jt/push-negotiation-fixes:
fetch: die on invalid --negotiation-tip hash
send-pack: fix push nego. when remote has refs
send-pack: fix push.negotiate with remote helper
Input validation of "git pack-objects --stdin-packs" has been
corrected.
* ab/pack-stdin-packs-fix:
pack-objects: fix segfault in --stdin-packs option
pack-objects tests: cover blindspots in stdin handling
"git maintenance" scheduler fix for macOS.
* js/maintenance-launchctl-fix:
maintenance: skip bootout/bootstrap when plist is registered
maintenance: create `launchctl` configuration using a lock file
Documentation updates.
* en/merge-strategy-docs:
Update error message and code comment
merge-strategies.txt: add coverage of the `ort` merge strategy
git-rebase.txt: correct out-of-date and misleading text about renames
merge-strategies.txt: fix simple capitalization error
merge-strategies.txt: avoid giving special preference to patience algorithm
merge-strategies.txt: do not imply using copy detection is desired
merge-strategies.txt: update wording for the resolve strategy
Documentation: edit awkward references to `git merge-recursive`
directory-rename-detection.txt: small updates due to merge-ort optimizations
git-rebase.txt: correct antiquated claims about --rebase-merges
"git cat-file --batch" with the "--batch-all-objects" option is
supposed to iterate over all the objects found in a repository, but
it used to translate these object names using the replace mechanism,
which defeats the point of enumerating all objects in the repository.
This has been corrected.
* jk/cat-file-batch-all-wo-replace:
cat-file: use packed_object_info() for --batch-all-objects
cat-file: split ordered/unordered batch-all-objects callbacks
cat-file: disable refs/replace with --batch-all-objects
cat-file: mention --unordered along with --batch-all-objects
t1006: clean up broken objects
Code clean-up.
* ab/designated-initializers-more:
builtin/remote.c: add and use SHOW_INFO_INIT
builtin/remote.c: add and use a REF_STATES_INIT
urlmatch.[ch]: add and use URLMATCH_CONFIG_INIT
builtin/blame.c: refactor commit_info_init() to COMMIT_INFO_INIT macro
daemon.c: refactor hostinfo_init() to HOSTINFO_INIT macro
Use ssh public crypto for object and push-cert signing.
* fs/ssh-signing:
ssh signing: test that gpg fails for unknown keys
ssh signing: tests for logs, tags & push certs
ssh signing: duplicate t7510 tests for commits
ssh signing: verify signatures using ssh-keygen
ssh signing: provide a textual signing_key_id
ssh signing: retrieve a default key from ssh-agent
ssh signing: add ssh key format and signing code
ssh signing: add test prereqs
ssh signing: preliminary refactoring and clean-up
When "cat-file --batch-all-objects" iterates over each object, it knows
where to find each one. But when we look up details of the object, we
don't use that information at all.
This patch teaches it to use the pack/offset pair when we're iterating
over objects in a pack. This yields a measurable speed improvement
(timings on a fully packed clone of linux.git):
Benchmark #1: ./git.old cat-file --batch-all-objects --unordered --batch-check="%(objecttype) %(objectname)"
Time (mean ± σ): 8.128 s ± 0.118 s [User: 7.968 s, System: 0.156 s]
Range (min … max): 8.007 s … 8.301 s 10 runs
Benchmark #2: ./git.new cat-file --batch-all-objects --unordered --batch-check="%(objecttype) %(objectname)"
Time (mean ± σ): 4.294 s ± 0.064 s [User: 4.167 s, System: 0.125 s]
Range (min … max): 4.227 s … 4.457 s 10 runs
Summary
'./git.new cat-file --batch-all-objects --unordered --batch-check="%(objecttype) %(objectname)"' ran
1.89 ± 0.04 times faster than './git.old cat-file --batch-all-objects --unordered --batch-check="%(objecttype) %(objectname)"
The implementation is pretty simple: we just call packed_object_info()
instead of oid_object_info_extended() when we can. Most of the changes
are just plumbing the pack/offset pair through the callstack. There is
one subtlety: replace lookups are not handled by packed_object_info().
But since those are disabled for --batch-all-objects, and since we'll
only have pack info when that option is in effect, we don't have to
worry about that.
There are a few limitations to this optimization which we could address
with further work:
- I didn't bother recording when we found an object loose. Technically
this could save us doing a fruitless lookup in the pack index. But
opening and mmap-ing a loose object is so expensive in the first
place that this doesn't matter much. And if your repository is large
enough to care about per-object performance, most objects are going
to be packed anyway.
- This works only in --unordered mode. For the sorted mode, we'd have
to record the pack/offset pair as part of our oid-collection. That's
more code, plus at least 16 extra bytes of heap per object. It would
probably still be a net win in runtime, but we'd need to measure.
- For --batch, this still helps us with getting the object metadata,
but we still do a from-scratch lookup for the object contents. This
probably doesn't matter that much, because the lookup cost will be
much smaller relative to the cost of actually unpacking and printing
the objects.
For small objects, we could probably swap out read_object_file() for
using packed_object_info() with a "object_info.contentp" to get the
contents. But we'd still need to deal with streaming for larger
objects. A better path forward here is to teach the initial
oid_object_info_extended() / packed_object_info() calls to retrieve
the contents of smaller objects while they are already being
accessed. That would save the extra lookup entirely. But it's a
non-trivial feature to add to the object_info code, so I left it for
now.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
When we originally added --batch-all-objects, it stuffed everything into
an oid_array(), and then iterated over that array with a callback to
write the actual output.
When we later added --unordered, that code path writes immediately as we
discover each object, but just calls the same batch_object_cb() as our
entry point to the writing code. That callback has a narrow interface;
it only receives the oid, but we know much more about each object in the
unordered write (which we'll make use of in the next patch). So let's
just call batch_object_write() directly. The callback wasn't saving us
much effort.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
When we're enumerating all objects in the object database, it doesn't
make sense to respect refs/replace. The point of this option is to
enumerate all of the objects in the database at a low level. By
definition we'd already show the replacement object's contents (under
its real oid), and showing those contents under another oid is almost
certainly working against what the user is trying to do.
Note that you could make the same argument for something like:
git show-index <foo.idx |
awk '{print $2}' |
git cat-file --batch
but there we can't know in cat-file exactly what the user intended,
because we don't know the source of the input. They could be trying to
do low-level debugging, or they could be doing something more high-level
(e.g., imagine a porcelain built around cat-file for its object
accesses). So in those cases, we'll have to rely on the user specifying
"git --no-replace-objects" to tell us what to do.
One _could_ make an argument that "cat-file --batch" is sufficiently
low-level plumbing that it should not respect replace-objects at all
(and the caller should do any replacement if they want it). But we have
been doing so for some time. The history is a little tangled:
- looking back as far as v1.6.6, we would not respect replace refs for
--batch-check, but would for --batch (because the former used
sha1_object_info(), and the replace mechanism only affected actual
object reads)
- this discrepancy was made even weirder by 98e2092b50 (cat-file:
teach --batch to stream blob objects, 2013-07-10), where we always
output the header using the --batch-check code, and then printed the
object separately. This could lead to "cat-file --batch" dying (when
it notices the size or type changed for a non-blob object) or even
producing bogus output (in streaming mode, we didn't notice that we
wrote the wrong number of bytes).
- that persisted until 1f7117ef7a (sha1_file: perform object
replacement in sha1_object_info_extended(), 2013-12-11), which then
respected replace refs for both forms.
So it has worked reliably this way for over 7 years, and we should make
sure it continues to do so. That could also be an argument that
--batch-all-objects should not change behavior (which this patch is
doing), but I really consider the current behavior to be an unintended
bug. It's a side effect of how the code is implemented (feeding the oids
back into oid_object_info() rather than looking at what we found while
reading the loose and packed object storage).
The implementation is straight-forward: we just disable the global
read_replace_refs flag when we're in --batch-all-objects mode. It would
perhaps be a little cleaner to change the flag we pass to
oid_object_info_extended(), but that's not enough. We also read objects
via read_object_file() and stream_blob_to_fd(). The former could switch
to its _extended() form, but the streaming code has no mechanism for
disabling replace refs. Setting the global flag works, and as a bonus,
it's impossible to have any "oops, we're sometimes replacing the object
and sometimes not" bugs in the output (like the ones caused by
98e2092b50 above).
The tests here cover the regular-input and --batch-all-objects cases,
for both --batch-check and --batch. There is a test in t6050 that covers
the regular-input case with --batch already, but this new one goes much
further in actually verifying the output (plus covering --batch-check
explicitly). This is perhaps a little overkill and the tests would be
simpler just covering --batch-check, but I wanted to make sure we're
checking that --batch output is consistent between the header and the
content. The global-flag technique used here makes that easy to get
right, but this is future-proofing us against regressions.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Stop using optname() in builtin/commit-graph.c to emit an error with
the --max-new-filters option. This changes code added in 809e0327f5
(builtin/commit-graph.c: introduce '--max-new-filters=<n>',
2020-09-18).
See 9440b831ad (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
f7e68a0878 (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>
Use the "enum parse_opt_result" instead of an "int flags" as the
return value of the applicable functions in parse-options.c.
This will help catch future bugs, such as the missing "case" arms in
the two existing users of the API in "blame.c" and "shortlog.c". A
third caller in 309be813c9 (update-index: migrate to parse-options
API, 2010-12-01) was already checking for these.
As can be seen when trying to sort through the deluge of warnings
produced when compiling this with CC=g++ (mostly unrelated to this
change) we're not consistently using "enum parse_opt_result" even now,
i.e. we'll return error() and "return 0;". See f41179f16b
(parse-options: avoid magic return codes, 2019-01-27) for a commit
which started changing some of that.
I'm not doing any more of that exhaustive migration here, and it's
probably not worthwhile past the point of being able to check "enum
parse_opt_result" in switch().
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
We strbuf_reset() this "struct strbuf" in a loop earlier, but never
freed it. Plugs a memory leak that's been here ever since this code
got introduced in 1c7b76be7d (Build in merge, 2008-07-07).
This takes us from 68 failed tests in "t7600-merge.sh" to 59 under
SANITIZE=leak, and makes "t7604-merge-custom-message.sh" pass!
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Fix a memory leak that's been here ever since 72aeb18772 (clean.c,
ls-files.c: respect encapsulation of exclude_list_groups, 2013-01-16),
we dup'd the argument in option_parse_exclude(), but never freed the
string_list.
This makes almost all of t3001-ls-files-others-exclude.sh pass (it had
a lot of failures before). Let's mark it as passing with
TEST_PASSES_SANITIZE_LEAK=true, and then exclude the tests that still
failed with a !SANITIZE_LEAK prerequisite check until we fix those
leaks. We can still see the failed tests under
GIT_TEST_FAIL_PREREQS=true.
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Fix an edge case that was missed when the dir_clear() call was added
in eceba53214 (dir: fix problematic API to avoid memory leaks,
2020-08-18), we need to also clean up when we're about to exit with
non-zero.
That commit says, on the topic of the dir_clear() API and UNLEAK():
[...]two of them clearly thought about leaks since they had an
UNLEAK(dir) directive, which to me suggests that the method to
free the data was too unclear.
I think that 0e5bba53af (add UNLEAK annotation for reducing leak
false positives, 2017-09-08) which added the UNLEAK() makes it clear
that that wasn't the case, rather it was the desire to avoid the
complexity of freeing the memory at the end of the program.
This does add a bit of complexity, but I think it's worth it to just
fix these leaks when it's easy in built-ins. It allows them to serve
as canaries for underlying APIs that shouldn't be leaking, it
encourages us to make those freeing APIs nicer for all their users,
and it prevents other leaking regressions by being able to mark the
entire test as TEST_PASSES_SANITIZE_LEAK=true.
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
"git repack" has been taught to generate multi-pack reachability
bitmaps.
* tb/repack-write-midx:
builtin/repack.c: pass `--refs-snapshot` when writing bitmaps
builtin/repack.c: make largest pack preferred
builtin/repack.c: support writing a MIDX while repacking
builtin/repack.c: extract showing progress to a variable
builtin/repack.c: rename variables that deal with non-kept packs
builtin/repack.c: keep track of existing packs unconditionally
midx: preliminary support for `--refs-snapshot`
builtin/multi-pack-index.c: support `--stdin-packs` mode
midx: expose `write_midx_file_only()` publicly
When "git cmd -h" shows more than one line of usage text (e.g.
the cmd subcommand may take sub-sub-command), parse-options API
learned to align these lines, even across i18n/l10n.
* ab/align-parse-options-help:
parse-options: properly align continued usage output
git rev-parse --parseopt tests: add more usagestr tests
send-pack: properly use parse_options() API for usage string
parse-options API users: align usage output in C-strings
Teach "git help -c" into helping the command line completion of
configuration variables.
* ab/help-config-vars:
help: move column config discovery to help.c library
help / completion: make "git help" do the hard work
help tests: test --config-for-completion option & output
help: simplify by moving to OPT_CMDMODE()
help: correct logic error in combining --all and --guides
help: correct logic error in combining --all and --config
help tests: add test for --config output
help: correct usage & behavior of "git help --guides"
help: correct the usage string in -h and documentation
Mostly preliminary clean-up in the hook API.
* ab/config-based-hooks-1:
hook-list.h: add a generated list of hooks, like config-list.h
hook.c users: use "hook_exists()" instead of "find_hook()"
hook.c: add a hook_exists() wrapper and use it in bugreport.c
hook.[ch]: move find_hook() from run-command.c to hook.c
Makefile: remove an out-of-date comment
Makefile: don't perform "mv $@+ $@" dance for $(GENERATED_H)
Makefile: stop hardcoding {command,config}-list.h
Makefile: mark "check" target as .PHONY
Various fixes in code paths that move untracked files away to make room.
* en/removing-untracked-fixes:
Documentation: call out commands that nuke untracked files/directories
Comment important codepaths regarding nuking untracked files/dirs
unpack-trees: avoid nuking untracked dir in way of locally deleted file
unpack-trees: avoid nuking untracked dir in way of unmerged file
Change unpack_trees' 'reset' flag into an enum
Remove ignored files by default when they are in the way
unpack-trees: make dir an internal-only struct
unpack-trees: introduce preserve_ignored to unpack_trees_options
read-tree, merge-recursive: overwrite ignored files by default
checkout, read-tree: fix leak of unpack_trees_options.dir
t2500: add various tests for nuking untracked files
"git add", "git mv", and "git rm" have been adjusted to avoid
updating paths outside of the sparse-checkout definition unless
the user specifies a "--sparse" option.
* ds/add-rm-with-sparse-index:
advice: update message to suggest '--sparse'
mv: refuse to move sparse paths
rm: skip sparse paths with missing SKIP_WORKTREE
rm: add --sparse option
add: update --renormalize to skip sparse paths
add: update --chmod to skip sparse paths
add: implement the --sparse option
add: skip tracked paths outside sparse-checkout cone
add: fail when adding an untracked sparse file
dir: fix pattern matching on dirs
dir: select directories correctly
t1092: behavior for adding sparse files
t3705: test that 'sparse_entry' is unstaged
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
"git rebase <upstream> <tag>" failed when aborted in the middle, as
it mistakenly tried to write the tag object instead of peeling it
to HEAD.
* pw/rebase-of-a-tag-fix:
rebase: dereference tags
rebase: use lookup_commit_reference_by_name()
rebase: use our standard error return value
t3407: rework rebase --quit tests
t3407: strengthen rebase --abort tests
t3407: use test_path_is_missing
t3407: rename a variable
t3407: use test_cmp_rev
t3407: use test_commit
t3407: run tests in $TEST_DIRECTORY
More code paths that use the hack to add submodule's object
database to the set of alternate object store have been cleaned up.
* jt/add-submodule-odb-clean-up:
revision: remove "submodule" from opt struct
repository: support unabsorbed in repo_submodule_init
submodule: remove unnecessary unabsorbed fallback
Junio C Hamano
Ævar Arnfjörð Bjarmason
Jeff King