| Message ID | 20210419225441.3139048-3-lukeshu@lukeshu.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | fast-export, fast-import: implement signed-commits | expand |
On Mon, Apr 19, 2021 at 04:54:40PM -0600, Luke Shumaker wrote: > From: Luke Shumaker <lukeshu@datawire.io> > > But still keep --signed-tags=warn as an undocumented alias. This name > is clearer as it has symmetry with warn-strip: > > action -> action > +----------------------------+ -> +----------------------------+ > msg? | verbatim | strip | -> msg? | verbatim | strip | > | warn | warn-strip | -> | warn-verbatim | warn-strip | > +----------------------------+ -> +----------------------------+ This table is rather confusing to me. What's unclear to me is what "msg?" and "action" are referring to. After reading your patch, I think it may be clearer to say: The --signed-tags option takes one of five arguments specifying how to handle singed tags during export. Among these arguments, strip is to warn-strip as verbatim is to warn. (The unmentioned argument is 'abort', which stops the fast-export process entirely). That is, signatures are either stripped or copied verbatim while exporting, with or without a warning. Make clear that the "warn" option instructs fast-export to copy signatures verbatim by matching the pattern (and calling the option "warn-verbatim"). To maintain backwards compatibility, "warn" is still recognized as an undocumented alias. > +test_expect_success 'signed-tags=warn' ' > + git fast-export --signed-tags=warn sign-your-name >output 2>err && > + grep PGP output && > + test -s err > +' > + > +test_expect_success 'signed-tags=warn-verbatim' ' > + git fast-export --signed-tags=warn sign-your-name >output 2>err && s/warn/warn-verbatim ? Thanks, Taylor
On Mon, 19 Apr 2021 18:27:35 -0600, Taylor Blau wrote: > > On Mon, Apr 19, 2021 at 04:54:40PM -0600, Luke Shumaker wrote: > > From: Luke Shumaker <lukeshu@datawire.io> > > > > But still keep --signed-tags=warn as an undocumented alias. This name > > is clearer as it has symmetry with warn-strip: > > > > action -> action > > +----------------------------+ -> +----------------------------+ > > msg? | verbatim | strip | -> msg? | verbatim | strip | > > | warn | warn-strip | -> | warn-verbatim | warn-strip | > > +----------------------------+ -> +----------------------------+ > > This table is rather confusing to me. What's unclear to me is what > "msg?" and "action" are referring to. After reading your patch, I think > it may be clearer to say: > > The --signed-tags option takes one of five arguments specifying how > to handle singed tags during export. Among these arguments, strip is > to warn-strip as verbatim is to warn. (The unmentioned argument is > 'abort', which stops the fast-export process entirely). That is, > signatures are either stripped or copied verbatim while exporting, > with or without a warning. > > Make clear that the "warn" option instructs fast-export to copy > signatures verbatim by matching the pattern (and calling the option > "warn-verbatim"). > > To maintain backwards compatibility, "warn" is still recognized as > an undocumented alias. Thank you, I'll take much of this wording. > > +test_expect_success 'signed-tags=warn' ' > > + git fast-export --signed-tags=warn sign-your-name >output 2>err && > > + grep PGP output && > > + test -s err > > +' > > + > > +test_expect_success 'signed-tags=warn-verbatim' ' > > + git fast-export --signed-tags=warn sign-your-name >output 2>err && > > s/warn/warn-verbatim ? Indeed, oops! I'll also add a comment clarifying that the signed-tags=warn test is testing for backward compatibility.
diff --git a/Documentation/git-fast-export.txt b/Documentation/git-fast-export.txt index 1978dbdc6a..d4a2bfe037 100644 --- a/Documentation/git-fast-export.txt +++ b/Documentation/git-fast-export.txt @@ -27,7 +27,7 @@ OPTIONS Insert 'progress' statements every <n> objects, to be shown by 'git fast-import' during import. ---signed-tags=(verbatim|warn|warn-strip|strip|abort):: +--signed-tags=(verbatim|warn-verbatim|warn-strip|strip|abort):: Specify how to handle signed tags. Since any transformation after the export can change the tag names (which can also happen when excluding revisions) the signatures will not match. @@ -36,8 +36,8 @@ When asking to 'abort' (which is the default), this program will die when encountering a signed tag. With 'strip', the tags will silently be made unsigned, with 'warn-strip' they will be made unsigned but a warning will be displayed, with 'verbatim', they will be silently -exported and with 'warn', they will be exported, but you will see a -warning. +exported and with 'warn-verbatim', they will be exported, but you will +see a warning. --tag-of-filtered-object=(abort|drop|rewrite):: Specify how to handle tags whose tagged object is filtered out. diff --git a/builtin/fast-export.c b/builtin/fast-export.c index 85a76e0ef8..d121dd2ee6 100644 --- a/builtin/fast-export.c +++ b/builtin/fast-export.c @@ -55,7 +55,7 @@ static int parse_opt_signed_tag_mode(const struct option *opt, signed_tag_mode = SIGNED_TAG_ABORT; else if (!strcmp(arg, "verbatim") || !strcmp(arg, "ignore")) signed_tag_mode = VERBATIM; - else if (!strcmp(arg, "warn")) + else if (!strcmp(arg, "warn-verbatim") || !strcmp(arg, "warn")) signed_tag_mode = WARN; else if (!strcmp(arg, "warn-strip")) signed_tag_mode = WARN_STRIP; diff --git a/t/t9350-fast-export.sh b/t/t9350-fast-export.sh index 409b48e244..db0e58b1e8 100755 --- a/t/t9350-fast-export.sh +++ b/t/t9350-fast-export.sh @@ -253,6 +253,18 @@ test_expect_success 'signed-tags=verbatim' ' ' +test_expect_success 'signed-tags=warn' ' + git fast-export --signed-tags=warn sign-your-name >output 2>err && + grep PGP output && + test -s err +' + +test_expect_success 'signed-tags=warn-verbatim' ' + git fast-export --signed-tags=warn sign-your-name >output 2>err && + grep PGP output && + test -s err +' + test_expect_success 'signed-tags=strip' ' git fast-export --signed-tags=strip sign-your-name > output &&