All of lore.kernel.org
 help / color / mirror / Atom feed
From: Elijah Newren <newren@gmail.com>
To: Luke Shumaker <lukeshu@lukeshu.com>
Cc: "Git Mailing List" <git@vger.kernel.org>,
	"Luke Shumaker" <lukeshu@datawire.io>,
	"Junio C Hamano" <gitster@pobox.com>, "Jeff King" <peff@peff.net>,
	"Johannes Schindelin" <Johannes.Schindelin@gmx.de>,
	"Nguyễn Thái Ngọc Duy" <pclouds@gmail.com>
Subject: Re: [PATCH 0/3] fast-export, fast-import: implement signed-commits
Date: Wed, 21 Apr 2021 11:12:40 -0700	[thread overview]
Message-ID: <CABPp-BEz-QykELpvofsrZzbQtQUE0fvijUcaJXhRFbCZpKJuYQ@mail.gmail.com> (raw)
In-Reply-To: <20210419225441.3139048-1-lukeshu@lukeshu.com>

On Mon, Apr 19, 2021 at 3:54 PM Luke Shumaker <lukeshu@lukeshu.com> wrote:
>
> From: Luke Shumaker <lukeshu@datawire.io>
>
> fast-export has an existing --signed-tags= flag that controls how to
> handle tag signatures.  However, there is no equivalent for commit
> signatures; it just silently strips the signature out of the commit
> (analogously to --signed-tags=strip).
>
> So implement a --signed-commits= flag in fast-export, and implement
> the receiving side of it in fast-import.

I understand adding an option to fast-export, but shouldn't there also
be one for fast-import?  In particular, I can see users wanting any of
the following:

* I want these signatures exported and imported because I know I won't
tweak them and they'll still be valid.
* I want these signatures even though they'll be invalid.  Whatever,
I'll just deal with it.
* I want the signatures exported and imported *when they will remain
valid*.  Always exporting them makes sense, because fast-export
doesn't know about tweaks I'll be making to its output before feeding
it to fast-import.  But fast-import should have options to
strip-if-invalid/warn-if-invalid/error-if-invalid/import-without-warning
for these tags (though they don't have to use these exact names).

I know fast-import doesn't do anything of the sort for signed tags,
but fast-import also doesn't support signed tags as per this comment
in the documentation:

"""
Signing annotated tags during import from within fast-import is not
supported.  Trying to include your own PGP/GPG signature is not
recommended, as the frontend does not (easily) have access to the
complete set of bytes which normally goes into such a signature.
If signing is required, create lightweight tags from within fast-import with
`reset`, then create the annotated versions of those tags offline
with the standard 'git tag' process.
"""

it just happens to "work" since the signature is part of the
annotation and fast-import doesn't attempt to read or validate the
annotation in any way, treating it as free-from text.  I'd say users
relying on this are on somewhat shaky ground.

But here you're adding explicit fast-import directives to the language
for signatures of commits, so you clearly do need to care.  And I
suspect fast-import's default should be error-if-invalid rather than
import-without-warning.

> Luke Shumaker (3):
>   git-fast-import.txt: add missing LF in the BNF
>   fast-export: rename --signed-tags='warn' to 'warn-verbatim'
>   fast-export, fast-import: implement signed-commits
>
>  Documentation/git-fast-export.txt | 18 +++++--
>  Documentation/git-fast-import.txt |  9 +++-
>  builtin/fast-export.c             | 88 +++++++++++++++++++++++++------
>  builtin/fast-import.c             | 15 ++++++
>  t/t9350-fast-export.sh            | 82 ++++++++++++++++++++++++++++
>  5 files changed, 191 insertions(+), 21 deletions(-)
>
> --
> 2.31.1
>
> Happy hacking,
> ~ Luke Shumaker

  parent reply	other threads:[~2021-04-21 18:12 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-04-19 22:54 [PATCH 0/3] fast-export, fast-import: implement signed-commits Luke Shumaker
2021-04-19 22:54 ` [PATCH 1/3] git-fast-import.txt: add missing LF in the BNF Luke Shumaker
2021-04-19 22:54 ` [PATCH 2/3] fast-export: rename --signed-tags='warn' to 'warn-verbatim' Luke Shumaker
2021-04-20  0:27   ` Taylor Blau
2021-04-20 15:45     ` Luke Shumaker
2021-04-19 22:54 ` [PATCH 3/3] fast-export, fast-import: implement signed-commits Luke Shumaker
2021-04-20  1:41   ` brian m. carlson
2021-04-20 17:15     ` Luke Shumaker
2021-04-20 23:07       ` brian m. carlson
2021-04-21 22:03         ` Luke Shumaker
2021-04-20  1:45   ` Taylor Blau
2021-04-20 16:23     ` Luke Shumaker
2021-04-20 15:51   ` Luke Shumaker
2021-04-21 18:12 ` Elijah Newren [this message]
2021-04-21 19:28   ` [PATCH 0/3] " Luke Shumaker

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CABPp-BEz-QykELpvofsrZzbQtQUE0fvijUcaJXhRFbCZpKJuYQ@mail.gmail.com \
    --to=newren@gmail.com \
    --cc=Johannes.Schindelin@gmx.de \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=lukeshu@datawire.io \
    --cc=lukeshu@lukeshu.com \
    --cc=pclouds@gmail.com \
    --cc=peff@peff.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.