Re: [PATCH v1 0/8] format-patch: introduce --confirm-overwrite

On Thu, May 06 2021, Firmin Martin wrote:

BACKGROUND
==========

Currently, git-format-patch, along with the option --cover-letter,
unconditionally overwrites a cover letter with the same name (if
present). Although this is a desired behaviour for patches which are
auto-generated from Git commits log, it might not be the case for a
cover letter whose the content is meticulously written manually.

Particulary, this behaviour could be awkward in the following
hypothetical situations:

* The user can easily erase a cover letter coming from prior versions or
another patch series by reusing an old command line (e.g. autocompleted
from the shell history).

* Assuming that the user is writing a cover letter and realizes that
small changes should be made. They make the change, amend and
format-patch again to regenerate patches. If it happens that they use
the same command again (e.g. with --cover-letter), the cover letter
being written is gone.

This patch series addresses this kind of issue by asking confirmation
from the user whenever a cover letter or a patch is subject to be
overwritten.

I like the goal here, I'm another person with ad-hoc tooling around
format-patch to manage / avoid this particular scenario and related
issues (e.g. I have a wrapper to rm -rf the output directory & re-crete
it, in case I want to rebase but use the same -vN version).

I wonder if you've considered some ways to automatically and more gently
detect these cases, such as:

 1. When we generate the patch, set the mtime manually to the time in
    the commit object. When clobbering a file see if they correspond. If
    mtime != expected => *boom* and ask for confirmation.

 2. We already include a blurb like "2.31.1.838.g7ac6e98bb53" (the git
    version) if you have format.signature set. How about making that
    include a short hash of the preceding lines. If it doesn't match we
    can ask, but if it does we clobber.

    This has the added benefit that other could script their MUAs to
    highlight manually edited patches.

 3. Ditto #2 but generate the new file in a tempfile, diff them, if
    they're different complain. This also opens the door to make this
    neatly integrate with git-diff's -I option, so you could specify
    regexes to ignore. By default we could ignore lines that change
    known headers we expect to change.

 4. The format we output would need parsing, but it's not that
    hard. I.e. we expect something like:

        [...]        
        Subject: [PATCH 0/1] *** SUBJECT HERE ***
        MIME-Version: 1.0
        Content-Type: text/plain; charset=UTF-8
        Content-Transfer-Encoding: 8bit
        
        *** BLURB HERE ***
        
        Ævar Arnfjörð Bjarmason (1):
        [...]


    And similarly for patches we could narrow things to look between the
    "---" and an expected diffstat. So we could complain just about
    changes there.

    But maybe that's a fool's errand, e.g. it would be hard to catch
    manually commented-on range-diffs without implementing a full parser
    for that format...

None of these suggestions should be read as making perfect the enemy of
the good. I *do* rely on the behavior of the setting you're introducing
and "breaking", but I think the user-base of advanced format-patch users
is small enough that we could just configure things to "never" and move
on, but accidentally losing data (as happens now) is a worse default...