Re: [PATCH v3 2/2] sequencer: fix quoting in write_author_script

From: Eric Sunshine <sunshine@sunshineco.com>
Date: Aug 3, 2018, 10:02 UTC
Message-ID: <CAPig+cTyTHxwFvk3ZtOq3L7KEtEjKLKu6-RnLC-_NuL1Xzhqzw@mail.gmail.com>
In-Reply-To: <c7b8629d-7b93-2fbf-6793-0d566e86a229@talktalk.net>

On Fri, Aug 3, 2018 at 5:33 AM Phillip Wood <phillip.wood@talktalk.net> wrote:

Show 7 quoted lines

> If there isn't some backward compatibility then if git gets upgraded
> while rebase is stopped then the author data will be silently corrupted
> if it contains "'". read_author_ident() will error out but that is only
> used for the root commit. read_env_script() which is used for normal
> picks will not dequote the badly quoted value correctly and will not
> return an error. It is unlikely but possible, I'll leave it to Junio to
> decide if it is worth it

If I understand correctly, the approach you implemented earlier[1] (perhaps coupled with the more robust detection suggested here[2]) would be sufficient to handle this backward compatibility concern. While it may not be as pretty or generalized as the current patch, it involves far less machinery, thus is less likely to harbor its own bugs. The earlier version is also much more self-contained, which makes it easier to drop at some point when backward compatibility is no longer a concern (if ever).

> There is a precedent for adding backwards compatibility 84df4560ed
> ("rebase: extract code for writing basic state", 2011-02-06) though it
> is much simpler.

Indeed, it is much simpler, adding a one-liner 'else' case to an 'if-then' for backward compatibility. Your earlier implementation[1] was pretty much the equivalent, just adding an extra one-liner arm to an 'if-then' statement.

The bug fix itself is important, and, while I do favor the cleaner approach of not worrying about backward compatibility for this fairly unlikely case, your earlier version seems a better compromise between having no backward compatibility and the much more heavyweight version implemented here.

Anyhow, I'm fine with whatever Junio decides.

[1]: https://public-inbox.org/git/20180731111532.9358-3-phillip.wood@talktalk.net/ [2]: https://public-inbox.org/git/CAPig+cTttbV2FjnoS_SZtwh2J4wwzsbK+48BAbt1cV0utynYzw@mail.gmail.com/