[PATCH v5 00/23] make "mktag" use fsck_tag() & more
- From
Ævar Arnfjörð Bjarmason <avarab@gmail.com>
- Date
- Jan 5, 2021, 19:42 UTC
- Message-ID
- <20210105194252.627-1-avarab@gmail.com>
- In-Reply-To
- <20201223013606.7972-1-avarab@gmail.com>
A late re-roll due to the holidays. Thanks all for the feedback on v4, this should address all of it.
Brief comments on each patch below, and then the range-diff.
Ævar Arnfjörð Bjarmason (23): mktag doc: say <hash> not <sha1> mktag doc: grammar fix, when exists -> when it exists
No changes.
mktag doc: update to explain why to use this
Got rid of the cross-linking of mktag and hash-object manpages as suggested by Junio.
mktag tests: don't needlessly use a subshell
no changes.
mktag tests: use "test_commit" helper
NEW: Another trivial mktag test cleanup, make it use test_commit instead of 3 boilerplate lines in its test. Made a later change smaller.
mktag tests: remove needless SHA-1 hardcoding
no changes.
mktag tests: don't redirect stderr to a file needlessly
Reworded the commit message, and fixed another copy/pasted mktag test (in the git-replace test file) that used the same pattern.
mktag tests: don't create "mytag" twice
The commit message notes that testing the tag at the end is pointless, but now later we'll do something with it...
mktag tests: run "fsck" after creating "mytag"
...i.e. run fsck to (potentially in the future) check the envelope v.s. refname.
mktag tests: stress test whitespace handling
Reworded commit message & test descriptions to make more sense, got rid of the one $space test, which didn't really add anything.
mktag tests: test "hash-object" compatibility
Gets rid of the "create tag refname" special case at the end of the tests by now doing it for every successful tag creation.
mktag tests: improve verify_object() test coverage
Commit message update, notes that we don't do "git replace" tests.
mktag tests: test verify_object() with replaced objects
NEW: test mktag with git-replace'd objects.
mktag: use default strbuf_read() hint
mktag: remove redundant braces in one-line body "if"
mktag: use puts(str) instead of printf("%s\n", str)no changes.
mktag: use fsck instead of custom verify_tag()
only carried-forward changes from rebasing on the changes above.
fsck: make fsck_config() re-usable mktag: allow turning off fsck.extraHeaderEntry
no changes.
mktag: allow omitting the header/body \n separator mktag: convert to parse-options
ditto carried-forward changes.
mktag: mark strings for translation mktag: add a --no-strict option
Use the more normal OPT_BOOL pattern, and fix a stupid error in GIT_TEST_GETTEXT_POISON=true testing that broke the CI.
Documentation/git-mktag.txt | 39 +++++- builtin/fsck.c | 20 +-- builtin/mktag.c | 235 +++++++++++++---------------------- fsck.c | 59 ++++++++- fsck.h | 16 +++ t/t1006-cat-file.sh | 2 +- t/t3800-mktag.sh | 236 ++++++++++++++++++++++++++++-------- t/t6050-replace.sh | 2 +- 8 files changed, 375 insertions(+), 234 deletions(-)
Range-diff:
1: a31c305cfc6 = 1: 74b035fb842 mktag doc: say <hash> not <sha1>
2: 81cb4cba5c0 = 2: 202bd33533f mktag doc: grammar fix, when exists -> when it exists
3: b4bc6f894cb ! 3: d1f650b6c4c mktag doc: update to explain why to use this
@@ Commit message
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
- ## Documentation/git-hash-object.txt ##
-@@ Documentation/git-hash-object.txt: OPTIONS
- stress-testing Git itself or reproducing characteristics of corrupt or
- bogus objects encountered in the wild.
-
-+SEE ALSO
-+--------
-+linkgit:git-mktag[1]
-+
- GIT
- ---
- Part of the linkgit:git[1] suite
-
## Documentation/git-mktag.txt ##
@@ Documentation/git-mktag.txt: git-mktag(1)
@@ Documentation/git-mktag.txt: SYNOPSIS
Tag Format
----------
-@@ Documentation/git-mktag.txt: exists, is separated by a blank line from the header. The
- message part may contain a signature that Git itself doesn't
- care about, but that can be verified with gpg.
-
-+SEE ALSO
-+--------
-+linkgit:git-hash-object[1],
-+
- GIT
- ---
- Part of the linkgit:git[1] suite
4: acb94e02895 = 4: 64432d22952 mktag tests: don't needlessly use a subshell
-: ----------- > 5: 57c7bdde80b mktag tests: use "test_commit" helper
5: 4ae76ec5e3f = 6: ba581a99c89 mktag tests: remove needless SHA-1 hardcoding
7: b81d31a917c ! 7: 974d1b9c6ec mktag tests: don't pipe to stderr needlessly
@@ Metadata
Author: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
## Commit message ##
- mktag tests: don't pipe to stderr needlessly
+ mktag tests: don't redirect stderr to a file needlessly
- Remove the piping of stderr to "message" in the valid tag test. This
- pattern seems to have been copy/pasted from the failure case in
- 446c6faec6 (New tests and en-passant modifications to mktag.,
+ Remove the redirection of stderr to "message" in the valid tag
+ test. This pattern seems to have been copy/pasted from the failure
+ case in 446c6faec6 (New tests and en-passant modifications to mktag.,
2006-07-29).
- Nothing is piped to "message" here, and in the event of the test
- failing it only serves to hide the error.
+ While I'm at it do the same for the "replace" tests. The tag creation
+ I'm changing here seems to have been copy/pasted from the "mktag"
+ tests to those tests in cc400f50112 (mktag: call
+ "check_sha1_signature" with the replacement sha1, 2009-01-23).
+
+ Nobody examines the contents of the resulting "message" file, so the
+ net result is that error messages cannot be seen in "sh t3800-mktag.sh
+ -v" output.
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
@@ t/t3800-mktag.sh: EOF
############################################################
# 25. check mytag
+
+ ## t/t6050-replace.sh ##
+@@ t/t6050-replace.sh: tagger T A Gger <> 0 +0000
+ EOF
+
+ test_expect_success 'tag replaced commit' '
+- git mktag <tag.sig >.git/refs/tags/mytag 2>message
++ git mktag <tag.sig >.git/refs/tags/mytag
+ '
+
+ test_expect_success '"git fsck" works' '
8: 11f59718b4b ! 8: 73d82dbca24 mktag tests: don't create "mytag" twice
@@ Commit message
it. Let's do this by moving the test to use "update-ref", instead of
our own homebrew ad-hoc refstore update.
+ We're not really testing for anything yet by creating the tag at the
+ end here. A subsequent commit will change that.
+
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
## t/t3800-mktag.sh ##
-: ----------- > 9: 0aa8190f404 mktag tests: run "fsck" after creating "mytag"
9: dd6b012b0c6 ! 10: 078e73c4221 mktag tests: stress test whitespace handling
@@ Metadata
## Commit message ##
mktag tests: stress test whitespace handling
- Add tests for various whitespace edge cases around the header/body
- boundary, such as two newlines separating the header and body, a space
- at the end of the "tagger" line before the header etc.
+ Add tests for a couple of whitespace edge cases around the header/body
+ boundary.
- Add a test showing that we insist that an empty body must be preceded
- by an empty line. This test for a long-standing regression which goes
- against the command's documented behavior. It'll be addressed in a
- follow-up change.
+ I consider the requirement for a blank line before the empty body a
+ bug, it's a long-standing regression which goes against the command's
+ documented behavior. This bug will be addressed in a follow-up change.
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
@@ t/t3800-mktag.sh: EOF
+
+EOF
+
-+test_expect_success 'allow extra newlines at end of headers' '
++test_expect_success 'require a blank line before an empty body (1)' '
+ git mktag <tag.sig
+'
+
-+space=' '
-+cat >tag.sig <<EOF
-+object $head
-+type commit
-+tag mytag
-+tagger T A Gger <tagger@example.com> 1206478233 -0500$space
-+
-+EOF
-+
-+check_verify_failure 'extra whitespace at end of headers' \
-+ '^error: char.*: malformed tag timezone$'
-+
+cat >tag.sig <<EOF
+object $head
+type commit
@@ t/t3800-mktag.sh: EOF
+tagger T A Gger <tagger@example.com> 1206478233 -0500
+EOF
+
-+check_verify_failure 'disallow no header / body newline separator' \
++check_verify_failure 'require a blank line before an empty body (2)' \
+ '^error: char.*: trailing garbage in tag header$'
+
############################################################
10: 56c6b562fd5 ! 11: 83d7ca86b8f mktag tests: test "hash-object" compatibility
@@ Commit message
This tests e.g. that "mktag" doesn't trim its input or otherwise munge
it in a way that "hash-object" doesn't.
+ Since we're doing an "fsck --strict" here at the end let's incorporate
+ the creation of the "mytag" name into this test, removing the
+ special-case at the end of the file.
+
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
## t/t3800-mktag.sh ##
@@ t/t3800-mktag.sh: check_verify_failure () {
+ test_expect_success "$1" '
+ git hash-object -t tag -w --stdin <tag.sig >expected &&
+ git fsck --strict &&
++
+ git mktag <tag.sig >hash &&
-+ git fsck --strict &&
-+ test_cmp expected hash
++ test_cmp expected hash &&
++ test_when_finished "git update-ref -d refs/tags/mytag $(cat hash)" &&
++ git update-ref refs/tags/mytag $(cat hash) $(test_oid zero) &&
++ git fsck --strict
+ '
+}
+
@@ t/t3800-mktag.sh: tagger T A Gger <tagger@example.com> 1206478233 -0500
EOF
--test_expect_success 'allow extra newlines at end of headers' '
+-test_expect_success 'require a blank line before an empty body (1)' '
- git mktag <tag.sig
-'
-+test_expect_mktag_success 'allow extra newlines at end of headers'
++test_expect_mktag_success 'require a blank line before an empty body (1)'
- space=' '
cat >tag.sig <<EOF
+ object $head
@@ t/t3800-mktag.sh: tagger T A Gger <tagger@example.com> 1206478233 -0500
EOF
-test_expect_success 'create valid tag' '
- git mktag <tag.sig >hash &&
+- git update-ref refs/tags/mytag $(cat hash) $(test_oid zero) &&
+- git fsck --strict
+-'
+test_expect_mktag_success 'create valid tag object'
-+test_expect_success 'create valid tag name' '
- git update-ref refs/tags/mytag $(cat hash) $(test_oid zero)
- '
+ test_done
6: 9effb4532bc ! 12: 6cbf9176c35 mktag tests: improve verify_object() test coverage
@@ Commit message
Let's split these tests up, so we're testing all combinations of a
non-existing object and in invalid/wrong "type" lines.
+ We need to provide GIT_TEST_GETTEXT_POISON=false here because the
+ "invalid object type" error is emitted by
+ parse_loose_header_extended(), which has that message already marked
+ for translation. Another option would be to use test_i18ngrep, but I
+ prefer always running the test, not skipping it under gettext poison
+ testing.
+
+ I'm not testing this in combination with "git replace". That'll be
+ done in a subsequent commit.
+
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
## t/t3800-mktag.sh ##
+@@ t/t3800-mktag.sh: test_description='git mktag: tag object verify test'
+ check_verify_failure () {
+ expect="$2"
+ test_expect_success "$1" '
+- test_must_fail git mktag <tag.sig 2>message &&
++ test_must_fail env GIT_TEST_GETTEXT_POISON=false \
++ git mktag <tag.sig 2>message &&
+ grep "$expect" message
+ '
+ }
@@ t/t3800-mktag.sh: check_verify_failure '"type" line type-name length check' \
'^error: char.*: type too long$'
@@ t/t3800-mktag.sh: check_verify_failure '"type" line type-name length check' \
EOF
-check_verify_failure 'verify object (SHA1/type) check' \
-+check_verify_failure 'verify object (hash/type) check -- made-up type, nonexisting object' \
++check_verify_failure 'verify object (hash/type) check -- made-up type, valid object' \
+ '^fatal: invalid object type'
+
+cat >tag.sig <<EOF
@@ t/t3800-mktag.sh: check_verify_failure '"type" line type-name length check' \
+
+EOF
+
-+check_verify_failure 'verify object (hash/type) check -- incorrect type, valid object' \
++check_verify_failure 'verify object (hash/type) check -- made-up type, nonexisting object' \
'^error: char7: could not verify object.*$'
+cat >tag.sig <<EOF
@@ t/t3800-mktag.sh: check_verify_failure '"type" line type-name length check' \
+
+EOF
+
-+check_verify_failure 'verify object (hash/type) check -- incorrect type, valid object' \
++check_verify_failure 'verify object (hash/type) check -- mismatched type, valid object' \
+ '^error: char7: could not verify object'
+
############################################################
-: ----------- > 13: ca1567866b0 mktag tests: test verify_object() with replaced objects
11: 1e2e4ec2697 = 14: ac31d8658f3 mktag: use default strbuf_read() hint
12: be2ab3edab0 = 15: e32292af8b4 mktag: remove redundant braces in one-line body "if"
13: d8514df970c = 16: 405fd2c747d mktag: use puts(str) instead of printf("%s\n", str)
14: 346d73cc972 ! 17: 6edaa992a25 mktag: use fsck instead of custom verify_tag()
@@ t/t3800-mktag.sh: tagger . <> 0 +0000
@@ t/t3800-mktag.sh: tagger . <> 0 +0000
EOF
- check_verify_failure 'verify object (hash/type) check -- made-up type, nonexisting object' \
+ check_verify_failure 'verify object (hash/type) check -- made-up type, valid object' \
- '^fatal: invalid object type'
+ '^error:.* badType:'
@@ t/t3800-mktag.sh: tagger . <> 0 +0000
@@ t/t3800-mktag.sh: tagger . <> 0 +0000
EOF
- check_verify_failure 'verify object (hash/type) check -- incorrect type, valid object' \
+ check_verify_failure 'verify object (hash/type) check -- made-up type, nonexisting object' \
- '^error: char7: could not verify object.*$'
+ '^error:.* badType:'
@@ t/t3800-mktag.sh: tagger . <> 0 +0000
@@ t/t3800-mktag.sh: tagger . <> 0 +0000
EOF
- check_verify_failure 'verify object (hash/type) check -- incorrect type, valid object' \
+ check_verify_failure 'verify object (hash/type) check -- mismatched type, valid object' \
- '^error: char7: could not verify object'
+ '^fatal: object.*tagged as.*tree.*but is.*commit'
+ ############################################################
+ # 9.5. verify object (hash/type) check -- replacement
+@@ t/t3800-mktag.sh: tagger . <> 0 +0000
+ EOF
+
+ check_verify_failure 'verify object (hash/type) check -- mismatched type, valid object' \
+- '^error: char7: could not verify object'
++ '^fatal: object.*tagged as.*tree.*but is.*blob'
+
############################################################
# 10. verify tag-name check
@@ t/t3800-mktag.sh: tagger . <> 0 +0000
@@ t/t3800-mktag.sh: this line should not be here
- '^error: char.*: trailing garbage in tag header$'
+ '^error:.* extraHeaderEntry:'
- cat >tag.sig <<EOF
- object $head
-@@ t/t3800-mktag.sh: tagger T A Gger <tagger@example.com> 1206478233 -0500$space
- EOF
-
- check_verify_failure 'extra whitespace at end of headers' \
-- '^error: char.*: malformed tag timezone$'
-+ '^error:.* badTimezone:'
-
cat >tag.sig <<EOF
object $head
@@ t/t3800-mktag.sh: tagger T A Gger <tagger@example.com> 1206478233 -0500
EOF
- check_verify_failure 'disallow no header / body newline separator' \
+ check_verify_failure 'require a blank line before an empty body (2)' \
- '^error: char.*: trailing garbage in tag header$'
+ '^error:.* extraHeaderEntry:'
15: 0e7994d8fc3 = 18: 4c3fbb7ed58 fsck: make fsck_config() re-usable
16: 5e8046022b6 = 19: 32fc2264ec1 mktag: allow turning off fsck.extraHeaderEntry
17: 32698e1d005 ! 20: 32922a42dda mktag: allow omitting the header/body \n separator
@@ fsck.c: int fsck_tag_standalone(const struct object_id *oid, const char *buffer,
/*
## t/t3800-mktag.sh ##
+@@ t/t3800-mktag.sh: tagger T A Gger <tagger@example.com> 1206478233 -0500
+
+ EOF
+
+-test_expect_mktag_success 'require a blank line before an empty body (1)'
++test_expect_mktag_success 'allow a blank line before an empty body (1)'
+
+ cat >tag.sig <<EOF
+ object $head
@@ t/t3800-mktag.sh: tag mytag
tagger T A Gger <tagger@example.com> 1206478233 -0500
EOF
--check_verify_failure 'disallow no header / body newline separator' \
+-check_verify_failure 'require a blank line before an empty body (2)' \
- '^error:.* extraHeaderEntry:'
-+test_expect_success 'allow no header / body newline separator' '
-+ git mktag <tag.sig
-+'
++test_expect_mktag_success 'allow no blank line before an empty body (2)'
############################################################
# 24. create valid tag
18: b6a22f2f992 ! 21: 3c991dda02b mktag: convert to parse-options
@@ builtin/mktag.c: static int verify_object_in_tag(struct object_id *tagged_oid, i
## t/t3800-mktag.sh ##
@@ t/t3800-mktag.sh: test_expect_success 'setup' '
- head=$(git rev-parse --verify HEAD)
+ blob=$(git rev-parse --verify HEAD:B.t)
'
+test_expect_success 'basic usage' '
19: 7fc0b81df7d ! 22: 16db5858a08 mktag: mark strings for translation
@@ builtin/mktag.c: int cmd_mktag(int argc, const char **argv, const char *prefix)
strbuf_release(&buf);
puts(oid_to_hex(&result));
-
- ## t/t3800-mktag.sh ##
-@@ t/t3800-mktag.sh: test_description='git mktag: tag object verify test'
- check_verify_failure () {
- expect="$2"
- test_expect_success "$1" '
-- test_must_fail git mktag <tag.sig 2>message &&
-+ test_must_fail env GIT_TEST_GETTEXT_POISON=false \
-+ git mktag <tag.sig 2>message &&
- grep "$expect" message
- '
- }
20: 6fa443d528c ! 23: 94d6aca6962 mktag: add a --no-strict option
@@ Documentation/git-mktag.txt: SYNOPSIS
+OPTIONS
+-------
+
-+--no-strict::
++--strict::
+ By default mktag turns on the equivalent of
-+ linkgit:git-fsck[1] `--strict` mode. This disables it.
++ linkgit:git-fsck[1] `--strict` mode. Use `--no-strict` to
++ disable it.
+
DESCRIPTION
-----------
@@ builtin/mktag.c: static char const * const builtin_mktag_usage[] = {
N_("git mktag"),
NULL
};
-+static int option_no_strict;
++static int option_strict = 1;
static struct fsck_options fsck_options = FSCK_OPTIONS_STRICT;
@@ builtin/mktag.c: static int mktag_fsck_error_func(struct fsck_options *o,
{
switch (msg_type) {
case FSCK_WARN:
-+ if (option_no_strict) {
++ if (!option_strict) {
+ fprintf_ln(stderr, _("warning: tag input does not pass fsck: %s"), message);
+ return 0;
+
@@ builtin/mktag.c: static int verify_object_in_tag(struct object_id *tagged_oid, i
int cmd_mktag(int argc, const char **argv, const char *prefix)
{
static struct option builtin_mktag_options[] = {
-+ OPT_NO_BOOL(0, "no-strict", &option_no_strict,
-+ N_("don't do strict fsck checks")),
++ OPT_BOOL(0, "strict", &option_strict,
++ N_("enable more strict checking")),
OPT_END(),
};
struct strbuf buf = STRBUF_INIT;
- ## parse-options.h ##
-@@ parse-options.h: struct option {
- #define OPT_COUNTUP(s, l, v, h) OPT_COUNTUP_F(s, l, v, h, 0)
- #define OPT_SET_INT(s, l, v, h, i) OPT_SET_INT_F(s, l, v, h, i, 0)
- #define OPT_BOOL(s, l, v, h) OPT_BOOL_F(s, l, v, h, 0)
-+#define OPT_NO_BOOL(s, l, v, h) OPT_BOOL_F(s, l, v, h, PARSE_OPT_NONEG)
- #define OPT_HIDDEN_BOOL(s, l, v, h) { OPTION_SET_INT, (s), (l), (v), NULL, \
- (h), PARSE_OPT_NOARG | PARSE_OPT_HIDDEN, NULL, 1}
- #define OPT_CMDMODE(s, l, v, h, i) { OPTION_SET_INT, (s), (l), (v), NULL, \
-
## t/t3800-mktag.sh ##
@@ t/t3800-mktag.sh: test_description='git mktag: tag object verify test'
# given in the expect.pat file.
@@ t/t3800-mktag.sh: test_description='git mktag: tag object verify test'
+ grep '$2' message &&
+ if test '$3' != '--no-strict'
+ then
-+ test_must_fail git mktag --no-strict <tag.sig 2>message.no-strict &&
++ test_must_fail env GIT_TEST_GETTEXT_POISON=false \
++ git mktag --no-strict <tag.sig 2>message.no-strict &&
+ grep '$2' message.no-strict
+ fi
+ "
}
test_expect_mktag_success() {
-@@ t/t3800-mktag.sh: test_expect_success 'basic usage' '
- EOF
- git mktag <tag.sig &&
- git mktag --end-of-options <tag.sig &&
-- test_expect_code 129 git mktag --unknown-option
-+ test_expect_code 129 git mktag --unknown-option &&
-+ test_expect_code 129 git mktag --strict
- '
-
- ############################################################
@@ t/t3800-mktag.sh: too short for a tag
EOF
-- 2.29.2.222.g5d2a92d10f8