Re: [PATCH 1/2] subtree: fix the GIT_EXEC_PATH sanity check to work on Windows

5 messages, 4 authors, 2021-06-15 · open the first message on its own page

Re: [PATCH 1/2] subtree: fix the GIT_EXEC_PATH sanity check to work on Windows

From: Junio C Hamano <hidden>
Date: 2021-06-15 02:34:04

Johannes Schindelin [off-list ref] writes:
quoted
`test FILE1 -ef FILE2` checks wether the inode is the same.  And it's
POSIX, so I'm assuming that it's sufficiently portable, though I
haven't actually tested whether things other than Bash implement it.
It's not POSIX. From
https://pubs.opengroup.org/onlinepubs/009695399/utilities/test.html:

	Some additional primaries newly invented or from the KornShell
	appeared in an early proposal as part of the conditional command
	([[]]): s1 > s2, s1 < s2, str = pattern, str != pattern,
	f1 -nt f2, f1 -ot f2, and f1 -ef f2.

Having said that, it appears that Bash implements it (what non-standard
behavior _doesn't_ it implement ;-))

And since Git for Windows ships with Bash, we can actually use it!
So,... is contrib/subtree for Windows only?

Re: [PATCH 1/2] subtree: fix the GIT_EXEC_PATH sanity check to work on Windows

From: Jeff King <hidden>
Date: 2021-06-15 10:56:34

On Tue, Jun 15, 2021 at 11:33:58AM +0900, Junio C Hamano wrote:
Johannes Schindelin [off-list ref] writes:
quoted
quoted
`test FILE1 -ef FILE2` checks wether the inode is the same.  And it's
POSIX, so I'm assuming that it's sufficiently portable, though I
haven't actually tested whether things other than Bash implement it.
It's not POSIX. From
https://pubs.opengroup.org/onlinepubs/009695399/utilities/test.html:

	Some additional primaries newly invented or from the KornShell
	appeared in an early proposal as part of the conditional command
	([[]]): s1 > s2, s1 < s2, str = pattern, str != pattern,
	f1 -nt f2, f1 -ot f2, and f1 -ef f2.

Having said that, it appears that Bash implements it (what non-standard
behavior _doesn't_ it implement ;-))

And since Git for Windows ships with Bash, we can actually use it!
So,... is contrib/subtree for Windows only?
I read it as "this workaround is needed only on Windows, and will kick
in only there; on other platforms, the "-ef" code will not run at all,
so we don't have to worry about its portability".

But having seen the earlier part of the thread, it looks like "are we on
Windows" is predicated on "! type -p cygpath", which seems a bit loose.
I also think "-p" is a bash-ism, so we'd want to avoid it before
determining whether we're on Windows to avoid a chicken-and-egg on other
platforms.

-Peff

Re: [PATCH 1/2] subtree: fix the GIT_EXEC_PATH sanity check to work on Windows

From: Bagas Sanjaya <hidden>
Date: 2021-06-15 11:05:20

Hi Jeff,
But having seen the earlier part of the thread, it looks like "are we on
Windows" is predicated on "! type -p cygpath", which seems a bit loose.
I also think "-p" is a bash-ism, so we'd want to avoid it before
determining whether we're on Windows to avoid a chicken-and-egg on other
platforms.

-Peff
What is the POSIX equivalent then of?

-- 
An old man doll... just what I always wanted! - Clara

Re: [PATCH 1/2] subtree: fix the GIT_EXEC_PATH sanity check to work on Windows

From: Jeff King <hidden>
Date: 2021-06-15 11:18:03

On Tue, Jun 15, 2021 at 06:05:08PM +0700, Bagas Sanjaya wrote:
Hi Jeff,
quoted
But having seen the earlier part of the thread, it looks like "are we on
Windows" is predicated on "! type -p cygpath", which seems a bit loose.
I also think "-p" is a bash-ism, so we'd want to avoid it before
determining whether we're on Windows to avoid a chicken-and-egg on other
platforms.

-Peff
What is the POSIX equivalent then of?
I don't think there is an equivalent for "-p". But regular "type" is
probably sufficient for this use (the "-p" is just suppressing aliases
and functions).

It would be nice if there was a more robust test in general, though
(after all, I could have something called "cygpath" on a non-Windows
system). I don't know what options there are to get info from bash,
though.

(I'd also clarify that I haven't been carefully following this thread,
so take any suggestion or comments from me with a grain of salt. I
mostly jumped in because it looked like there was a communication
confusion).

-Peff

Re: [PATCH 1/2] subtree: fix the GIT_EXEC_PATH sanity check to work on Windows

From: Johannes Schindelin <hidden>
Date: 2021-06-15 11:27:28

Hi,

On Tue, 15 Jun 2021, Jeff King wrote:
On Tue, Jun 15, 2021 at 06:05:08PM +0700, Bagas Sanjaya wrote:
quoted
quoted
But having seen the earlier part of the thread, it looks like "are we on
Windows" is predicated on "! type -p cygpath", which seems a bit loose.
I also think "-p" is a bash-ism, so we'd want to avoid it before
determining whether we're on Windows to avoid a chicken-and-egg on other
platforms.

-Peff
What is the POSIX equivalent then of?
I don't think there is an equivalent for "-p". But regular "type" is
probably sufficient for this use (the "-p" is just suppressing aliases
and functions).

It would be nice if there was a more robust test in general, though
(after all, I could have something called "cygpath" on a non-Windows
system). I don't know what options there are to get info from bash,
though.

(I'd also clarify that I haven't been carefully following this thread,
so take any suggestion or comments from me with a grain of salt. I
mostly jumped in because it looked like there was a communication
confusion).
Please don't worry about `type` vs `type -p` here, as that is no longer
used in v2.

Please also don't worry about perceived brittleness of relying on `-ef`:
The only thing my patch now does is to _fall back_ after the
previously-already-existing test verifying that `$PATH` starts with
`$GIT_EXEC_PREFIX:`.

In the worst case, this will simply behave the same. In the best case
(which is the case on Windows), it will rely on Git for Windows' Bash to
_have_ support for `-ef` and no longer do the wrong thing.

Ciao,
Dscho
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help