Re: [PATCH v3 3/3] fast-export, fast-import: implement signed-commits

[Luke Shumaker <lukeshu@lukeshu.com>]

On Thu, 29 Apr 2021 17:42:24 -0600,
Junio C Hamano wrote:
> 
> Elijah Newren <newren@gmail.com> writes:
> 
> > I do get that we might have to use warn-strip as the default anyway
> > just because some existing tools might rely on it, but do you have any
> > examples outside of git-filter-repo?  Given the filter-repo bug
> > reports I've gotten with users being surprised at commit signatures
> > being stripped (despite the fact that this is documented -- users
> > don't always read the documentation), I'd argue that changing to
> > --signed-commits=abort as the default is probably a good bugfix for
> > both fast-export and for filter-repo.
> 
> Thanks.  The "filter-repo already gets bug reports from the users"
> is a valuable input when deciding if it is reasonable to sell the
> behaviour change as a bugfix to our users.
> 
> Perhaps teaching fast-export to pay attention to two environment
> variables that say "when no --signed-{tag,commit}=<disposition>"
> command line option is given, use this behaviour" would be a good
> enough escape hatch for existing tools and their users, while they
> are waiting for their tools to get updated with the new option you
> are planning to add?

Between Elijah being on-board with changing the default, and the
suggested env-var escape hatch, you've won me over.

I'll change the default to 'abort' and implement an env-var escape
hatch.  Any suggestions on how to name it?
`FAST_EXPORT_SIGNED_COMMITS`?  Should I give it a `GIT_` prefix?
`FILTER_BRANCH_SQUELCH_WARNING` doesn't have a `GIT_` prefix...

> Also, I am glad that you brought up another possible behaviour that
> Luke's patch did not add.  Exporting existing signatures that may
> become invalid and deciding what to do with them on the receiving
> end would be a good option to have.  And that would most likely have
> to be done at "fast-import" end, as a commit that "fast-export"
> expected to retain its object name if its export stream were applied
> as-is may not retain the object name when the export stream gets
> preprocessed before being fed to "fast-import".

Elijah suggested that on an earlier version of the patchset too.  I
agree that it's a splendid idea, but I'm not willing to be the one to
do the work of implementing it... at least not in the next few months.

-- 
Happy hacking,
~ Luke Shumaker