Hi Jeremy,
On Tue, 29 Jan 2019, Jeremy Huddleston Sequoia wrote:
> 480871e09e ("format-patch: show base info before email signature",
> 2016-09-07) added a helper function to recreate the signature at the end
> of the e-mail, i.e. "-- " line followed by the version string of Git,
> using output from "git --version" and stripping everything before the last
> SP.
>
> Because the default Git version string looks like "git version
> 2.10.0-1-g480871e09e", this was mostly OK, but people can change this
> version string to arbitrary thing while compiling, which can break the
> assumption if they had SP in it. Notably, Apple ships modified Git with
> " (Apple Git-xx)" appended to its version number.
Here would be a fine place to add Junio's explanation that `git version`
always prefixes "git version " to the `git_version_string` and that the
default signature in `builtin/log.c` is defined as said
`git_version_string`.
>
> Instead, come up with the version string by stripping the "git version "
> from the beginning.
>
> Regressed-in: 480871e09ed2e5275b4ba16b278681e5a8c122ae
This is really not a good way to reference a commit, what with our
intention to switch to SHA-256 at some stage.
Besides, this footer is completely redundant with the information that
starts the very first paragraph.
Ciao,
Johannes
> Signed-off-by: Jeremy Huddleston Sequoia <jeremyhu@xxxxxxxxx>
> Reviewed-by: Josh Triplett <josh@xxxxxxxxxxxxxxxx>
> ---
> t/t4014-format-patch.sh | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh
> index 909c743c13..414c56fcff 100755
> --- a/t/t4014-format-patch.sh
> +++ b/t/t4014-format-patch.sh
> @@ -757,7 +757,7 @@ test_expect_success 'format-patch --ignore-if-in-upstream HEAD' '
> git format-patch --ignore-if-in-upstream HEAD
> '
>
> -git_version="$(git --version | sed "s/.* //")"
> +git_version="$(git --version | sed "s/git version //")"
As Junio said, this should be anchored. And for extra safety, in case some
even more unreasonable company decides to change the output of `git
--version` itself, it should probably use
git_version="$(expr "$(git --version)" : "^git version \(.*\)")"
Ciao,
Johannes
>
> signature() {
> printf "%s\n%s\n\n" "-- " "${1:-$git_version}"
> --
> 2.20.0 (Apple Git-115)
>
>
