Remove gnatmake_version_at_least

Message ID 20250109202407.4163030-1-tromey@adacore.com
State New
Headers
Series Remove gnatmake_version_at_least |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_gdb_build--master-aarch64 success Build passed
linaro-tcwg-bot/tcwg_gdb_build--master-arm success Build passed
linaro-tcwg-bot/tcwg_gdb_check--master-aarch64 success Test passed
linaro-tcwg-bot/tcwg_gdb_check--master-arm success Test passed

Commit Message

Tom Tromey Jan. 9, 2025, 8:24 p.m. UTC
  This removes gnatmake_version_at_least in favor of using the more
flexible gnat_version_compare.  I think these two version numbers
should be the same, as gnatmake is shipped with the compiler.
---
 gdb/testsuite/gdb.ada/mi_prot.exp             |  2 +-
 .../gdb.ada/rename_subscript_param.exp        |  2 +-
 gdb/testsuite/gdb.ada/scalar_storage.exp      |  2 +-
 gdb/testsuite/gdb.ada/verylong.exp            |  2 +-
 gdb/testsuite/lib/ada.exp                     | 21 -------------------
 5 files changed, 4 insertions(+), 25 deletions(-)
  

Comments

Tom Tromey Jan. 22, 2025, 7:55 p.m. UTC | #1
>>>>> "Tom" == Tom Tromey <tromey@adacore.com> writes:

Tom> This removes gnatmake_version_at_least in favor of using the more
Tom> flexible gnat_version_compare.  I think these two version numbers
Tom> should be the same, as gnatmake is shipped with the compiler.

I'm checking this in.

Tom
  
Tom de Vries Feb. 26, 2025, 4:08 p.m. UTC | #2
On 1/9/25 21:24, Tom Tromey wrote:
> This removes gnatmake_version_at_least in favor of using the more
> flexible gnat_version_compare.  I think these two version numbers
> should be the same, as gnatmake is shipped with the compiler.

Hi Tom,

starting with this commit, test-case gdb.ada/scalar_storage.exp fails 
again for me on debian testing, for the exact same reason as I described 
in commit 1da36057976 ("[gdb/testsuite] Check gnatmake version in 
gdb.ada/scalar_storage.exp").

I checked the gnat version and gnatmake version, and they're identical 
(13.3.0).

Looking at the implementation of gnat_version_compare, it doesn't look 
at gnat, but gcc, which is 14.2.0.

Is this a problem with the implementation of gnat_version_compare, or am 
I misunderstanding something?

Thanks,
- Tom

> ---
>   gdb/testsuite/gdb.ada/mi_prot.exp             |  2 +-
>   .../gdb.ada/rename_subscript_param.exp        |  2 +-
>   gdb/testsuite/gdb.ada/scalar_storage.exp      |  2 +-
>   gdb/testsuite/gdb.ada/verylong.exp            |  2 +-
>   gdb/testsuite/lib/ada.exp                     | 21 -------------------
>   5 files changed, 4 insertions(+), 25 deletions(-)
> 
> diff --git a/gdb/testsuite/gdb.ada/mi_prot.exp b/gdb/testsuite/gdb.ada/mi_prot.exp
> index 5a3ab10b16c..8525c1fcf32 100644
> --- a/gdb/testsuite/gdb.ada/mi_prot.exp
> +++ b/gdb/testsuite/gdb.ada/mi_prot.exp
> @@ -16,7 +16,7 @@
>   load_lib "ada.exp"
>   
>   require allow_ada_tests
> -require {gnatmake_version_at_least 8}
> +require {gnat_version_compare >= 8}
>   
>   standard_ada_testfile prot
>   
> diff --git a/gdb/testsuite/gdb.ada/rename_subscript_param.exp b/gdb/testsuite/gdb.ada/rename_subscript_param.exp
> index bd49b5b02b6..efb29f40f62 100644
> --- a/gdb/testsuite/gdb.ada/rename_subscript_param.exp
> +++ b/gdb/testsuite/gdb.ada/rename_subscript_param.exp
> @@ -17,7 +17,7 @@ load_lib "ada.exp"
>   
>   require allow_ada_tests
>   
> -require {gnatmake_version_at_least 8}
> +require {gnat_version_compare >= 8}
>   
>   standard_ada_testfile pb30_012
>   
> diff --git a/gdb/testsuite/gdb.ada/scalar_storage.exp b/gdb/testsuite/gdb.ada/scalar_storage.exp
> index 5dccaef8f14..f8fc46974b1 100644
> --- a/gdb/testsuite/gdb.ada/scalar_storage.exp
> +++ b/gdb/testsuite/gdb.ada/scalar_storage.exp
> @@ -36,7 +36,7 @@ if {![runto "storage.adb:$bp_location"]} {
>   gdb_test "print V_LE" "= \\(value => 126, another_value => 12, color => green\\)"
>   
>   # This requires a compiler fix that is in GCC 14.
> -if { ![gnatmake_version_at_least 14] }	{
> +if { ![gnat_version_compare >= 14] }	{
>       setup_kfail "DW_AT_endianity on enum types" *-*-*
>   }
>   gdb_test "print V_BE" "= \\(value => 126, another_value => 12, color => green\\)"
> diff --git a/gdb/testsuite/gdb.ada/verylong.exp b/gdb/testsuite/gdb.ada/verylong.exp
> index 4a3e0b9eeb2..c8574f34e9d 100644
> --- a/gdb/testsuite/gdb.ada/verylong.exp
> +++ b/gdb/testsuite/gdb.ada/verylong.exp
> @@ -16,7 +16,7 @@
>   load_lib "ada.exp"
>   
>   require allow_ada_tests
> -require {gnatmake_version_at_least 11}
> +require {gnat_version_compare >= 11}
>   
>   standard_ada_testfile prog
>   
> diff --git a/gdb/testsuite/lib/ada.exp b/gdb/testsuite/lib/ada.exp
> index 05151d3d8f1..83b419ae210 100644
> --- a/gdb/testsuite/lib/ada.exp
> +++ b/gdb/testsuite/lib/ada.exp
> @@ -167,27 +167,6 @@ proc find_ada_tool {tool} {
>       return $result
>   }
>   
> -# Return 1 if gnatmake is at least version $MAJOR.x.x
> -
> -proc gnatmake_version_at_least { major } {
> -    set gnatmake [find_gnatmake]
> -    set gnatmake [lindex [split $gnatmake] 0]
> -    if {[catch {exec $gnatmake --version} output]} {
> -	return 0
> -    }
> -    if { [regexp {GNATMAKE ([^ .]+).([^ .]+).([^ .]+)} $output \
> -	      match gnatmake_major gnatmake_minor gnatmake_micro] } {
> -	if { $gnatmake_major >= $major } {
> -	    return 1
> -	} else {
> -	    return 0
> -	}
> -    }
> -
> -    # Unknown, return 1
> -    return 1
> -}
> -
>   # Compare the GNAT version against L2 using version_compare.  If the
>   # compiler does not appear to be GCC, this will always return false.
>
  
Tom de Vries March 20, 2025, 10:23 a.m. UTC | #3
On 2/26/25 17:08, Tom de Vries wrote:
> On 1/9/25 21:24, Tom Tromey wrote:
>> This removes gnatmake_version_at_least in favor of using the more
>> flexible gnat_version_compare.  I think these two version numbers
>> should be the same, as gnatmake is shipped with the compiler.
> 
> Hi Tom,
> 
> starting with this commit, test-case gdb.ada/scalar_storage.exp fails 
> again for me on debian testing, for the exact same reason as I described 
> in commit 1da36057976 ("[gdb/testsuite] Check gnatmake version in 
> gdb.ada/scalar_storage.exp").
> 
> I checked the gnat version and gnatmake version, and they're identical 
> (13.3.0).
> 
> Looking at the implementation of gnat_version_compare, it doesn't look 
> at gnat, but gcc, which is 14.2.0.
> 
> Is this a problem with the implementation of gnat_version_compare, or am 
> I misunderstanding something?
> 

Pinging this question.

Thanks,
- Tom

> Thanks,
> - Tom
> 
>> ---
>>   gdb/testsuite/gdb.ada/mi_prot.exp             |  2 +-
>>   .../gdb.ada/rename_subscript_param.exp        |  2 +-
>>   gdb/testsuite/gdb.ada/scalar_storage.exp      |  2 +-
>>   gdb/testsuite/gdb.ada/verylong.exp            |  2 +-
>>   gdb/testsuite/lib/ada.exp                     | 21 -------------------
>>   5 files changed, 4 insertions(+), 25 deletions(-)
>>
>> diff --git a/gdb/testsuite/gdb.ada/mi_prot.exp b/gdb/testsuite/ 
>> gdb.ada/mi_prot.exp
>> index 5a3ab10b16c..8525c1fcf32 100644
>> --- a/gdb/testsuite/gdb.ada/mi_prot.exp
>> +++ b/gdb/testsuite/gdb.ada/mi_prot.exp
>> @@ -16,7 +16,7 @@
>>   load_lib "ada.exp"
>>   require allow_ada_tests
>> -require {gnatmake_version_at_least 8}
>> +require {gnat_version_compare >= 8}
>>   standard_ada_testfile prot
>> diff --git a/gdb/testsuite/gdb.ada/rename_subscript_param.exp b/gdb/ 
>> testsuite/gdb.ada/rename_subscript_param.exp
>> index bd49b5b02b6..efb29f40f62 100644
>> --- a/gdb/testsuite/gdb.ada/rename_subscript_param.exp
>> +++ b/gdb/testsuite/gdb.ada/rename_subscript_param.exp
>> @@ -17,7 +17,7 @@ load_lib "ada.exp"
>>   require allow_ada_tests
>> -require {gnatmake_version_at_least 8}
>> +require {gnat_version_compare >= 8}
>>   standard_ada_testfile pb30_012
>> diff --git a/gdb/testsuite/gdb.ada/scalar_storage.exp b/gdb/testsuite/ 
>> gdb.ada/scalar_storage.exp
>> index 5dccaef8f14..f8fc46974b1 100644
>> --- a/gdb/testsuite/gdb.ada/scalar_storage.exp
>> +++ b/gdb/testsuite/gdb.ada/scalar_storage.exp
>> @@ -36,7 +36,7 @@ if {![runto "storage.adb:$bp_location"]} {
>>   gdb_test "print V_LE" "= \\(value => 126, another_value => 12, color 
>> => green\\)"
>>   # This requires a compiler fix that is in GCC 14.
>> -if { ![gnatmake_version_at_least 14] }    {
>> +if { ![gnat_version_compare >= 14] }    {
>>       setup_kfail "DW_AT_endianity on enum types" *-*-*
>>   }
>>   gdb_test "print V_BE" "= \\(value => 126, another_value => 12, color 
>> => green\\)"
>> diff --git a/gdb/testsuite/gdb.ada/verylong.exp b/gdb/testsuite/ 
>> gdb.ada/verylong.exp
>> index 4a3e0b9eeb2..c8574f34e9d 100644
>> --- a/gdb/testsuite/gdb.ada/verylong.exp
>> +++ b/gdb/testsuite/gdb.ada/verylong.exp
>> @@ -16,7 +16,7 @@
>>   load_lib "ada.exp"
>>   require allow_ada_tests
>> -require {gnatmake_version_at_least 11}
>> +require {gnat_version_compare >= 11}
>>   standard_ada_testfile prog
>> diff --git a/gdb/testsuite/lib/ada.exp b/gdb/testsuite/lib/ada.exp
>> index 05151d3d8f1..83b419ae210 100644
>> --- a/gdb/testsuite/lib/ada.exp
>> +++ b/gdb/testsuite/lib/ada.exp
>> @@ -167,27 +167,6 @@ proc find_ada_tool {tool} {
>>       return $result
>>   }
>> -# Return 1 if gnatmake is at least version $MAJOR.x.x
>> -
>> -proc gnatmake_version_at_least { major } {
>> -    set gnatmake [find_gnatmake]
>> -    set gnatmake [lindex [split $gnatmake] 0]
>> -    if {[catch {exec $gnatmake --version} output]} {
>> -    return 0
>> -    }
>> -    if { [regexp {GNATMAKE ([^ .]+).([^ .]+).([^ .]+)} $output \
>> -          match gnatmake_major gnatmake_minor gnatmake_micro] } {
>> -    if { $gnatmake_major >= $major } {
>> -        return 1
>> -    } else {
>> -        return 0
>> -    }
>> -    }
>> -
>> -    # Unknown, return 1
>> -    return 1
>> -}
>> -
>>   # Compare the GNAT version against L2 using version_compare.  If the
>>   # compiler does not appear to be GCC, this will always return false.
>
  
Andrew Burgess March 20, 2025, 11:33 a.m. UTC | #4
Tom de Vries <tdevries@suse.de> writes:

> On 1/9/25 21:24, Tom Tromey wrote:
>> This removes gnatmake_version_at_least in favor of using the more
>> flexible gnat_version_compare.  I think these two version numbers
>> should be the same, as gnatmake is shipped with the compiler.
>
> Hi Tom,
>
> starting with this commit, test-case gdb.ada/scalar_storage.exp fails 
> again for me on debian testing, for the exact same reason as I described 
> in commit 1da36057976 ("[gdb/testsuite] Check gnatmake version in 
> gdb.ada/scalar_storage.exp").
>
> I checked the gnat version and gnatmake version, and they're identical 
> (13.3.0).
>
> Looking at the implementation of gnat_version_compare, it doesn't look 
> at gnat, but gcc, which is 14.2.0.
>
> Is this a problem with the implementation of gnat_version_compare, or am 
> I misunderstanding something?

I can't speak to Tom's original intent, but looking at the code, it does
seem weird that we check the gcc version when gnat can print its own
version number.

Unless Tom Tromey says not to, I think gnat_version_compare should be
changed to check the gnat version.

Just my $0.02 worth.

Thanks,
Andrew
  
Tom Tromey March 31, 2025, 5:55 p.m. UTC | #5
>>>>> "Tom" == Tom de Vries <tdevries@suse.de> writes:

Tom> Is this a problem with the implementation of gnat_version_compare, or
Tom> am I misunderstanding something?

Sorry about the delay on this.  I forgot about your email and then it
got buried.

I agree this is an oversight.  I'll send a fix momentarily.

Tom
  
Tom de Vries March 31, 2025, 6:56 p.m. UTC | #6
On 3/31/25 19:55, Tom Tromey wrote:
>>>>>> "Tom" == Tom de Vries <tdevries@suse.de> writes:
> 
> Tom> Is this a problem with the implementation of gnat_version_compare, or
> Tom> am I misunderstanding something?
> 
> Sorry about the delay on this.  I forgot about your email and then it
> got buried.
> 

Hi Tom,

thanks for following up on this.

I also saw Andrew's reply and then planned to fix this myself but ... oh 
well.

> I agree this is an oversight.  I'll send a fix momentarily.

I'll take a look.

Thanks,
- Tom
  

Patch

diff --git a/gdb/testsuite/gdb.ada/mi_prot.exp b/gdb/testsuite/gdb.ada/mi_prot.exp
index 5a3ab10b16c..8525c1fcf32 100644
--- a/gdb/testsuite/gdb.ada/mi_prot.exp
+++ b/gdb/testsuite/gdb.ada/mi_prot.exp
@@ -16,7 +16,7 @@ 
 load_lib "ada.exp"
 
 require allow_ada_tests
-require {gnatmake_version_at_least 8}
+require {gnat_version_compare >= 8}
 
 standard_ada_testfile prot
 
diff --git a/gdb/testsuite/gdb.ada/rename_subscript_param.exp b/gdb/testsuite/gdb.ada/rename_subscript_param.exp
index bd49b5b02b6..efb29f40f62 100644
--- a/gdb/testsuite/gdb.ada/rename_subscript_param.exp
+++ b/gdb/testsuite/gdb.ada/rename_subscript_param.exp
@@ -17,7 +17,7 @@  load_lib "ada.exp"
 
 require allow_ada_tests
 
-require {gnatmake_version_at_least 8}
+require {gnat_version_compare >= 8}
 
 standard_ada_testfile pb30_012
 
diff --git a/gdb/testsuite/gdb.ada/scalar_storage.exp b/gdb/testsuite/gdb.ada/scalar_storage.exp
index 5dccaef8f14..f8fc46974b1 100644
--- a/gdb/testsuite/gdb.ada/scalar_storage.exp
+++ b/gdb/testsuite/gdb.ada/scalar_storage.exp
@@ -36,7 +36,7 @@  if {![runto "storage.adb:$bp_location"]} {
 gdb_test "print V_LE" "= \\(value => 126, another_value => 12, color => green\\)"
 
 # This requires a compiler fix that is in GCC 14.
-if { ![gnatmake_version_at_least 14] }	{
+if { ![gnat_version_compare >= 14] }	{
     setup_kfail "DW_AT_endianity on enum types" *-*-*
 }
 gdb_test "print V_BE" "= \\(value => 126, another_value => 12, color => green\\)"
diff --git a/gdb/testsuite/gdb.ada/verylong.exp b/gdb/testsuite/gdb.ada/verylong.exp
index 4a3e0b9eeb2..c8574f34e9d 100644
--- a/gdb/testsuite/gdb.ada/verylong.exp
+++ b/gdb/testsuite/gdb.ada/verylong.exp
@@ -16,7 +16,7 @@ 
 load_lib "ada.exp"
 
 require allow_ada_tests
-require {gnatmake_version_at_least 11}
+require {gnat_version_compare >= 11}
 
 standard_ada_testfile prog
 
diff --git a/gdb/testsuite/lib/ada.exp b/gdb/testsuite/lib/ada.exp
index 05151d3d8f1..83b419ae210 100644
--- a/gdb/testsuite/lib/ada.exp
+++ b/gdb/testsuite/lib/ada.exp
@@ -167,27 +167,6 @@  proc find_ada_tool {tool} {
     return $result
 }
 
-# Return 1 if gnatmake is at least version $MAJOR.x.x
-
-proc gnatmake_version_at_least { major } {
-    set gnatmake [find_gnatmake]
-    set gnatmake [lindex [split $gnatmake] 0]
-    if {[catch {exec $gnatmake --version} output]} {
-	return 0
-    }
-    if { [regexp {GNATMAKE ([^ .]+).([^ .]+).([^ .]+)} $output \
-	      match gnatmake_major gnatmake_minor gnatmake_micro] } {
-	if { $gnatmake_major >= $major } {
-	    return 1
-	} else {
-	    return 0
-	}
-    }
-
-    # Unknown, return 1
-    return 1
-}
-
 # Compare the GNAT version against L2 using version_compare.  If the
 # compiler does not appear to be GCC, this will always return false.