Git Mailing List Archive mirror
 help / color / mirror / Atom feed
* [PATCH] Handle compiler versions containing a dash
@ 2023-04-23  9:12 Mike Hommey
  2023-04-24 21:40 ` Junio C Hamano
  0 siblings, 1 reply; 3+ messages in thread
From: Mike Hommey @ 2023-04-23  9:12 UTC (permalink / raw)
  To: git; +Cc: gitster, Mike Hommey

The version reported by e.g. x86_64-w64-mingw32-gcc on Debian bullseye
looks like:
  gcc version 10-win32 20210110 (GCC)

This ends up with detect-compiler failing with:
  ./detect-compiler: 30: test: Illegal number: 10-win32

This change removes the -win32 part by excluding the dash and everything
that follows from the version.
---
 detect-compiler | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/detect-compiler b/detect-compiler
index 50087f5670..d961df5fb5 100755
--- a/detect-compiler
+++ b/detect-compiler
@@ -17,7 +17,7 @@ get_family() {
 }
 
 get_version() {
-	get_version_line | sed 's/^.* version \([0-9][^ ]*\).*/\1/'
+	get_version_line | sed 's/^.* version \([0-9][^ -]*\).*/\1/'
 }
 
 print_flags() {
-- 
2.40.0.1.gc689dad23e


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] Handle compiler versions containing a dash
  2023-04-23  9:12 [PATCH] Handle compiler versions containing a dash Mike Hommey
@ 2023-04-24 21:40 ` Junio C Hamano
  2023-04-26  0:48   ` [PATCH] Handle some " Mike Hommey
  0 siblings, 1 reply; 3+ messages in thread
From: Junio C Hamano @ 2023-04-24 21:40 UTC (permalink / raw)
  To: Mike Hommey; +Cc: git

Mike Hommey <mh@glandium.org> writes:

> The version reported by e.g. x86_64-w64-mingw32-gcc on Debian bullseye
> looks like:
>   gcc version 10-win32 20210110 (GCC)
>
> This ends up with detect-compiler failing with:
>   ./detect-compiler: 30: test: Illegal number: 10-win32
>
> This change removes the -win32 part by excluding the dash and everything
> that follows from the version.

This may help the "test "$version" -gt 0" check and $((version - 1))
to pass.  It is not quite clear if/why it gives sensible results,
though.

> ---
>  detect-compiler | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Missing sign-off?

> diff --git a/detect-compiler b/detect-compiler
> index 50087f5670..d961df5fb5 100755
> --- a/detect-compiler
> +++ b/detect-compiler
> @@ -17,7 +17,7 @@ get_family() {
>  }
>  
>  get_version() {
> -	get_version_line | sed 's/^.* version \([0-9][^ ]*\).*/\1/'
> +	get_version_line | sed 's/^.* version \([0-9][^ -]*\).*/\1/'

The original is bad enough in that it says "We take anything that
begins with a digit up to (but not including) the first SP, and then
it assumes that it is getting an integer.  This one is not all that
better in that it can still accept a garbage like "version 01xx-foo"
and "test "$version" -gt 0" would fail the same way, no?

If we are sure that "version N-win32" is always equivalent to
"version N" for the purpose of print_flags() helper function, it may
be more prudent to allow the known-good ones, with something like

	# A string that begins with a digit up to the next SP
	ver=$(get_version_line | sed 's/^.* version \([0-9][^ ]*\).*/\1/')

	# There are known -variant suffixes that do not affect the
	# meaning of the main version number.  Strip them.
	ver=${ver%-win32}
	ver=${ver%-win64}
	...
	echo "$ver"

while keeping the ones that are not "known-to-be-good" as-is.

That way, non-numeric numbers that we do not know about how to
interpret will continue to stop detect-compiler from giving a bogus
answer, which would be better than silently accepting anything with
dash and treat as if the string after the number does not make any
difference.

One thing I am worried about is "10-prerelease" that is not quite
"10" yet to be treated just like "10" and causing problems.

Thanks.



^ permalink raw reply	[flat|nested] 3+ messages in thread

* [PATCH] Handle some compiler versions containing a dash
  2023-04-24 21:40 ` Junio C Hamano
@ 2023-04-26  0:48   ` Mike Hommey
  0 siblings, 0 replies; 3+ messages in thread
From: Mike Hommey @ 2023-04-26  0:48 UTC (permalink / raw)
  To: git; +Cc: gitster, Mike Hommey

The version reported by e.g. x86_64-w64-mingw32-gcc on Debian bullseye
looks like:
  gcc version 10-win32 20210110 (GCC)

This ends up with detect-compiler failing with:
  ./detect-compiler: 30: test: Illegal number: 10-win32

This change removes the two known suffixes known to exist in GCC versions
in Debian: -win32 and -posix.

Signed-off-by: Mike Hommey <mh@glandium.org>
---
 detect-compiler | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/detect-compiler b/detect-compiler
index 50087f5670..a87650b71b 100755
--- a/detect-compiler
+++ b/detect-compiler
@@ -17,7 +17,15 @@ get_family() {
 }
 
 get_version() {
-	get_version_line | sed 's/^.* version \([0-9][^ ]*\).*/\1/'
+	# A string that begins with a digit up to the next SP
+	ver=$(get_version_line | sed 's/^.* version \([0-9][^ ]*\).*/\1/')
+
+	# There are known -variant suffixes that do not affect the
+	# meaning of the main version number.  Strip them.
+	ver=${ver%-win32}
+	ver=${ver%-posix}
+
+	echo "$ver"
 }
 
 print_flags() {
-- 
2.40.0.1.gc689dad23e


^ permalink raw reply related	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2023-04-26  0:49 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-04-23  9:12 [PATCH] Handle compiler versions containing a dash Mike Hommey
2023-04-24 21:40 ` Junio C Hamano
2023-04-26  0:48   ` [PATCH] Handle some " Mike Hommey

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 read-only IMAP folder(s) and NNTP newsgroup(s).