From: Michael Haggerty <mhagger@alum.mit.edu>
To: Junio C Hamano <gitster@pobox.com>
Cc: Johannes Sixt <j6t@kdbg.org>,
Tarmigan Casebolt <tarmigan+git@gmail.com>,
git@vger.kernel.org, "Shawn O. Pearce" <spearce@spearce.org>
Subject: Re: [PATCH v2] Test t5560: Fix test when run with dash
Date: Thu, 21 Jan 2010 17:15:42 +0100 [thread overview]
Message-ID: <4B587DAE.9030208@alum.mit.edu> (raw)
In-Reply-To: <7v4ommoo4p.fsf@alter.siamese.dyndns.org>
Junio C Hamano wrote:
> Johannes Sixt <j6t@kdbg.org> writes:
>>> Yesterday, I saw rebase--interactive has a few codepaths where "output"
>>> shell function was used with the single-shot export; perhaps they need to
>>> also be fixed.
>> I knew these spots, and they were discussed when that code was introduced.
>> Before I sent out the mail you were responding to, I tried various ways to
>> show the failure in rebase--interactive, but it didn't fail...
>
> It may be the case that the single-shot-ness of these GIT_AUTHOR_NAME
> exports do not matter at all in that program, even though the original
> versions may have been written carefully not to leak the value suitable
> for the current commit to later rounds.
>
> I think the recent updates from Michael actually depends on the
> distinction not to matter. For example, do_with_author() in 7756ecf
> (rebase -i: Extract function do_with_author, 2010-01-14) invokes "$@"
> that could be a shell function.
I have to say that I am a little bit over my head here. I didn't try to
follow the complete data path of the GIT_AUTHOR_* shell variables, nor
do I know exactly what git commands they affect. I just tried to
locally refactor the code based on my mistaken assumption that shell
functions are treated much like external commands WRT export of shell
variables.
The use of the GIT_AUTHOR_* variables in git-rebase--interactive.sh were
and are a bit peculiar anyway, since the variables are already set
before do_with_author() is invoked, and the values are left to hang
around afterwards. The do_with_author() function only tries to export
these already-set variables.
So I suppose that the simplest solution is to export these variables
explicitly in do_with_author(), something like this (similar to the
third code block that was replaced by the do_with_author() function):
do_with_author() {
export GIT_AUTHOR_NAME GIT_AUTHOR_EMAIL GIT_AUTHOR_DATE &&
"$@"
}
But to ensure that this is a correct solution would require verification
that these now-exported variables don't cause unwanted side-effects
during any other external command invocations. Alternatively, I suppose
that the variables could be exported within a subshell that also invokes
the "$@" command; this subshell could even source the $AUTHOR_SCRIPT
file if it were thought advantageous not to set the GIT_AUTHOR_*
variables in the git-rebase--interactive.sh script at all.
Help would be most appreciated; I probably won't have time to work on
this myself for a week or two.
Michael
prev parent reply other threads:[~2010-01-21 16:16 UTC|newest]
Thread overview: 31+ messages / expand[flat|nested] mbox.gz Atom feed top
2009-12-28 22:04 [PATCH RFC 1/2] Smart-http tests: Break test t5560-http-backend into pieces Tarmigan Casebolt
2009-12-28 22:04 ` [PATCH RFC 2/2] Smart-http tests: Test http-backend without curl or a webserver Tarmigan Casebolt
2009-12-30 17:54 ` [PATCH RFC 1/2] Smart-http tests: Break test t5560-http-backend into pieces Junio C Hamano
2009-12-30 18:09 ` Tarmigan
2009-12-30 18:59 ` Tarmigan Casebolt
2009-12-30 18:59 ` [PATCH RFC 2/2] Smart-http tests: Test http-backend without curl or a webserver Tarmigan Casebolt
2010-01-01 5:15 ` [PATCH RFC 1/2] Smart-http tests: Break test t5560-http-backend into pieces Junio C Hamano
2010-01-02 20:44 ` Tarmigan
2010-01-02 20:45 ` [PATCH v3 RFC 1/3] Smart-http tests: Improve coverage in test t5560 Tarmigan Casebolt
2010-01-02 20:54 ` Shawn O. Pearce
2010-01-02 20:45 ` [PATCH v3 2/3] Smart-http tests: Break test t5560-http-backend into pieces Tarmigan Casebolt
2010-01-02 20:59 ` Shawn O. Pearce
2010-01-02 21:38 ` [PATCH v4 1/3] Smart-http tests: Improve coverage in test t5560 Tarmigan Casebolt
2010-01-02 21:38 ` [PATCH v4 2/3] Smart-http tests: Break test t5560-http-backend into pieces Tarmigan Casebolt
2010-01-02 21:38 ` [PATCH v4 3/3] Smart-http tests: Test http-backend without curl or a webserver Tarmigan Casebolt
2010-01-02 20:45 ` [PATCH v3 " Tarmigan Casebolt
2010-01-02 21:03 ` Shawn O. Pearce
2010-01-02 21:37 ` Tarmigan
2010-01-02 21:41 ` Shawn O. Pearce
2010-01-02 21:43 ` [PATCH v4 " Tarmigan Casebolt
2010-01-14 5:27 ` Michael Haggerty
2010-01-14 7:01 ` [PATCH] Test t5560: Fix test when run with dash Tarmigan Casebolt
2010-01-14 8:23 ` Michael Haggerty
2010-01-14 8:41 ` Junio C Hamano
2010-01-15 6:44 ` [PATCH v2] " Tarmigan Casebolt
2010-01-15 8:30 ` Johannes Sixt
2010-01-15 18:18 ` Junio C Hamano
2010-01-15 19:16 ` Johannes Sixt
2010-01-15 19:53 ` Junio C Hamano
2010-01-16 1:05 ` Junio C Hamano
2010-01-21 16:15 ` Michael Haggerty [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=4B587DAE.9030208@alum.mit.edu \
--to=mhagger@alum.mit.edu \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=j6t@kdbg.org \
--cc=spearce@spearce.org \
--cc=tarmigan+git@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).