public inbox for libc-alpha@sourceware.org
 help / color / mirror / Atom feed
* [PATCH] build-many-glibcs: relax version check to allow non-digit characters
@ 2024-01-31  4:02 Fangrui Song
  2024-01-31 21:42 ` DJ Delorie
  0 siblings, 1 reply; 2+ messages in thread
From: Fangrui Song @ 2024-01-31  4:02 UTC (permalink / raw)
  To: DJ Delorie, Lukasz Majewski, Adhemerval Zanella Netto
  Cc: libc-alpha, Fangrui Song

A version string may contain non-digit characters, commonly found in
built-from-VCS tools, e.g.
```
git version 2.39.GIT
git version 2.43.0.493.gbc7ee2e5e1
```

`int()` will raise a ValueError, leading to a spurious 'missing'.
---
 scripts/build-many-glibcs.py | 16 +++++++---------
 1 file changed, 7 insertions(+), 9 deletions(-)

diff --git a/scripts/build-many-glibcs.py b/scripts/build-many-glibcs.py
index 784c80d132..84418e8de1 100755
--- a/scripts/build-many-glibcs.py
+++ b/scripts/build-many-glibcs.py
@@ -1888,7 +1888,7 @@ def get_parser():
     return parser
 
 
-def get_version_common(progname,line,word,delchars,arg1):
+def get_version_common(progname,line,word,arg1):
     try:
         out = subprocess.run([progname, arg1],
                              stdout=subprocess.PIPE,
@@ -1896,13 +1896,12 @@ def get_version_common(progname,line,word,delchars,arg1):
                              stdin=subprocess.DEVNULL,
                              check=True, universal_newlines=True)
         v = out.stdout.splitlines()[line].split()[word]
-        if delchars:
-            v = v.replace(delchars,'')
+        v = re.match(r'[0-9]+(.[0-9]+)*', v).group()
         return [int(x) for x in v.split('.')]
     except:
         return 'missing';
 
-def get_version_common_stderr(progname,line,word,delchars,arg1):
+def get_version_common_stderr(progname,line,word,arg1):
     try:
         out = subprocess.run([progname, arg1],
                              stdout=subprocess.DEVNULL,
@@ -1910,20 +1909,19 @@ def get_version_common_stderr(progname,line,word,delchars,arg1):
                              stdin=subprocess.DEVNULL,
                              check=True, universal_newlines=True)
         v = out.stderr.splitlines()[line].split()[word]
-        if delchars:
-            v = v.replace(delchars,'')
+        v = re.match(r'[0-9]+(.[0-9]+)*', v).group()
         return [int(x) for x in v.split('.')]
     except:
         return 'missing';
 
 def get_version(progname):
-    return get_version_common (progname, 0, -1, None, '--version');
+    return get_version_common(progname, 0, -1, '--version');
 
 def get_version_awk(progname):
-    return get_version_common (progname, 0, 2, ',', '--version');
+    return get_version_common(progname, 0, 2, '--version');
 
 def get_version_bzip2(progname):
-    return get_version_common_stderr (progname, 0, 6, ',', '-h');
+    return get_version_common_stderr(progname, 0, 6, '-h');
 
 def check_version(ver, req):
     for v, r in zip(ver, req):
-- 
2.43.0.429.g432eaa2c6b-goog


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

* Re: [PATCH] build-many-glibcs: relax version check to allow non-digit characters
  2024-01-31  4:02 [PATCH] build-many-glibcs: relax version check to allow non-digit characters Fangrui Song
@ 2024-01-31 21:42 ` DJ Delorie
  0 siblings, 0 replies; 2+ messages in thread
From: DJ Delorie @ 2024-01-31 21:42 UTC (permalink / raw)
  To: Fangrui Song; +Cc: lukma, adhemerval.zanella, libc-alpha, maskray

Fangrui Song <maskray@google.com> writes:
> A version string may contain non-digit characters, commonly found in
> built-from-VCS tools, e.g.

And we assume that these extra characters are not part of the
comparable, hopefully monotonically-increasing, portion of the version
number ;-)

> -def get_version_common(progname,line,word,delchars,arg1):
> +def get_version_common(progname,line,word,arg1):

Ok.

>          v = out.stdout.splitlines()[line].split()[word]
> -        if delchars:
> -            v = v.replace(delchars,'')
> +        v = re.match(r'[0-9]+(.[0-9]+)*', v).group()
>          return [int(x) for x in v.split('.')]

I confirmed that this will include the first number despite it not being
in ()'s.  Ok.

> -def get_version_common_stderr(progname,line,word,delchars,arg1):
> +def get_version_common_stderr(progname,line,word,arg1):

Ok.

> -        if delchars:
> -            v = v.replace(delchars,'')
> +        v = re.match(r'[0-9]+(.[0-9]+)*', v).group()

Ok.

>  def get_version(progname):
> -    return get_version_common (progname, 0, -1, None, '--version');
> +    return get_version_common(progname, 0, -1, '--version');
>  
>  def get_version_awk(progname):
> -    return get_version_common (progname, 0, 2, ',', '--version');
> +    return get_version_common(progname, 0, 2, '--version');
>  
>  def get_version_bzip2(progname):
> -    return get_version_common_stderr (progname, 0, 6, ',', '-h');
> +    return get_version_common_stderr(progname, 0, 6, '-h');

Ok.

LGTM.

Reviewed-by: DJ Delorie <dj@redhat.com>


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

end of thread, other threads:[~2024-01-31 21:42 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-01-31  4:02 [PATCH] build-many-glibcs: relax version check to allow non-digit characters Fangrui Song
2024-01-31 21:42 ` DJ Delorie

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).