[PATCH v4 00/20] make "mktag" use fsck_tag() & more
From: Ævar Arnfjörð Bjarmason <hidden>
Date: 2020-12-23 01:38:27
So, when re-rolling this with Junio's small fixup this grew in scope a
bit, but should paradoxically be easier to deal with even though it's
2x the size now. Read on:
Ævar Arnfjörð Bjarmason (20):
mktag doc: say <hash> not <sha1>
mktag doc: grammar fix, when exists -> when it exists
mktag doc: update to explain why to use this
mktag tests: don't needlessly use a subshell
mktag tests: remove needless SHA-1 hardcoding
mktag tests: improve verify_object() test coverage
mktag tests: don't pipe to stderr needlessly
mktag tests: don't create "mytag" twice
mktag tests: stress test whitespace handling
mktag tests: test "hash-object" compatibility
I re-arranged this series so the doc/test patches for existing
behavior all come first now. There's some new patches there (see
range-diff), but all rather easy-to review fixes or tests for existing
behavior.
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)
Trivial coding style changes, the puts() patch is new.
mktag: use fsck instead of custom verify_tag()
Still the real meat of the series, unchanged in any meaningful way,
except in (as seen in the range-diff) carrying forward doc/test
changes made earlier.
fsck: make fsck_config() re-usable
mktag: allow turning off fsck.extraHeaderEntry
ditto unchanged.
mktag: allow omitting the header/body \n separator
I discovered a regression in mktag in git since 2008 where it refuses
to accept input without an empty newline separating the body & message
in cases where there's no message.
Now we again accept the same input as hash-object, and with the new
"hash-object" test integration earlier in the series we're confident
that mktag & hash-object do the same thing in all these cases.
mktag: convert to parse-options
mktag: mark strings for translation
mktag: add a --no-strict option
The #leftoverbits I suggested in v3 of converting to parse-options &
doing i18n for mktag, and finally supporting --no-strict so you can
make it behave like "fsck" does in its default mode.
Documentation/git-hash-object.txt | 4 +
Documentation/git-mktag.txt | 42 +++++-
builtin/fsck.c | 20 +--
builtin/mktag.c | 235 +++++++++++-------------------
fsck.c | 59 +++++++-
fsck.h | 16 ++
parse-options.h | 1 +
t/t1006-cat-file.sh | 2 +-
t/t3800-mktag.sh | 211 +++++++++++++++++++++------
9 files changed, 361 insertions(+), 229 deletions(-)
Range-diff:
1: aee3f52a47 = 1: a31c305cfc mktag doc: say <hash> not <sha1>
-: ---------- > 2: 81cb4cba5c mktag doc: grammar fix, when exists -> when it exists
8: fa04664f7f ! 3: b4bc6f894c mktag doc: update to explain why to use this
@@ Commit message
documentation wouldn't have much of an idea what the difference
was.
- Let's make it clear that it's to do with slightly different fsck
- validation logic, and cross-link the "mktag" and "hash-object"
- documentation to aid discover-ability.
+ Let's allude to our own validation logic, and cross-link the "mktag"
+ and "hash-object" documentation to aid discover-ability. A follow-up
+ change to migrate "mktag" to use "fsck" validation will make the part
+ about validation logic clearer.
Signed-off-by: Ævar Arnfjörð Bjarmason [off-list ref]
@@ Documentation/git-mktag.txt: SYNOPSIS
+ git hash-object -t tag -w --stdin <my-tag
+
+The difference is that mktag will die before writing the tag if the
-+tag doesn't pass a linkgit:git-fsck[1] check.
-+
-+The "fsck" check done mktag is is stricter than what
-+linkgit:git-fsck[1] would run by default in that all `fsck.<msg-id>`
-+messages are promoted from warnings to errors (so e.g. a missing
-+"tagger" line is an error). Extra headers in the object are also an
-+error under mktag, but ignored by linkgit:git-fsck[1].
++tag doesn't pass a sanity check.
Tag Format
----------
4: 1f06b9c0cf = 4: acb94e0289 mktag tests: don't needlessly use a subshell
5: 5d1cb73ca3 = 5: 4ae76ec5e3 mktag tests: remove needless SHA-1 hardcoding
6: cf86f4ca37 = 6: 9effb4532b mktag tests: improve verify_object() test coverage
-: ---------- > 7: b81d31a917 mktag tests: don't pipe to stderr needlessly
-: ---------- > 8: 11f59718b4 mktag tests: don't create "mytag" twice
-: ---------- > 9: dd6b012b0c mktag tests: stress test whitespace handling
-: ---------- > 10: 56c6b562fd mktag tests: test "hash-object" compatibility
2: 6e98557709 = 11: 1e2e4ec269 mktag: use default strbuf_read() hint
3: 8e5fe08f15 = 12: be2ab3edab mktag: remove redundant braces in one-line body "if"
-: ---------- > 13: d8514df970 mktag: use puts(str) instead of printf("%s\n", str)
7: 5812ee53c9 ! 14: 346d73cc97 mktag: use fsck instead of custom verify_tag()
@@ Commit message
Signed-off-by: Ævar Arnfjörð Bjarmason [off-list ref]
+ ## Documentation/git-mktag.txt ##
+@@ Documentation/git-mktag.txt: write a tag found in `my-tag`:
+ git hash-object -t tag -w --stdin <my-tag
+
+ The difference is that mktag will die before writing the tag if the
+-tag doesn't pass a sanity check.
++tag doesn't pass a linkgit:git-fsck[1] check.
++
++The "fsck" check done mktag is stricter than what linkgit:git-fsck[1]
++would run by default in that all `fsck.<msg-id>` messages are promoted
++from warnings to errors (so e.g. a missing "tagger" line is an error).
++
++Extra headers in the object are also an error under mktag, but ignored
++by linkgit:git-fsck[1]
+
+ Tag Format
+ ----------
+
## builtin/mktag.c ##
@@
#include "tag.h"
@@ builtin/mktag.c
+
+ buffer = read_object_file(tagged_oid, &type, &size);
+ if (!buffer)
-+ die("could not read tagged object '%s'\n",
++ die("could not read tagged object '%s'",
+ oid_to_hex(tagged_oid));
+ if (type != *tagged_type)
-+ die("object '%s' tagged as '%s', but is a '%s' type\n",
++ die("object '%s' tagged as '%s', but is a '%s' type",
+ oid_to_hex(tagged_oid),
+ type_name(*tagged_type), type_name(type));
+
@@ t/t3800-mktag.sh: tagger <> 0 +0000
-check_verify_failure 'disallow missing tag author name' \
- '^error: char.*: missing tagger name$'
-+test_expect_success 'allow missing tag author name' '
-+ git mktag <tag.sig
-+'
++test_expect_mktag_success 'allow missing tag author name'
############################################################
# 14. disallow missing tag author name
@@ t/t3800-mktag.sh: tagger T A Gger <
############################################################
# 15. allow empty tag email
-@@ t/t3800-mktag.sh: test_expect_success \
- 'git mktag <tag.sig >.git/refs/tags/mytag 2>message'
+@@ t/t3800-mktag.sh: EOF
+ test_expect_mktag_success 'allow empty tag email'
############################################################
-# 16. disallow spaces in tag email
@@ t/t3800-mktag.sh: tagger T A Gger <tag ger@example.com> 0 +0000
-check_verify_failure 'disallow spaces in tag email' \
- '^error: char.*: malformed tagger field$'
-+test_expect_success 'allow spaces in tag email like fsck' '
-+ git mktag <tag.sig
-+'
++test_expect_mktag_success 'allow spaces in tag email like fsck'
############################################################
# 17. disallow missing tag timestamp
@@ t/t3800-mktag.sh: tagger T A Gger [off-list ref] 1206478233 -1430
-check_verify_failure 'detect invalid tag timezone3' \
- '^error: char.*: malformed tag timezone$'
-+test_expect_success 'allow invalid tag timezone' '
-+ git mktag <tag.sig
-+'
++test_expect_mktag_success 'allow invalid tag timezone'
############################################################
# 23. detect invalid header entry
@@ t/t3800-mktag.sh: this line should not be here
check_verify_failure 'detect invalid header entry' \
- '^error: char.*: trailing garbage in tag header$'
+ '^error:.* extraHeaderEntry:'
-+
-+cat >tag.sig <<EOF
-+object $head
-+type commit
-+tag mytag
-+tagger T A Gger [off-list ref] 1206478233 -0500
-+
-+
-+this line comes after an extra newline
-+EOF
-+
-+test_expect_success \
-+ 'allow extra newlines at start of body' \
-+ 'git mktag <tag.sig >.git/refs/tags/mytag 2>message'
+
+ cat >tag.sig <<EOF
+ object $head
+@@ t/t3800-mktag.sh: tagger T A Gger [off-list ref] 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 [off-list ref] 1206478233 -0500
+ EOF
+
+ check_verify_failure 'disallow no header / body newline separator' \
+- '^error: char.*: trailing garbage in tag header$'
++ '^error:.* extraHeaderEntry:'
############################################################
# 24. create valid tag
9: 30eff9170f = 15: 0e7994d8fc fsck: make fsck_config() re-usable
10: 11139ec2b8 ! 16: 5e8046022b mktag: allow turning off fsck.extraHeaderEntry
@@ Commit message
Signed-off-by: Ævar Arnfjörð Bjarmason [off-list ref]
## Documentation/git-mktag.txt ##
-@@ Documentation/git-mktag.txt: tag doesn't pass a linkgit:git-fsck[1] check.
- The "fsck" check done mktag is is stricter than what
- linkgit:git-fsck[1] would run by default in that all `fsck.<msg-id>`
- messages are promoted from warnings to errors (so e.g. a missing
--"tagger" line is an error). Extra headers in the object are also an
--error under mktag, but ignored by linkgit:git-fsck[1].
-+"tagger" line is an error).
-+
-+Extra headers in the object are also an error under mktag, but ignored
+@@ Documentation/git-mktag.txt: would run by default in that all `fsck.<msg-id>` messages are promoted
+ from warnings to errors (so e.g. a missing "tagger" line is an error).
+
+ Extra headers in the object are also an error under mktag, but ignored
+-by linkgit:git-fsck[1]
+by linkgit:git-fsck[1]. This extra check can be turned off by setting
+the appropriate `fsck.<msg-id>` varible:
+
-: ---------- > 17: 32698e1d00 mktag: allow omitting the header/body \n separator
-: ---------- > 18: b6a22f2f99 mktag: convert to parse-options
-: ---------- > 19: 7fc0b81df7 mktag: mark strings for translation
-: ---------- > 20: 6fa443d528 mktag: add a --no-strict option
--
2.29.2.222.g5d2a92d10f8