On Thu, Feb 14, 2013 at 12:29:36PM -0800, Junio C Hamano wrote:

> Quote the variable in double-quotes, like this:
>       if [ "$GIT_AUTHOR_EMAIL" = j...@old.example.com ]
> Otherwise the comparison will break, if the e-mail part had a
> whitespace in it, or if it were empty, which is an example of a more
> likely situation where you would want to fix commits using a
> procedure like this, no?

Yeah, definitely. If you are cleaning up a broken ident that cannot be
parsed, the failure mode is to put the whole author line in
$GIT_AUTHOR_EMAIL, which would almost certainly include spaces.

> Taking all of the above, the added text may look more like this, I
> think:
>       The `--env-filter` can be used to modify committer and/or
>       author identity.  For example, if you found out that your
>       commits have wrong identity of yours due to misconfigured
>       user.email, you can make correction, before publishing the
>       project, like this:
>       --------------------------------------------------------
>         git filter-branch --env-filter '
>               if test "$GIT_AUTHOR_EMAIL" = "root@localhost"
>                 then
>                       GIT_AUTHOR_EMAIL=y...@example.com
>                       export GIT_AUTHOR_EMAIL
>               fi
>               if test "$GIT_COMMITTER_EMAIL" = "root@localhost"
>                 then
>                       GIT_COMMITTER_EMAIL=y...@example.com
>                       export GIT_COMMITTER_EMAIL
>               fi
>       ' -- --all
>       --------------------------------------------------------

That looks better, though there are a few English nits; here's my edited

        The `--env-filter` option can be used to modify committer and/or
        author identity.  For example, if you found out that your
        commits have the wrong identity due to a misconfigured
        user.email, you can make a correction, before publishing the
        project, like this:

> By the way, I left the "export" in; "git filter-branch --help"
> explicitly says that you need to re-export it.  But I am not sure if
> they are necessary, especially after 3c730fab2cae (filter-branch:
> use git-sh-setup's ident parsing functions, 2012-10-18) by Peff,
> which added extra "export" to make sure all six identity variables
> are exported.  After applying the above rewrite, we may want to do
> the following as a separate, follow-up patch.

I think it has always been the case that we export them after setting
them; just look at the preimage from 3c730fab, and you can see exports

I think the advice in the documentation about re-exporting is because
some versions of the bourne shell will not reliably pass the new version
of the variable when you do this:

  export VAR
  some_subprocess ;# we see $VAR=old here!

I do not recall ever running across such a shell myself, but rather
hearing about it third-hand in a portability guide somewhere. Apple's
shell documentation seems to indicate that /bin/sh in older versions of
OS X had this behavior:


which makes me think that BSD ash may behave that way. It is certainly
not necessary to re-export under bash or dash. I shudder to think what
horrible, 1980's-era behavior is codified in Solaris /bin/sh.

We could explicitly re-export all of the ident variables preemptively
before calling commit-tree, just to save the user the hassle of
remembering to do so.  It would be a no-op on sane shells, and I doubt
the runtime cost is very high. I suppose it would break somebody who
explicitly did:

  unset GIT_COMMITTER_NAME ;# use the value from user.name

in their env-filter, but that seems like a pretty unlikely corner case.

To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to