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

On Thu, Aug 2, 2018 at 1:27 PM Junio C Hamano [off-list ref] wrote:

Phillip Wood [off-list ref] writes:

quoted

For other interactive rebases this only affects external scripts that
read the author script and users whose git is upgraded from the shell
version of rebase -i while rebase was stopped when the author contains
"'". This is because the parsing in read_env_script() expected the
broken quoting.

I wasn't following the discussion, but is it the general consensus
that reading the broken a-i file is a requirement for the new code?
Not an objection phrased as a question.

I do not think it is worth worrying about the "upgrade while rebase
was in progress" case, if it involves much more code than necessary
without its support, especially if the only thing the user needs to
do recover from such a situation is to say "rebase --abort" and then
to retry the same rebase with the fixed version that was installed
in the meantime. [...]

[...] It still does look
unnecessarily ugly and over-engineered to have this (and the
"version" reading code), though, at least to me, but perhaps it is
just me.

It's not just you. I also questioned[1] if such backward compatibility
was needed, and had concerns[2] about a version file being heavyweight
and over-engineered.

This is a lot of new code (possibly harboring its own bugs) for a
situation unlikely to arise, and which becomes ever more unlikely as
time passes. Also, unlike long-lived (years or decades) resources,
such as a repository or pack file, for instance, for which a version
number makes sense, this file is very short-lived, which makes it even
more difficult to justify adding this much machinery for something so
unlikely to arise in practice.

The overall aim of this series to fix these bugs is laudable, but I
would be happy to see this one reduced to just a "bug fix" patch
without all the backward-compatibility machinery (and wouldn't mind
seeing patch 1/2 simplified[3], as well).

Thanks.

[1]: https://public-inbox.org/git/CAPig+cR5VHP8muo5_A_9t7OPZam8O_uPb0nd73B15Ye92n+p7Q@mail.gmail.com/
[2]: https://public-inbox.org/git/CAPig+cTttbV2FjnoS_SZtwh2J4wwzsbK+48BAbt1cV0utynYzw@mail.gmail.com/
[3]: https://public-inbox.org/git/CAPig+cSZ3Zm=qFcvGjyj_uStn=JMAYuskMa0O_2yxkKjaRWTSg@mail.gmail.com/