Re: [PATCH v2 3/5] t1500: avoid changing working directory outside of tests

3 messages, 3 authors, 2016-06-16 · open the first message on its own page

Re: [PATCH v2 3/5] t1500: avoid changing working directory outside of tests

From: Junio C Hamano <hidden>
Date: 2016-06-16 02:19:26

Jeff King [off-list ref] writes:
On Tue, May 17, 2016 at 04:48:33PM -0400, Eric Sunshine wrote:
quoted
On Tue, May 17, 2016 at 4:37 PM, Junio C Hamano [off-list ref] wrote:
quoted
Eric Sunshine [off-list ref] writes:
quoted
+                     git ${dir:+-C "$dir"} rev-parse --$o >actual &&
This is kosher POSIX, but I vaguely recall some shells had trouble
with the SP between -C and "$dir" in the past.  Let's see if anybody
screams; hopefully I am misremembering or buggy shells died out.
I also am bothered by a vague recollection of some issue (possibly
involving the internal space and lack of quotes around the entire
${...}), but couldn't remember nor find a reference to the specific
details. Perhaps someone reading the patch has a better memory than I.
Probably:

  http://thread.gmane.org/gmane.comp.version-control.git/265094
Yikes, you're right.  Does anybody know if FreeBSD shell is still
buggy?

Re: [PATCH v2 3/5] t1500: avoid changing working directory outside of tests

From: SZEDER Gábor <hidden>
Date: 2016-06-16 02:19:26

Quoting Junio C Hamano [off-list ref]:
Jeff King [off-list ref] writes:
quoted
On Tue, May 17, 2016 at 04:48:33PM -0400, Eric Sunshine wrote:
quoted
On Tue, May 17, 2016 at 4:37 PM, Junio C Hamano [off-list ref] wrote:
quoted
Eric Sunshine [off-list ref] writes:
quoted
+                     git ${dir:+-C "$dir"} rev-parse --$o >actual &&
This is kosher POSIX, but I vaguely recall some shells had trouble
with the SP between -C and "$dir" in the past.  Let's see if anybody
screams; hopefully I am misremembering or buggy shells died out.
I also am bothered by a vague recollection of some issue (possibly
involving the internal space and lack of quotes around the entire
${...}), but couldn't remember nor find a reference to the specific
details. Perhaps someone reading the patch has a better memory than I.
Probably:

 http://thread.gmane.org/gmane.comp.version-control.git/265094
And ea10b60c910e (Work around ash "alternate value" expansion bug,
2009-04-18) as well.

    http://thread.gmane.org/gmane.comp.version-control.git/116816
Yikes, you're right.  Does anybody know if FreeBSD shell is still
buggy?
git-submodule.sh contains a few offending constructs:

	git submodule--helper update-clone ${GIT_QUIET:+--quiet} \
		${wt_prefix:+--prefix "$wt_prefix"} \
		${prefix:+--recursive-prefix "$prefix"} \
		${update:+--update "$update"} \
		${reference:+--reference "$reference"} \
		${depth:+--depth "$depth"} \
		"$@" || echo "#unmatched"
	} | {

They were added recently in 48308681b072 (git submodule update: have
a dedicated helper for cloning, 2016-02-29), which is not in any
release yet, perhaps that's why noone complained yet.

Re: [PATCH v2 3/5] t1500: avoid changing working directory outside of tests

From: Eric Sunshine <hidden>
Date: 2016-06-16 02:19:26

On Tue, May 17, 2016 at 7:06 PM, SZEDER Gábor [off-list ref] wrote:
Quoting Junio C Hamano [off-list ref]:
quoted
Jeff King [off-list ref] writes:
quoted
On Tue, May 17, 2016 at 04:48:33PM -0400, Eric Sunshine wrote:
quoted
On Tue, May 17, 2016 at 4:37 PM, Junio C Hamano [off-list ref]
wrote:
quoted
Eric Sunshine [off-list ref] writes:
quoted
+                     git ${dir:+-C "$dir"} rev-parse --$o >actual &&
This is kosher POSIX, but I vaguely recall some shells had trouble
with the SP between -C and "$dir" in the past.  Let's see if anybody
screams; hopefully I am misremembering or buggy shells died out.
I also am bothered by a vague recollection of some issue (possibly
involving the internal space and lack of quotes around the entire
${...}), but couldn't remember nor find a reference to the specific
details. Perhaps someone reading the patch has a better memory than I.
Probably:
 http://thread.gmane.org/gmane.comp.version-control.git/265094
And ea10b60c910e (Work around ash "alternate value" expansion bug,
2009-04-18) as well.

   http://thread.gmane.org/gmane.comp.version-control.git/116816
Thanks for the additional link. I have v3 ready to roll and will send
it out within the next day if no more actionable review comments
arrive.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help