From 31dd24eae963c77530bd0e9b7693e6c60774268b Mon Sep 17 00:00:00 2001 From: Jeff King Date: Sat, 1 May 2021 11:41:19 -0400 Subject: [PATCH 01/31] t7415: remove out-dated comment about translation Since GETTEXT_POISON does not exist anymore, there is no point warning people about whether we should use test_i18ngrep. This is doubly confusing because the comment was describing why it was OK to use grep, but it got caught up in the mass conversion of 674ba34038 (fsck: mark strings for translation, 2018-11-10). Note there are other uses of test_i18ngrep in this script which are now obsolete; I'll save those for a mass-cleanup. My goal here was just to fix the confusing comment in code I'm about to refactor. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- t/t7415-submodule-names.sh | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/t/t7415-submodule-names.sh b/t/t7415-submodule-names.sh index 6bf098a6bea308..f46c4f604bfefd 100755 --- a/t/t7415-submodule-names.sh +++ b/t/t7415-submodule-names.sh @@ -151,10 +151,9 @@ test_expect_success 'fsck detects symlinked .gitmodules file' ' } | git mktree && # Check not only that we fail, but that it is due to the - # symlink detector; this grep string comes from the config - # variable name and will not be translated. + # symlink detector test_must_fail git fsck 2>output && - test_i18ngrep gitmodulesSymlink output + grep gitmodulesSymlink output ) ' From ecc13a8c983b307e3e45e7b8f1491c9b6a650081 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Sat, 1 May 2021 11:41:38 -0400 Subject: [PATCH 02/31] fsck_tree(): fix shadowed variable Commit b2f2039c2b (fsck: accept an oid instead of a "struct tree" for fsck_tree(), 2019-10-18) introduced a new "oid" parameter to fsck_tree(), and we pass it to the report() function when we find problems. However, that is shadowed within the tree-walking loop by the existing "oid" variable which we use to store the oid of each tree entry. As a result, we may report the wrong oid for some problems we detect within the loop (the entry oid, instead of the tree oid). Our tests didn't catch this because they checked only that we found the expected fsck problem, not that it was attached to the correct object. Let's rename both variables in the function to avoid confusion. This makes the diff a little noisy (e.g., all of the report() calls outside the loop were already correct but need to be touched), but makes sure we catch all cases and will avoid similar confusion in the future. And we can update the test to be a bit more specific and catch this problem. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- fsck.c | 42 ++++++++++++++++++++------------------ t/t7415-submodule-names.sh | 5 +++-- 2 files changed, 25 insertions(+), 22 deletions(-) diff --git a/fsck.c b/fsck.c index f5ed6a263583f0..dd31ed1413b8d5 100644 --- a/fsck.c +++ b/fsck.c @@ -558,7 +558,7 @@ static int verify_ordered(unsigned mode1, const char *name1, return c1 < c2 ? 0 : TREE_UNORDERED; } -static int fsck_tree(const struct object_id *oid, +static int fsck_tree(const struct object_id *tree_oid, const char *buffer, unsigned long size, struct fsck_options *options) { @@ -579,7 +579,7 @@ static int fsck_tree(const struct object_id *oid, struct name_stack df_dup_candidates = { NULL }; if (init_tree_desc_gently(&desc, buffer, size)) { - retval += report(options, oid, OBJ_TREE, FSCK_MSG_BAD_TREE, "cannot be parsed as a tree"); + retval += report(options, tree_oid, OBJ_TREE, FSCK_MSG_BAD_TREE, "cannot be parsed as a tree"); return retval; } @@ -589,11 +589,11 @@ static int fsck_tree(const struct object_id *oid, while (desc.size) { unsigned short mode; const char *name, *backslash; - const struct object_id *oid; + const struct object_id *entry_oid; - oid = tree_entry_extract(&desc, &name, &mode); + entry_oid = tree_entry_extract(&desc, &name, &mode); - has_null_sha1 |= is_null_oid(oid); + has_null_sha1 |= is_null_oid(entry_oid); has_full_path |= !!strchr(name, '/'); has_empty_name |= !*name; has_dot |= !strcmp(name, "."); @@ -603,10 +603,11 @@ static int fsck_tree(const struct object_id *oid, if (is_hfs_dotgitmodules(name) || is_ntfs_dotgitmodules(name)) { if (!S_ISLNK(mode)) - oidset_insert(&options->gitmodules_found, oid); + oidset_insert(&options->gitmodules_found, + entry_oid); else retval += report(options, - oid, OBJ_TREE, + tree_oid, OBJ_TREE, FSCK_MSG_GITMODULES_SYMLINK, ".gitmodules is a symbolic link"); } @@ -617,9 +618,10 @@ static int fsck_tree(const struct object_id *oid, has_dotgit |= is_ntfs_dotgit(backslash); if (is_ntfs_dotgitmodules(backslash)) { if (!S_ISLNK(mode)) - oidset_insert(&options->gitmodules_found, oid); + oidset_insert(&options->gitmodules_found, + entry_oid); else - retval += report(options, oid, OBJ_TREE, + retval += report(options, tree_oid, OBJ_TREE, FSCK_MSG_GITMODULES_SYMLINK, ".gitmodules is a symbolic link"); } @@ -628,7 +630,7 @@ static int fsck_tree(const struct object_id *oid, } if (update_tree_entry_gently(&desc)) { - retval += report(options, oid, OBJ_TREE, FSCK_MSG_BAD_TREE, "cannot be parsed as a tree"); + retval += report(options, tree_oid, OBJ_TREE, FSCK_MSG_BAD_TREE, "cannot be parsed as a tree"); break; } @@ -676,25 +678,25 @@ static int fsck_tree(const struct object_id *oid, name_stack_clear(&df_dup_candidates); if (has_null_sha1) - retval += report(options, oid, OBJ_TREE, FSCK_MSG_NULL_SHA1, "contains entries pointing to null sha1"); + retval += report(options, tree_oid, OBJ_TREE, FSCK_MSG_NULL_SHA1, "contains entries pointing to null sha1"); if (has_full_path) - retval += report(options, oid, OBJ_TREE, FSCK_MSG_FULL_PATHNAME, "contains full pathnames"); + retval += report(options, tree_oid, OBJ_TREE, FSCK_MSG_FULL_PATHNAME, "contains full pathnames"); if (has_empty_name) - retval += report(options, oid, OBJ_TREE, FSCK_MSG_EMPTY_NAME, "contains empty pathname"); + retval += report(options, tree_oid, OBJ_TREE, FSCK_MSG_EMPTY_NAME, "contains empty pathname"); if (has_dot) - retval += report(options, oid, OBJ_TREE, FSCK_MSG_HAS_DOT, "contains '.'"); + retval += report(options, tree_oid, OBJ_TREE, FSCK_MSG_HAS_DOT, "contains '.'"); if (has_dotdot) - retval += report(options, oid, OBJ_TREE, FSCK_MSG_HAS_DOTDOT, "contains '..'"); + retval += report(options, tree_oid, OBJ_TREE, FSCK_MSG_HAS_DOTDOT, "contains '..'"); if (has_dotgit) - retval += report(options, oid, OBJ_TREE, FSCK_MSG_HAS_DOTGIT, "contains '.git'"); + retval += report(options, tree_oid, OBJ_TREE, FSCK_MSG_HAS_DOTGIT, "contains '.git'"); if (has_zero_pad) - retval += report(options, oid, OBJ_TREE, FSCK_MSG_ZERO_PADDED_FILEMODE, "contains zero-padded file modes"); + retval += report(options, tree_oid, OBJ_TREE, FSCK_MSG_ZERO_PADDED_FILEMODE, "contains zero-padded file modes"); if (has_bad_modes) - retval += report(options, oid, OBJ_TREE, FSCK_MSG_BAD_FILEMODE, "contains bad file modes"); + retval += report(options, tree_oid, OBJ_TREE, FSCK_MSG_BAD_FILEMODE, "contains bad file modes"); if (has_dup_entries) - retval += report(options, oid, OBJ_TREE, FSCK_MSG_DUPLICATE_ENTRIES, "contains duplicate file entries"); + retval += report(options, tree_oid, OBJ_TREE, FSCK_MSG_DUPLICATE_ENTRIES, "contains duplicate file entries"); if (not_properly_sorted) - retval += report(options, oid, OBJ_TREE, FSCK_MSG_TREE_NOT_SORTED, "not properly sorted"); + retval += report(options, tree_oid, OBJ_TREE, FSCK_MSG_TREE_NOT_SORTED, "not properly sorted"); return retval; } diff --git a/t/t7415-submodule-names.sh b/t/t7415-submodule-names.sh index f46c4f604bfefd..96ee8f3b28cf09 100755 --- a/t/t7415-submodule-names.sh +++ b/t/t7415-submodule-names.sh @@ -148,12 +148,13 @@ test_expect_success 'fsck detects symlinked .gitmodules file' ' { printf "100644 blob $content\t$tricky\n" && printf "120000 blob $target\t.gitmodules\n" - } | git mktree && + } >bad-tree && + tree=$(git mktree output && - grep gitmodulesSymlink output + grep "tree $tree: gitmodulesSymlink" output ) ' From e5f803e84e4d791e0b3d491760f93940d2274d93 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Sat, 1 May 2021 11:41:43 -0400 Subject: [PATCH 03/31] fsck_tree(): wrap some long lines Many calls to report() in fsck_tree() are kept on a single line and are quite long. Most were pretty big to begin with, but have gotten even longer over the years as we've added more parameters. Let's accept the churn of wrapping them in order to conform to our usual line limits. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- fsck.c | 48 ++++++++++++++++++++++++++++++++++++------------ 1 file changed, 36 insertions(+), 12 deletions(-) diff --git a/fsck.c b/fsck.c index dd31ed1413b8d5..db948178980be2 100644 --- a/fsck.c +++ b/fsck.c @@ -579,7 +579,9 @@ static int fsck_tree(const struct object_id *tree_oid, struct name_stack df_dup_candidates = { NULL }; if (init_tree_desc_gently(&desc, buffer, size)) { - retval += report(options, tree_oid, OBJ_TREE, FSCK_MSG_BAD_TREE, "cannot be parsed as a tree"); + retval += report(options, tree_oid, OBJ_TREE, + FSCK_MSG_BAD_TREE, + "cannot be parsed as a tree"); return retval; } @@ -630,7 +632,9 @@ static int fsck_tree(const struct object_id *tree_oid, } if (update_tree_entry_gently(&desc)) { - retval += report(options, tree_oid, OBJ_TREE, FSCK_MSG_BAD_TREE, "cannot be parsed as a tree"); + retval += report(options, tree_oid, OBJ_TREE, + FSCK_MSG_BAD_TREE, + "cannot be parsed as a tree"); break; } @@ -678,25 +682,45 @@ static int fsck_tree(const struct object_id *tree_oid, name_stack_clear(&df_dup_candidates); if (has_null_sha1) - retval += report(options, tree_oid, OBJ_TREE, FSCK_MSG_NULL_SHA1, "contains entries pointing to null sha1"); + retval += report(options, tree_oid, OBJ_TREE, + FSCK_MSG_NULL_SHA1, + "contains entries pointing to null sha1"); if (has_full_path) - retval += report(options, tree_oid, OBJ_TREE, FSCK_MSG_FULL_PATHNAME, "contains full pathnames"); + retval += report(options, tree_oid, OBJ_TREE, + FSCK_MSG_FULL_PATHNAME, + "contains full pathnames"); if (has_empty_name) - retval += report(options, tree_oid, OBJ_TREE, FSCK_MSG_EMPTY_NAME, "contains empty pathname"); + retval += report(options, tree_oid, OBJ_TREE, + FSCK_MSG_EMPTY_NAME, + "contains empty pathname"); if (has_dot) - retval += report(options, tree_oid, OBJ_TREE, FSCK_MSG_HAS_DOT, "contains '.'"); + retval += report(options, tree_oid, OBJ_TREE, + FSCK_MSG_HAS_DOT, + "contains '.'"); if (has_dotdot) - retval += report(options, tree_oid, OBJ_TREE, FSCK_MSG_HAS_DOTDOT, "contains '..'"); + retval += report(options, tree_oid, OBJ_TREE, + FSCK_MSG_HAS_DOTDOT, + "contains '..'"); if (has_dotgit) - retval += report(options, tree_oid, OBJ_TREE, FSCK_MSG_HAS_DOTGIT, "contains '.git'"); + retval += report(options, tree_oid, OBJ_TREE, + FSCK_MSG_HAS_DOTGIT, + "contains '.git'"); if (has_zero_pad) - retval += report(options, tree_oid, OBJ_TREE, FSCK_MSG_ZERO_PADDED_FILEMODE, "contains zero-padded file modes"); + retval += report(options, tree_oid, OBJ_TREE, + FSCK_MSG_ZERO_PADDED_FILEMODE, + "contains zero-padded file modes"); if (has_bad_modes) - retval += report(options, tree_oid, OBJ_TREE, FSCK_MSG_BAD_FILEMODE, "contains bad file modes"); + retval += report(options, tree_oid, OBJ_TREE, + FSCK_MSG_BAD_FILEMODE, + "contains bad file modes"); if (has_dup_entries) - retval += report(options, tree_oid, OBJ_TREE, FSCK_MSG_DUPLICATE_ENTRIES, "contains duplicate file entries"); + retval += report(options, tree_oid, OBJ_TREE, + FSCK_MSG_DUPLICATE_ENTRIES, + "contains duplicate file entries"); if (not_properly_sorted) - retval += report(options, tree_oid, OBJ_TREE, FSCK_MSG_TREE_NOT_SORTED, "not properly sorted"); + retval += report(options, tree_oid, OBJ_TREE, + FSCK_MSG_TREE_NOT_SORTED, + "not properly sorted"); return retval; } From 903741fe8000f4b3348c48dac829446cc6ebfef9 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Sat, 1 May 2021 11:42:08 -0400 Subject: [PATCH 04/31] t7415: rename to expand scope This script has already expanded beyond its original intent of ".. in submodule names" to include other malicious submodule bits. Let's update the name and description to reflect that, as well as the fact that we'll soon be adding similar tests for other dotfiles (.gitattributes, etc). We'll also renumber it to move it out of the group of submodule-specific tests. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- ...submodule-names.sh => t7450-bad-git-dotfiles.sh} | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) rename t/{t7415-submodule-names.sh => t7450-bad-git-dotfiles.sh} (95%) diff --git a/t/t7415-submodule-names.sh b/t/t7450-bad-git-dotfiles.sh similarity index 95% rename from t/t7415-submodule-names.sh rename to t/t7450-bad-git-dotfiles.sh index 96ee8f3b28cf09..169abd4f6310bd 100755 --- a/t/t7415-submodule-names.sh +++ b/t/t7450-bad-git-dotfiles.sh @@ -1,9 +1,16 @@ #!/bin/sh -test_description='check handling of .. in submodule names +test_description='check broken or malicious patterns in .git* files -Exercise the name-checking function on a variety of names, and then give a -real-world setup that confirms we catch this in practice. +Such as: + + - presence of .. in submodule names; + Exercise the name-checking function on a variety of names, and then give a + real-world setup that confirms we catch this in practice. + + - nested submodule names + + - symlinked .gitmodules, etc ' . ./test-lib.sh . "$TEST_DIRECTORY"/lib-pack.sh From 1eb72ef85db07e2f1e47eb6f1100c8c8d2609f79 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Mon, 3 May 2021 16:43:15 -0400 Subject: [PATCH 05/31] t7450: test verify_path() handling of gitmodules Commit 10ecfa7649 (verify_path: disallow symlinks in .gitmodules, 2018-05-04) made it impossible to load a symlink .gitmodules file into the index. However, there are no tests of this behavior. Let's make sure this case is covered. We can easily reuse the test setup created by the matching b7b1fca175 (fsck: complain when .gitmodules is a symlink, 2018-05-04). Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- t/t7450-bad-git-dotfiles.sh | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-) diff --git a/t/t7450-bad-git-dotfiles.sh b/t/t7450-bad-git-dotfiles.sh index 169abd4f6310bd..dc43bc3e9d684e 100755 --- a/t/t7450-bad-git-dotfiles.sh +++ b/t/t7450-bad-git-dotfiles.sh @@ -139,7 +139,7 @@ test_expect_success 'index-pack --strict works for non-repo pack' ' grep gitmodulesName output ' -test_expect_success 'fsck detects symlinked .gitmodules file' ' +test_expect_success 'set up repo with symlinked .gitmodules file' ' git init symlink && ( cd symlink && @@ -155,8 +155,14 @@ test_expect_success 'fsck detects symlinked .gitmodules file' ' { printf "100644 blob $content\t$tricky\n" && printf "120000 blob $target\t.gitmodules\n" - } >bad-tree && - tree=$(git mktree bad-tree + ) && + tree=$(git -C symlink mktree err && + grep "invalid path.*gitmodules" err && + git -C symlink ls-files >out && + test_must_be_empty out +' + test_expect_success 'fsck detects non-blob .gitmodules' ' git init non-blob && ( From 8f9eaaf36c6d5f0cf7160288ea38fc4561400d00 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Mon, 3 May 2021 16:43:18 -0400 Subject: [PATCH 06/31] t7450: test .gitmodules symlink matching against obscured names In t7450 we check that both verify_path() and fsck catch malformed .gitmodules entries in trees. However, we don't check that we catch filesystem-equivalent forms of these (e.g., ".GITMOD~1" on Windows). Our name-matching functions are exercised well in t0060, but there's nothing to test that we correctly call the matching functions from the actual fsck and verify_path() code. So instead of testing just .gitmodules, let's repeat our tests for a few basic cases. We don't need to be exhaustive here (t0060 handles that), but just make sure we hit one name of each type. Besides pushing the tests into a function that takes the path as a parameter, we'll need to do a few things: - adjust the directory name to accommodate the tests running multiple times - set core.protecthfs for index checks. Fsck always protects all types by default, but we want to be able to exercise the HFS routines on every system. Note that core.protectntfs is already the default these days, but it doesn't hurt to explicitly label our need for it. - we'll also take the filename ("gitmodules") as a parameter. All calls use the same name for now, but a future patch will extend this to handle other .gitfoo files. Note that our fake-content symlink destination is somewhat .gitmodules specific. But it isn't necessary for other files (which don't do a content check). And it happens to be a valid attribute and ignore file anyway. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- t/t7450-bad-git-dotfiles.sh | 91 +++++++++++++++++++++---------------- 1 file changed, 53 insertions(+), 38 deletions(-) diff --git a/t/t7450-bad-git-dotfiles.sh b/t/t7450-bad-git-dotfiles.sh index dc43bc3e9d684e..5b7f63f4aa1e46 100755 --- a/t/t7450-bad-git-dotfiles.sh +++ b/t/t7450-bad-git-dotfiles.sh @@ -139,44 +139,59 @@ test_expect_success 'index-pack --strict works for non-repo pack' ' grep gitmodulesName output ' -test_expect_success 'set up repo with symlinked .gitmodules file' ' - git init symlink && - ( - cd symlink && - - # Make the tree directly to avoid index restrictions. - # - # Because symlinks store the target as a blob, choose - # a pathname that could be parsed as a .gitmodules file - # to trick naive non-symlink-aware checking. - tricky="[foo]bar=true" && - content=$(git hash-object -w ../.gitmodules) && - target=$(printf "$tricky" | git hash-object -w --stdin) && - { - printf "100644 blob $content\t$tricky\n" && - printf "120000 blob $target\t.gitmodules\n" - } >bad-tree - ) && - tree=$(git -C symlink mktree output && - grep "tree $tree: gitmodulesSymlink" output - ) -' - -test_expect_success 'refuse to load symlinked .gitmodules into index' ' - test_must_fail git -C symlink read-tree $tree 2>err && - grep "invalid path.*gitmodules" err && - git -C symlink ls-files >out && - test_must_be_empty out -' +check_dotx_symlink () { + name=$1 + type=$2 + path=$3 + dir=symlink-$name-$type + + test_expect_success "set up repo with symlinked $name ($type)" ' + git init $dir && + ( + cd $dir && + + # Make the tree directly to avoid index restrictions. + # + # Because symlinks store the target as a blob, choose + # a pathname that could be parsed as a .gitmodules file + # to trick naive non-symlink-aware checking. + tricky="[foo]bar=true" && + content=$(git hash-object -w ../.gitmodules) && + target=$(printf "$tricky" | git hash-object -w --stdin) && + { + printf "100644 blob $content\t$tricky\n" && + printf "120000 blob $target\t$path\n" + } >bad-tree + ) && + tree=$(git -C $dir mktree <$dir/bad-tree) + ' + + test_expect_success "fsck detects symlinked $name ($type)" ' + ( + cd $dir && + + # Check not only that we fail, but that it is due to the + # symlink detector + test_must_fail git fsck 2>output && + grep "tree $tree: ${name}Symlink" output + ) + ' + + test_expect_success "refuse to load symlinked $name into index ($type)" ' + test_must_fail \ + git -C $dir \ + -c core.protectntfs \ + -c core.protecthfs \ + read-tree $tree 2>err && + grep "invalid path.*$name" err && + git -C $dir ls-files -s >out && + test_must_be_empty out + ' +} + +check_dotx_symlink gitmodules vanilla .gitmodules +check_dotx_symlink gitmodules ntfs ".gitmodules ." +check_dotx_symlink gitmodules hfs ".${u200c}gitmodules" test_expect_success 'fsck detects non-blob .gitmodules' ' git init non-blob && From ef23c95e986f53a7b30a1fae56e06c398565e71e Mon Sep 17 00:00:00 2001 From: Jeff King Date: Mon, 3 May 2021 16:43:22 -0400 Subject: [PATCH 07/31] t0060: test ntfs/hfs-obscured dotfiles We have tests that cover various filesystem-specific spellings of ".gitmodules", because we need to reliably identify that path for some security checks. These are from dc2d9ba318 (is_{hfs,ntfs}_dotgitmodules: add tests, 2018-05-12), with the actual code coming from e7cb0b4455 (is_ntfs_dotgit: match other .git files, 2018-05-11) and 0fc333ba20 (is_hfs_dotgit: match other .git files, 2018-05-02). Those latter two commits also added similar matching functions for .gitattributes and .gitignore. These ended up not being used in the final series, and are currently dead code. But in preparation for them being used in some fsck checks, let's make sure they actually work by throwing a few basic tests at them. Likewise, let's cover .mailmap (which does need matching code added). I didn't bother with the whole battery of tests that we cover for .gitmodules. These functions are all based on the same generic matcher, so it's sufficient to test most of the corner cases just once. Note that the ntfs magic prefix names in the tests come from the algorithm described in e7cb0b4455 (and are different for each file). Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- cache.h | 1 + path.c | 5 +++++ t/helper/test-path-utils.c | 46 +++++++++++++++++++++++++++----------- t/t0060-path-utils.sh | 30 +++++++++++++++++++++++++ utf8.c | 5 +++++ utf8.h | 1 + 6 files changed, 75 insertions(+), 13 deletions(-) diff --git a/cache.h b/cache.h index 1666c38edba9c1..abeaec2b2baaa0 100644 --- a/cache.h +++ b/cache.h @@ -1272,6 +1272,7 @@ int is_ntfs_dotgit(const char *name); int is_ntfs_dotgitmodules(const char *name); int is_ntfs_dotgitignore(const char *name); int is_ntfs_dotgitattributes(const char *name); +int is_ntfs_dotmailmap(const char *name); /* * Returns true iff "str" could be confused as a command-line option when diff --git a/path.c b/path.c index 9e883eb52446d3..7bccd830e95890 100644 --- a/path.c +++ b/path.c @@ -1493,6 +1493,11 @@ int is_ntfs_dotgitattributes(const char *name) return is_ntfs_dot_str(name, "gitattributes", "gi7d29"); } +int is_ntfs_dotmailmap(const char *name) +{ + return is_ntfs_dot_str(name, "mailmap", "maba30"); +} + int looks_like_command_line_option(const char *str) { return str && str[0] == '-'; diff --git a/t/helper/test-path-utils.c b/t/helper/test-path-utils.c index 313a153209c48f..229ed416b0e678 100644 --- a/t/helper/test-path-utils.c +++ b/t/helper/test-path-utils.c @@ -172,9 +172,22 @@ static struct test_data dirname_data[] = { { NULL, NULL } }; -static int is_dotgitmodules(const char *path) +static int check_dotfile(const char *x, const char **argv, + int (*is_hfs)(const char *), + int (*is_ntfs)(const char *)) { - return is_hfs_dotgitmodules(path) || is_ntfs_dotgitmodules(path); + int res = 0, expect = 1; + for (; *argv; argv++) { + if (!strcmp("--not", *argv)) + expect = !expect; + else if (expect != (is_hfs(*argv) || is_ntfs(*argv))) + res = error("'%s' is %s.git%s", *argv, + expect ? "not " : "", x); + else + fprintf(stderr, "ok: '%s' is %s.git%s\n", + *argv, expect ? "" : "not ", x); + } + return !!res; } static int cmp_by_st_size(const void *a, const void *b) @@ -382,17 +395,24 @@ int cmd__path_utils(int argc, const char **argv) return test_function(dirname_data, posix_dirname, argv[1]); if (argc > 2 && !strcmp(argv[1], "is_dotgitmodules")) { - int res = 0, expect = 1, i; - for (i = 2; i < argc; i++) - if (!strcmp("--not", argv[i])) - expect = !expect; - else if (expect != is_dotgitmodules(argv[i])) - res = error("'%s' is %s.gitmodules", argv[i], - expect ? "not " : ""); - else - fprintf(stderr, "ok: '%s' is %s.gitmodules\n", - argv[i], expect ? "" : "not "); - return !!res; + return check_dotfile("modules", argv + 2, + is_hfs_dotgitmodules, + is_ntfs_dotgitmodules); + } + if (argc > 2 && !strcmp(argv[1], "is_dotgitignore")) { + return check_dotfile("ignore", argv + 2, + is_hfs_dotgitignore, + is_ntfs_dotgitignore); + } + if (argc > 2 && !strcmp(argv[1], "is_dotgitattributes")) { + return check_dotfile("attributes", argv + 2, + is_hfs_dotgitattributes, + is_ntfs_dotgitattributes); + } + if (argc > 2 && !strcmp(argv[1], "is_dotmailmap")) { + return check_dotfile("mailmap", argv + 2, + is_hfs_dotmailmap, + is_ntfs_dotmailmap); } if (argc > 2 && !strcmp(argv[1], "file-size")) { diff --git a/t/t0060-path-utils.sh b/t/t0060-path-utils.sh index 0ff06b5d1b30a8..de4960783f071a 100755 --- a/t/t0060-path-utils.sh +++ b/t/t0060-path-utils.sh @@ -468,6 +468,36 @@ test_expect_success 'match .gitmodules' ' .gitmodules,:\$DATA ' +test_expect_success 'match .gitattributes' ' + test-tool path-utils is_dotgitattributes \ + .gitattributes \ + .git${u200c}attributes \ + .Gitattributes \ + .gitattributeS \ + GITATT~1 \ + GI7D29~1 +' + +test_expect_success 'match .gitignore' ' + test-tool path-utils is_dotgitignore \ + .gitignore \ + .git${u200c}ignore \ + .Gitignore \ + .gitignorE \ + GITIGN~1 \ + GI250A~1 +' + +test_expect_success 'match .mailmap' ' + test-tool path-utils is_dotmailmap \ + .mailmap \ + .mail${u200c}map \ + .Mailmap \ + .mailmaP \ + MAILMA~1 \ + MABA30~1 +' + test_expect_success MINGW 'is_valid_path() on Windows' ' test-tool path-utils is_valid_path \ win32 \ diff --git a/utf8.c b/utf8.c index 5b39361ada0bba..de4ce5c0e68adc 100644 --- a/utf8.c +++ b/utf8.c @@ -777,6 +777,11 @@ int is_hfs_dotgitattributes(const char *path) return is_hfs_dot_str(path, "gitattributes"); } +int is_hfs_dotmailmap(const char *path) +{ + return is_hfs_dot_str(path, "mailmap"); +} + const char utf8_bom[] = "\357\273\277"; int skip_utf8_bom(char **text, size_t len) diff --git a/utf8.h b/utf8.h index fcd5167bafb38d..9a16c8679dcd08 100644 --- a/utf8.h +++ b/utf8.h @@ -61,6 +61,7 @@ int is_hfs_dotgit(const char *path); int is_hfs_dotgitmodules(const char *path); int is_hfs_dotgitignore(const char *path); int is_hfs_dotgitattributes(const char *path); +int is_hfs_dotmailmap(const char *path); typedef enum { ALIGN_LEFT, From 2ed21cc4733096fbe69d31a42b5904b8a075208e Mon Sep 17 00:00:00 2001 From: Jeff King Date: Mon, 3 May 2021 16:43:25 -0400 Subject: [PATCH 08/31] fsck: warn about symlinked dotfiles we'll open with O_NOFOLLOW In the commits merged in via 204333b015 (Merge branch 'jk/open-dotgitx-with-nofollow', 2021-03-22), we stopped following symbolic links for .gitattributes, .gitignore, and .mailmap files. Let's teach fsck to warn that these symlinks are not going to do anything. Note that this is just a warning, and won't block the objects via transfer.fsckObjects, since there are reported to be cases of this in the wild (and even once fixed, they will continue to exist in the commit history of those projects, but are not particularly dangerous). Note that we won't add these to the existing gitmodules block in the fsck code. The logic for gitmodules is a bit more complicated, as we also check the content of non-symlink instances we find. But for these new files, there is no content check; we're just looking at the name and mode of the tree entry (and we can avoid even the complicated name checks in the common case that the mode doesn't indicate a symlink). We can reuse the test helper function we defined for .gitmodules, though (it needs some slight adjustments for the fsck error code, and because we don't block these symlinks via verify_path()). Note that I didn't explicitly test the transfer.fsckObjects case here (nor does the existing .gitmodules test that it blocks a push). The translation of fsck severities to outcomes is covered in general in t5504. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- fsck.c | 18 ++++++++++++++++++ fsck.h | 3 +++ t/t7450-bad-git-dotfiles.sh | 29 +++++++++++++++++++++++++++-- 3 files changed, 48 insertions(+), 2 deletions(-) diff --git a/fsck.c b/fsck.c index db948178980be2..3ec500d707aaf6 100644 --- a/fsck.c +++ b/fsck.c @@ -614,6 +614,24 @@ static int fsck_tree(const struct object_id *tree_oid, ".gitmodules is a symbolic link"); } + if (S_ISLNK(mode)) { + if (is_hfs_dotgitignore(name) || + is_ntfs_dotgitignore(name)) + retval += report(options, tree_oid, OBJ_TREE, + FSCK_MSG_GITIGNORE_SYMLINK, + ".gitignore is a symlink"); + if (is_hfs_dotgitattributes(name) || + is_ntfs_dotgitattributes(name)) + retval += report(options, tree_oid, OBJ_TREE, + FSCK_MSG_GITATTRIBUTES_SYMLINK, + ".gitattributes is a symlink"); + if (is_hfs_dotmailmap(name) || + is_ntfs_dotmailmap(name)) + retval += report(options, tree_oid, OBJ_TREE, + FSCK_MSG_MAILMAP_SYMLINK, + ".mailmap is a symlink"); + } + if ((backslash = strchr(name, '\\'))) { while (backslash) { backslash++; diff --git a/fsck.h b/fsck.h index 7202c3c87e8b94..d07f7a2459e8a2 100644 --- a/fsck.h +++ b/fsck.h @@ -67,6 +67,9 @@ enum fsck_msg_type { FUNC(NUL_IN_COMMIT, WARN) \ /* infos (reported as warnings, but ignored by default) */ \ FUNC(GITMODULES_PARSE, INFO) \ + FUNC(GITIGNORE_SYMLINK, INFO) \ + FUNC(GITATTRIBUTES_SYMLINK, INFO) \ + FUNC(MAILMAP_SYMLINK, INFO) \ FUNC(BAD_TAG_NAME, INFO) \ FUNC(MISSING_TAGGER_ENTRY, INFO) \ /* ignored (elevated when requested) */ \ diff --git a/t/t7450-bad-git-dotfiles.sh b/t/t7450-bad-git-dotfiles.sh index 5b7f63f4aa1e46..41706c1c9ff91d 100755 --- a/t/t7450-bad-git-dotfiles.sh +++ b/t/t7450-bad-git-dotfiles.sh @@ -140,6 +140,18 @@ test_expect_success 'index-pack --strict works for non-repo pack' ' ' check_dotx_symlink () { + fsck_must_fail=test_must_fail + fsck_prefix=error + refuse_index=t + case "$1" in + --warning) + fsck_must_fail= + fsck_prefix=warning + refuse_index= + shift + ;; + esac + name=$1 type=$2 path=$3 @@ -172,11 +184,12 @@ check_dotx_symlink () { # Check not only that we fail, but that it is due to the # symlink detector - test_must_fail git fsck 2>output && - grep "tree $tree: ${name}Symlink" output + $fsck_must_fail git fsck 2>output && + grep "$fsck_prefix.*tree $tree: ${name}Symlink" output ) ' + test -n "$refuse_index" && test_expect_success "refuse to load symlinked $name into index ($type)" ' test_must_fail \ git -C $dir \ @@ -193,6 +206,18 @@ check_dotx_symlink gitmodules vanilla .gitmodules check_dotx_symlink gitmodules ntfs ".gitmodules ." check_dotx_symlink gitmodules hfs ".${u200c}gitmodules" +check_dotx_symlink --warning gitattributes vanilla .gitattributes +check_dotx_symlink --warning gitattributes ntfs ".gitattributes ." +check_dotx_symlink --warning gitattributes hfs ".${u200c}gitattributes" + +check_dotx_symlink --warning gitignore vanilla .gitignore +check_dotx_symlink --warning gitignore ntfs ".gitignore ." +check_dotx_symlink --warning gitignore hfs ".${u200c}gitignore" + +check_dotx_symlink --warning mailmap vanilla .mailmap +check_dotx_symlink --warning mailmap ntfs ".mailmap ." +check_dotx_symlink --warning mailmap hfs ".${u200c}mailmap" + test_expect_success 'fsck detects non-blob .gitmodules' ' git init non-blob && ( From 274d4c3ba30179427b4b9278bedf7ce1a5effab2 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Mon, 3 May 2021 16:43:28 -0400 Subject: [PATCH 09/31] docs: document symlink restrictions for dot-files We stopped allowing symlinks for .gitmodules files in 10ecfa7649 (verify_path: disallow symlinks in .gitmodules, 2018-05-04), and we stopped following symlinks for .gitattributes, .gitignore, and .mailmap in the commits from 204333b015 (Merge branch 'jk/open-dotgitx-with-nofollow', 2021-03-22). The reasons are discussed in detail there, but we never adjusted the documentation to let users know. This hasn't been a big deal since the point is that such setups were mildly broken and thought to be unusual anyway. But it certainly doesn't hurt to be clear and explicit about it. Suggested-by: Philip Oakley Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- Documentation/gitattributes.txt | 6 ++++++ Documentation/gitignore.txt | 4 ++++ Documentation/gitmailmap.txt | 7 +++++++ Documentation/gitmodules.txt | 8 ++++++++ 4 files changed, 25 insertions(+) diff --git a/Documentation/gitattributes.txt b/Documentation/gitattributes.txt index cfcfa800c2d057..83fd4e19a410ae 100644 --- a/Documentation/gitattributes.txt +++ b/Documentation/gitattributes.txt @@ -1247,6 +1247,12 @@ to: [attr]binary -diff -merge -text ------------ +NOTES +----- + +Git does not follow symbolic links when accessing a `.gitattributes` +file in the working tree. This keeps behavior consistent when the file +is accessed from the index or a tree versus from the filesystem. EXAMPLES -------- diff --git a/Documentation/gitignore.txt b/Documentation/gitignore.txt index 5751603b13f40c..53e7d5c914d2d2 100644 --- a/Documentation/gitignore.txt +++ b/Documentation/gitignore.txt @@ -149,6 +149,10 @@ not tracked by Git remain untracked. To stop tracking a file that is currently tracked, use 'git rm --cached'. +Git does not follow symbolic links when accessing a `.gitignore` file in +the working tree. This keeps behavior consistent when the file is +accessed from the index or a tree versus from the filesystem. + EXAMPLES -------- diff --git a/Documentation/gitmailmap.txt b/Documentation/gitmailmap.txt index 3fb39f801fb13c..06f4af93fe6213 100644 --- a/Documentation/gitmailmap.txt +++ b/Documentation/gitmailmap.txt @@ -55,6 +55,13 @@ this would also match the 'Commit Name ' above: Proper Name CoMmIt NaMe -- +NOTES +----- + +Git does not follow symbolic links when accessing a `.mailmap` file in +the working tree. This keeps behavior consistent when the file is +accessed from the index or a tree versus from the filesystem. + EXAMPLES -------- diff --git a/Documentation/gitmodules.txt b/Documentation/gitmodules.txt index 8e333dde1baeea..dcee09b5001dc8 100644 --- a/Documentation/gitmodules.txt +++ b/Documentation/gitmodules.txt @@ -98,6 +98,14 @@ submodule..shallow:: shallow clone (with a history depth of 1) unless the user explicitly asks for a non-shallow clone. +NOTES +----- + +Git does not allow the `.gitmodules` file within a working tree to be a +symbolic link, and will refuse to check out such a tree entry. This +keeps behavior consistent when the file is accessed from the index or a +tree versus from the filesystem, and helps Git reliably enforce security +checks of the file contents. EXAMPLES -------- From c5ed6cfcbac6554710f718319f012eec17f5b33f Mon Sep 17 00:00:00 2001 From: ZheNing Hu Date: Mon, 3 May 2021 15:41:04 +0000 Subject: [PATCH 10/31] docs: correct descript of trailer..command In the original documentation of `trailer..command`, some descriptions are easily misunderstood. So let's modify it to increase its readability. In addition, clarify that `$ARG` in command can only be replaced once. Signed-off-by: ZheNing Hu Signed-off-by: Junio C Hamano --- Documentation/git-interpret-trailers.txt | 37 ++++++++++++++---------- 1 file changed, 21 insertions(+), 16 deletions(-) diff --git a/Documentation/git-interpret-trailers.txt b/Documentation/git-interpret-trailers.txt index 96ec6499f001e6..6f2a7a13046491 100644 --- a/Documentation/git-interpret-trailers.txt +++ b/Documentation/git-interpret-trailers.txt @@ -232,25 +232,30 @@ trailer..ifmissing:: that option for trailers with the specified . trailer..command:: - This option can be used to specify a shell command that will - be called to automatically add or modify a trailer with the - specified . + This option can be used to specify a shell command that will be called: + once to automatically add a trailer with the specified , and then + each time a '--trailer =' argument to modify the of + the trailer that this option would produce. + -When this option is specified, the behavior is as if a special -'=' argument were added at the beginning of the command -line, where is taken to be the standard output of the -specified command with any leading and trailing whitespace trimmed -off. +When the specified command is first called to add a trailer +with the specified , the behavior is as if a special +'--trailer =' argument was added at the beginning +of the "git interpret-trailers" command, where +is taken to be the standard output of the command with any +leading and trailing whitespace trimmed off. + -If the command contains the `$ARG` string, this string will be -replaced with the part of an existing trailer with the same -, if any, before the command is launched. +If some '--trailer =' arguments are also passed +on the command line, the command is called again once for each +of these arguments with the same . And the part +of these arguments, if any, will be used to replace the first +occurrence of substring `$ARG` in the command. This way the +command can produce a computed from the passed +in the '--trailer =' argument. + -If some '=' arguments are also passed on the command -line, when a 'trailer..command' is configured, the command will -also be executed for each of these arguments. And the part of -these arguments, if any, will be used to replace the `$ARG` string in -the command. +For consistency, the first occurrence of substring `$ARG` is +also replaced, this time with the empty string, in the command +when the command is first called to add a trailer with the +specified . EXAMPLES -------- From fcae59509a24ce32df0192f4d056240dc10f1392 Mon Sep 17 00:00:00 2001 From: ZheNing Hu Date: Mon, 3 May 2021 15:41:05 +0000 Subject: [PATCH 11/31] trailer: add new .cmd config option The `trailer..command` configuration variable specifies a command (run via the shell, so it does not have to be a single name or path to the command, but can be a shell script), and the first occurrence of substring $ARG is replaced with the value given to the `interpret-trailer` command for the token in a '--trailer =' argument. This has three downsides: * The use of $ARG in the mechanism misleads the users that the value is passed in the shell variable, and tempt them to use $ARG more than once, but that would not work, as the second and subsequent $ARG are not replaced. * Because $ARG is textually replaced without regard to the shell language syntax, even '$ARG' (inside a single-quote pair), which a user would expect to stay intact, would be replaced, and worse, if the value had an unmatched single quote (imagine a name like "O'Connor", substituted into NAME='$ARG' to make it NAME='O'Connor'), it would result in a broken command that is not syntactically correct (or worse). * The first occurrence of substring `$ARG` will be replaced with the empty string, in the command when the command is first called to add a trailer with the specified . This is a bad design, the nature of automatic execution causes it to add a trailer that we don't expect. Introduce a new `trailer..cmd` configuration that takes higher precedence to deprecate and eventually remove `trailer..command`, which passes the value as an argument to the command. Instead of "$ARG", users can refer to the value as positional argument, $1, in their scripts. At the same time, in order to allow `git interpret-trailers` to better simulate the behavior of `git command -s`, 'trailer..cmd' will not automatically execute. Helped-by: Junio C Hamano Helped-by: Christian Couder Signed-off-by: ZheNing Hu Signed-off-by: Junio C Hamano --- Documentation/git-interpret-trailers.txt | 75 ++++++++++++++++++--- t/t7513-interpret-trailers.sh | 84 ++++++++++++++++++++++++ trailer.c | 35 +++++++--- 3 files changed, 175 insertions(+), 19 deletions(-) diff --git a/Documentation/git-interpret-trailers.txt b/Documentation/git-interpret-trailers.txt index 6f2a7a13046491..956a01d184f788 100644 --- a/Documentation/git-interpret-trailers.txt +++ b/Documentation/git-interpret-trailers.txt @@ -232,6 +232,20 @@ trailer..ifmissing:: that option for trailers with the specified . trailer..command:: + This option behaves in the same way as 'trailer..cmd', except + that it doesn't pass anything as argument to the specified command. + Instead the first occurrence of substring $ARG is replaced by the + value that would be passed as argument. ++ +The 'trailer..command' option has been deprecated in favor of +'trailer..cmd' due to the fact that $ARG in the user's command is +only replaced once and that the original way of replacing $ARG is not safe. ++ +When both 'trailer..cmd' and 'trailer..command' are given +for the same , 'trailer..cmd' is used and +'trailer..command' is ignored. + +trailer..cmd:: This option can be used to specify a shell command that will be called: once to automatically add a trailer with the specified , and then each time a '--trailer =' argument to modify the of @@ -247,15 +261,9 @@ leading and trailing whitespace trimmed off. If some '--trailer =' arguments are also passed on the command line, the command is called again once for each of these arguments with the same . And the part -of these arguments, if any, will be used to replace the first -occurrence of substring `$ARG` in the command. This way the -command can produce a computed from the passed -in the '--trailer =' argument. -+ -For consistency, the first occurrence of substring `$ARG` is -also replaced, this time with the empty string, in the command -when the command is first called to add a trailer with the -specified . +of these arguments, if any, will be passed to the command as its +first argument. This way the command can produce a computed +from the passed in the '--trailer =' argument. EXAMPLES -------- @@ -338,6 +346,55 @@ subject Fix #42 ------------ +* Configure a 'help' trailer with a cmd use a script `glog-find-author` + which search specified author identity from git log in git repository + and show how it works: ++ +------------ +$ cat ~/bin/glog-find-author +#!/bin/sh +test -n "$1" && git log --author="$1" --pretty="%an <%ae>" -1 || true +$ git config trailer.help.key "Helped-by: " +$ git config trailer.help.ifExists "addIfDifferentNeighbor" +$ git config trailer.help.cmd "~/bin/glog-find-author" +$ git interpret-trailers --trailer="help:Junio" --trailer="help:Couder" < subject +> +> message +> +> EOF +subject + +message + +Helped-by: Junio C Hamano +Helped-by: Christian Couder +------------ + +* Configure a 'ref' trailer with a cmd use a script `glog-grep` + to grep last relevant commit from git log in the git repository + and show how it works: ++ +------------ +$ cat ~/bin/glog-grep +#!/bin/sh +test -n "$1" && git log --grep "$1" --pretty=reference -1 || true +$ git config trailer.ref.key "Reference-to: " +$ git config trailer.ref.ifExists "replace" +$ git config trailer.ref.cmd "~/bin/glog-grep" +$ git interpret-trailers --trailer="ref:Add copyright notices." < subject +> +> message +> +> EOF +subject + +message + +Reference-to: 8bc9a0c769 (Add copyright notices., 2005-04-07) +------------ + * Configure a 'see' trailer with a command to show the subject of a commit that is related, and show how it works: + diff --git a/t/t7513-interpret-trailers.sh b/t/t7513-interpret-trailers.sh index 6602790b5f4c4a..04885d0a5e5c26 100755 --- a/t/t7513-interpret-trailers.sh +++ b/t/t7513-interpret-trailers.sh @@ -51,6 +51,69 @@ test_expect_success 'setup' ' EOF ' +test_expect_success 'with cmd' ' + test_when_finished "git config --remove-section trailer.bug" && + git config trailer.bug.key "Bug-maker: " && + git config trailer.bug.ifExists "add" && + git config trailer.bug.cmd "echo \"maybe is\"" && + cat >expected2 <<-EOF && + + Bug-maker: maybe is him + Bug-maker: maybe is me + EOF + git interpret-trailers --trailer "bug: him" --trailer "bug:me" \ + >actual2 && + test_cmp expected2 actual2 +' + +test_expect_success 'with cmd and $1' ' + test_when_finished "git config --remove-section trailer.bug" && + git config trailer.bug.key "Bug-maker: " && + git config trailer.bug.ifExists "add" && + git config trailer.bug.cmd "echo \"\$1\" is" && + cat >expected2 <<-EOF && + + Bug-maker: him is him + Bug-maker: me is me + EOF + git interpret-trailers --trailer "bug: him" --trailer "bug:me" \ + >actual2 && + test_cmp expected2 actual2 +' + +test_expect_success 'with cmd and $1 with sh -c' ' + test_when_finished "git config --remove-section trailer.bug" && + git config trailer.bug.key "Bug-maker: " && + git config trailer.bug.ifExists "replace" && + git config trailer.bug.cmd "sh -c \"echo who is \"\$1\"\"" && + cat >expected2 <<-EOF && + + Bug-maker: who is me + EOF + git interpret-trailers --trailer "bug: him" --trailer "bug:me" \ + >actual2 && + test_cmp expected2 actual2 +' + +test_expect_success 'with cmd and $1 with shell script' ' + test_when_finished "git config --remove-section trailer.bug" && + git config trailer.bug.key "Bug-maker: " && + git config trailer.bug.ifExists "replace" && + git config trailer.bug.cmd "./echoscript" && + cat >expected2 <<-EOF && + + Bug-maker: who is me + EOF + cat >echoscript <<-EOF && + #!/bin/sh + echo who is "\$1" + EOF + chmod +x echoscript && + git interpret-trailers --trailer "bug: him" --trailer "bug:me" \ + >actual2 && + test_cmp expected2 actual2 +' + test_expect_success 'without config' ' sed -e "s/ Z\$/ /" >expected <<-\EOF && @@ -1274,6 +1337,27 @@ test_expect_success 'setup a commit' ' git commit -m "Add file a.txt" ' +test_expect_success 'cmd takes precedence over command' ' + test_when_finished "git config --unset trailer.fix.cmd" && + git config trailer.fix.ifExists "replace" && + git config trailer.fix.cmd "test -n \"\$1\" && git log -1 --oneline --format=\"%h (%aN)\" \ + --abbrev-commit --abbrev=14 \"\$1\" || true" && + git config trailer.fix.command "git log -1 --oneline --format=\"%h (%s)\" \ + --abbrev-commit --abbrev=14 \$ARG" && + FIXED=$(git log -1 --oneline --format="%h (%aN)" --abbrev-commit --abbrev=14 HEAD) && + cat complex_message_body >expected2 && + sed -e "s/ Z\$/ /" >>expected2 <<-EOF && + Fixes: $FIXED + Acked-by= Z + Reviewed-by: + Signed-off-by: Z + Signed-off-by: A U Thor + EOF + git interpret-trailers --trailer "review:" --trailer "fix=HEAD" \ + actual2 && + test_cmp expected2 actual2 +' + test_expect_success 'with command using $ARG' ' git config trailer.fix.ifExists "replace" && git config trailer.fix.command "git log -1 --oneline --format=\"%h (%s)\" --abbrev-commit --abbrev=14 \$ARG" && diff --git a/trailer.c b/trailer.c index be4e9726421cc8..7c7cb61a945c7b 100644 --- a/trailer.c +++ b/trailer.c @@ -14,6 +14,7 @@ struct conf_info { char *name; char *key; char *command; + char *cmd; enum trailer_where where; enum trailer_if_exists if_exists; enum trailer_if_missing if_missing; @@ -127,6 +128,7 @@ static void free_arg_item(struct arg_item *item) free(item->conf.name); free(item->conf.key); free(item->conf.command); + free(item->conf.cmd); free(item->token); free(item->value); free(item); @@ -216,18 +218,24 @@ static int check_if_different(struct trailer_item *in_tok, return 1; } -static char *apply_command(const char *command, const char *arg) +static char *apply_command(struct conf_info *conf, const char *arg) { struct strbuf cmd = STRBUF_INIT; struct strbuf buf = STRBUF_INIT; struct child_process cp = CHILD_PROCESS_INIT; char *result; - strbuf_addstr(&cmd, command); - if (arg) - strbuf_replace(&cmd, TRAILER_ARG_STRING, arg); - - strvec_push(&cp.args, cmd.buf); + if (conf->cmd) { + strbuf_addstr(&cmd, conf->cmd); + strvec_push(&cp.args, cmd.buf); + if (arg) + strvec_push(&cp.args, arg); + } else if (conf->command) { + strbuf_addstr(&cmd, conf->command); + if (arg) + strbuf_replace(&cmd, TRAILER_ARG_STRING, arg); + strvec_push(&cp.args, cmd.buf); + } cp.env = local_repo_env; cp.no_stdin = 1; cp.use_shell = 1; @@ -247,7 +255,7 @@ static char *apply_command(const char *command, const char *arg) static void apply_item_command(struct trailer_item *in_tok, struct arg_item *arg_tok) { - if (arg_tok->conf.command) { + if (arg_tok->conf.command || arg_tok->conf.cmd) { const char *arg; if (arg_tok->value && arg_tok->value[0]) { arg = arg_tok->value; @@ -257,7 +265,7 @@ static void apply_item_command(struct trailer_item *in_tok, struct arg_item *arg else arg = xstrdup(""); } - arg_tok->value = apply_command(arg_tok->conf.command, arg); + arg_tok->value = apply_command(&arg_tok->conf, arg); free((char *)arg); } } @@ -430,6 +438,7 @@ static void duplicate_conf(struct conf_info *dst, const struct conf_info *src) dst->name = xstrdup_or_null(src->name); dst->key = xstrdup_or_null(src->key); dst->command = xstrdup_or_null(src->command); + dst->cmd = xstrdup_or_null(src->cmd); } static struct arg_item *get_conf_item(const char *name) @@ -454,8 +463,8 @@ static struct arg_item *get_conf_item(const char *name) return item; } -enum trailer_info_type { TRAILER_KEY, TRAILER_COMMAND, TRAILER_WHERE, - TRAILER_IF_EXISTS, TRAILER_IF_MISSING }; +enum trailer_info_type { TRAILER_KEY, TRAILER_COMMAND, TRAILER_CMD, + TRAILER_WHERE, TRAILER_IF_EXISTS, TRAILER_IF_MISSING }; static struct { const char *name; @@ -463,6 +472,7 @@ static struct { } trailer_config_items[] = { { "key", TRAILER_KEY }, { "command", TRAILER_COMMAND }, + { "cmd", TRAILER_CMD }, { "where", TRAILER_WHERE }, { "ifexists", TRAILER_IF_EXISTS }, { "ifmissing", TRAILER_IF_MISSING } @@ -542,6 +552,11 @@ static int git_trailer_config(const char *conf_key, const char *value, void *cb) warning(_("more than one %s"), conf_key); conf->command = xstrdup(value); break; + case TRAILER_CMD: + if (conf->cmd) + warning(_("more than one %s"), conf_key); + conf->cmd = xstrdup(value); + break; case TRAILER_WHERE: if (trailer_set_where(&conf->where, value)) warning(_("unknown value '%s' for key '%s'"), value, conf_key); From 471302934371f8adaff69b1d75f72132c1400a56 Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Tue, 11 May 2021 15:26:42 +0900 Subject: [PATCH 12/31] The sixteenth batch Signed-off-by: Junio C Hamano --- Documentation/RelNotes/2.32.0.txt | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/Documentation/RelNotes/2.32.0.txt b/Documentation/RelNotes/2.32.0.txt index f6feab46b628b4..b562eda2d2d96d 100644 --- a/Documentation/RelNotes/2.32.0.txt +++ b/Documentation/RelNotes/2.32.0.txt @@ -109,6 +109,18 @@ UI, Workflows & Features * "git subtree" updates. + * It is now documented that "format-patch" skips merges. + + * Options to "git pack-objects" that take numeric values like + --window and --depth should not accept negative values; the input + validation has been tightened. + + * The way the command line specified by the trailer..command + configuration variable receives the end-user supplied value was + both error prone and misleading. An alternative to achieve the + same goal in a safer and more intuitive way has been added, as + the trailer..cmd configuration variable, to replace it. + Performance, Internal Implementation, Development Support etc. @@ -319,6 +331,20 @@ Fixes since v2.31 * "git repack -A -d" in a partial clone unnecessarily loosened objects in promisor pack. + * "git bisect skip" when custom words are used for new/old did not + work, which has been corrected. + + * A few variants of informational message "Already up-to-date" has + been rephrased. + (merge ad9322da03 js/merge-already-up-to-date-message-reword later to maint). + + * "git submodule update --quiet" did not propagate the quiet option + down to underlying "git fetch", which has been corrected. + (merge 62af4bdd42 nc/submodule-update-quiet later to maint). + + * Document that our test can use "local" keyword. + (merge a84fd3bcc6 jc/test-allows-local later to maint). + * Other code cleanup, docfix, build fix, etc. (merge f451960708 dl/cat-file-doc-cleanup later to maint). (merge 12604a8d0c sv/t9801-test-path-is-file-cleanup later to maint). From 423767e5ec2da02a8464adf8751b48982cbd5453 Mon Sep 17 00:00:00 2001 From: ZheNing Hu Date: Thu, 13 May 2021 15:15:37 +0000 Subject: [PATCH 13/31] ref-filter: add objectsize to used_atom When the support for "objectsize:disk" was bolted onto the existing support for "objectsize", it didn't follow the usual pattern for handling "atomtype:modifier", which reads the part just once while parsing the format string, and store the parsed result in the union in the used_atom structure, so that the string form of it does not have to be parsed over and over at runtime (e.g. in grab_common_values()). Add a new member `objectsize` to the union `used_atom.u`, so that we can separate the check of from the check of , this will bring scalability to atom `%(objectsize)`. Signed-off-by: ZheNing Hu Signed-off-by: Junio C Hamano --- ref-filter.c | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/ref-filter.c b/ref-filter.c index e2eac50d9508ee..8cb622b95db2a2 100644 --- a/ref-filter.c +++ b/ref-filter.c @@ -146,6 +146,9 @@ static struct used_atom { enum { O_FULL, O_LENGTH, O_SHORT } option; unsigned int length; } oid; + struct { + enum { O_SIZE, O_SIZE_DISK } option; + } objectsize; struct email_option { enum { EO_RAW, EO_TRIM, EO_LOCALPART } option; } email_option; @@ -269,11 +272,13 @@ static int objectsize_atom_parser(const struct ref_format *format, struct used_a const char *arg, struct strbuf *err) { if (!arg) { + atom->u.objectsize.option = O_SIZE; if (*atom->name == '*') oi_deref.info.sizep = &oi_deref.size; else oi.info.sizep = &oi.size; } else if (!strcmp(arg, "disk")) { + atom->u.objectsize.option = O_SIZE_DISK; if (*atom->name == '*') oi_deref.info.disk_sizep = &oi_deref.disk_size; else @@ -967,12 +972,14 @@ static void grab_common_values(struct atom_value *val, int deref, struct expand_ name++; if (!strcmp(name, "objecttype")) v->s = xstrdup(type_name(oi->type)); - else if (!strcmp(name, "objectsize:disk")) { - v->value = oi->disk_size; - v->s = xstrfmt("%"PRIuMAX, (uintmax_t)oi->disk_size); - } else if (!strcmp(name, "objectsize")) { - v->value = oi->size; - v->s = xstrfmt("%"PRIuMAX , (uintmax_t)oi->size); + else if (starts_with(name, "objectsize")) { + if (used_atom[i].u.objectsize.option == O_SIZE_DISK) { + v->value = oi->disk_size; + v->s = xstrfmt("%"PRIuMAX, (uintmax_t)oi->disk_size); + } else if (used_atom[i].u.objectsize.option == O_SIZE) { + v->value = oi->size; + v->s = xstrfmt("%"PRIuMAX , (uintmax_t)oi->size); + } } else if (!strcmp(name, "deltabase")) v->s = xstrdup(oid_to_hex(&oi->delta_base_oid)); else if (deref) From c60e72e2c6a046698ed8d64d7272306e9c8a66c5 Mon Sep 17 00:00:00 2001 From: ZheNing Hu Date: Thu, 13 May 2021 15:15:38 +0000 Subject: [PATCH 14/31] ref-filter: introduce enum atom_type In the original ref-filter design, it will copy the parsed atom's name and attributes to `used_atom[i].name` in the atom's parsing step, and use it again for string matching in the later specific ref attributes filling step. It use a lot of string matching to determine which atom we need. Introduce the enum "atom_type", each enum value is named as `ATOM_*`, which is the index of each corresponding valid_atom entry. In the first step of the atom parsing, `used_atom.atom_type` will record corresponding enum value from valid_atom entry index, and then in specific reference attribute filling step, only need to compare the value of the `used_atom[i].atom_type` to check the atom type. Helped-by: Junio C Hamano Helped-by: Christian Couder Signed-off-by: ZheNing Hu Signed-off-by: Junio C Hamano --- ref-filter.c | 197 +++++++++++++++++++++++++++++++++------------------ 1 file changed, 128 insertions(+), 69 deletions(-) diff --git a/ref-filter.c b/ref-filter.c index 8cb622b95db2a2..4db0e40ff4c625 100644 --- a/ref-filter.c +++ b/ref-filter.c @@ -108,6 +108,56 @@ static struct ref_to_worktree_map { struct worktree **worktrees; } ref_to_worktree_map; +/* + * The enum atom_type is used as the index of valid_atom array. + * In the atom parsing stage, it will be passed to used_atom.atom_type + * as the identifier of the atom type. We can check the type of used_atom + * entry by `if (used_atom[i].atom_type == ATOM_*)`. + */ +enum atom_type { + ATOM_REFNAME, + ATOM_OBJECTTYPE, + ATOM_OBJECTSIZE, + ATOM_OBJECTNAME, + ATOM_DELTABASE, + ATOM_TREE, + ATOM_PARENT, + ATOM_NUMPARENT, + ATOM_OBJECT, + ATOM_TYPE, + ATOM_TAG, + ATOM_AUTHOR, + ATOM_AUTHORNAME, + ATOM_AUTHOREMAIL, + ATOM_AUTHORDATE, + ATOM_COMMITTER, + ATOM_COMMITTERNAME, + ATOM_COMMITTEREMAIL, + ATOM_COMMITTERDATE, + ATOM_TAGGER, + ATOM_TAGGERNAME, + ATOM_TAGGEREMAIL, + ATOM_TAGGERDATE, + ATOM_CREATOR, + ATOM_CREATORDATE, + ATOM_SUBJECT, + ATOM_BODY, + ATOM_TRAILERS, + ATOM_CONTENTS, + ATOM_UPSTREAM, + ATOM_PUSH, + ATOM_SYMREF, + ATOM_FLAG, + ATOM_HEAD, + ATOM_COLOR, + ATOM_WORKTREEPATH, + ATOM_ALIGN, + ATOM_END, + ATOM_IF, + ATOM_THEN, + ATOM_ELSE, +}; + /* * An atom is a valid field atom listed below, possibly prefixed with * a "*" to denote deref_tag(). @@ -119,6 +169,7 @@ static struct ref_to_worktree_map { * array. */ static struct used_atom { + enum atom_type atom_type; const char *name; cmp_type type; info_source source; @@ -506,47 +557,47 @@ static struct { int (*parser)(const struct ref_format *format, struct used_atom *atom, const char *arg, struct strbuf *err); } valid_atom[] = { - { "refname", SOURCE_NONE, FIELD_STR, refname_atom_parser }, - { "objecttype", SOURCE_OTHER, FIELD_STR, objecttype_atom_parser }, - { "objectsize", SOURCE_OTHER, FIELD_ULONG, objectsize_atom_parser }, - { "objectname", SOURCE_OTHER, FIELD_STR, oid_atom_parser }, - { "deltabase", SOURCE_OTHER, FIELD_STR, deltabase_atom_parser }, - { "tree", SOURCE_OBJ, FIELD_STR, oid_atom_parser }, - { "parent", SOURCE_OBJ, FIELD_STR, oid_atom_parser }, - { "numparent", SOURCE_OBJ, FIELD_ULONG }, - { "object", SOURCE_OBJ }, - { "type", SOURCE_OBJ }, - { "tag", SOURCE_OBJ }, - { "author", SOURCE_OBJ }, - { "authorname", SOURCE_OBJ }, - { "authoremail", SOURCE_OBJ, FIELD_STR, person_email_atom_parser }, - { "authordate", SOURCE_OBJ, FIELD_TIME }, - { "committer", SOURCE_OBJ }, - { "committername", SOURCE_OBJ }, - { "committeremail", SOURCE_OBJ, FIELD_STR, person_email_atom_parser }, - { "committerdate", SOURCE_OBJ, FIELD_TIME }, - { "tagger", SOURCE_OBJ }, - { "taggername", SOURCE_OBJ }, - { "taggeremail", SOURCE_OBJ, FIELD_STR, person_email_atom_parser }, - { "taggerdate", SOURCE_OBJ, FIELD_TIME }, - { "creator", SOURCE_OBJ }, - { "creatordate", SOURCE_OBJ, FIELD_TIME }, - { "subject", SOURCE_OBJ, FIELD_STR, subject_atom_parser }, - { "body", SOURCE_OBJ, FIELD_STR, body_atom_parser }, - { "trailers", SOURCE_OBJ, FIELD_STR, trailers_atom_parser }, - { "contents", SOURCE_OBJ, FIELD_STR, contents_atom_parser }, - { "upstream", SOURCE_NONE, FIELD_STR, remote_ref_atom_parser }, - { "push", SOURCE_NONE, FIELD_STR, remote_ref_atom_parser }, - { "symref", SOURCE_NONE, FIELD_STR, refname_atom_parser }, - { "flag", SOURCE_NONE }, - { "HEAD", SOURCE_NONE, FIELD_STR, head_atom_parser }, - { "color", SOURCE_NONE, FIELD_STR, color_atom_parser }, - { "worktreepath", SOURCE_NONE }, - { "align", SOURCE_NONE, FIELD_STR, align_atom_parser }, - { "end", SOURCE_NONE }, - { "if", SOURCE_NONE, FIELD_STR, if_atom_parser }, - { "then", SOURCE_NONE }, - { "else", SOURCE_NONE }, + [ATOM_REFNAME] = { "refname", SOURCE_NONE, FIELD_STR, refname_atom_parser }, + [ATOM_OBJECTTYPE] = { "objecttype", SOURCE_OTHER, FIELD_STR, objecttype_atom_parser }, + [ATOM_OBJECTSIZE] = { "objectsize", SOURCE_OTHER, FIELD_ULONG, objectsize_atom_parser }, + [ATOM_OBJECTNAME] = { "objectname", SOURCE_OTHER, FIELD_STR, oid_atom_parser }, + [ATOM_DELTABASE] = { "deltabase", SOURCE_OTHER, FIELD_STR, deltabase_atom_parser }, + [ATOM_TREE] = { "tree", SOURCE_OBJ, FIELD_STR, oid_atom_parser }, + [ATOM_PARENT] = { "parent", SOURCE_OBJ, FIELD_STR, oid_atom_parser }, + [ATOM_NUMPARENT] = { "numparent", SOURCE_OBJ, FIELD_ULONG }, + [ATOM_OBJECT] = { "object", SOURCE_OBJ }, + [ATOM_TYPE] = { "type", SOURCE_OBJ }, + [ATOM_TAG] = { "tag", SOURCE_OBJ }, + [ATOM_AUTHOR] = { "author", SOURCE_OBJ }, + [ATOM_AUTHORNAME] = { "authorname", SOURCE_OBJ }, + [ATOM_AUTHOREMAIL] = { "authoremail", SOURCE_OBJ, FIELD_STR, person_email_atom_parser }, + [ATOM_AUTHORDATE] = { "authordate", SOURCE_OBJ, FIELD_TIME }, + [ATOM_COMMITTER] = { "committer", SOURCE_OBJ }, + [ATOM_COMMITTERNAME] = { "committername", SOURCE_OBJ }, + [ATOM_COMMITTEREMAIL] = { "committeremail", SOURCE_OBJ, FIELD_STR, person_email_atom_parser }, + [ATOM_COMMITTERDATE] = { "committerdate", SOURCE_OBJ, FIELD_TIME }, + [ATOM_TAGGER] = { "tagger", SOURCE_OBJ }, + [ATOM_TAGGERNAME] = { "taggername", SOURCE_OBJ }, + [ATOM_TAGGEREMAIL] = { "taggeremail", SOURCE_OBJ, FIELD_STR, person_email_atom_parser }, + [ATOM_TAGGERDATE] = { "taggerdate", SOURCE_OBJ, FIELD_TIME }, + [ATOM_CREATOR] = { "creator", SOURCE_OBJ }, + [ATOM_CREATORDATE] = { "creatordate", SOURCE_OBJ, FIELD_TIME }, + [ATOM_SUBJECT] = { "subject", SOURCE_OBJ, FIELD_STR, subject_atom_parser }, + [ATOM_BODY] = { "body", SOURCE_OBJ, FIELD_STR, body_atom_parser }, + [ATOM_TRAILERS] = { "trailers", SOURCE_OBJ, FIELD_STR, trailers_atom_parser }, + [ATOM_CONTENTS] = { "contents", SOURCE_OBJ, FIELD_STR, contents_atom_parser }, + [ATOM_UPSTREAM] = { "upstream", SOURCE_NONE, FIELD_STR, remote_ref_atom_parser }, + [ATOM_PUSH] = { "push", SOURCE_NONE, FIELD_STR, remote_ref_atom_parser }, + [ATOM_SYMREF] = { "symref", SOURCE_NONE, FIELD_STR, refname_atom_parser }, + [ATOM_FLAG] = { "flag", SOURCE_NONE }, + [ATOM_HEAD] = { "HEAD", SOURCE_NONE, FIELD_STR, head_atom_parser }, + [ATOM_COLOR] = { "color", SOURCE_NONE, FIELD_STR, color_atom_parser }, + [ATOM_WORKTREEPATH] = { "worktreepath", SOURCE_NONE }, + [ATOM_ALIGN] = { "align", SOURCE_NONE, FIELD_STR, align_atom_parser }, + [ATOM_END] = { "end", SOURCE_NONE }, + [ATOM_IF] = { "if", SOURCE_NONE, FIELD_STR, if_atom_parser }, + [ATOM_THEN] = { "then", SOURCE_NONE }, + [ATOM_ELSE] = { "else", SOURCE_NONE }, /* * Please update $__git_ref_fieldlist in git-completion.bash * when you add new atoms @@ -628,6 +679,7 @@ static int parse_ref_filter_atom(const struct ref_format *format, at = used_atom_cnt; used_atom_cnt++; REALLOC_ARRAY(used_atom, used_atom_cnt); + used_atom[at].atom_type = i; used_atom[at].name = xmemdupz(atom, ep - atom); used_atom[at].type = valid_atom[i].cmp_type; used_atom[at].source = valid_atom[i].source; @@ -652,7 +704,7 @@ static int parse_ref_filter_atom(const struct ref_format *format, return -1; if (*atom == '*') need_tagged = 1; - if (!strcmp(valid_atom[i].name, "symref")) + if (i == ATOM_SYMREF) need_symref = 1; return at; } @@ -965,14 +1017,15 @@ static void grab_common_values(struct atom_value *val, int deref, struct expand_ for (i = 0; i < used_atom_cnt; i++) { const char *name = used_atom[i].name; + enum atom_type atom_type = used_atom[i].atom_type; struct atom_value *v = &val[i]; if (!!deref != (*name == '*')) continue; if (deref) name++; - if (!strcmp(name, "objecttype")) + if (atom_type == ATOM_OBJECTTYPE) v->s = xstrdup(type_name(oi->type)); - else if (starts_with(name, "objectsize")) { + else if (atom_type == ATOM_OBJECTSIZE) { if (used_atom[i].u.objectsize.option == O_SIZE_DISK) { v->value = oi->disk_size; v->s = xstrfmt("%"PRIuMAX, (uintmax_t)oi->disk_size); @@ -980,9 +1033,9 @@ static void grab_common_values(struct atom_value *val, int deref, struct expand_ v->value = oi->size; v->s = xstrfmt("%"PRIuMAX , (uintmax_t)oi->size); } - } else if (!strcmp(name, "deltabase")) + } else if (atom_type == ATOM_DELTABASE) v->s = xstrdup(oid_to_hex(&oi->delta_base_oid)); - else if (deref) + else if (atom_type == ATOM_OBJECTNAME && deref) grab_oid(name, "objectname", &oi->oid, v, &used_atom[i]); } } @@ -995,16 +1048,17 @@ static void grab_tag_values(struct atom_value *val, int deref, struct object *ob for (i = 0; i < used_atom_cnt; i++) { const char *name = used_atom[i].name; + enum atom_type atom_type = used_atom[i].atom_type; struct atom_value *v = &val[i]; if (!!deref != (*name == '*')) continue; if (deref) name++; - if (!strcmp(name, "tag")) + if (atom_type == ATOM_TAG) v->s = xstrdup(tag->tag); - else if (!strcmp(name, "type") && tag->tagged) + else if (atom_type == ATOM_TYPE && tag->tagged) v->s = xstrdup(type_name(tag->tagged->type)); - else if (!strcmp(name, "object") && tag->tagged) + else if (atom_type == ATOM_OBJECT && tag->tagged) v->s = xstrdup(oid_to_hex(&tag->tagged->oid)); } } @@ -1017,18 +1071,20 @@ static void grab_commit_values(struct atom_value *val, int deref, struct object for (i = 0; i < used_atom_cnt; i++) { const char *name = used_atom[i].name; + enum atom_type atom_type = used_atom[i].atom_type; struct atom_value *v = &val[i]; if (!!deref != (*name == '*')) continue; if (deref) name++; - if (grab_oid(name, "tree", get_commit_tree_oid(commit), v, &used_atom[i])) + if (atom_type == ATOM_TREE && + grab_oid(name, "tree", get_commit_tree_oid(commit), v, &used_atom[i])) continue; - if (!strcmp(name, "numparent")) { + if (atom_type == ATOM_NUMPARENT) { v->value = commit_list_count(commit->parents); v->s = xstrfmt("%lu", (unsigned long)v->value); } - else if (starts_with(name, "parent")) { + else if (atom_type == ATOM_PARENT) { struct commit_list *parents; struct strbuf s = STRBUF_INIT; for (parents = commit->parents; parents; parents = parents->next) { @@ -1208,15 +1264,16 @@ static void grab_person(const char *who, struct atom_value *val, int deref, void return; for (i = 0; i < used_atom_cnt; i++) { const char *name = used_atom[i].name; + enum atom_type atom_type = used_atom[i].atom_type; struct atom_value *v = &val[i]; if (!!deref != (*name == '*')) continue; if (deref) name++; - if (starts_with(name, "creatordate")) + if (atom_type == ATOM_CREATORDATE) grab_date(wholine, v, name); - else if (!strcmp(name, "creator")) + else if (atom_type == ATOM_CREATOR) v->s = copy_line(wholine); } } @@ -1696,6 +1753,7 @@ static int populate_value(struct ref_array_item *ref, struct strbuf *err) /* Fill in specials first */ for (i = 0; i < used_atom_cnt; i++) { struct used_atom *atom = &used_atom[i]; + enum atom_type atom_type = atom->atom_type; const char *name = used_atom[i].name; struct atom_value *v = &ref->value[i]; int deref = 0; @@ -1710,18 +1768,18 @@ static int populate_value(struct ref_array_item *ref, struct strbuf *err) name++; } - if (starts_with(name, "refname")) + if (atom_type == ATOM_REFNAME) refname = get_refname(atom, ref); - else if (!strcmp(name, "worktreepath")) { + else if (atom_type == ATOM_WORKTREEPATH) { if (ref->kind == FILTER_REFS_BRANCHES) v->s = get_worktree_path(atom, ref); else v->s = xstrdup(""); continue; } - else if (starts_with(name, "symref")) + else if (atom_type == ATOM_SYMREF) refname = get_symref(atom, ref); - else if (starts_with(name, "upstream")) { + else if (atom_type == ATOM_UPSTREAM) { const char *branch_name; /* only local branches may have an upstream */ if (!skip_prefix(ref->refname, "refs/heads/", @@ -1737,7 +1795,7 @@ static int populate_value(struct ref_array_item *ref, struct strbuf *err) else v->s = xstrdup(""); continue; - } else if (atom->u.remote_ref.push) { + } else if (atom_type == ATOM_PUSH && atom->u.remote_ref.push) { const char *branch_name; v->s = xstrdup(""); if (!skip_prefix(ref->refname, "refs/heads/", @@ -1756,10 +1814,10 @@ static int populate_value(struct ref_array_item *ref, struct strbuf *err) free((char *)v->s); fill_remote_ref_details(atom, refname, branch, &v->s); continue; - } else if (starts_with(name, "color:")) { + } else if (atom_type == ATOM_COLOR) { v->s = xstrdup(atom->u.color); continue; - } else if (!strcmp(name, "flag")) { + } else if (atom_type == ATOM_FLAG) { char buf[256], *cp = buf; if (ref->flag & REF_ISSYMREF) cp = copy_advance(cp, ",symref"); @@ -1772,23 +1830,24 @@ static int populate_value(struct ref_array_item *ref, struct strbuf *err) v->s = xstrdup(buf + 1); } continue; - } else if (!deref && grab_oid(name, "objectname", &ref->objectname, v, atom)) { - continue; - } else if (!strcmp(name, "HEAD")) { + } else if (!deref && atom_type == ATOM_OBJECTNAME && + grab_oid(name, "objectname", &ref->objectname, v, atom)) { + continue; + } else if (atom_type == ATOM_HEAD) { if (atom->u.head && !strcmp(ref->refname, atom->u.head)) v->s = xstrdup("*"); else v->s = xstrdup(" "); continue; - } else if (starts_with(name, "align")) { + } else if (atom_type == ATOM_ALIGN) { v->handler = align_atom_handler; v->s = xstrdup(""); continue; - } else if (!strcmp(name, "end")) { + } else if (atom_type == ATOM_END) { v->handler = end_atom_handler; v->s = xstrdup(""); continue; - } else if (starts_with(name, "if")) { + } else if (atom_type == ATOM_IF) { const char *s; if (skip_prefix(name, "if:", &s)) v->s = xstrdup(s); @@ -1796,11 +1855,11 @@ static int populate_value(struct ref_array_item *ref, struct strbuf *err) v->s = xstrdup(""); v->handler = if_atom_handler; continue; - } else if (!strcmp(name, "then")) { + } else if (atom_type == ATOM_THEN) { v->handler = then_atom_handler; v->s = xstrdup(""); continue; - } else if (!strcmp(name, "else")) { + } else if (atom_type == ATOM_ELSE) { v->handler = else_atom_handler; v->s = xstrdup(""); continue; From 6be0a7235236afd9c9ae5fb2af34afc07afe565b Mon Sep 17 00:00:00 2001 From: ZheNing Hu Date: Thu, 3 Jun 2021 16:29:25 +0000 Subject: [PATCH 15/31] cat-file: handle trivial --batch format with --batch-all-objects The --batch code to print an object assumes we found out the type of the object from calling oid_object_info_extended(). This is true for the default format, but even in a custom format, we manually modify the object_info struct to ask for the type. This assumption was broken by 845de33a5b (cat-file: avoid noop calls to sha1_object_info_extended, 2016-05-18). That commit skips the call to oid_object_info_extended() entirely when --batch-all-objects is in use, and the custom format does not include any placeholders that require calling it. Or when the custom format only include placeholders like %(objectname) or %(rest), oid_object_info_extended() will not get the type of the object. This results in an error when we try to confirm that the type didn't change: $ git cat-file --batch=batman --batch-all-objects batman fatal: object 000023961a0c02d6e21dc51ea3484ff71abf1c74 changed type!? and also has other subtle effects (e.g., we'd fail to stream a blob, since we don't realize it's a blob in the first place). We can fix this by flipping the order of the setup. The check for "do we need to get the object info" must come _after_ we've decided whether we need to look up the type. Helped-by: Jeff King Signed-off-by: ZheNing Hu Acked-by: Jeff King Signed-off-by: Junio C Hamano --- builtin/cat-file.c | 13 +++++++------ t/t1006-cat-file.sh | 22 ++++++++++++++++++++++ 2 files changed, 29 insertions(+), 6 deletions(-) diff --git a/builtin/cat-file.c b/builtin/cat-file.c index 5ebf13359e839a..02461bb5ea6f97 100644 --- a/builtin/cat-file.c +++ b/builtin/cat-file.c @@ -512,12 +512,6 @@ static int batch_objects(struct batch_options *opt) if (opt->cmdmode) data.split_on_whitespace = 1; - if (opt->all_objects) { - struct object_info empty = OBJECT_INFO_INIT; - if (!memcmp(&data.info, &empty, sizeof(empty))) - data.skip_object_info = 1; - } - /* * If we are printing out the object, then always fill in the type, * since we will want to decide whether or not to stream. @@ -525,6 +519,13 @@ static int batch_objects(struct batch_options *opt) if (opt->print_contents) data.info.typep = &data.type; + if (opt->all_objects) { + struct object_info empty = OBJECT_INFO_INIT; + + if (!memcmp(&data.info, &empty, sizeof(empty))) + data.skip_object_info = 1; + } + if (opt->all_objects) { struct object_cb_data cb; diff --git a/t/t1006-cat-file.sh b/t/t1006-cat-file.sh index 5d2dc99b74ad00..18b3779ccb6014 100755 --- a/t/t1006-cat-file.sh +++ b/t/t1006-cat-file.sh @@ -586,4 +586,26 @@ test_expect_success 'cat-file --unordered works' ' test_cmp expect actual ' +test_expect_success 'set up object list for --batch-all-objects tests' ' + git -C all-two cat-file --batch-all-objects --batch-check="%(objectname)" >objects +' + +test_expect_success 'cat-file --batch="%(objectname)" with --batch-all-objects will work' ' + git -C all-two cat-file --batch="%(objectname)" expect && + git -C all-two cat-file --batch-all-objects --batch="%(objectname)" >actual && + cmp expect actual +' + +test_expect_success 'cat-file --batch="%(rest)" with --batch-all-objects will work' ' + git -C all-two cat-file --batch="%(rest)" expect && + git -C all-two cat-file --batch-all-objects --batch="%(rest)" >actual && + cmp expect actual +' + +test_expect_success 'cat-file --batch="batman" with --batch-all-objects will work' ' + git -C all-two cat-file --batch="batman" expect && + git -C all-two cat-file --batch-all-objects --batch="batman" >actual && + cmp expect actual +' + test_done From 3f886be8e9fbad8d3c347815e44311244bba2de6 Mon Sep 17 00:00:00 2001 From: ZheNing Hu Date: Thu, 3 Jun 2021 16:29:26 +0000 Subject: [PATCH 16/31] cat-file: merge two block into one There are two "if (opt->all_objects)" blocks next to each other, merge them into one to provide better readability. Helped-by: Jeff King Signed-off-by: ZheNing Hu Acked-by: Jeff King Signed-off-by: Junio C Hamano --- builtin/cat-file.c | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/builtin/cat-file.c b/builtin/cat-file.c index 02461bb5ea6f97..243fe6844bc67a 100644 --- a/builtin/cat-file.c +++ b/builtin/cat-file.c @@ -520,14 +520,11 @@ static int batch_objects(struct batch_options *opt) data.info.typep = &data.type; if (opt->all_objects) { + struct object_cb_data cb; struct object_info empty = OBJECT_INFO_INIT; if (!memcmp(&data.info, &empty, sizeof(empty))) data.skip_object_info = 1; - } - - if (opt->all_objects) { - struct object_cb_data cb; if (has_promisor_remote()) warning("This repository uses promisor remotes. Some objects may not be loaded."); From fd12fe805d9524b2927506496fcc4b683c5569f1 Mon Sep 17 00:00:00 2001 From: ZheNing Hu Date: Sat, 5 Jun 2021 09:13:29 +0000 Subject: [PATCH 17/31] [GSOC] ref-filter: add obj-type check in grab contents Only tag and commit objects use `grab_sub_body_contents()` to grab object contents in the current codebase. We want to teach the function to also handle blobs and trees to get their raw data, without parsing a blob (whose contents looks like a commit or a tag) incorrectly as a commit or a tag. Skip the block of code that is specific to handling commits and tags early when the given object is of a wrong type to help later addition to handle other types of objects in this function. Mentored-by: Christian Couder Mentored-by: Hariom Verma Helped-by: Junio C Hamano Signed-off-by: ZheNing Hu --- ref-filter.c | 24 +++++++++++++++--------- 1 file changed, 15 insertions(+), 9 deletions(-) diff --git a/ref-filter.c b/ref-filter.c index 4db0e40ff4c625..5cee6512fbafa8 100644 --- a/ref-filter.c +++ b/ref-filter.c @@ -1356,11 +1356,12 @@ static void append_lines(struct strbuf *out, const char *buf, unsigned long size } /* See grab_values */ -static void grab_sub_body_contents(struct atom_value *val, int deref, void *buf) +static void grab_sub_body_contents(struct atom_value *val, int deref, struct expand_data *data) { int i; const char *subpos = NULL, *bodypos = NULL, *sigpos = NULL; size_t sublen = 0, bodylen = 0, nonsiglen = 0, siglen = 0; + void *buf = data->content; for (i = 0; i < used_atom_cnt; i++) { struct used_atom *atom = &used_atom[i]; @@ -1371,10 +1372,13 @@ static void grab_sub_body_contents(struct atom_value *val, int deref, void *buf) continue; if (deref) name++; - if (strcmp(name, "body") && - !starts_with(name, "subject") && - !starts_with(name, "trailers") && - !starts_with(name, "contents")) + + if ((data->type != OBJ_TAG && + data->type != OBJ_COMMIT) || + (strcmp(name, "body") && + !starts_with(name, "subject") && + !starts_with(name, "trailers") && + !starts_with(name, "contents"))) continue; if (!subpos) find_subpos(buf, @@ -1438,17 +1442,19 @@ static void fill_missing_values(struct atom_value *val) * pointed at by the ref itself; otherwise it is the object the * ref (which is a tag) refers to. */ -static void grab_values(struct atom_value *val, int deref, struct object *obj, void *buf) +static void grab_values(struct atom_value *val, int deref, struct object *obj, struct expand_data *data) { + void *buf = data->content; + switch (obj->type) { case OBJ_TAG: grab_tag_values(val, deref, obj); - grab_sub_body_contents(val, deref, buf); + grab_sub_body_contents(val, deref, data); grab_person("tagger", val, deref, buf); break; case OBJ_COMMIT: grab_commit_values(val, deref, obj); - grab_sub_body_contents(val, deref, buf); + grab_sub_body_contents(val, deref, data); grab_person("author", val, deref, buf); grab_person("committer", val, deref, buf); break; @@ -1678,7 +1684,7 @@ static int get_object(struct ref_array_item *ref, int deref, struct object **obj return strbuf_addf_ret(err, -1, _("parse_object_buffer failed on %s for %s"), oid_to_hex(&oi->oid), ref->refname); } - grab_values(ref->value, deref, *obj, oi->content); + grab_values(ref->value, deref, *obj, oi); } grab_common_values(ref->value, deref, oi); From d703156a5fef04286de024676f0aedbbade6c930 Mon Sep 17 00:00:00 2001 From: ZheNing Hu Date: Wed, 9 Jun 2021 16:11:59 +0000 Subject: [PATCH 18/31] [GSOC] ref-filter: add %(raw) atom MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add new formatting option `%(raw)`, which will print the raw object data without any changes. It will help further to migrate all cat-file formatting logic from cat-file to ref-filter. The raw data of blob, tree objects may contain '\0', but most of the logic in `ref-filter` depends on the output of the atom being text (specifically, no embedded NULs in it). E.g. `quote_formatting()` use `strbuf_addstr()` or `*._quote_buf()` add the data to the buffer. The raw data of a tree object is `100644 one\0...`, only the `100644 one` will be added to the buffer, which is incorrect. Therefore, we need to find a way to record the length of the atom_value's member `s`. Although strbuf can already record the string and its length, if we want to replace the type of atom_value's member `s` with strbuf, many places in ref-filter that are filled with dynamically allocated mermory in `v->s` are not easy to replace. At the same time, we need to check if `v->s == NULL` in populate_value(), and strbuf cannot easily distinguish NULL and empty strings, but c-style "const char *" can do it. So add a new member in `struct atom_value`: `s_size`, which can record raw object size, it can help us add raw object data to the buffer or compare two buffers which contain raw object data. Note that `--format=%(raw)` cannot be used with `--python`, `--shell`, `--tcl`, and `--perl` because if the binary raw data is passed to a variable in such languages, these may not support arbitrary binary data in their string variable type. Mentored-by: Christian Couder Mentored-by: Hariom Verma Helped-by: Bagas Sanjaya Helped-by: Ævar Arnfjörð Bjarmason Helped-by: Felipe Contreras Helped-by: Phillip Wood Helped-by: Junio C Hamano Based-on-patch-by: Olga Telezhnaya Signed-off-by: ZheNing Hu --- Documentation/git-for-each-ref.txt | 9 ++ ref-filter.c | 139 +++++++++++++++---- t/t6300-for-each-ref.sh | 216 +++++++++++++++++++++++++++++ 3 files changed, 337 insertions(+), 27 deletions(-) diff --git a/Documentation/git-for-each-ref.txt b/Documentation/git-for-each-ref.txt index 2ae2478de706ce..cbb6f87d13f776 100644 --- a/Documentation/git-for-each-ref.txt +++ b/Documentation/git-for-each-ref.txt @@ -235,6 +235,15 @@ and `date` to extract the named component. For email fields (`authoremail`, without angle brackets, and `:localpart` to get the part before the `@` symbol out of the trimmed email. +The raw data in an object is `raw`. + +raw:size:: + The raw data size of the object. + +Note that `--format=%(raw)` can not be used with `--python`, `--shell`, `--tcl`, +`--perl` because such language may not support arbitrary binary data in their +string variable type. + The message in a commit or a tag object is `contents`, from which `contents:` can be used to extract various parts out of: diff --git a/ref-filter.c b/ref-filter.c index 5cee6512fbafa8..7822be903071c0 100644 --- a/ref-filter.c +++ b/ref-filter.c @@ -144,6 +144,7 @@ enum atom_type { ATOM_BODY, ATOM_TRAILERS, ATOM_CONTENTS, + ATOM_RAW, ATOM_UPSTREAM, ATOM_PUSH, ATOM_SYMREF, @@ -189,6 +190,9 @@ static struct used_atom { struct process_trailer_options trailer_opts; unsigned int nlines; } contents; + struct { + enum { RAW_BARE, RAW_LENGTH } option; + } raw_data; struct { cmp_status cmp_status; const char *str; @@ -426,6 +430,18 @@ static int contents_atom_parser(const struct ref_format *format, struct used_ato return 0; } +static int raw_atom_parser(const struct ref_format *format, struct used_atom *atom, + const char *arg, struct strbuf *err) +{ + if (!arg) + atom->u.raw_data.option = RAW_BARE; + else if (!strcmp(arg, "size")) + atom->u.raw_data.option = RAW_LENGTH; + else + return strbuf_addf_ret(err, -1, _("unrecognized %%(raw) argument: %s"), arg); + return 0; +} + static int oid_atom_parser(const struct ref_format *format, struct used_atom *atom, const char *arg, struct strbuf *err) { @@ -586,6 +602,7 @@ static struct { [ATOM_BODY] = { "body", SOURCE_OBJ, FIELD_STR, body_atom_parser }, [ATOM_TRAILERS] = { "trailers", SOURCE_OBJ, FIELD_STR, trailers_atom_parser }, [ATOM_CONTENTS] = { "contents", SOURCE_OBJ, FIELD_STR, contents_atom_parser }, + [ATOM_RAW] = { "raw", SOURCE_OBJ, FIELD_STR, raw_atom_parser }, [ATOM_UPSTREAM] = { "upstream", SOURCE_NONE, FIELD_STR, remote_ref_atom_parser }, [ATOM_PUSH] = { "push", SOURCE_NONE, FIELD_STR, remote_ref_atom_parser }, [ATOM_SYMREF] = { "symref", SOURCE_NONE, FIELD_STR, refname_atom_parser }, @@ -620,12 +637,15 @@ struct ref_formatting_state { struct atom_value { const char *s; + size_t s_size; int (*handler)(struct atom_value *atomv, struct ref_formatting_state *state, struct strbuf *err); uintmax_t value; /* used for sorting when not FIELD_STR */ struct used_atom *atom; }; +#define ATOM_VALUE_S_SIZE_INIT (-1) + /* * Used to parse format string and sort specifiers */ @@ -644,13 +664,6 @@ static int parse_ref_filter_atom(const struct ref_format *format, return strbuf_addf_ret(err, -1, _("malformed field name: %.*s"), (int)(ep-atom), atom); - /* Do we have the atom already used elsewhere? */ - for (i = 0; i < used_atom_cnt; i++) { - int len = strlen(used_atom[i].name); - if (len == ep - atom && !memcmp(used_atom[i].name, atom, len)) - return i; - } - /* * If the atom name has a colon, strip it and everything after * it off - it specifies the format for this entry, and @@ -660,6 +673,13 @@ static int parse_ref_filter_atom(const struct ref_format *format, arg = memchr(sp, ':', ep - sp); atom_len = (arg ? arg : ep) - sp; + /* Do we have the atom already used elsewhere? */ + for (i = 0; i < used_atom_cnt; i++) { + int len = strlen(used_atom[i].name); + if (len == ep - atom && !memcmp(used_atom[i].name, atom, len)) + return i; + } + /* Is the atom a valid one? */ for (i = 0; i < ARRAY_SIZE(valid_atom); i++) { int len = strlen(valid_atom[i].name); @@ -709,11 +729,14 @@ static int parse_ref_filter_atom(const struct ref_format *format, return at; } -static void quote_formatting(struct strbuf *s, const char *str, int quote_style) +static void quote_formatting(struct strbuf *s, const char *str, size_t len, int quote_style) { switch (quote_style) { case QUOTE_NONE: - strbuf_addstr(s, str); + if (len != ATOM_VALUE_S_SIZE_INIT) + strbuf_add(s, str, len); + else + strbuf_addstr(s, str); break; case QUOTE_SHELL: sq_quote_buf(s, str); @@ -740,9 +763,12 @@ static int append_atom(struct atom_value *v, struct ref_formatting_state *state, * encountered. */ if (!state->stack->prev) - quote_formatting(&state->stack->output, v->s, state->quote_style); + quote_formatting(&state->stack->output, v->s, v->s_size, state->quote_style); else - strbuf_addstr(&state->stack->output, v->s); + if (v->s_size != ATOM_VALUE_S_SIZE_INIT) + strbuf_add(&state->stack->output, v->s, v->s_size); + else + strbuf_addstr(&state->stack->output, v->s); return 0; } @@ -842,21 +868,23 @@ static int if_atom_handler(struct atom_value *atomv, struct ref_formatting_state return 0; } -static int is_empty(const char *s) +static int is_empty(struct strbuf *buf) { - while (*s != '\0') { - if (!isspace(*s)) - return 0; - s++; - } - return 1; -} + const char *cur = buf->buf; + const char *end = buf->buf + buf->len; + + while (cur != end && (isspace(*cur))) + cur++; + + return cur == end; + } static int then_atom_handler(struct atom_value *atomv, struct ref_formatting_state *state, struct strbuf *err) { struct ref_formatting_stack *cur = state->stack; struct if_then_else *if_then_else = NULL; + size_t str_len = 0; if (cur->at_end == if_then_else_handler) if_then_else = (struct if_then_else *)cur->at_end_data; @@ -867,18 +895,22 @@ static int then_atom_handler(struct atom_value *atomv, struct ref_formatting_sta if (if_then_else->else_atom_seen) return strbuf_addf_ret(err, -1, _("format: %%(then) atom used after %%(else)")); if_then_else->then_atom_seen = 1; + if (if_then_else->str) + str_len = strlen(if_then_else->str); /* * If the 'equals' or 'notequals' attribute is used then * perform the required comparison. If not, only non-empty * strings satisfy the 'if' condition. */ if (if_then_else->cmp_status == COMPARE_EQUAL) { - if (!strcmp(if_then_else->str, cur->output.buf)) + if (str_len == cur->output.len && + !memcmp(if_then_else->str, cur->output.buf, cur->output.len)) if_then_else->condition_satisfied = 1; } else if (if_then_else->cmp_status == COMPARE_UNEQUAL) { - if (strcmp(if_then_else->str, cur->output.buf)) + if (str_len != cur->output.len || + memcmp(if_then_else->str, cur->output.buf, cur->output.len)) if_then_else->condition_satisfied = 1; - } else if (cur->output.len && !is_empty(cur->output.buf)) + } else if (cur->output.len && !is_empty(&cur->output)) if_then_else->condition_satisfied = 1; strbuf_reset(&cur->output); return 0; @@ -924,7 +956,7 @@ static int end_atom_handler(struct atom_value *atomv, struct ref_formatting_stat * only on the topmost supporting atom. */ if (!current->prev->prev) { - quote_formatting(&s, current->output.buf, state->quote_style); + quote_formatting(&s, current->output.buf, current->output.len, state->quote_style); strbuf_swap(¤t->output, &s); } strbuf_release(&s); @@ -974,6 +1006,10 @@ int verify_ref_format(struct ref_format *format) at = parse_ref_filter_atom(format, sp + 2, ep, &err); if (at < 0) die("%s", err.buf); + if (format->quote_style && used_atom[at].atom_type == ATOM_RAW && + used_atom[at].u.raw_data.option == RAW_BARE) + die(_("--format=%.*s cannot be used with" + "--python, --shell, --tcl, --perl"), (int)(ep - sp - 2), sp + 2); cp = ep + 1; if (skip_prefix(used_atom[at].name, "color:", &color)) @@ -1362,17 +1398,29 @@ static void grab_sub_body_contents(struct atom_value *val, int deref, struct exp const char *subpos = NULL, *bodypos = NULL, *sigpos = NULL; size_t sublen = 0, bodylen = 0, nonsiglen = 0, siglen = 0; void *buf = data->content; + unsigned long buf_size = data->size; for (i = 0; i < used_atom_cnt; i++) { struct used_atom *atom = &used_atom[i]; const char *name = atom->name; struct atom_value *v = &val[i]; + enum atom_type atom_type = atom->atom_type; if (!!deref != (*name == '*')) continue; if (deref) name++; + if (atom_type == ATOM_RAW) { + if (atom->u.raw_data.option == RAW_BARE) { + v->s = xmemdupz(buf, buf_size); + v->s_size = buf_size; + } else if (atom->u.raw_data.option == RAW_LENGTH) { + v->s = xstrfmt("%"PRIuMAX, (uintmax_t)buf_size); + } + continue; + } + if ((data->type != OBJ_TAG && data->type != OBJ_COMMIT) || (strcmp(name, "body") && @@ -1460,9 +1508,11 @@ static void grab_values(struct atom_value *val, int deref, struct object *obj, s break; case OBJ_TREE: /* grab_tree_values(val, deref, obj, buf, sz); */ + grab_sub_body_contents(val, deref, data); break; case OBJ_BLOB: /* grab_blob_values(val, deref, obj, buf, sz); */ + grab_sub_body_contents(val, deref, data); break; default: die("Eh? Object of type %d?", obj->type); @@ -1766,6 +1816,7 @@ static int populate_value(struct ref_array_item *ref, struct strbuf *err) const char *refname; struct branch *branch = NULL; + v->s_size = ATOM_VALUE_S_SIZE_INIT; v->handler = append_atom; v->atom = atom; @@ -2369,6 +2420,19 @@ static int compare_detached_head(struct ref_array_item *a, struct ref_array_item return 0; } +static int memcasecmp(const void *vs1, const void *vs2, size_t n) +{ + const char *s1 = vs1, *s2 = vs2; + const char *end = s1 + n; + + for (; s1 < end; s1++, s2++) { + int diff = tolower(*s1) - tolower(*s2); + if (diff) + return diff; + } + return 0; +} + static int cmp_ref_sorting(struct ref_sorting *s, struct ref_array_item *a, struct ref_array_item *b) { struct atom_value *va, *vb; @@ -2389,10 +2453,30 @@ static int cmp_ref_sorting(struct ref_sorting *s, struct ref_array_item *a, stru } else if (s->sort_flags & REF_SORTING_VERSION) { cmp = versioncmp(va->s, vb->s); } else if (cmp_type == FIELD_STR) { - int (*cmp_fn)(const char *, const char *); - cmp_fn = s->sort_flags & REF_SORTING_ICASE - ? strcasecmp : strcmp; - cmp = cmp_fn(va->s, vb->s); + if (va->s_size == ATOM_VALUE_S_SIZE_INIT && + vb->s_size == ATOM_VALUE_S_SIZE_INIT) { + int (*cmp_fn)(const char *, const char *); + cmp_fn = s->sort_flags & REF_SORTING_ICASE + ? strcasecmp : strcmp; + cmp = cmp_fn(va->s, vb->s); + } else { + size_t a_size = va->s_size == ATOM_VALUE_S_SIZE_INIT ? + strlen(va->s) : va->s_size; + size_t b_size = vb->s_size == ATOM_VALUE_S_SIZE_INIT ? + strlen(vb->s) : vb->s_size; + int (*cmp_fn)(const void *, const void *, size_t); + cmp_fn = s->sort_flags & REF_SORTING_ICASE + ? memcasecmp : memcmp; + + cmp = cmp_fn(va->s, vb->s, b_size > a_size ? + a_size : b_size); + if (!cmp) { + if (a_size > b_size) + cmp = 1; + else if (a_size < b_size) + cmp = -1; + } + } } else { if (va->value < vb->value) cmp = -1; @@ -2492,6 +2576,7 @@ int format_ref_array_item(struct ref_array_item *info, } if (format->need_color_reset_at_eol) { struct atom_value resetv; + resetv.s_size = ATOM_VALUE_S_SIZE_INIT; resetv.s = GIT_COLOR_RESET; if (append_atom(&resetv, &state, error_buf)) { pop_stack_element(&state.stack); diff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh index 9e0214076b4d7e..18554f62d94109 100755 --- a/t/t6300-for-each-ref.sh +++ b/t/t6300-for-each-ref.sh @@ -130,6 +130,8 @@ test_atom head parent:short=10 '' test_atom head numparent 0 test_atom head object '' test_atom head type '' +test_atom head raw "$(git cat-file commit refs/heads/main) +" test_atom head '*objectname' '' test_atom head '*objecttype' '' test_atom head author 'A U Thor 1151968724 +0200' @@ -221,6 +223,15 @@ test_atom tag contents 'Tagging at 1151968727 ' test_atom tag HEAD ' ' +test_expect_success 'basic atom: refs/tags/testtag *raw' ' + git cat-file commit refs/tags/testtag^{} >expected && + git for-each-ref --format="%(*raw)" refs/tags/testtag >actual && + sanitize_pgp expected.clean && + echo >>expected.clean && + sanitize_pgp actual.clean && + test_cmp expected.clean actual.clean +' + test_expect_success 'Check invalid atoms names are errors' ' test_must_fail git for-each-ref --format="%(INVALID)" refs/heads ' @@ -686,6 +697,15 @@ test_atom refs/tags/signed-empty contents:body '' test_atom refs/tags/signed-empty contents:signature "$sig" test_atom refs/tags/signed-empty contents "$sig" +test_expect_success GPG 'basic atom: refs/tags/signed-empty raw' ' + git cat-file tag refs/tags/signed-empty >expected && + git for-each-ref --format="%(raw)" refs/tags/signed-empty >actual && + sanitize_pgp expected.clean && + echo >>expected.clean && + sanitize_pgp actual.clean && + test_cmp expected.clean actual.clean +' + test_atom refs/tags/signed-short subject 'subject line' test_atom refs/tags/signed-short subject:sanitize 'subject-line' test_atom refs/tags/signed-short contents:subject 'subject line' @@ -695,6 +715,15 @@ test_atom refs/tags/signed-short contents:signature "$sig" test_atom refs/tags/signed-short contents "subject line $sig" +test_expect_success GPG 'basic atom: refs/tags/signed-short raw' ' + git cat-file tag refs/tags/signed-short >expected && + git for-each-ref --format="%(raw)" refs/tags/signed-short >actual && + sanitize_pgp expected.clean && + echo >>expected.clean && + sanitize_pgp actual.clean && + test_cmp expected.clean actual.clean +' + test_atom refs/tags/signed-long subject 'subject line' test_atom refs/tags/signed-long subject:sanitize 'subject-line' test_atom refs/tags/signed-long contents:subject 'subject line' @@ -708,6 +737,15 @@ test_atom refs/tags/signed-long contents "subject line body contents $sig" +test_expect_success GPG 'basic atom: refs/tags/signed-long raw' ' + git cat-file tag refs/tags/signed-long >expected && + git for-each-ref --format="%(raw)" refs/tags/signed-long >actual && + sanitize_pgp expected.clean && + echo >>expected.clean && + sanitize_pgp actual.clean && + test_cmp expected.clean actual.clean +' + test_expect_success 'set up refs pointing to tree and blob' ' git update-ref refs/mytrees/first refs/heads/main^{tree} && git update-ref refs/myblobs/first refs/heads/main:one @@ -720,6 +758,16 @@ test_atom refs/mytrees/first contents:body "" test_atom refs/mytrees/first contents:signature "" test_atom refs/mytrees/first contents "" +test_expect_success 'basic atom: refs/mytrees/first raw' ' + git cat-file tree refs/mytrees/first >expected && + echo >>expected && + git for-each-ref --format="%(raw)" refs/mytrees/first >actual && + test_cmp expected actual && + git cat-file -s refs/mytrees/first >expected && + git for-each-ref --format="%(raw:size)" refs/mytrees/first >actual && + test_cmp expected actual +' + test_atom refs/myblobs/first subject "" test_atom refs/myblobs/first contents:subject "" test_atom refs/myblobs/first body "" @@ -727,6 +775,174 @@ test_atom refs/myblobs/first contents:body "" test_atom refs/myblobs/first contents:signature "" test_atom refs/myblobs/first contents "" +test_expect_success 'basic atom: refs/myblobs/first raw' ' + git cat-file blob refs/myblobs/first >expected && + echo >>expected && + git for-each-ref --format="%(raw)" refs/myblobs/first >actual && + test_cmp expected actual && + git cat-file -s refs/myblobs/first >expected && + git for-each-ref --format="%(raw:size)" refs/myblobs/first >actual && + test_cmp expected actual +' + +test_expect_success 'set up refs pointing to binary blob' ' + printf "a\0b\0c" >blob1 && + printf "a\0c\0b" >blob2 && + printf "\0a\0b\0c" >blob3 && + printf "abc" >blob4 && + printf "\0 \0 \0 " >blob5 && + printf "\0 \0a\0 " >blob6 && + printf " " >blob7 && + >blob8 && + obj=$(git hash-object -w blob1) && + git update-ref refs/myblobs/blob1 "$obj" && + obj=$(git hash-object -w blob2) && + git update-ref refs/myblobs/blob2 "$obj" && + obj=$(git hash-object -w blob3) && + git update-ref refs/myblobs/blob3 "$obj" && + obj=$(git hash-object -w blob4) && + git update-ref refs/myblobs/blob4 "$obj" && + obj=$(git hash-object -w blob5) && + git update-ref refs/myblobs/blob5 "$obj" && + obj=$(git hash-object -w blob6) && + git update-ref refs/myblobs/blob6 "$obj" && + obj=$(git hash-object -w blob7) && + git update-ref refs/myblobs/blob7 "$obj" && + obj=$(git hash-object -w blob8) && + git update-ref refs/myblobs/blob8 "$obj" +' + +test_expect_success 'Verify sorts with raw' ' + cat >expected <<-EOF && + refs/myblobs/blob8 + refs/myblobs/blob5 + refs/myblobs/blob6 + refs/myblobs/blob3 + refs/myblobs/blob7 + refs/mytrees/first + refs/myblobs/first + refs/myblobs/blob1 + refs/myblobs/blob2 + refs/myblobs/blob4 + refs/heads/main + EOF + git for-each-ref --format="%(refname)" --sort=raw \ + refs/heads/main refs/myblobs/ refs/mytrees/first >actual && + test_cmp expected actual +' + +test_expect_success 'Verify sorts with raw:size' ' + cat >expected <<-EOF && + refs/myblobs/blob8 + refs/myblobs/first + refs/myblobs/blob7 + refs/heads/main + refs/myblobs/blob4 + refs/myblobs/blob1 + refs/myblobs/blob2 + refs/myblobs/blob3 + refs/myblobs/blob5 + refs/myblobs/blob6 + refs/mytrees/first + EOF + git for-each-ref --format="%(refname)" --sort=raw:size \ + refs/heads/main refs/myblobs/ refs/mytrees/first >actual && + test_cmp expected actual +' + +test_expect_success 'validate raw atom with %(if:equals)' ' + cat >expected <<-EOF && + not equals + not equals + not equals + not equals + not equals + not equals + refs/myblobs/blob4 + not equals + not equals + not equals + not equals + not equals + EOF + git for-each-ref --format="%(if:equals=abc)%(raw)%(then)%(refname)%(else)not equals%(end)" \ + refs/myblobs/ refs/heads/ >actual && + test_cmp expected actual +' + +test_expect_success 'validate raw atom with %(if:notequals)' ' + cat >expected <<-EOF && + refs/heads/ambiguous + refs/heads/main + refs/heads/newtag + refs/myblobs/blob1 + refs/myblobs/blob2 + refs/myblobs/blob3 + equals + refs/myblobs/blob5 + refs/myblobs/blob6 + refs/myblobs/blob7 + refs/myblobs/blob8 + refs/myblobs/first + EOF + git for-each-ref --format="%(if:notequals=abc)%(raw)%(then)%(refname)%(else)equals%(end)" \ + refs/myblobs/ refs/heads/ >actual && + test_cmp expected actual +' + +test_expect_success 'empty raw refs with %(if)' ' + cat >expected <<-EOF && + refs/myblobs/blob1 not empty + refs/myblobs/blob2 not empty + refs/myblobs/blob3 not empty + refs/myblobs/blob4 not empty + refs/myblobs/blob5 not empty + refs/myblobs/blob6 not empty + refs/myblobs/blob7 empty + refs/myblobs/blob8 empty + refs/myblobs/first not empty + EOF + git for-each-ref --format="%(refname) %(if)%(raw)%(then)not empty%(else)empty%(end)" \ + refs/myblobs/ >actual && + test_cmp expected actual +' + +test_expect_success '%(raw) with --python must fail' ' + test_must_fail git for-each-ref --format="%(raw)" --python +' + +test_expect_success '%(raw) with --tcl must fail' ' + test_must_fail git for-each-ref --format="%(raw)" --tcl +' + +test_expect_success '%(raw) with --perl must fail' ' + test_must_fail git for-each-ref --format="%(raw)" --perl +' + +test_expect_success '%(raw) with --shell must fail' ' + test_must_fail git for-each-ref --format="%(raw)" --shell +' + +test_expect_success '%(raw) with --shell and --sort=raw must fail' ' + test_must_fail git for-each-ref --format="%(raw)" --sort=raw --shell +' + +test_expect_success '%(raw:size) with --shell' ' + git for-each-ref --format="%(raw:size)" | while read line + do + echo "'\''$line'\''" >>expect + done && + git for-each-ref --format="%(raw:size)" --shell >actual && + test_cmp expect actual +' + +test_expect_success 'for-each-ref --format compare with cat-file --batch' ' + git rev-parse refs/mytrees/first | git cat-file --batch >expected && + git for-each-ref --format="%(objectname) %(objecttype) %(objectsize) +%(raw)" refs/mytrees/first >actual && + test_cmp expected actual +' + test_expect_success 'set up multiple-sort tags' ' for when in 100000 200000 do From a091df82b7de77b862d5e2090a65f6b2ad3a774c Mon Sep 17 00:00:00 2001 From: ZheNing Hu Date: Sat, 19 Jun 2021 11:55:03 +0800 Subject: [PATCH 19/31] [GSOC] ref-filter: --format=%(raw) re-support --perl MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Because the perl language can handle binary data correctly, add the function perl_quote_buf_with_len(), which can specify the length of the data and prevent the data from being truncated at '\0' to help `--format="%(raw)"` re-support `--perl`. Helped-by: Ævar Arnfjörð Bjarmason Signed-off-by: ZheNing Hu --- Documentation/git-for-each-ref.txt | 2 +- quote.c | 17 +++++++++++++++++ quote.h | 1 + ref-filter.c | 15 +++++++++++---- t/t6300-for-each-ref.sh | 19 +++++++++++++++++-- 5 files changed, 47 insertions(+), 7 deletions(-) diff --git a/Documentation/git-for-each-ref.txt b/Documentation/git-for-each-ref.txt index cbb6f87d13f776..86d0c0d621b5b0 100644 --- a/Documentation/git-for-each-ref.txt +++ b/Documentation/git-for-each-ref.txt @@ -241,7 +241,7 @@ raw:size:: The raw data size of the object. Note that `--format=%(raw)` can not be used with `--python`, `--shell`, `--tcl`, -`--perl` because such language may not support arbitrary binary data in their +because such language may not support arbitrary binary data in their string variable type. The message in a commit or a tag object is `contents`, from which diff --git a/quote.c b/quote.c index 8a3a5e39eb12ad..26719d21d1e755 100644 --- a/quote.c +++ b/quote.c @@ -471,6 +471,23 @@ void perl_quote_buf(struct strbuf *sb, const char *src) strbuf_addch(sb, sq); } +void perl_quote_buf_with_len(struct strbuf *sb, const char *src, size_t len) +{ + const char sq = '\''; + const char bq = '\\'; + const char *c = src; + const char *end = src + len; + + strbuf_addch(sb, sq); + while (c != end) { + if (*c == sq || *c == bq) + strbuf_addch(sb, bq); + strbuf_addch(sb, *c); + c++; + } + strbuf_addch(sb, sq); +} + void python_quote_buf(struct strbuf *sb, const char *src) { const char sq = '\''; diff --git a/quote.h b/quote.h index 768cc6338e27d7..0fe69e264b0e70 100644 --- a/quote.h +++ b/quote.h @@ -94,6 +94,7 @@ char *quote_path(const char *in, const char *prefix, struct strbuf *out, unsigne /* quoting as a string literal for other languages */ void perl_quote_buf(struct strbuf *sb, const char *src); +void perl_quote_buf_with_len(struct strbuf *sb, const char *src, size_t len); void python_quote_buf(struct strbuf *sb, const char *src); void tcl_quote_buf(struct strbuf *sb, const char *src); void basic_regex_quote_buf(struct strbuf *sb, const char *src); diff --git a/ref-filter.c b/ref-filter.c index 7822be903071c0..797b20ffa61247 100644 --- a/ref-filter.c +++ b/ref-filter.c @@ -742,7 +742,10 @@ static void quote_formatting(struct strbuf *s, const char *str, size_t len, int sq_quote_buf(s, str); break; case QUOTE_PERL: - perl_quote_buf(s, str); + if (len != ATOM_VALUE_S_SIZE_INIT) + perl_quote_buf_with_len(s, str, len); + else + perl_quote_buf(s, str); break; case QUOTE_PYTHON: python_quote_buf(s, str); @@ -1006,10 +1009,14 @@ int verify_ref_format(struct ref_format *format) at = parse_ref_filter_atom(format, sp + 2, ep, &err); if (at < 0) die("%s", err.buf); - if (format->quote_style && used_atom[at].atom_type == ATOM_RAW && - used_atom[at].u.raw_data.option == RAW_BARE) + + if ((format->quote_style == QUOTE_PYTHON || + format->quote_style == QUOTE_SHELL || + format->quote_style == QUOTE_TCL) && + used_atom[at].atom_type == ATOM_RAW && + used_atom[at].u.raw_data.option == RAW_BARE) die(_("--format=%.*s cannot be used with" - "--python, --shell, --tcl, --perl"), (int)(ep - sp - 2), sp + 2); + "--python, --shell, --tcl"), (int)(ep - sp - 2), sp + 2); cp = ep + 1; if (skip_prefix(used_atom[at].name, "color:", &color)) diff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh index 18554f62d94109..0b66e743c58e73 100755 --- a/t/t6300-for-each-ref.sh +++ b/t/t6300-for-each-ref.sh @@ -915,8 +915,23 @@ test_expect_success '%(raw) with --tcl must fail' ' test_must_fail git for-each-ref --format="%(raw)" --tcl ' -test_expect_success '%(raw) with --perl must fail' ' - test_must_fail git for-each-ref --format="%(raw)" --perl +test_expect_success '%(raw) with --perl' ' + git for-each-ref --format="\$name= %(raw); +print \"\$name\"" refs/myblobs/blob1 --perl | perl > actual && + cmp blob1 actual && + git for-each-ref --format="\$name= %(raw); +print \"\$name\"" refs/myblobs/blob3 --perl | perl > actual && + cmp blob3 actual && + git for-each-ref --format="\$name= %(raw); +print \"\$name\"" refs/myblobs/blob8 --perl | perl > actual && + cmp blob8 actual && + git for-each-ref --format="\$name= %(raw); +print \"\$name\"" refs/myblobs/first --perl | perl > actual && + cmp one actual && + git cat-file tree refs/mytrees/first > expected && + git for-each-ref --format="\$name= %(raw); +print \"\$name\"" refs/mytrees/first --perl | perl > actual && + cmp expected actual ' test_expect_success '%(raw) with --shell must fail' ' From 1729a5f0c8d7dbb48e52275136ba33ae2564a8be Mon Sep 17 00:00:00 2001 From: ZheNing Hu Date: Wed, 9 Jun 2021 16:12:00 +0000 Subject: [PATCH 20/31] [GSOC] ref-filter: use non-const ref_format in *_atom_parser() Use non-const ref_format in *_atom_parser(), which can help us modify the members of ref_format in *_atom_parser(). Mentored-by: Christian Couder Mentored-by: Hariom Verma Signed-off-by: ZheNing Hu --- builtin/tag.c | 2 +- ref-filter.c | 44 ++++++++++++++++++++++---------------------- ref-filter.h | 4 ++-- 3 files changed, 25 insertions(+), 25 deletions(-) diff --git a/builtin/tag.c b/builtin/tag.c index 82fcfc0982423f..452558ec957519 100644 --- a/builtin/tag.c +++ b/builtin/tag.c @@ -146,7 +146,7 @@ static int verify_tag(const char *name, const char *ref, const struct object_id *oid, void *cb_data) { int flags; - const struct ref_format *format = cb_data; + struct ref_format *format = cb_data; flags = GPG_VERIFY_VERBOSE; if (format->format) diff --git a/ref-filter.c b/ref-filter.c index 797b20ffa61247..d01a0266fb89da 100644 --- a/ref-filter.c +++ b/ref-filter.c @@ -226,7 +226,7 @@ static int strbuf_addf_ret(struct strbuf *sb, int ret, const char *fmt, ...) return ret; } -static int color_atom_parser(const struct ref_format *format, struct used_atom *atom, +static int color_atom_parser(struct ref_format *format, struct used_atom *atom, const char *color_value, struct strbuf *err) { if (!color_value) @@ -264,7 +264,7 @@ static int refname_atom_parser_internal(struct refname_atom *atom, const char *a return 0; } -static int remote_ref_atom_parser(const struct ref_format *format, struct used_atom *atom, +static int remote_ref_atom_parser(struct ref_format *format, struct used_atom *atom, const char *arg, struct strbuf *err) { struct string_list params = STRING_LIST_INIT_DUP; @@ -311,7 +311,7 @@ static int remote_ref_atom_parser(const struct ref_format *format, struct used_a return 0; } -static int objecttype_atom_parser(const struct ref_format *format, struct used_atom *atom, +static int objecttype_atom_parser(struct ref_format *format, struct used_atom *atom, const char *arg, struct strbuf *err) { if (arg) @@ -323,7 +323,7 @@ static int objecttype_atom_parser(const struct ref_format *format, struct used_a return 0; } -static int objectsize_atom_parser(const struct ref_format *format, struct used_atom *atom, +static int objectsize_atom_parser(struct ref_format *format, struct used_atom *atom, const char *arg, struct strbuf *err) { if (!arg) { @@ -343,7 +343,7 @@ static int objectsize_atom_parser(const struct ref_format *format, struct used_a return 0; } -static int deltabase_atom_parser(const struct ref_format *format, struct used_atom *atom, +static int deltabase_atom_parser(struct ref_format *format, struct used_atom *atom, const char *arg, struct strbuf *err) { if (arg) @@ -355,7 +355,7 @@ static int deltabase_atom_parser(const struct ref_format *format, struct used_at return 0; } -static int body_atom_parser(const struct ref_format *format, struct used_atom *atom, +static int body_atom_parser(struct ref_format *format, struct used_atom *atom, const char *arg, struct strbuf *err) { if (arg) @@ -364,7 +364,7 @@ static int body_atom_parser(const struct ref_format *format, struct used_atom *a return 0; } -static int subject_atom_parser(const struct ref_format *format, struct used_atom *atom, +static int subject_atom_parser(struct ref_format *format, struct used_atom *atom, const char *arg, struct strbuf *err) { if (!arg) @@ -376,7 +376,7 @@ static int subject_atom_parser(const struct ref_format *format, struct used_atom return 0; } -static int trailers_atom_parser(const struct ref_format *format, struct used_atom *atom, +static int trailers_atom_parser(struct ref_format *format, struct used_atom *atom, const char *arg, struct strbuf *err) { atom->u.contents.trailer_opts.no_divider = 1; @@ -402,7 +402,7 @@ static int trailers_atom_parser(const struct ref_format *format, struct used_ato return 0; } -static int contents_atom_parser(const struct ref_format *format, struct used_atom *atom, +static int contents_atom_parser(struct ref_format *format, struct used_atom *atom, const char *arg, struct strbuf *err) { if (!arg) @@ -430,7 +430,7 @@ static int contents_atom_parser(const struct ref_format *format, struct used_ato return 0; } -static int raw_atom_parser(const struct ref_format *format, struct used_atom *atom, +static int raw_atom_parser(struct ref_format *format, struct used_atom *atom, const char *arg, struct strbuf *err) { if (!arg) @@ -442,7 +442,7 @@ static int raw_atom_parser(const struct ref_format *format, struct used_atom *at return 0; } -static int oid_atom_parser(const struct ref_format *format, struct used_atom *atom, +static int oid_atom_parser(struct ref_format *format, struct used_atom *atom, const char *arg, struct strbuf *err) { if (!arg) @@ -461,7 +461,7 @@ static int oid_atom_parser(const struct ref_format *format, struct used_atom *at return 0; } -static int person_email_atom_parser(const struct ref_format *format, struct used_atom *atom, +static int person_email_atom_parser(struct ref_format *format, struct used_atom *atom, const char *arg, struct strbuf *err) { if (!arg) @@ -475,7 +475,7 @@ static int person_email_atom_parser(const struct ref_format *format, struct used return 0; } -static int refname_atom_parser(const struct ref_format *format, struct used_atom *atom, +static int refname_atom_parser(struct ref_format *format, struct used_atom *atom, const char *arg, struct strbuf *err) { return refname_atom_parser_internal(&atom->u.refname, arg, atom->name, err); @@ -492,7 +492,7 @@ static align_type parse_align_position(const char *s) return -1; } -static int align_atom_parser(const struct ref_format *format, struct used_atom *atom, +static int align_atom_parser(struct ref_format *format, struct used_atom *atom, const char *arg, struct strbuf *err) { struct align *align = &atom->u.align; @@ -544,7 +544,7 @@ static int align_atom_parser(const struct ref_format *format, struct used_atom * return 0; } -static int if_atom_parser(const struct ref_format *format, struct used_atom *atom, +static int if_atom_parser(struct ref_format *format, struct used_atom *atom, const char *arg, struct strbuf *err) { if (!arg) { @@ -559,7 +559,7 @@ static int if_atom_parser(const struct ref_format *format, struct used_atom *ato return 0; } -static int head_atom_parser(const struct ref_format *format, struct used_atom *atom, +static int head_atom_parser(struct ref_format *format, struct used_atom *atom, const char *arg, struct strbuf *unused_err) { atom->u.head = resolve_refdup("HEAD", RESOLVE_REF_READING, NULL, NULL); @@ -570,7 +570,7 @@ static struct { const char *name; info_source source; cmp_type cmp_type; - int (*parser)(const struct ref_format *format, struct used_atom *atom, + int (*parser)(struct ref_format *format, struct used_atom *atom, const char *arg, struct strbuf *err); } valid_atom[] = { [ATOM_REFNAME] = { "refname", SOURCE_NONE, FIELD_STR, refname_atom_parser }, @@ -649,7 +649,7 @@ struct atom_value { /* * Used to parse format string and sort specifiers */ -static int parse_ref_filter_atom(const struct ref_format *format, +static int parse_ref_filter_atom(struct ref_format *format, const char *atom, const char *ep, struct strbuf *err) { @@ -2553,9 +2553,9 @@ static void append_literal(const char *cp, const char *ep, struct ref_formatting } int format_ref_array_item(struct ref_array_item *info, - const struct ref_format *format, - struct strbuf *final_buf, - struct strbuf *error_buf) + struct ref_format *format, + struct strbuf *final_buf, + struct strbuf *error_buf) { const char *cp, *sp, *ep; struct ref_formatting_state state = REF_FORMATTING_STATE_INIT; @@ -2600,7 +2600,7 @@ int format_ref_array_item(struct ref_array_item *info, } void pretty_print_ref(const char *name, const struct object_id *oid, - const struct ref_format *format) + struct ref_format *format) { struct ref_array_item *ref_item; struct strbuf output = STRBUF_INIT; diff --git a/ref-filter.h b/ref-filter.h index baf72a71896527..74fb423fc89f8d 100644 --- a/ref-filter.h +++ b/ref-filter.h @@ -116,7 +116,7 @@ void ref_array_sort(struct ref_sorting *sort, struct ref_array *array); void ref_sorting_set_sort_flags_all(struct ref_sorting *sorting, unsigned int mask, int on); /* Based on the given format and quote_style, fill the strbuf */ int format_ref_array_item(struct ref_array_item *info, - const struct ref_format *format, + struct ref_format *format, struct strbuf *final_buf, struct strbuf *error_buf); /* Parse a single sort specifier and add it to the list */ @@ -137,7 +137,7 @@ void setup_ref_filter_porcelain_msg(void); * name must be a fully qualified refname. */ void pretty_print_ref(const char *name, const struct object_id *oid, - const struct ref_format *format); + struct ref_format *format); /* * Push a single ref onto the array; this can be used to construct your own From 55f8b27ae23a023b92151549bbea62740744da7b Mon Sep 17 00:00:00 2001 From: ZheNing Hu Date: Wed, 9 Jun 2021 16:12:01 +0000 Subject: [PATCH 21/31] [GSOC] ref-filter: add %(rest) atom In order to let "cat-file --batch=%(rest)" use the ref-filter interface, add %(rest) atom for ref-filter. "git for-each-ref", "git branch", "git tag" and "git verify-tag" will reject %(rest) by default. Mentored-by: Christian Couder Mentored-by: Hariom Verma Signed-off-by: ZheNing Hu --- ref-filter.c | 21 +++++++++++++++++++++ ref-filter.h | 5 ++++- t/t3203-branch-output.sh | 4 ++++ t/t6300-for-each-ref.sh | 4 ++++ t/t7004-tag.sh | 4 ++++ t/t7030-verify-tag.sh | 4 ++++ 6 files changed, 41 insertions(+), 1 deletion(-) diff --git a/ref-filter.c b/ref-filter.c index d01a0266fb89da..10c78de9cfa424 100644 --- a/ref-filter.c +++ b/ref-filter.c @@ -157,6 +157,7 @@ enum atom_type { ATOM_IF, ATOM_THEN, ATOM_ELSE, + ATOM_REST, }; /* @@ -559,6 +560,15 @@ static int if_atom_parser(struct ref_format *format, struct used_atom *atom, return 0; } +static int rest_atom_parser(struct ref_format *format, struct used_atom *atom, + const char *arg, struct strbuf *err) +{ + if (arg) + return strbuf_addf_ret(err, -1, _("%%(rest) does not take arguments")); + format->use_rest = 1; + return 0; +} + static int head_atom_parser(struct ref_format *format, struct used_atom *atom, const char *arg, struct strbuf *unused_err) { @@ -615,6 +625,7 @@ static struct { [ATOM_IF] = { "if", SOURCE_NONE, FIELD_STR, if_atom_parser }, [ATOM_THEN] = { "then", SOURCE_NONE }, [ATOM_ELSE] = { "else", SOURCE_NONE }, + [ATOM_REST] = { "rest", SOURCE_NONE, FIELD_STR, rest_atom_parser }, /* * Please update $__git_ref_fieldlist in git-completion.bash * when you add new atoms @@ -1010,6 +1021,9 @@ int verify_ref_format(struct ref_format *format) if (at < 0) die("%s", err.buf); + if (used_atom[at].atom_type == ATOM_REST) + die("this command reject atom %%(%.*s)", (int)(ep - sp - 2), sp + 2); + if ((format->quote_style == QUOTE_PYTHON || format->quote_style == QUOTE_SHELL || format->quote_style == QUOTE_TCL) && @@ -1927,6 +1941,12 @@ static int populate_value(struct ref_array_item *ref, struct strbuf *err) v->handler = else_atom_handler; v->s = xstrdup(""); continue; + } else if (atom_type == ATOM_REST) { + if (ref->rest) + v->s = xstrdup(ref->rest); + else + v->s = xstrdup(""); + continue; } else continue; @@ -2144,6 +2164,7 @@ static struct ref_array_item *new_ref_array_item(const char *refname, FLEX_ALLOC_STR(ref, refname, refname); oidcpy(&ref->objectname, oid); + ref->rest = NULL; return ref; } diff --git a/ref-filter.h b/ref-filter.h index 74fb423fc89f8d..c15dee8d6b95ba 100644 --- a/ref-filter.h +++ b/ref-filter.h @@ -38,6 +38,7 @@ struct ref_sorting { struct ref_array_item { struct object_id objectname; + const char *rest; int flag; unsigned int kind; const char *symref; @@ -76,14 +77,16 @@ struct ref_format { * verify_ref_format() afterwards to finalize. */ const char *format; + const char *rest; int quote_style; + int use_rest; int use_color; /* Internal state to ref-filter */ int need_color_reset_at_eol; }; -#define REF_FORMAT_INIT { NULL, 0, -1 } +#define REF_FORMAT_INIT { .use_color = -1 } /* Macros for checking --merged and --no-merged options */ #define _OPT_MERGED_NO_MERGED(option, filter, h) \ diff --git a/t/t3203-branch-output.sh b/t/t3203-branch-output.sh index 5325b9f67a0078..6e94c6db7b5aa9 100755 --- a/t/t3203-branch-output.sh +++ b/t/t3203-branch-output.sh @@ -340,6 +340,10 @@ test_expect_success 'git branch --format option' ' test_cmp expect actual ' +test_expect_success 'git branch with --format=%(rest) must fail' ' + test_must_fail git branch --format="%(rest)" >actual +' + test_expect_success 'worktree colors correct' ' cat >expect <<-EOF && * (HEAD detached from fromtag) diff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh index 0b66e743c58e73..6ca5c2cc19cfb7 100755 --- a/t/t6300-for-each-ref.sh +++ b/t/t6300-for-each-ref.sh @@ -1211,6 +1211,10 @@ test_expect_success 'basic atom: head contents:trailers' ' test_cmp expect actual.clean ' +test_expect_success 'basic atom: rest must fail' ' + test_must_fail git for-each-ref --format="%(rest)" refs/heads/main +' + test_expect_success 'trailer parsing not fooled by --- line' ' git commit --allow-empty -F - <<-\EOF && this is the subject diff --git a/t/t7004-tag.sh b/t/t7004-tag.sh index 2f72c5c6883e3e..082be85dffc7b1 100755 --- a/t/t7004-tag.sh +++ b/t/t7004-tag.sh @@ -1998,6 +1998,10 @@ test_expect_success '--format should list tags as per format given' ' test_cmp expect actual ' +test_expect_success 'git tag -l with --format="%(rest)" must fail' ' + test_must_fail git tag -l --format="%(rest)" "v1*" +' + test_expect_success "set up color tests" ' echo "v1.0" >expect.color && echo "v1.0" >expect.bare && diff --git a/t/t7030-verify-tag.sh b/t/t7030-verify-tag.sh index 3cefde9602bf73..10faa645157ea4 100755 --- a/t/t7030-verify-tag.sh +++ b/t/t7030-verify-tag.sh @@ -194,6 +194,10 @@ test_expect_success GPG 'verifying tag with --format' ' test_cmp expect actual ' +test_expect_success GPG 'verifying tag with --format="%(rest)" must fail' ' + test_must_fail git verify-tag --format="%(rest)" "fourth-signed" +' + test_expect_success GPG 'verifying a forged tag with --format should fail silently' ' test_must_fail git verify-tag --format="tagname : %(tag)" $(cat forged1.tag) >actual-forged && test_must_be_empty actual-forged From f02e7c348903462bb42960db32dfb8a1a1585395 Mon Sep 17 00:00:00 2001 From: ZheNing Hu Date: Sat, 12 Jun 2021 14:54:46 +0800 Subject: [PATCH 22/31] [GSOC] ref-filter: pass get_object() return value to their callers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Because in the refactor of `git cat-file --batch` later, oid_object_info_extended() in get_object() will be used to obtain the info of an object with it's oid. When the object cannot be obtained in the git repository, `cat-file --batch` expects to output " missing" and continue the next oid query instead of letting Git exit. In other error conditions, Git should exit normally. So we can achieve this function by passing the return value of get_object(). Mentored-by: Christian Couder Mentored-by: Hariom Verma Helped-by: Ævar Arnfjörð Bjarmason Signed-off-by: ZheNing Hu --- ref-filter.c | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/ref-filter.c b/ref-filter.c index 10c78de9cfa424..e4988aa8a24fe6 100644 --- a/ref-filter.c +++ b/ref-filter.c @@ -1816,6 +1816,7 @@ static int populate_value(struct ref_array_item *ref, struct strbuf *err) { struct object *obj; int i; + int ret; struct object_info empty = OBJECT_INFO_INIT; CALLOC_ARRAY(ref->value, used_atom_cnt); @@ -1972,8 +1973,9 @@ static int populate_value(struct ref_array_item *ref, struct strbuf *err) oi.oid = ref->objectname; - if (get_object(ref, 0, &obj, &oi, err)) - return -1; + ret = get_object(ref, 0, &obj, &oi, err); + if (ret) + return ret; /* * If there is no atom that wants to know about tagged @@ -2005,8 +2007,10 @@ static int get_ref_atom_value(struct ref_array_item *ref, int atom, struct atom_value **v, struct strbuf *err) { if (!ref->value) { - if (populate_value(ref, err)) - return -1; + int ret = populate_value(ref, err); + + if (ret) + return ret; fill_missing_values(ref->value); } *v = &ref->value[atom]; @@ -2580,6 +2584,7 @@ int format_ref_array_item(struct ref_array_item *info, { const char *cp, *sp, *ep; struct ref_formatting_state state = REF_FORMATTING_STATE_INIT; + int ret; state.quote_style = format->quote_style; push_stack_element(&state.stack); @@ -2592,10 +2597,10 @@ int format_ref_array_item(struct ref_array_item *info, if (cp < sp) append_literal(cp, sp, &state); pos = parse_ref_filter_atom(format, sp + 2, ep, error_buf); - if (pos < 0 || get_ref_atom_value(info, pos, &atomv, error_buf) || + if (pos < 0 || (ret = get_ref_atom_value(info, pos, &atomv, error_buf)) || atomv->handler(atomv, &state, error_buf)) { pop_stack_element(&state.stack); - return -1; + return ret ? ret : -1; } } if (*cp) { From d3a986d5ccc2f3ee752f7a66367594b1b8c9e8f9 Mon Sep 17 00:00:00 2001 From: ZheNing Hu Date: Tue, 15 Jun 2021 19:33:17 +0800 Subject: [PATCH 23/31] [GSOC] ref-filter: introduce free_ref_array_item_value() function When we use ref_array_item which is not dynamically allocated and want to free the space of its member "value" after the end of use, free_array_item() does not meet our needs, because it tries to free ref_array_item itself and its member "symref". Introduce free_ref_array_item_value() for freeing ref_array_item value. It will be called internally by free_array_item(), and it will help `cat-file --batch` free ref_array_item's value memory later. Helped-by: Junio C Hamano Mentored-by: Christian Couder Mentored-by: Hariom Verma Signed-off-by: ZheNing Hu --- ref-filter.c | 11 ++++++++--- ref-filter.h | 2 ++ 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/ref-filter.c b/ref-filter.c index e4988aa8a24fe6..731e596eaa6e43 100644 --- a/ref-filter.c +++ b/ref-filter.c @@ -2291,16 +2291,21 @@ static int ref_filter_handler(const char *refname, const struct object_id *oid, return 0; } -/* Free memory allocated for a ref_array_item */ -static void free_array_item(struct ref_array_item *item) +void free_ref_array_item_value(struct ref_array_item *item) { - free((char *)item->symref); if (item->value) { int i; for (i = 0; i < used_atom_cnt; i++) free((char *)item->value[i].s); free(item->value); } +} + +/* Free memory allocated for a ref_array_item */ +static void free_array_item(struct ref_array_item *item) +{ + free((char *)item->symref); + free_ref_array_item_value(item); free(item); } diff --git a/ref-filter.h b/ref-filter.h index c15dee8d6b95ba..44e6dc05ac2fbc 100644 --- a/ref-filter.h +++ b/ref-filter.h @@ -111,6 +111,8 @@ struct ref_format { int filter_refs(struct ref_array *array, struct ref_filter *filter, unsigned int type); /* Clear all memory allocated to ref_array */ void ref_array_clear(struct ref_array *array); +/* Free ref_array_item's value */ +void free_ref_array_item_value(struct ref_array_item *item); /* Used to verify if the given format is correct and to parse out the used atoms */ int verify_ref_format(struct ref_format *format); /* Sort the given ref_array as per the ref_sorting provided */ From 4ae142a2d38970febeb549241e744b8247c06cf3 Mon Sep 17 00:00:00 2001 From: ZheNing Hu Date: Mon, 21 Jun 2021 22:56:27 +0800 Subject: [PATCH 24/31] [GSOC] ref-filter: add cat_file_mode in struct ref_format Add `cat_file_mode` member in struct `ref_format`, when `cat-file --batch` use ref-filter logic later, it can help us reject atoms in verify_ref_format() which cat-file cannot use, e.g. `%(refname)`, `%(push)`, `%(upstream)`... Mentored-by: Christian Couder Mentored-by: Hariom Verma Signed-off-by: ZheNing Hu --- ref-filter.c | 11 +++++++++-- ref-filter.h | 1 + 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/ref-filter.c b/ref-filter.c index 731e596eaa6e43..45122959eef12b 100644 --- a/ref-filter.c +++ b/ref-filter.c @@ -1021,8 +1021,15 @@ int verify_ref_format(struct ref_format *format) if (at < 0) die("%s", err.buf); - if (used_atom[at].atom_type == ATOM_REST) - die("this command reject atom %%(%.*s)", (int)(ep - sp - 2), sp + 2); + if ((!format->cat_file_mode && used_atom[at].atom_type == ATOM_REST) || + (format->cat_file_mode && (used_atom[at].atom_type == ATOM_FLAG || + used_atom[at].atom_type == ATOM_HEAD || + used_atom[at].atom_type == ATOM_PUSH || + used_atom[at].atom_type == ATOM_REFNAME || + used_atom[at].atom_type == ATOM_SYMREF || + used_atom[at].atom_type == ATOM_UPSTREAM || + used_atom[at].atom_type == ATOM_WORKTREEPATH))) + die(_("this command reject atom %%(%.*s)"), (int)(ep - sp - 2), sp + 2); if ((format->quote_style == QUOTE_PYTHON || format->quote_style == QUOTE_SHELL || diff --git a/ref-filter.h b/ref-filter.h index 44e6dc05ac2fbc..053980a6a42667 100644 --- a/ref-filter.h +++ b/ref-filter.h @@ -78,6 +78,7 @@ struct ref_format { */ const char *format; const char *rest; + int cat_file_mode; int quote_style; int use_rest; int use_color; From 1bb0754a880cb6aac0e4422e6967493774180d62 Mon Sep 17 00:00:00 2001 From: ZheNing Hu Date: Mon, 21 Jun 2021 23:01:20 +0800 Subject: [PATCH 25/31] [GSOC] ref-filter: modify the error message and value in get_object Let get_object() return 1 and print " missing" instead of returning -1 and printing "missing object for " if oid_object_info_extended() unable to find the data corresponding to oid. When `cat-file --batch` use ref-filter logic later it can help `format_ref_array_item()` just report that the object is missing without letting Git exit. Mentored-by: Christian Couder Mentored-by: Hariom Verma Signed-off-by: ZheNing Hu --- ref-filter.c | 4 ++-- t/t6301-for-each-ref-errors.sh | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/ref-filter.c b/ref-filter.c index 45122959eef12b..9ca3dd5557dc5f 100644 --- a/ref-filter.c +++ b/ref-filter.c @@ -1749,8 +1749,8 @@ static int get_object(struct ref_array_item *ref, int deref, struct object **obj } if (oid_object_info_extended(the_repository, &oi->oid, &oi->info, OBJECT_INFO_LOOKUP_REPLACE)) - return strbuf_addf_ret(err, -1, _("missing object %s for %s"), - oid_to_hex(&oi->oid), ref->refname); + return strbuf_addf_ret(err, 1, _("%s missing"), + oid_to_hex(&oi->oid)); if (oi->info.disk_sizep && oi->disk_size < 0) BUG("Object size is less than zero."); diff --git a/t/t6301-for-each-ref-errors.sh b/t/t6301-for-each-ref-errors.sh index 40edf9dab53402..3553f84a00c153 100755 --- a/t/t6301-for-each-ref-errors.sh +++ b/t/t6301-for-each-ref-errors.sh @@ -41,7 +41,7 @@ test_expect_success 'Missing objects are reported correctly' ' r=refs/heads/missing && echo $MISSING >.git/$r && test_when_finished "rm -f .git/$r" && - echo "fatal: missing object $MISSING for $r" >missing-err && + echo "fatal: $MISSING missing" >missing-err && test_must_fail git for-each-ref 2>err && test_cmp missing-err err && ( From 1a668d8fe1302a6dfc205871e1e724f473d3a74d Mon Sep 17 00:00:00 2001 From: ZheNing Hu Date: Mon, 21 Jun 2021 22:46:50 +0800 Subject: [PATCH 26/31] [GSOC] cat-file: add has_object_file() check Use `has_object_file()` in `batch_one_object()` to check whether the input object exists. This can help us reject the missing oid when we let `cat-file --batch` use ref-filter logic later. Mentored-by: Christian Couder Mentored-by: Hariom Verma Signed-off-by: ZheNing Hu --- builtin/cat-file.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/builtin/cat-file.c b/builtin/cat-file.c index 243fe6844bc67a..59a86412fd0eea 100644 --- a/builtin/cat-file.c +++ b/builtin/cat-file.c @@ -428,6 +428,13 @@ static void batch_one_object(const char *obj_name, return; } + if (!has_object_file(&data->oid)) { + printf("%s missing\n", + obj_name ? obj_name : oid_to_hex(&data->oid)); + fflush(stdout); + return; + } + batch_object_write(obj_name, scratch, opt, data); } From 8f6239d90dd9ee2b76afd95fbebb11305ea000a8 Mon Sep 17 00:00:00 2001 From: ZheNing Hu Date: Tue, 22 Jun 2021 10:09:14 +0800 Subject: [PATCH 27/31] [GSOC] cat-file: change batch_objects parameter name Because later cat-file reuses ref-filter logic that will add parameter "const struct option *options" to batch_objects(), the two synonymous parameters of "opt" and "options" may confuse readers, so change batch_options parameter of batch_objects() from "opt" to "batch". Mentored-by: Christian Couder Mentored-by: Hariom Verma Signed-off-by: ZheNing Hu --- builtin/cat-file.c | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/builtin/cat-file.c b/builtin/cat-file.c index 59a86412fd0eea..41d407638d529d 100644 --- a/builtin/cat-file.c +++ b/builtin/cat-file.c @@ -495,7 +495,7 @@ static int batch_unordered_packed(const struct object_id *oid, return batch_unordered_object(oid, data); } -static int batch_objects(struct batch_options *opt) +static int batch_objects(struct batch_options *batch) { struct strbuf input = STRBUF_INIT; struct strbuf output = STRBUF_INIT; @@ -503,8 +503,8 @@ static int batch_objects(struct batch_options *opt) int save_warning; int retval = 0; - if (!opt->format) - opt->format = "%(objectname) %(objecttype) %(objectsize)"; + if (!batch->format) + batch->format = "%(objectname) %(objecttype) %(objectsize)"; /* * Expand once with our special mark_query flag, which will prime the @@ -513,20 +513,20 @@ static int batch_objects(struct batch_options *opt) */ memset(&data, 0, sizeof(data)); data.mark_query = 1; - strbuf_expand(&output, opt->format, expand_format, &data); + strbuf_expand(&output, batch->format, expand_format, &data); data.mark_query = 0; strbuf_release(&output); - if (opt->cmdmode) + if (batch->cmdmode) data.split_on_whitespace = 1; /* * If we are printing out the object, then always fill in the type, * since we will want to decide whether or not to stream. */ - if (opt->print_contents) + if (batch->print_contents) data.info.typep = &data.type; - if (opt->all_objects) { + if (batch->all_objects) { struct object_cb_data cb; struct object_info empty = OBJECT_INFO_INIT; @@ -536,11 +536,11 @@ static int batch_objects(struct batch_options *opt) if (has_promisor_remote()) warning("This repository uses promisor remotes. Some objects may not be loaded."); - cb.opt = opt; + cb.opt = batch; cb.expand = &data; cb.scratch = &output; - if (opt->unordered) { + if (batch->unordered) { struct oidset seen = OIDSET_INIT; cb.seen = &seen; @@ -590,7 +590,7 @@ static int batch_objects(struct batch_options *opt) data.rest = p; } - batch_one_object(input.buf, &output, opt, &data); + batch_one_object(input.buf, &output, batch, &data); } strbuf_release(&input); From 8c2ebaf93cda791c601cf3b0887a7b43dc8e5218 Mon Sep 17 00:00:00 2001 From: ZheNing Hu Date: Thu, 10 Jun 2021 16:50:13 +0800 Subject: [PATCH 28/31] [GSOC] cat-file: reuse ref-filter logic MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In order to let cat-file use ref-filter logic, let's do the following: 1. Change the type of member `format` in struct `batch_options` to `ref_format`, we will pass it to ref-filter later. 2. Let `batch_objects()` add atoms to format, and use `verify_ref_format()` to check atoms. 3. Use `format_ref_array_item()` in `batch_object_write()` to get the formatted data corresponding to the object. If the return value of `format_ref_array_item()` is equals to zero, use `batch_write()` to print object data; else if the return value is less than zero, use `die()` to print the error message and exit; else if return value is greater than zero, only print the error message, but don't exit. 4. Use free_ref_array_item_value() to free ref_array_item's value. Most of the atoms in `for-each-ref --format` are now supported, such as `%(tree)`, `%(parent)`, `%(author)`, `%(tagger)`, `%(if)`, `%(then)`, `%(else)`, `%(end)`. But these atoms will be rejected: `%(refname)`, `%(symref)`, `%(upstream)`, `%(push)`, `%(worktreepath)`, `%(flag)`, `%(HEAD)`, because these atoms are unique to those objects that pointed to by a ref, "for-each-ref"'s family can naturally use these atoms, but not all objects are pointed to be a ref, so "cat-file" will not be able to use them. The performance for `git cat-file --batch-all-objects --batch-check` on the Git repository itself with performance testing tool `hyperfine` changes from 669.4 ms ± 31.1 ms to 1.134 s ± 0.063 s. The performance for `git cat-file --batch-all-objects --batch >/dev/null` on the Git repository itself with performance testing tool `time` change from "27.37s user 0.29s system 98% cpu 28.089 total" to "33.69s user 1.54s system 87% cpu 40.258 total". Mentored-by: Christian Couder Mentored-by: Hariom Verma Signed-off-by: ZheNing Hu --- Documentation/git-cat-file.txt | 6 + builtin/cat-file.c | 242 ++++++------------------------- t/t1006-cat-file.sh | 252 +++++++++++++++++++++++++++++++++ 3 files changed, 305 insertions(+), 195 deletions(-) diff --git a/Documentation/git-cat-file.txt b/Documentation/git-cat-file.txt index 4eb0421b3fd946..ef8ab952b2fa05 100644 --- a/Documentation/git-cat-file.txt +++ b/Documentation/git-cat-file.txt @@ -226,6 +226,12 @@ newline. The available atoms are: after that first run of whitespace (i.e., the "rest" of the line) are output in place of the `%(rest)` atom. +Note that most of the atoms in `for-each-ref --format` are now supported, +such as `%(tree)`, `%(parent)`, `%(author)`, `%(tagger)`, `%(if)`, +`%(then)`, `%(else)`, `%(end)`. But these atoms will be rejected: +`%(refname)`, `%(symref)`, `%(upstream)`, `%(push)`, `%(worktreepath)`, +`%(flag)`, `%(HEAD)`. See linkgit:git-for-each-ref[1]. + If no format is specified, the default format is `%(objectname) %(objecttype) %(objectsize)`. diff --git a/builtin/cat-file.c b/builtin/cat-file.c index 41d407638d529d..5b163551fc687e 100644 --- a/builtin/cat-file.c +++ b/builtin/cat-file.c @@ -16,6 +16,7 @@ #include "packfile.h" #include "object-store.h" #include "promisor-remote.h" +#include "ref-filter.h" struct batch_options { int enabled; @@ -25,7 +26,7 @@ struct batch_options { int all_objects; int unordered; int cmdmode; /* may be 'w' or 'c' for --filters or --textconv */ - const char *format; + struct ref_format format; }; static const char *force_path; @@ -195,99 +196,10 @@ static int cat_one_file(int opt, const char *exp_type, const char *obj_name, struct expand_data { struct object_id oid; - enum object_type type; - unsigned long size; - off_t disk_size; const char *rest; - struct object_id delta_base_oid; - - /* - * If mark_query is true, we do not expand anything, but rather - * just mark the object_info with items we wish to query. - */ - int mark_query; - - /* - * Whether to split the input on whitespace before feeding it to - * get_sha1; this is decided during the mark_query phase based on - * whether we have a %(rest) token in our format. - */ int split_on_whitespace; - - /* - * After a mark_query run, this object_info is set up to be - * passed to oid_object_info_extended. It will point to the data - * elements above, so you can retrieve the response from there. - */ - struct object_info info; - - /* - * This flag will be true if the requested batch format and options - * don't require us to call oid_object_info, which can then be - * optimized out. - */ - unsigned skip_object_info : 1; }; -static int is_atom(const char *atom, const char *s, int slen) -{ - int alen = strlen(atom); - return alen == slen && !memcmp(atom, s, alen); -} - -static void expand_atom(struct strbuf *sb, const char *atom, int len, - void *vdata) -{ - struct expand_data *data = vdata; - - if (is_atom("objectname", atom, len)) { - if (!data->mark_query) - strbuf_addstr(sb, oid_to_hex(&data->oid)); - } else if (is_atom("objecttype", atom, len)) { - if (data->mark_query) - data->info.typep = &data->type; - else - strbuf_addstr(sb, type_name(data->type)); - } else if (is_atom("objectsize", atom, len)) { - if (data->mark_query) - data->info.sizep = &data->size; - else - strbuf_addf(sb, "%"PRIuMAX , (uintmax_t)data->size); - } else if (is_atom("objectsize:disk", atom, len)) { - if (data->mark_query) - data->info.disk_sizep = &data->disk_size; - else - strbuf_addf(sb, "%"PRIuMAX, (uintmax_t)data->disk_size); - } else if (is_atom("rest", atom, len)) { - if (data->mark_query) - data->split_on_whitespace = 1; - else if (data->rest) - strbuf_addstr(sb, data->rest); - } else if (is_atom("deltabase", atom, len)) { - if (data->mark_query) - data->info.delta_base_oid = &data->delta_base_oid; - else - strbuf_addstr(sb, - oid_to_hex(&data->delta_base_oid)); - } else - die("unknown format element: %.*s", len, atom); -} - -static size_t expand_format(struct strbuf *sb, const char *start, void *data) -{ - const char *end; - - if (*start != '(') - return 0; - end = strchr(start + 1, ')'); - if (!end) - die("format element '%s' does not end in ')'", start); - - expand_atom(sb, start + 1, end - start - 1, data); - - return end - start + 1; -} - static void batch_write(struct batch_options *opt, const void *data, int len) { if (opt->buffer_output) { @@ -297,87 +209,34 @@ static void batch_write(struct batch_options *opt, const void *data, int len) write_or_die(1, data, len); } -static void print_object_or_die(struct batch_options *opt, struct expand_data *data) -{ - const struct object_id *oid = &data->oid; - - assert(data->info.typep); - - if (data->type == OBJ_BLOB) { - if (opt->buffer_output) - fflush(stdout); - if (opt->cmdmode) { - char *contents; - unsigned long size; - - if (!data->rest) - die("missing path for '%s'", oid_to_hex(oid)); - - if (opt->cmdmode == 'w') { - if (filter_object(data->rest, 0100644, oid, - &contents, &size)) - die("could not convert '%s' %s", - oid_to_hex(oid), data->rest); - } else if (opt->cmdmode == 'c') { - enum object_type type; - if (!textconv_object(the_repository, - data->rest, 0100644, oid, - 1, &contents, &size)) - contents = read_object_file(oid, - &type, - &size); - if (!contents) - die("could not convert '%s' %s", - oid_to_hex(oid), data->rest); - } else - BUG("invalid cmdmode: %c", opt->cmdmode); - batch_write(opt, contents, size); - free(contents); - } else { - stream_blob(oid); - } - } - else { - enum object_type type; - unsigned long size; - void *contents; - - contents = read_object_file(oid, &type, &size); - if (!contents) - die("object %s disappeared", oid_to_hex(oid)); - if (type != data->type) - die("object %s changed type!?", oid_to_hex(oid)); - if (data->info.sizep && size != data->size) - die("object %s changed size!?", oid_to_hex(oid)); - - batch_write(opt, contents, size); - free(contents); - } -} static void batch_object_write(const char *obj_name, struct strbuf *scratch, struct batch_options *opt, struct expand_data *data) { - if (!data->skip_object_info && - oid_object_info_extended(the_repository, &data->oid, &data->info, - OBJECT_INFO_LOOKUP_REPLACE) < 0) { - printf("%s missing\n", - obj_name ? obj_name : oid_to_hex(&data->oid)); - fflush(stdout); - return; - } + int ret; + struct strbuf err = STRBUF_INIT; + struct ref_array_item item = { data->oid, data->rest }; strbuf_reset(scratch); - strbuf_expand(scratch, opt->format, expand_format, data); - strbuf_addch(scratch, '\n'); - batch_write(opt, scratch->buf, scratch->len); - if (opt->print_contents) { - print_object_or_die(opt, data); - batch_write(opt, "\n", 1); + ret = format_ref_array_item(&item, &opt->format, scratch, &err); + if (ret < 0) + die("%s\n", err.buf); + if (ret) { + /* ret > 0 means when the object corresponding to oid + * cannot be found in format_ref_array_item(), we only print + * the error message. + */ + printf("%s\n", err.buf); + fflush(stdout); + } else { + strbuf_addch(scratch, '\n'); + batch_write(opt, scratch->buf, scratch->len); } + free_ref_array_item_value(&item); + strbuf_release(&err); } static void batch_one_object(const char *obj_name, @@ -495,43 +354,37 @@ static int batch_unordered_packed(const struct object_id *oid, return batch_unordered_object(oid, data); } -static int batch_objects(struct batch_options *batch) +static const char * const cat_file_usage[] = { + N_("git cat-file (-t [--allow-unknown-type] | -s [--allow-unknown-type] | -e | -p | | --textconv | --filters) [--path=] "), + N_("git cat-file (--batch[=] | --batch-check[=]) [--follow-symlinks] [--textconv | --filters]"), + NULL +}; + +static int batch_objects(struct batch_options *batch, const struct option *options) { struct strbuf input = STRBUF_INIT; struct strbuf output = STRBUF_INIT; + struct strbuf format = STRBUF_INIT; struct expand_data data; int save_warning; int retval = 0; - if (!batch->format) - batch->format = "%(objectname) %(objecttype) %(objectsize)"; - - /* - * Expand once with our special mark_query flag, which will prime the - * object_info to be handed to oid_object_info_extended for each - * object. - */ memset(&data, 0, sizeof(data)); - data.mark_query = 1; - strbuf_expand(&output, batch->format, expand_format, &data); - data.mark_query = 0; - strbuf_release(&output); - if (batch->cmdmode) - data.split_on_whitespace = 1; - - /* - * If we are printing out the object, then always fill in the type, - * since we will want to decide whether or not to stream. - */ + if (batch->format.format) + strbuf_addstr(&format, batch->format.format); + else + strbuf_addstr(&format, "%(objectname) %(objecttype) %(objectsize)"); if (batch->print_contents) - data.info.typep = &data.type; + strbuf_addstr(&format, "\n%(raw)"); + batch->format.format = format.buf; + if (verify_ref_format(&batch->format)) + usage_with_options(cat_file_usage, options); + + if (batch->cmdmode || batch->format.use_rest) + data.split_on_whitespace = 1; if (batch->all_objects) { struct object_cb_data cb; - struct object_info empty = OBJECT_INFO_INIT; - - if (!memcmp(&data.info, &empty, sizeof(empty))) - data.skip_object_info = 1; if (has_promisor_remote()) warning("This repository uses promisor remotes. Some objects may not be loaded."); @@ -561,6 +414,7 @@ static int batch_objects(struct batch_options *batch) oid_array_clear(&sa); } + strbuf_release(&format); strbuf_release(&output); return 0; } @@ -593,18 +447,13 @@ static int batch_objects(struct batch_options *batch) batch_one_object(input.buf, &output, batch, &data); } + strbuf_release(&format); strbuf_release(&input); strbuf_release(&output); warn_on_object_refname_ambiguity = save_warning; return retval; } -static const char * const cat_file_usage[] = { - N_("git cat-file (-t [--allow-unknown-type] | -s [--allow-unknown-type] | -e | -p | | --textconv | --filters) [--path=] "), - N_("git cat-file (--batch[=] | --batch-check[=]) [--follow-symlinks] [--textconv | --filters]"), - NULL -}; - static int git_cat_file_config(const char *var, const char *value, void *cb) { if (userdiff_config(var, value) < 0) @@ -627,7 +476,7 @@ static int batch_option_callback(const struct option *opt, bo->enabled = 1; bo->print_contents = !strcmp(opt->long_name, "batch"); - bo->format = arg; + bo->format.format = arg; return 0; } @@ -636,7 +485,9 @@ int cmd_cat_file(int argc, const char **argv, const char *prefix) { int opt = 0; const char *exp_type = NULL, *obj_name = NULL; - struct batch_options batch = {0}; + struct batch_options batch = { + .format = REF_FORMAT_INIT + }; int unknown_type = 0; const struct option options[] = { @@ -675,6 +526,7 @@ int cmd_cat_file(int argc, const char **argv, const char *prefix) git_config(git_cat_file_config, NULL); batch.buffer_output = -1; + batch.format.cat_file_mode = 1; argc = parse_options(argc, argv, prefix, options, cat_file_usage, 0); if (opt) { @@ -718,7 +570,7 @@ int cmd_cat_file(int argc, const char **argv, const char *prefix) batch.buffer_output = batch.all_objects; if (batch.enabled) - return batch_objects(&batch); + return batch_objects(&batch, options); if (unknown_type && opt != 't' && opt != 's') die("git cat-file --allow-unknown-type: use with -s or -t"); diff --git a/t/t1006-cat-file.sh b/t/t1006-cat-file.sh index 18b3779ccb6014..97e355930b2330 100755 --- a/t/t1006-cat-file.sh +++ b/t/t1006-cat-file.sh @@ -608,4 +608,256 @@ test_expect_success 'cat-file --batch="batman" with --batch-all-objects will wor cmp expect actual ' +. "$TEST_DIRECTORY"/lib-gpg.sh +. "$TEST_DIRECTORY"/lib-terminal.sh + +test_expect_success 'cat-file --batch|--batch-check setup' ' + echo 1>blob1 && + printf "a\0b\0\c" >blob2 && + git add blob1 blob2 && + git commit -m "Commit Message" && + git branch -M main && + git tag -a -m "v0.0.0" testtag && + git update-ref refs/myblobs/blob1 HEAD:blob1 && + git update-ref refs/myblobs/blob2 HEAD:blob2 && + git update-ref refs/mytrees/tree1 HEAD^{tree} +' + +batch_test_atom() { + if test "$3" = "fail" + then + test_expect_${4:-success} $PREREQ "basic atom: $1 $2 must fail" " + test_must_fail git cat-file --batch-check='$2' >bad <<-EOF + $1 + EOF + " + else + test_expect_${4:-success} $PREREQ "basic atom: $1 $2" " + git for-each-ref --format='$2' $1 >expected && + git cat-file --batch-check='$2' >actual <<-EOF && + $1 + EOF + sanitize_pgp actual.clean && + cmp expected actual.clean + " + fi +} + +batch_test_atom refs/heads/main '%(refname)' fail +batch_test_atom refs/heads/main '%(refname:)' fail +batch_test_atom refs/heads/main '%(refname:short)' fail +batch_test_atom refs/heads/main '%(refname:lstrip=1)' fail +batch_test_atom refs/heads/main '%(refname:lstrip=2)' fail +batch_test_atom refs/heads/main '%(refname:lstrip=-1)' fail +batch_test_atom refs/heads/main '%(refname:lstrip=-2)' fail +batch_test_atom refs/heads/main '%(refname:rstrip=1)' fail +batch_test_atom refs/heads/main '%(refname:rstrip=2)' fail +batch_test_atom refs/heads/main '%(refname:rstrip=-1)' fail +batch_test_atom refs/heads/main '%(refname:rstrip=-2)' fail +batch_test_atom refs/heads/main '%(refname:strip=1)' fail +batch_test_atom refs/heads/main '%(refname:strip=2)' fail +batch_test_atom refs/heads/main '%(refname:strip=-1)' fail +batch_test_atom refs/heads/main '%(refname:strip=-2)' fail +batch_test_atom refs/heads/main '%(upstream)' fail +batch_test_atom refs/heads/main '%(upstream:short)' fail +batch_test_atom refs/heads/main '%(upstream:lstrip=2)' fail +batch_test_atom refs/heads/main '%(upstream:lstrip=-2)' fail +batch_test_atom refs/heads/main '%(upstream:rstrip=2)' fail +batch_test_atom refs/heads/main '%(upstream:rstrip=-2)' fail +batch_test_atom refs/heads/main '%(upstream:strip=2)' fail +batch_test_atom refs/heads/main '%(upstream:strip=-2)' fail +batch_test_atom refs/heads/main '%(push)' fail +batch_test_atom refs/heads/main '%(push:short)' fail +batch_test_atom refs/heads/main '%(push:lstrip=1)' fail +batch_test_atom refs/heads/main '%(push:lstrip=-1)' fail +batch_test_atom refs/heads/main '%(push:rstrip=1)' fail +batch_test_atom refs/heads/main '%(push:rstrip=-1)' fail +batch_test_atom refs/heads/main '%(push:strip=1)' fail +batch_test_atom refs/heads/main '%(push:strip=-1)' fail +batch_test_atom refs/heads/main '%(objecttype)' +batch_test_atom refs/heads/main '%(objectsize)' +batch_test_atom refs/heads/main '%(objectsize:disk)' +batch_test_atom refs/heads/main '%(deltabase)' +batch_test_atom refs/heads/main '%(objectname)' +batch_test_atom refs/heads/main '%(objectname:short)' +batch_test_atom refs/heads/main '%(objectname:short=1)' +batch_test_atom refs/heads/main '%(objectname:short=10)' +batch_test_atom refs/heads/main '%(tree)' +batch_test_atom refs/heads/main '%(tree:short)' +batch_test_atom refs/heads/main '%(tree:short=1)' +batch_test_atom refs/heads/main '%(tree:short=10)' +batch_test_atom refs/heads/main '%(parent)' +batch_test_atom refs/heads/main '%(parent:short)' +batch_test_atom refs/heads/main '%(parent:short=1)' +batch_test_atom refs/heads/main '%(parent:short=10)' +batch_test_atom refs/heads/main '%(numparent)' +batch_test_atom refs/heads/main '%(object)' +batch_test_atom refs/heads/main '%(type)' +batch_test_atom refs/heads/main '%(raw)' +batch_test_atom refs/heads/main '%(*objectname)' +batch_test_atom refs/heads/main '%(*objecttype)' +batch_test_atom refs/heads/main '%(author)' +batch_test_atom refs/heads/main '%(authorname)' +batch_test_atom refs/heads/main '%(authoremail)' +batch_test_atom refs/heads/main '%(authoremail:trim)' +batch_test_atom refs/heads/main '%(authoremail:localpart)' +batch_test_atom refs/heads/main '%(authordate)' +batch_test_atom refs/heads/main '%(committer)' +batch_test_atom refs/heads/main '%(committername)' +batch_test_atom refs/heads/main '%(committeremail)' +batch_test_atom refs/heads/main '%(committeremail:trim)' +batch_test_atom refs/heads/main '%(committeremail:localpart)' +batch_test_atom refs/heads/main '%(committerdate)' +batch_test_atom refs/heads/main '%(tag)' +batch_test_atom refs/heads/main '%(tagger)' +batch_test_atom refs/heads/main '%(taggername)' +batch_test_atom refs/heads/main '%(taggeremail)' +batch_test_atom refs/heads/main '%(taggeremail:trim)' +batch_test_atom refs/heads/main '%(taggeremail:localpart)' +batch_test_atom refs/heads/main '%(taggerdate)' +batch_test_atom refs/heads/main '%(creator)' +batch_test_atom refs/heads/main '%(creatordate)' +batch_test_atom refs/heads/main '%(subject)' +batch_test_atom refs/heads/main '%(subject:sanitize)' +batch_test_atom refs/heads/main '%(contents:subject)' +batch_test_atom refs/heads/main '%(body)' +batch_test_atom refs/heads/main '%(contents:body)' +batch_test_atom refs/heads/main '%(contents:signature)' +batch_test_atom refs/heads/main '%(contents)' +batch_test_atom refs/heads/main '%(HEAD)' fail +batch_test_atom refs/heads/main '%(upstream:track)' fail +batch_test_atom refs/heads/main '%(upstream:trackshort)' fail +batch_test_atom refs/heads/main '%(upstream:track,nobracket)' fail +batch_test_atom refs/heads/main '%(upstream:nobracket,track)' fail +batch_test_atom refs/heads/main '%(push:track)' fail +batch_test_atom refs/heads/main '%(push:trackshort)' fail +batch_test_atom refs/heads/main '%(worktreepath)' fail +batch_test_atom refs/heads/main '%(symref)' fail +batch_test_atom refs/heads/main '%(flag)' fail + +batch_test_atom refs/tags/testtag '%(refname)' fail +batch_test_atom refs/tags/testtag '%(refname:short)' fail +batch_test_atom refs/tags/testtag '%(upstream)' fail +batch_test_atom refs/tags/testtag '%(push)' fail +batch_test_atom refs/tags/testtag '%(objecttype)' +batch_test_atom refs/tags/testtag '%(objectsize)' +batch_test_atom refs/tags/testtag '%(objectsize:disk)' +batch_test_atom refs/tags/testtag '%(*objectsize:disk)' +batch_test_atom refs/tags/testtag '%(deltabase)' +batch_test_atom refs/tags/testtag '%(*deltabase)' +batch_test_atom refs/tags/testtag '%(objectname)' +batch_test_atom refs/tags/testtag '%(objectname:short)' +batch_test_atom refs/tags/testtag '%(tree)' +batch_test_atom refs/tags/testtag '%(tree:short)' +batch_test_atom refs/tags/testtag '%(tree:short=1)' +batch_test_atom refs/tags/testtag '%(tree:short=10)' +batch_test_atom refs/tags/testtag '%(parent)' +batch_test_atom refs/tags/testtag '%(parent:short)' +batch_test_atom refs/tags/testtag '%(parent:short=1)' +batch_test_atom refs/tags/testtag '%(parent:short=10)' +batch_test_atom refs/tags/testtag '%(numparent)' +batch_test_atom refs/tags/testtag '%(object)' +batch_test_atom refs/tags/testtag '%(type)' +batch_test_atom refs/tags/testtag '%(*objectname)' +batch_test_atom refs/tags/testtag '%(*objecttype)' +batch_test_atom refs/tags/testtag '%(author)' +batch_test_atom refs/tags/testtag '%(authorname)' +batch_test_atom refs/tags/testtag '%(authoremail)' +batch_test_atom refs/tags/testtag '%(authoremail:trim)' +batch_test_atom refs/tags/testtag '%(authoremail:localpart)' +batch_test_atom refs/tags/testtag '%(authordate)' +batch_test_atom refs/tags/testtag '%(committer)' +batch_test_atom refs/tags/testtag '%(committername)' +batch_test_atom refs/tags/testtag '%(committeremail)' +batch_test_atom refs/tags/testtag '%(committeremail:trim)' +batch_test_atom refs/tags/testtag '%(committeremail:localpart)' +batch_test_atom refs/tags/testtag '%(committerdate)' +batch_test_atom refs/tags/testtag '%(tag)' +batch_test_atom refs/tags/testtag '%(tagger)' +batch_test_atom refs/tags/testtag '%(taggername)' +batch_test_atom refs/tags/testtag '%(taggeremail)' +batch_test_atom refs/tags/testtag '%(taggeremail:trim)' +batch_test_atom refs/tags/testtag '%(taggeremail:localpart)' +batch_test_atom refs/tags/testtag '%(taggerdate)' +batch_test_atom refs/tags/testtag '%(creator)' +batch_test_atom refs/tags/testtag '%(creatordate)' +batch_test_atom refs/tags/testtag '%(subject)' +batch_test_atom refs/tags/testtag '%(subject:sanitize)' +batch_test_atom refs/tags/testtag '%(contents:subject)' +batch_test_atom refs/tags/testtag '%(body)' +batch_test_atom refs/tags/testtag '%(contents:body)' +batch_test_atom refs/tags/testtag '%(contents:signature)' +batch_test_atom refs/tags/testtag '%(contents)' +batch_test_atom refs/tags/testtag '%(HEAD)' fail + +batch_test_atom refs/myblobs/blob1 '%(refname)' fail +batch_test_atom refs/myblobs/blob1 '%(upstream)' fail +batch_test_atom refs/myblobs/blob1 '%(push)' fail +batch_test_atom refs/myblobs/blob1 '%(HEAD)' fail + +batch_test_atom refs/myblobs/blob1 '%(objectname)' +batch_test_atom refs/myblobs/blob1 '%(objecttype)' +batch_test_atom refs/myblobs/blob1 '%(objectsize)' +batch_test_atom refs/myblobs/blob1 '%(objectsize:disk)' +batch_test_atom refs/myblobs/blob1 '%(deltabase)' + +batch_test_atom refs/myblobs/blob1 '%(contents)' +batch_test_atom refs/myblobs/blob2 '%(contents)' + +batch_test_atom refs/myblobs/blob1 '%(raw)' +batch_test_atom refs/myblobs/blob2 '%(raw)' +batch_test_atom refs/mytrees/tree1 '%(raw)' + +batch_test_atom refs/myblobs/blob1 '%(raw:size)' +batch_test_atom refs/myblobs/blob2 '%(raw:size)' +batch_test_atom refs/mytrees/tree1 '%(raw:size)' + +batch_test_atom refs/myblobs/blob1 '%(if:equals=blob)%(objecttype)%(then)commit%(else)not commit%(end)' +batch_test_atom refs/myblobs/blob2 '%(if:equals=blob)%(objecttype)%(then)commit%(else)not commit%(end)' +batch_test_atom refs/mytrees/tree1 '%(if:equals=tree)%(objecttype)%(then)tree%(else)not tree%(end)' + +batch_test_atom refs/heads/main '%(align:60) objectname is %(objectname)%(end)|%(objectname)' +batch_test_atom refs/heads/main '%(align:left,60) objectname is %(objectname)%(end)|%(objectname)' +batch_test_atom refs/heads/main '%(align:middle,60) objectname is %(objectname)%(end)|%(objectname)' +batch_test_atom refs/heads/main '%(align:60,right) objectname is %(objectname)%(end)|%(objectname)' + +batch_test_atom refs/heads/main 'VALID' +batch_test_atom refs/heads/main '%(INVALID)' fail +batch_test_atom refs/heads/main '%(authordate:INVALID)' fail + +test_expect_success '%(rest) works with both a branch and a tag' ' + cat >expected <<-EOF && + 123 commit 123 + 456 tag 456 + EOF + git cat-file --batch-check="%(rest) %(objecttype) %(rest)" >actual <<-EOF && + refs/heads/main 123 + refs/tags/testtag 456 + EOF + test_cmp expected actual +' + +batch_test_atom refs/heads/main '%(objectname) %(objecttype) %(objectsize) +%(raw)' +batch_test_atom refs/tags/testtag '%(objectname) %(objecttype) %(objectsize) +%(raw)' +batch_test_atom refs/myblobs/blob1 '%(objectname) %(objecttype) %(objectsize) +%(raw)' +batch_test_atom refs/myblobs/blob2 '%(objectname) %(objecttype) %(objectsize) +%(raw)' + + +test_expect_success 'cat-file --batch equals to --batch-check with atoms' ' + git cat-file --batch-check="%(objectname) %(objecttype) %(objectsize) +%(raw)" >expected <<-EOF && + refs/heads/main + refs/tags/testtag + EOF + git cat-file --batch >actual <<-EOF && + refs/heads/main + refs/tags/testtag + EOF + cmp expected actual +' + test_done From 13e94685da1df8a86407f045fb6cd601da6d4efc Mon Sep 17 00:00:00 2001 From: ZheNing Hu Date: Sat, 12 Jun 2021 15:12:44 +0800 Subject: [PATCH 29/31] [GSOC] cat-file: reuse err buf in batch_object_write() Reuse the `err` buffer in batch_object_write(), as the buffer `scratch` does. This will reduce the overhead of multiple allocations of memory of the err buffer. Mentored-by: Christian Couder Mentored-by: Hariom Verma Signed-off-by: ZheNing Hu --- builtin/cat-file.c | 22 ++++++++++++++-------- 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/builtin/cat-file.c b/builtin/cat-file.c index 5b163551fc687e..dc604a9879d6ea 100644 --- a/builtin/cat-file.c +++ b/builtin/cat-file.c @@ -212,35 +212,36 @@ static void batch_write(struct batch_options *opt, const void *data, int len) static void batch_object_write(const char *obj_name, struct strbuf *scratch, + struct strbuf *err, struct batch_options *opt, struct expand_data *data) { int ret; - struct strbuf err = STRBUF_INIT; struct ref_array_item item = { data->oid, data->rest }; strbuf_reset(scratch); + strbuf_reset(err); - ret = format_ref_array_item(&item, &opt->format, scratch, &err); + ret = format_ref_array_item(&item, &opt->format, scratch, err); if (ret < 0) - die("%s\n", err.buf); + die("%s\n", err->buf); if (ret) { /* ret > 0 means when the object corresponding to oid * cannot be found in format_ref_array_item(), we only print * the error message. */ - printf("%s\n", err.buf); + printf("%s\n", err->buf); fflush(stdout); } else { strbuf_addch(scratch, '\n'); batch_write(opt, scratch->buf, scratch->len); } free_ref_array_item_value(&item); - strbuf_release(&err); } static void batch_one_object(const char *obj_name, struct strbuf *scratch, + struct strbuf *err, struct batch_options *opt, struct expand_data *data) { @@ -294,7 +295,7 @@ static void batch_one_object(const char *obj_name, return; } - batch_object_write(obj_name, scratch, opt, data); + batch_object_write(obj_name, scratch, err, opt, data); } struct object_cb_data { @@ -302,13 +303,14 @@ struct object_cb_data { struct expand_data *expand; struct oidset *seen; struct strbuf *scratch; + struct strbuf *err; }; static int batch_object_cb(const struct object_id *oid, void *vdata) { struct object_cb_data *data = vdata; oidcpy(&data->expand->oid, oid); - batch_object_write(NULL, data->scratch, data->opt, data->expand); + batch_object_write(NULL, data->scratch, data->err, data->opt, data->expand); return 0; } @@ -364,6 +366,7 @@ static int batch_objects(struct batch_options *batch, const struct option *optio { struct strbuf input = STRBUF_INIT; struct strbuf output = STRBUF_INIT; + struct strbuf err = STRBUF_INIT; struct strbuf format = STRBUF_INIT; struct expand_data data; int save_warning; @@ -392,6 +395,7 @@ static int batch_objects(struct batch_options *batch, const struct option *optio cb.opt = batch; cb.expand = &data; cb.scratch = &output; + cb.err = &err; if (batch->unordered) { struct oidset seen = OIDSET_INIT; @@ -416,6 +420,7 @@ static int batch_objects(struct batch_options *batch, const struct option *optio strbuf_release(&format); strbuf_release(&output); + strbuf_release(&err); return 0; } @@ -444,12 +449,13 @@ static int batch_objects(struct batch_options *batch, const struct option *optio data.rest = p; } - batch_one_object(input.buf, &output, batch, &data); + batch_one_object(input.buf, &output, &err, batch, &data); } strbuf_release(&format); strbuf_release(&input); strbuf_release(&output); + strbuf_release(&err); warn_on_object_refname_ambiguity = save_warning; return retval; } From 60bf953451b33f04f5b761c9526e90b6437071db Mon Sep 17 00:00:00 2001 From: ZheNing Hu Date: Thu, 10 Jun 2021 20:38:56 +0800 Subject: [PATCH 30/31] [GSOC] cat-file: re-implement --textconv, --filters options After cat-file reuses the ref-filter logic, we re-implement the functions of --textconv and --filters options. Add members `use_textconv` and `use_filters` in struct `ref_format`, and use global variables `use_filters` and `use_textconv` in `ref-filter.c`, so that we can filter the content of the object in get_object(). Use `actual_oi` to record the real expand_data: it may point to the original `oi` or the `act_oi` processed by `textconv_object()` or `convert_to_working_tree()`. `grab_values()` will grab the contents of `actual_oi` and `grab_common_values()` to grab the contents of origin `oi`, this ensures that `%(objectsize)` still uses the size of the unfiltered data. In `get_object()`, we made an optimization: Firstly, get the size and type of the object instead of directly getting the object data. If using --textconv, after successfully obtaining the filtered object data, an extra oid_object_info_extended() will be skipped, which can reduce the cost of object data copy; If using --filter, the data of the object first will be getted first, and then convert_to_working_tree() will be used to get the filtered object data. Mentored-by: Christian Couder Mentored-by: Hariom Verma Signed-off-by: ZheNing Hu --- builtin/cat-file.c | 6 +++++ ref-filter.c | 59 ++++++++++++++++++++++++++++++++++++++++++++-- ref-filter.h | 2 ++ 3 files changed, 65 insertions(+), 2 deletions(-) diff --git a/builtin/cat-file.c b/builtin/cat-file.c index dc604a9879d6ea..710085b7d72e27 100644 --- a/builtin/cat-file.c +++ b/builtin/cat-file.c @@ -380,6 +380,12 @@ static int batch_objects(struct batch_options *batch, const struct option *optio if (batch->print_contents) strbuf_addstr(&format, "\n%(raw)"); batch->format.format = format.buf; + + if (batch->cmdmode == 'c') + batch->format.use_textconv = 1; + else if (batch->cmdmode == 'w') + batch->format.use_filters = 1; + if (verify_ref_format(&batch->format)) usage_with_options(cat_file_usage, options); diff --git a/ref-filter.c b/ref-filter.c index 9ca3dd5557dc5f..427552b81082ab 100644 --- a/ref-filter.c +++ b/ref-filter.c @@ -1,3 +1,4 @@ +#define USE_THE_INDEX_COMPATIBILITY_MACROS #include "builtin.h" #include "cache.h" #include "parse-options.h" @@ -84,6 +85,9 @@ static struct expand_data { struct object_info info; } oi, oi_deref; +int use_filters; +int use_textconv; + struct ref_to_worktree_entry { struct hashmap_entry ent; struct worktree *wt; /* key is wt->head_ref */ @@ -1031,6 +1035,9 @@ int verify_ref_format(struct ref_format *format) used_atom[at].atom_type == ATOM_WORKTREEPATH))) die(_("this command reject atom %%(%.*s)"), (int)(ep - sp - 2), sp + 2); + use_filters = format->use_filters; + use_textconv = format->use_textconv; + if ((format->quote_style == QUOTE_PYTHON || format->quote_style == QUOTE_SHELL || format->quote_style == QUOTE_TCL) && @@ -1742,10 +1749,38 @@ static int get_object(struct ref_array_item *ref, int deref, struct object **obj { /* parse_object_buffer() will set eaten to 0 if free() will be needed */ int eaten = 1; + struct expand_data *actual_oi = oi; + struct expand_data act_oi = {0}; + if (oi->info.contentp) { /* We need to know that to use parse_object_buffer properly */ + void **temp_contentp = oi->info.contentp; + oi->info.contentp = NULL; oi->info.sizep = &oi->size; oi->info.typep = &oi->type; + + /* get the type and size */ + if (oid_object_info_extended(the_repository, &oi->oid, &oi->info, + OBJECT_INFO_LOOKUP_REPLACE)) + return strbuf_addf_ret(err, 1, _("%s missing"), + oid_to_hex(&oi->oid)); + + oi->info.sizep = NULL; + oi->info.typep = NULL; + oi->info.contentp = temp_contentp; + + if (use_textconv && !ref->rest) + return strbuf_addf_ret(err, -1, _("missing path for '%s'"), + oid_to_hex(&act_oi.oid)); + if (use_textconv && oi->type == OBJ_BLOB) { + act_oi = *oi; + if (textconv_object(the_repository, + ref->rest, 0100644, &act_oi.oid, + 1, (char **)(&act_oi.content), &act_oi.size)) { + actual_oi = &act_oi; + goto success; + } + } } if (oid_object_info_extended(the_repository, &oi->oid, &oi->info, OBJECT_INFO_LOOKUP_REPLACE)) @@ -1755,19 +1790,39 @@ static int get_object(struct ref_array_item *ref, int deref, struct object **obj BUG("Object size is less than zero."); if (oi->info.contentp) { - *obj = parse_object_buffer(the_repository, &oi->oid, oi->type, oi->size, oi->content, &eaten); + if (use_filters && !ref->rest) + return strbuf_addf_ret(err, -1, _("missing path for '%s'"), + oid_to_hex(&oi->oid)); + if (use_filters && oi->type == OBJ_BLOB) { + struct strbuf strbuf = STRBUF_INIT; + struct checkout_metadata meta; + act_oi = *oi; + + init_checkout_metadata(&meta, NULL, NULL, &act_oi.oid); + if (!convert_to_working_tree(&the_index, ref->rest, act_oi.content, act_oi.size, &strbuf, &meta)) + die("could not convert '%s' %s", + oid_to_hex(&oi->oid), ref->rest); + act_oi.size = strbuf.len; + act_oi.content = strbuf_detach(&strbuf, NULL); + actual_oi = &act_oi; + } + +success: + *obj = parse_object_buffer(the_repository, &actual_oi->oid, actual_oi->type, actual_oi->size, actual_oi->content, &eaten); if (!*obj) { if (!eaten) free(oi->content); return strbuf_addf_ret(err, -1, _("parse_object_buffer failed on %s for %s"), oid_to_hex(&oi->oid), ref->refname); } - grab_values(ref->value, deref, *obj, oi); + grab_values(ref->value, deref, *obj, actual_oi); } grab_common_values(ref->value, deref, oi); if (!eaten) free(oi->content); + if (actual_oi != oi) + free(actual_oi->content); return 0; } diff --git a/ref-filter.h b/ref-filter.h index 053980a6a42667..497e3e93632f85 100644 --- a/ref-filter.h +++ b/ref-filter.h @@ -80,6 +80,8 @@ struct ref_format { const char *rest; int cat_file_mode; int quote_style; + int use_textconv; + int use_filters; int use_rest; int use_color; From c3b60b905048cb5c634969fd01dfc3f29df9d1dc Mon Sep 17 00:00:00 2001 From: ZheNing Hu Date: Fri, 25 Jun 2021 23:05:33 +0800 Subject: [PATCH 31/31] [GSOC] ref-filter: remove grab_oid() function Because "atom_type == ATOM_OBJECTNAME" implies the condition of `starts_with(name, "objectname")`, "atom_type == ATOM_TREE" implies the condition of `starts_with(name, "tree")`, so the check for `starts_with(name, field)` in grab_oid() is redundant. So Remove the grab_oid() from ref-filter, to reduce repeated check. Mentored-by: Christian Couder Mentored-by: Hariom Verma Signed-off-by: ZheNing Hu --- ref-filter.c | 26 +++++++++----------------- 1 file changed, 9 insertions(+), 17 deletions(-) diff --git a/ref-filter.c b/ref-filter.c index 427552b81082ab..55bff4ed9e7fc1 100644 --- a/ref-filter.c +++ b/ref-filter.c @@ -1071,16 +1071,6 @@ static const char *do_grab_oid(const char *field, const struct object_id *oid, } } -static int grab_oid(const char *name, const char *field, const struct object_id *oid, - struct atom_value *v, struct used_atom *atom) -{ - if (starts_with(name, field)) { - v->s = xstrdup(do_grab_oid(field, oid, atom)); - return 1; - } - return 0; -} - /* See grab_values */ static void grab_common_values(struct atom_value *val, int deref, struct expand_data *oi) { @@ -1106,8 +1096,9 @@ static void grab_common_values(struct atom_value *val, int deref, struct expand_ } } else if (atom_type == ATOM_DELTABASE) v->s = xstrdup(oid_to_hex(&oi->delta_base_oid)); - else if (atom_type == ATOM_OBJECTNAME && deref) - grab_oid(name, "objectname", &oi->oid, v, &used_atom[i]); + else if (atom_type == ATOM_OBJECTNAME && deref) { + v->s = xstrdup(do_grab_oid("objectname", &oi->oid, &used_atom[i])); + } } } @@ -1148,9 +1139,10 @@ static void grab_commit_values(struct atom_value *val, int deref, struct object continue; if (deref) name++; - if (atom_type == ATOM_TREE && - grab_oid(name, "tree", get_commit_tree_oid(commit), v, &used_atom[i])) + if (atom_type == ATOM_TREE) { + v->s = xstrdup(do_grab_oid("tree", get_commit_tree_oid(commit), &used_atom[i])); continue; + } if (atom_type == ATOM_NUMPARENT) { v->value = commit_list_count(commit->parents); v->s = xstrfmt("%lu", (unsigned long)v->value); @@ -1971,9 +1963,9 @@ static int populate_value(struct ref_array_item *ref, struct strbuf *err) v->s = xstrdup(buf + 1); } continue; - } else if (!deref && atom_type == ATOM_OBJECTNAME && - grab_oid(name, "objectname", &ref->objectname, v, atom)) { - continue; + } else if (!deref && atom_type == ATOM_OBJECTNAME) { + v->s = xstrdup(do_grab_oid("objectname", &ref->objectname, atom)); + continue; } else if (atom_type == ATOM_HEAD) { if (atom->u.head && !strcmp(ref->refname, atom->u.head)) v->s = xstrdup("*");