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

From: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Date: May 10, 2021, 12:02 UTC
Message-ID: <87r1ieu6q4.fsf@evledraar.gmail.com>
In-Reply-To: <20210506165102.123739-1-firminmartin24@gmail.com>

On Thu, May 06 2021, Firmin Martin wrote:

Show 25 quoted lines

> 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...