Johannes Schindelin <Johannes.Schindelin@xxxxxx> writes:
> You stated elsewhere that converting a script into a builtin should focus
> on a faithful conversion.
>
> The original code is:
>
> . "$author_script"
>
> Granted, this *cannot* be converted faithfully without reimplementing a
> shell interpreter. So I did the next best thing: I converted it into code
> that reads a block of environment variable settings.
It is unfortunate that you took "faithful" too literally. While I
do appreciate it if your conversion aims to faithfully replicate the
original, even to be bug-to-bug compatible, but obviously we cannot
replicate everything the above original _could_ do.
By "a faithful conversion", I meant that the behaviour of the
reimplementation should be as faithful to the original's intent as
possible. Nothing more.
The intent of the above original is to read back what we wrote to
preserve the author identity we learned earlier when we wrote the
file. We read the "author" line from the commit object header, and
write assignments to GIT_AUTHOR_{NAME,EMAIL,DATE} variables. Nothing
more is intended.
The end-users COULD abuse the original code to cause it to do a lot
more than that, e.g. by adding "export FOO=BAR" at the end and have
the value of the new environment variable propagate throughout the
code and even down to subprocesses. They can even assign to some
variables we use internally for bookkeeping and break "rebase -i"
machinery. But that is outside the intent of the original code--we
do not need to or want to replicate that faithfully.
I also need to react to the "environment variable settings" at the
end of the quoted paragraph.
If the code in the sequencer.c reads things other than the three
variables we ourselves set, and make them into environment variables
and propagate to subprocesses (hooks and editors), it would be a
bug. The original did not intend to do that (the dot-sourcing is
overly loose than reading three known variables and nothing else,
but is OK because we do not support the case where end users muck
with the file). Also, writing FOO=BAR alone (not "export FOO=BAR"
or "FOO=BAR; export FOO") to the file wouldn't have exported FOO to
subprocesses anyway.