From a53343e0fd72ee871aef994bf1352be320dbf710 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=86var=20Arnfj=C3=B6r=C3=B0=20Bjarmason?= Date: Wed, 23 Mar 2022 17:13:01 +0800 Subject: ls-tree tests: add tests for --name-status MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The --name-status synonym for --name-only added in c639a5548a5 (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 Signed-off-by: Teng Long Signed-off-by: Junio C Hamano --- t/t3101-ls-tree-dirname.sh | 55 ++++++++++++++++++++++++---------------------- 1 file changed, 29 insertions(+), 26 deletions(-) (limited to 't') diff --git a/t/t3101-ls-tree-dirname.sh b/t/t3101-ls-tree-dirname.sh index 05fde64225..217006d1bf 100755 --- a/t/t3101-ls-tree-dirname.sh +++ b/t/t3101-ls-tree-dirname.sh @@ -201,31 +201,34 @@ EOF test_cmp expected check ' -test_expect_success 'ls-tree --name-only' ' - git ls-tree --name-only $tree >current && - cat >expected <<\EOF && -1.txt -2.txt -path0 -path1 -path2 -path3 -EOF - test_output -' - -test_expect_success 'ls-tree --name-only -r' ' - git ls-tree --name-only -r $tree >current && - cat >expected <<\EOF && -1.txt -2.txt -path0/a/b/c/1.txt -path1/b/c/1.txt -path2/1.txt -path3/1.txt -path3/2.txt -EOF - test_output -' +for opt in --name-only --name-status +do + test_expect_success "ls-tree $opt" ' + git ls-tree $opt $tree >current && + cat >expected <<-\EOF && + 1.txt + 2.txt + path0 + path1 + path2 + path3 + EOF + test_output + ' + + test_expect_success "ls-tree $opt -r" ' + git ls-tree $opt -r $tree >current && + cat >expected <<-\EOF && + 1.txt + 2.txt + path0/a/b/c/1.txt + path1/b/c/1.txt + path2/1.txt + path3/1.txt + path3/2.txt + EOF + test_output + ' +done test_done -- cgit v1.2.3 From f6b224d5ebfe9fcf58ef34eddea68d9fb4384d50 Mon Sep 17 00:00:00 2001 From: Teng Long Date: Wed, 23 Mar 2022 17:13:08 +0800 Subject: ls-tree: fix "--name-only" and "--long" combined use bug MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 Signed-off-by: Teng Long Signed-off-by: Junio C Hamano --- t/t3103-ls-tree-misc.sh | 9 +++++++++ 1 file changed, 9 insertions(+) (limited to 't') diff --git a/t/t3103-ls-tree-misc.sh b/t/t3103-ls-tree-misc.sh index d18ba1bd84..d9d7fa932f 100755 --- a/t/t3103-ls-tree-misc.sh +++ b/t/t3103-ls-tree-misc.sh @@ -23,4 +23,13 @@ test_expect_success 'ls-tree fails with non-zero exit code on broken tree' ' test_must_fail git ls-tree -r HEAD ' +for opts in \ + "--name-only --long" \ + "--name-status --long" +do + test_expect_success "usage: incompatible options: $opts" ' + test_expect_code 129 git ls-tree $opts $tree + ' +done + test_done -- cgit v1.2.3 From 455923e0a152420054ad74f1af36336d5fa7be75 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=86var=20Arnfj=C3=B6r=C3=B0=20Bjarmason?= Date: Wed, 23 Mar 2022 17:13:12 +0800 Subject: ls-tree: introduce "--format" option MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 " 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 " 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 Signed-off-by: Teng Long Signed-off-by: Junio C Hamano --- t/t3104-ls-tree-format.sh | 67 +++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 67 insertions(+) create mode 100755 t/t3104-ls-tree-format.sh (limited to 't') diff --git a/t/t3104-ls-tree-format.sh b/t/t3104-ls-tree-format.sh new file mode 100755 index 0000000000..7f1eb699d3 --- /dev/null +++ b/t/t3104-ls-tree-format.sh @@ -0,0 +1,67 @@ +#!/bin/sh + +test_description='ls-tree --format' + +TEST_PASSES_SANITIZE_LEAK=true +. ./test-lib.sh + +test_expect_success 'ls-tree --format usage' ' + test_expect_code 129 git ls-tree --format=fmt -l HEAD && + test_expect_code 129 git ls-tree --format=fmt --name-only HEAD && + test_expect_code 129 git ls-tree --format=fmt --name-status HEAD +' + +test_expect_success 'setup' ' + mkdir dir && + test_commit dir/sub-file && + test_commit top-file +' + +test_ls_tree_format () { + format=$1 && + opts=$2 && + fmtopts=$3 && + shift 2 && + + test_expect_success "ls-tree '--format=<$format>' is like options '$opts $fmtopts'" ' + git ls-tree $opts -r HEAD >expect && + git ls-tree --format="$format" -r $fmtopts HEAD >actual && + test_cmp expect actual + ' + + test_expect_success "ls-tree '--format=<$format>' on optimized v.s. non-optimized path" ' + git ls-tree --format="$format" -r $fmtopts HEAD >expect && + git ls-tree --format="> $format" -r $fmtopts HEAD >actual.raw && + sed "s/^> //" >actual Date: Wed, 23 Mar 2022 17:13:13 +0800 Subject: ls-tree: support --object-only option for "git-ls-tree" MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit '--object-only' is an alias for '--format=%(objectname)'. It cannot be used together other format-altering options like '--name-only', '--long' or '--format', they are mutually exclusive. The "--name-only" option outputs only. Likewise, is another high frequency used field, so implement '--object-only' option will bring intuitive and clear semantics for this scenario. Using '--format=%(objectname)' we can achieve a similar effect, but the former is with a lower learning cost(without knowing the format requirement of '--format' option). Even so, if a user is prefer to use "--format=%(objectname)", this is entirely welcome because they are not only equivalent in function, but also have almost identical performance. The reason is this commit also add the specific of "--format=%(objectname)" to the current fast-pathes (builtin formats) to avoid running unnecessary parsing mechanisms. The following performance benchmarks are based on torvalds/linux.git: When hit the fast-path: Benchmark 1: /opt/git/ls-tree-oid-only/bin/git ls-tree -r --object-only HEAD Time (mean ± σ): 83.6 ms ± 2.0 ms [User: 59.4 ms, System: 24.1 ms] Range (min … max): 80.4 ms … 87.2 ms 35 runs Benchmark 1: /opt/git/ls-tree-oid-only/bin/git ls-tree -r --format='%(objectname)' HEAD Time (mean ± σ): 84.1 ms ± 1.8 ms [User: 61.7 ms, System: 22.3 ms] Range (min … max): 80.9 ms … 87.5 ms 35 runs But for a customized format, it will be slower: Benchmark 1: /opt/git/ls-tree-oid-only/bin/git ls-tree -r --format='oid: %(objectname)' HEAD Time (mean ± σ): 96.5 ms ± 2.5 ms [User: 72.9 ms, System: 23.5 ms] Range (min … max): 93.1 ms … 104.1 ms 31 runs Helped-by: Ævar Arnfjörð Bjarmason Signed-off-by: Teng Long Signed-off-by: Junio C Hamano --- t/t3103-ls-tree-misc.sh | 7 +++++-- t/t3104-ls-tree-format.sh | 9 +++++++++ 2 files changed, 14 insertions(+), 2 deletions(-) (limited to 't') diff --git a/t/t3103-ls-tree-misc.sh b/t/t3103-ls-tree-misc.sh index d9d7fa932f..d979c0df5d 100755 --- a/t/t3103-ls-tree-misc.sh +++ b/t/t3103-ls-tree-misc.sh @@ -25,11 +25,14 @@ test_expect_success 'ls-tree fails with non-zero exit code on broken tree' ' for opts in \ "--name-only --long" \ - "--name-status --long" + "--name-status --long" \ + "--name-only --object-only" \ + "--name-status --object-only" \ + "--object-only --long" \ + "--object-only --format" do test_expect_success "usage: incompatible options: $opts" ' test_expect_code 129 git ls-tree $opts $tree ' done - test_done diff --git a/t/t3104-ls-tree-format.sh b/t/t3104-ls-tree-format.sh index 7f1eb699d3..0769a933d6 100755 --- a/t/t3104-ls-tree-format.sh +++ b/t/t3104-ls-tree-format.sh @@ -49,6 +49,15 @@ test_ls_tree_format \ "%(path)" \ "--name-only" +test_ls_tree_format \ + "%(objectname)" \ + "--object-only" + +test_ls_tree_format \ + "%(objectname)" \ + "--object-only --abbrev" \ + "--abbrev" + test_ls_tree_format \ "%(objectmode) %(objecttype) %(objectname)%x09%(path)" \ "-t" \ -- cgit v1.2.3 From 0f8878359207191195867ebd499abba3710f9f4b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=86var=20Arnfj=C3=B6r=C3=B0=20Bjarmason?= Date: Wed, 23 Mar 2022 17:13:14 +0800 Subject: ls-tree: detect and error on --name-only --name-status MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The --name-only and --name-status options are synonyms, but let's detect and error if both are provided. In addition let's add explicit --format tests for the combination of these various options. Signed-off-by: Ævar Arnfjörð Bjarmason Signed-off-by: Teng Long Signed-off-by: Junio C Hamano --- t/t3103-ls-tree-misc.sh | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) (limited to 't') diff --git a/t/t3103-ls-tree-misc.sh b/t/t3103-ls-tree-misc.sh index d979c0df5d..81c6343962 100755 --- a/t/t3103-ls-tree-misc.sh +++ b/t/t3103-ls-tree-misc.sh @@ -24,15 +24,18 @@ test_expect_success 'ls-tree fails with non-zero exit code on broken tree' ' ' for opts in \ - "--name-only --long" \ - "--name-status --long" \ - "--name-only --object-only" \ + "--long --name-only" \ + "--name-only --name-status" \ "--name-status --object-only" \ - "--object-only --long" \ - "--object-only --format" + "--object-only --long" do test_expect_success "usage: incompatible options: $opts" ' test_expect_code 129 git ls-tree $opts $tree - ' + ' + + one_opt=$(echo "$opts" | cut -d' ' -f1) + test_expect_success "usage: incompatible options: $one_opt and --format" ' + test_expect_code 129 git ls-tree $one_opt --format=fmt $tree + ' done test_done -- cgit v1.2.3