expand_show_tree() borrows the base strbuf given to us by read_tree() to
build the full path of the current entry when handling %(path). Only
its indirect caller, show_tree_fmt(), removes the added entry name.
That works fine as long as %(path) is only included once in the format
string, but accumulates duplicates if it's repeated:
$ git ls-tree --format='%(path) %(path) %(path)' HEAD M*
Makefile MakefileMakefile MakefileMakefileMakefile
Reset the length after each use to get the same expansion every time;
here's the behavior with this patch:
$ ./git ls-tree --format='%(path) %(path) %(path)' HEAD M*
Makefile Makefile Makefile
Signed-off-by: René Scharfe <l.s.r@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Add a --format option to ls-tree. It has an existing default output,
and then --long and --name-only options to emit the default output
along with the objectsize and, or to only emit object paths.
Rather than add --type-only, --object-only etc. we can just support a
--format using a strbuf_expand() similar to "for-each-ref
--format". We might still add such options in the future for
convenience.
The --format implementation is slower than the existing code, but this
change does not cause any performance regressions. We'll leave the
existing show_tree() unchanged, and only run show_tree_fmt() in if
a --format different than the hardcoded built-in ones corresponding to
the existing modes is provided.
I.e. something like the "--long" output would be much slower with
this, mainly due to how we need to allocate various things to do with
quote.c instead of spewing the output directly to stdout.
The new option of '--format' comes from Ævar Arnfjörð Bjarmasonn's
idea and suggestion, this commit makes modifications in terms of the
original discussion on community [1].
In [1] there was a "GIT_TEST_LS_TREE_FORMAT_BACKEND" variable to
ensure that we had test coverage for passing tests that would
otherwise use show_tree() through show_tree_fmt(), and thus that the
formatting mechanism could handle all the same cases as the
non-formatting options.
Somewhere in subsequent re-rolls of that we seem to have drifted away
from what the goal of these tests should be. We're trying to ensure
correctness of show_tree_fmt(). We can't tell if we "hit [the]
fast-path" here, and instead of having an explicit test for that, we
can just add it to something our "test_ls_tree_format" tests for.
Here is the statistics about performance tests:
1. Default format (hitten the builtin formats):
"git ls-tree <tree-ish>" vs "--format='%(mode) %(type) %(object)%x09%(file)'"
$hyperfine --warmup=10 "/opt/git/master/bin/git ls-tree -r HEAD"
Benchmark 1: /opt/git/master/bin/git ls-tree -r HEAD
Time (mean ± σ): 105.2 ms ± 3.3 ms [User: 84.3 ms, System: 20.8 ms]
Range (min … max): 99.2 ms … 113.2 ms 28 runs
$hyperfine --warmup=10 "/opt/git/ls-tree-oid-only/bin/git ls-tree -r --format='%(mode) %(type) %(object)%x09%(file)' HEAD"
Benchmark 1: /opt/git/ls-tree-oid-only/bin/git ls-tree -r --format='%(mode) %(type) %(object)%x09%(file)' HEAD
Time (mean ± σ): 106.4 ms ± 2.7 ms [User: 86.1 ms, System: 20.2 ms]
Range (min … max): 100.2 ms … 110.5 ms 29 runs
2. Default format includes object size (hitten the builtin formats):
"git ls-tree -l <tree-ish>" vs "--format='%(mode) %(type) %(object) %(size:padded)%x09%(file)'"
$hyperfine --warmup=10 "/opt/git/master/bin/git ls-tree -r -l HEAD"
Benchmark 1: /opt/git/master/bin/git ls-tree -r -l HEAD
Time (mean ± σ): 335.1 ms ± 6.5 ms [User: 304.6 ms, System: 30.4 ms]
Range (min … max): 327.5 ms … 348.4 ms 10 runs
$hyperfine --warmup=10 "/opt/git/ls-tree-oid-only/bin/git ls-tree -r --format='%(mode) %(type) %(object) %(size:padded)%x09%(file)' HEAD"
Benchmark 1: /opt/git/ls-tree-oid-only/bin/git ls-tree -r --format='%(mode) %(type) %(object) %(size:padded)%x09%(file)' HEAD
Time (mean ± σ): 337.2 ms ± 8.2 ms [User: 309.2 ms, System: 27.9 ms]
Range (min … max): 328.8 ms … 349.4 ms 10 runs
Links:
[1] https://public-inbox.org/git/RFC-patch-6.7-eac299f06ff-20211217T131635Z-avarab@gmail.com/
[2] https://lore.kernel.org/git/cb717d08be87e3239117c6c667cb32caabaad33d.1646390152.git.dyroneteng@gmail.com/
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Teng Long <dyroneteng@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
A convenient way to pad strings is to use something like
`strbuf_addf(&buf, "%20s", "Hello, world!")`.
However, the Coccinelle rule that forbids a format `"%s"` with a
constant string argument cast too wide a net, and also forbade such
padding.
The original rule was introduced by commit:
28c23cd4c3 (strbuf.cocci: suggest strbuf_addbuf() to add one strbuf to an other, 2019-01-25)
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Teng Long <dyroneteng@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
"show_tree_data" is a struct that packages the necessary fields for
"show_tree()". This commit is a pre-prepared commit for supporting
"--format" option and it does not affect any existing functionality.
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Teng Long <dyroneteng@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
This is a non-functional change, we introduce an enum "ls_tree_cmdmode"
then use it to mark which columns to output.
This has the advantage of making the show_tree logic simpler and more
readable, as well as making it easier to extend new options (for example,
if we want to add a "--object-only" option, we just need to add a similar
"short-circuit logic in "show_tree()").
Helped-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Teng Long <dyroneteng@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
If we execute "git ls-tree" with combined "--name-only" and "--long"
, only the pathname will be printed, the size is omitted (the original
discoverer was Peff in [1]).
This commit fix this issue by using `OPT_CMDMODE()` instead to make both
of them mutually exclusive.
[1] https://public-inbox.org/git/YZK0MKCYAJmG+pSU@coredump.intra.peff.net/
Helped-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Teng Long <dyroneteng@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Use the object_type() function to determine the object type from the
"mode" passed to us by read_tree(), instead of doing so with the S_*()
macros.
Helped-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Teng Long <dyronetengb@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The variable which "show_tree()" return is named "retval", a name that's
a little hard to understand. The commit rename "retval" to "recurse"
which is a more meaningful name than before in the context. We do not
need to take a look at "read_tree_at()" in "tree.c" to make sure what
does "retval" mean.
Signed-off-by: Teng Long <dyroneteng@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The "struct strbuf"'s "len" member is a "size_t", not an "int", so
let's change our corresponding types accordingly. This also changes
the "len" and "speclen" variables, which are likewise used to store
the return value of strlen(), which returns "size_t", not "int".
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Change the ls-tree.c code to use type_name() on the enum instead of
using the string constants. This doesn't matter either way for
performance, but makes this a bit easier to read as we'll no longer
need a strcmp() here.
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Add missing {} to the "else" arms in show_tree() per the
CodingGuidelines.
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Remove code added in f35a6d3bce (Teach core object handling functions
about gitlinks, 2007-04-09), later patched in 7d0b18a4da (Add output
flushing before fork(), 2008-08-04), and then finally ending up in its
current form in d3bee161fe (tree.c: allow read_tree_recursive() to
traverse gitlink entries, 2009-01-25). All while being commented-out!
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The --name-status synonym for --name-only added in
c639a5548a (ls-tree: --name-only, 2005-12-01) had no tests, let's
make sure it works the same way as its sibling.
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Teng Long <dyroneteng@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
When checking typos in file "po/ko.po", "git-po-helper" reports lots of
false positives because there are no spaces between ASCII and Korean
characters. After applied commit adee197 "(dict: add smudge table for
Korean language, 2021-11-11)" of "git-l10n/git-po-helper" to suppress
these false positives, some easy-to-fix typos are found and fixed.
Signed-off-by: Jiang Xin <worldhello.net@gmail.com>
When we added a new event type to trace2 event stream, we forgot to
raise the format version number, which has been corrected.
* js/trace2-raise-format-version:
trace2: increment event format version
Regression fix.
* ab/fsck-unexpected-type:
object-file: free(*contents) only in read_loose_object() caller
object-file: fix SEGV on free() regression in v2.34.0-rc2
In 64bc752 (trace2: add trace2_child_ready() to report on background
children, 2021-09-20), we added a new "child_ready" event. In
Documentation/technical/api-trace2.txt, we promise that adding a new
event type will result in incrementing the trace2 event format version
number, but this was not done. Correct this in code & docs.
Signed-off-by: Josh Steadmon <steadmon@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
In the preceding commit a free() of uninitialized memory regression in
96e41f58fe (fsck: report invalid object type-path combinations,
2021-10-01) was fixed, but we'd still have an issue with leaking
memory from fsck_loose(). Let's fix that issue too.
That issue was introduced in my 31deb28f5e (fsck: don't hard die on
invalid object types, 2021-10-01). It can be reproduced under
SANITIZE=leak with the test I added in 093fffdfbe (fsck tests: add
test for fsck-ing an unknown type, 2021-10-01):
./t1450-fsck.sh --run=84 -vixd
In some sense it's not a problem, we lost the same amount of memory in
terms of things malloc'd and not free'd. It just moved from the "still
reachable" to "definitely lost" column in valgrind(1) nomenclature[1],
since we'd have die()'d before.
But now that we don't hard die() anymore in the library let's properly
free() it. Doing so makes this code much easier to follow, since we'll
now have one function owning the freeing of the "contents" variable,
not two.
For context on that memory management pattern the read_loose_object()
function was added in f6371f9210 (sha1_file: add read_loose_object()
function, 2017-01-13) and subsequently used in c68b489e56 (fsck:
parse loose object paths directly, 2017-01-13). The pattern of it
being the task of both sides to free() the memory has been there in
this form since its inception.
1. https://valgrind.org/docs/manual/mc-manual.html#mc-manual.leaks
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
This reverts commit f45022dc2f,
as this is like breakage in the traversal more likely. In a
history with 10 single strand of pearls,
1-->2-->3--...->7-->8-->9-->10
asking "rev-list --unsorted-input 1 10 --not 9 8 7 6 5 4" fails to
paint the bottom 1 uninteresting as the traversal stops, without
completing the propagation of uninteresting bit starting at 4 down
through 3 and 2 to 1.
Fix a regression introduced in my 96e41f58fe (fsck: report invalid
object type-path combinations, 2021-10-01). When fsck-ing blobs larger
than core.bigFileThreshold, we'd free() a pointer to uninitialized
memory.
This issue would have been caught by SANITIZE=address, but since it
involves core.bigFileThreshold, none of the existing tests in our test
suite covered it.
Running them with the "big_file_threshold" in "environment.c" changed
to say "6" would have shown this failure, but let's add a dedicated
test for this scenario based on Han Xin's report[1].
The bug was introduced between v9 and v10[2] of the fsck series merged
in 061a21d36d (Merge branch 'ab/fsck-unexpected-type', 2021-10-25).
1. https://lore.kernel.org/git/20211111030302.75694-1-hanxin.hx@alibaba-inc.com/
2. https://lore.kernel.org/git/cover-v10-00.17-00000000000-20211001T091051Z-avarab@gmail.com/
Reported-by: Han Xin <chiyutianyi@gmail.com>
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The way Cygwin emulates a unix-domain socket, on top of which the
simple-ipc mechanism is implemented, can race with the program on
the other side that wants to use the socket, and briefly make it
appear as a regular file before lstat(2) starts reporting it as a
socket. We now have a workaround on the side that connects to a
unix domain socket.
* js/simple-ipc-cygwin-socket-fix:
simple-ipc: work around issues with Cygwin's Unix socket emulation
"git maintenance run" learned to use system supplied scheduler
backend, but cron on macOS turns out to be unusable for this
purpose.
* ds/no-usable-cron-on-macos:
maintenance: disable cron on macOS
"git pull --ff-only" and "git pull --rebase --ff-only" should make
it a no-op to attempt pulling from a remote that is behind us, but
instead the command errored out by saying it was impossible to
fast-forward, which may technically be true, but not a useful thing
to diagnose as an error. This has been corrected.
* jc/fix-pull-ff-only-when-already-up-to-date:
pull: --ff-only should make it a noop when already-up-to-date
The "-Y find-principals" option of ssh-keygen seems to be broken in
Debian's openssh-client 1:8.7p1-1, whereas it works fine in 1:8.4p1-5.
This causes several failures for GPGSSH tests. We fulfill the
prerequisite because generating the keys works fine, but actually
verifying a signature causes results ranging from bogus results to
ssh-keygen segfaulting.
We can find the broken version during the prereq check by feeding it
empty input. This should result in it complaining to stderr, but in the
broken version it triggers the segfault, causing the GPGSSH tests to be
skipped.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
In eba1ba9 (maintenance: `git maintenance run` learned
`--scheduler=<scheduler>`, 2021-09-04), we introduced the ability to
specify a scheduler explicitly. This led to some extra checks around
whether an alternative scheduler was available. This added the
functionality of removing background maintenance from schedulers other
than the one selected.
On macOS, cron is technically available, but running 'crontab' triggers
a UI prompt asking for special permissions. This is the major reason why
launchctl is used as the default scheduler. The is_crontab_available()
method triggers this UI prompt, causing user disruption.
Remove this disruption by using an #ifdef to prevent running crontab
this way on macOS. This has the unfortunate downside that if a user
manually selects cron via the '--scheduler' option, then adjusting the
scheduler later will not remove the schedule from cron. The
'--scheduler' option ignores the is_available checks, which is how we
can get into this situation.
Extract the new check_crontab_process() method to avoid making the
'child' variable unused on macOS. The method is marked MAYBE_UNUSED
because it has no callers on macOS.
Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Cygwin emulates Unix sockets by writing files with custom contents and
then marking them as system files.
The tricky problem is that while the file is written and its `system`
bit is set, it is still identified as a file. This caused test failures
when Git is too fast looking for the Unix sockets and then complains
that there is a plain file in the way.
Let's work around this by adding a delayed retry loop, specifically for
Cygwin.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Tested-by: Ramsay Jones <ramsay@ramsayjones.plus.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
