Hi Eric
On 03/08/18 11:02, Eric Sunshine wrote:
On Fri, Aug 3, 2018 at 5:33 AM Phillip Wood <phillip.wood@xxxxxxxxxxxx> 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
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@xxxxxxxxxxxx/
[2]: https://public-inbox.org/git/CAPig+cTttbV2FjnoS_SZtwh2J4wwzsbK+48BAbt1cV0utynYzw@xxxxxxxxxxxxxx/
