Re: [PATCH v3 2/2] sequencer: fix quoting in write_author_script
- From
- Phillip Wood <phillip.wood@talktalk.net>
- Date
- Aug 3, 2018, 14:12 UTC
- Message-ID
- <455fafb5-3c92-4348-0c2c-0a4ab62cf2ce@talktalk.net>
- In-Reply-To
- <CAPig+cTyTHxwFvk3ZtOq3L7KEtEjKLKu6-RnLC-_NuL1Xzhqzw@mail.gmail.com>
Hi Eric On 03/08/18 11:02, Eric Sunshine wrote:
Show 17 quoted lines
> On Fri, Aug 3, 2018 at 5:33 AM Phillip Wood <phillip.wood@talktalk.net> wrote: >> 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).
Yes I think the earlier approach with the more robust detection you suggested is probably a good compromise. Junio does that sound good to you?
Best Wishes
Phillip
Show 20 quoted lines
>> 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/
>