ci: Check for necessary Debian packages when running build-many-glibcs.py

Message ID 20211108165255.15600-1-lukma@denx.de
State Accepted, archived
Delegated to: DJ Delorie
Headers
Series ci: Check for necessary Debian packages when running build-many-glibcs.py |

Checks

Context Check Description
dj/TryBot-apply_patch success Patch applied to master at the time it was sent
dj/TryBot-32bit success Build for i686

Commit Message

Lukasz Majewski Nov. 8, 2021, 4:52 p.m. UTC
  The build-many-glibc.py can be run on a 'vanila' Debian distribution
(as for example in docker container), which don't have by default
installed some packages (like flex).

This causes build break at late stage of the full build;
../src/scripts/build-many-glibcs.py . checkout --replace-sources &&
../src/scripts/build-many-glibcs.py . host-libraries &&
../src/scripts/build-many-glibcs.py . compilers &&
../src/scripts/build-many-glibcs.py . glibcs

To avoid such situation, this check has been added to inform user
early of required (and missing) Debian packages.

The same approach (with using the 'distro' python module) can be
applied to Fedora or Suse.

Signed-off-by: Lukasz Majewski <lukma@denx.de>
---
 scripts/build-many-glibcs.py | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)
  

Comments

Joseph Myers Nov. 8, 2021, 4:59 p.m. UTC | #1
On Mon, 8 Nov 2021, Lukasz Majewski wrote:

> The same approach (with using the 'distro' python module) can be
> applied to Fedora or Suse.

That module isn't part of the Python standard library.  I don't think we 
should introduce a dependency on it; rather, any use of it should be 
appropriately conditional, so the code still runs (without these checks) 
if the module is unavailable (importing produces an ImportError).

In particular, even if the OS Python installation includes that module, 
the script should work with a separately built copy of Python without any 
such modules from the OS.

> +def check_os_requirements():
> +    if distro.id() == "debian" and distro.version() == "10":
> +        # List 'Debian' specific packages requirements (different than
> +        # vanila distro) to run this test without errors.
> +        debian_requirements = ['flex', 'bison', 'dnsutils', 'texinfo']

Why is dnsutils needed?
  
Adhemerval Zanella Netto Nov. 8, 2021, 5:06 p.m. UTC | #2
On 08/11/2021 13:59, Joseph Myers wrote:
> On Mon, 8 Nov 2021, Lukasz Majewski wrote:
> 
>> The same approach (with using the 'distro' python module) can be
>> applied to Fedora or Suse.
> 
> That module isn't part of the Python standard library.  I don't think we 
> should introduce a dependency on it; rather, any use of it should be 
> appropriately conditional, so the code still runs (without these checks) 
> if the module is unavailable (importing produces an ImportError).
> 
> In particular, even if the OS Python installation includes that module, 
> the script should work with a separately built copy of Python without any 
> such modules from the OS.

Maybe add a check without tying to any distribution (tool -v and some version
parsing).

> 
>> +def check_os_requirements():
>> +    if distro.id() == "debian" and distro.version() == "10":
>> +        # List 'Debian' specific packages requirements (different than
>> +        # vanila distro) to run this test without errors.
>> +        debian_requirements = ['flex', 'bison', 'dnsutils', 'texinfo']
> 
> Why is dnsutils needed?

Also, strictly to build and check 'texinfo' is not required either.
  
Florian Weimer Nov. 8, 2021, 5:15 p.m. UTC | #3
* Adhemerval Zanella:

>>> +def check_os_requirements():
>>> +    if distro.id() == "debian" and distro.version() == "10":
>>> +        # List 'Debian' specific packages requirements (different than
>>> +        # vanila distro) to run this test without errors.
>>> +        debian_requirements = ['flex', 'bison', 'dnsutils', 'texinfo']
>> 
>> Why is dnsutils needed?
>
> Also, strictly to build and check 'texinfo' is not required either.

I think binutils cannot be built without makeinfo.  At least for
somewhat older branches.  If it's a problem, I should finally work out a
fix for it.  The makeinfo builds part of the cross-toolchain are fairly
costly, too.

Thanks,
Florian
  
Florian Weimer Nov. 8, 2021, 5:17 p.m. UTC | #4
* Lukasz Majewski:

> The build-many-glibc.py can be run on a 'vanila' Debian distribution
> (as for example in docker container), which don't have by default
> installed some packages (like flex).
>
> This causes build break at late stage of the full build;
> ../src/scripts/build-many-glibcs.py . checkout --replace-sources &&
> ../src/scripts/build-many-glibcs.py . host-libraries &&
> ../src/scripts/build-many-glibcs.py . compilers &&
> ../src/scripts/build-many-glibcs.py . glibcs
>
> To avoid such situation, this check has been added to inform user
> early of required (and missing) Debian packages.
>
> The same approach (with using the 'distro' python module) can be
> applied to Fedora or Suse.

I would recommend searching for required tools on PATH, using
shutil.which.  This should work on any distribution.  If I recall
correctly, the non-obvious tools are:

bison
flex
git
makeinfo
patch
tar

Thanks,
Florian
  
Lukasz Majewski Nov. 8, 2021, 6:15 p.m. UTC | #5
Hi Joseph,

> On Mon, 8 Nov 2021, Lukasz Majewski wrote:
> 
> > The same approach (with using the 'distro' python module) can be
> > applied to Fedora or Suse.  
> 
> That module isn't part of the Python standard library.  I don't think
> we should introduce a dependency on it; rather, any use of it should
> be appropriately conditional, so the code still runs (without these
> checks) if the module is unavailable (importing produces an
> ImportError).

The 'distro' module is the most elegant (and pythonic) way to get the
info about running OS.
And it is by default available on Debian as part of 
/usr/lib/python3/dist-packages

> 
> In particular, even if the OS Python installation includes that
> module, the script should work with a separately built copy of Python
> without any such modules from the OS.

As I've said before - it looks like the most elegant approach...

> 
> > +def check_os_requirements():
> > +    if distro.id() == "debian" and distro.version() == "10":
> > +        # List 'Debian' specific packages requirements (different
> > than
> > +        # vanila distro) to run this test without errors.
> > +        debian_requirements = ['flex', 'bison', 'dnsutils',
> > 'texinfo']  
> 
> Why is dnsutils needed?
> 

configure.ac:83: installing 'build-aux/compile'
configure.ac:46: installing 'build-aux/config.guess'
configure.ac:46: installing 'build-aux/config.sub'
configure.ac:26: installing 'build-aux/install-sh'
configure.ac:26: installing 'build-aux/missing'
Makefile.am: installing './INSTALL'
Makefile.am: installing 'build-aux/depcomp'
Makefile.am:32: installing 'build-aux/mdate-sh'
doc/Makefrag.am:106: warning: user target '$(srcdir)/doc/version.texi'
defined here ... Makefile.am:155:   'doc/Makefrag.am' included from here
/usr/share/automake-1.16/am/texi-vers.am: ... overrides Automake target
'$(srcdir)/doc/version.texi' defined here Makefile.am:32: installing
'build-aux/texinfo.tex' parallel-tests: installing
'build-aux/test-driver' Traceback (most recent call last):
  File "/usr/lib/python3.7/urllib/request.py", line 1324, in do_open
    encode_chunked=req.has_header('Transfer-encoding'))
  File "/usr/lib/python3.7/http/client.py", line 1260, in request
    self._send_request(method, url, body, headers, encode_chunked)
  File "/usr/lib/python3.7/http/client.py", line 1306, in _send_request
    self.endheaders(body, encode_chunked=encode_chunked)
  File "/usr/lib/python3.7/http/client.py", line 1255, in endheaders
    self._send_output(message_body, encode_chunked=encode_chunked)
  File "/usr/lib/python3.7/http/client.py", line 1030, in _send_output
    self.send(msg)
  File "/usr/lib/python3.7/http/client.py", line 970, in send
    self.connect()
  File "/usr/lib/python3.7/http/client.py", line 1415, in connect
    super().connect()
  File "/usr/lib/python3.7/http/client.py", line 942, in connect
    (self.host,self.port), self.timeout, self.source_address)
  File "/usr/lib/python3.7/socket.py", line 707, in create_connection
    for res in getaddrinfo(host, port, 0, SOCK_STREAM):
  File "/usr/lib/python3.7/socket.py", line 748, in getaddrinfo
    for res in _socket.getaddrinfo(host, port, family, type, proto,
flags): socket.gaierror: [Errno -2] Name or service not known



Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma@denx.de
  
Joseph Myers Nov. 8, 2021, 6:31 p.m. UTC | #6
On Mon, 8 Nov 2021, Lukasz Majewski wrote:

> The 'distro' module is the most elegant (and pythonic) way to get the
> info about running OS.

Florian's suggestion of checking for specific programs, so staying 
completely independent of the running OS and its version and of whether 
those programs are provided by the OS or not, seems much better to me.

> > Why is dnsutils needed?

>   File "/usr/lib/python3.7/socket.py", line 748, in getaddrinfo
>     for res in _socket.getaddrinfo(host, port, family, type, proto,
> flags): socket.gaierror: [Errno -2] Name or service not known

That doesn't answer my question, since getaddrinfo (implemented using the 
C library function of that name, presumably) shouldn't be using any of the 
programs in dnsutils "Transitional package for bind9-dnsutils".  "Can do 
working DNS client lookups from library code" is not the same as "has 
certain executables installed that a user might use from the command line 
to look things up in the DNS".

If you want to check for working DNS lookups / network access from library 
code, then trying to look up www.gnu.org seems a better approach than 
checking for any installed programs.
  
Joseph Myers Nov. 8, 2021, 6:52 p.m. UTC | #7
On Mon, 8 Nov 2021, Florian Weimer via Libc-alpha wrote:

> I would recommend searching for required tools on PATH, using
> shutil.which.  This should work on any distribution.  If I recall
> correctly, the non-obvious tools are:
> 
> bison
> flex
> git
> makeinfo
> patch
> tar

What needs patch?  I agree the rest of the tools you list are needed by 
the checkout or build process.
  
Adhemerval Zanella Netto Nov. 8, 2021, 6:56 p.m. UTC | #8
On 08/11/2021 14:17, Florian Weimer wrote:
> * Lukasz Majewski:
> 
>> The build-many-glibc.py can be run on a 'vanila' Debian distribution
>> (as for example in docker container), which don't have by default
>> installed some packages (like flex).
>>
>> This causes build break at late stage of the full build;
>> ../src/scripts/build-many-glibcs.py . checkout --replace-sources &&
>> ../src/scripts/build-many-glibcs.py . host-libraries &&
>> ../src/scripts/build-many-glibcs.py . compilers &&
>> ../src/scripts/build-many-glibcs.py . glibcs
>>
>> To avoid such situation, this check has been added to inform user
>> early of required (and missing) Debian packages.
>>
>> The same approach (with using the 'distro' python module) can be
>> applied to Fedora or Suse.
> 
> I would recommend searching for required tools on PATH, using
> shutil.which.  This should work on any distribution.  If I recall
> correctly, the non-obvious tools are:
> 
> bison
> flex
> git
> makeinfo
> patch
> tar

I think we might need gunzip, bunzip2, and unxz as well.
  
Florian Weimer Nov. 8, 2021, 7:03 p.m. UTC | #9
* Joseph Myers:

> On Mon, 8 Nov 2021, Florian Weimer via Libc-alpha wrote:
>
>> I would recommend searching for required tools on PATH, using
>> shutil.which.  This should work on any distribution.  If I recall
>> correctly, the non-obvious tools are:
>> 
>> bison
>> flex
>> git
>> makeinfo
>> patch
>> tar
>
> What needs patch?

gnumach, see its configure.ac towards the end (“The remaining ugly, dark
corners”).  It patches the just-generated config.status file.

Thanks,
Florian
  
Lukasz Majewski Nov. 8, 2021, 7:44 p.m. UTC | #10
Hi Adhemerval,

> On 08/11/2021 13:59, Joseph Myers wrote:
> > On Mon, 8 Nov 2021, Lukasz Majewski wrote:
> >   
> >> The same approach (with using the 'distro' python module) can be
> >> applied to Fedora or Suse.  
> > 
> > That module isn't part of the Python standard library.  I don't
> > think we should introduce a dependency on it; rather, any use of it
> > should be appropriately conditional, so the code still runs
> > (without these checks) if the module is unavailable (importing
> > produces an ImportError).
> > 
> > In particular, even if the OS Python installation includes that
> > module, the script should work with a separately built copy of
> > Python without any such modules from the OS.  
> 
> Maybe add a check without tying to any distribution (tool -v and some
> version parsing).

This would require some extra, work but then we would avoid 'distro'
module as the dependency.

> 
> >   
> >> +def check_os_requirements():
> >> +    if distro.id() == "debian" and distro.version() == "10":
> >> +        # List 'Debian' specific packages requirements (different
> >> than
> >> +        # vanila distro) to run this test without errors.
> >> +        debian_requirements = ['flex', 'bison', 'dnsutils',
> >> 'texinfo']  
> > 
> > Why is dnsutils needed?  
> 
> Also, strictly to build and check 'texinfo' is not required either.

if makeinfo --split-size=5000000 --split-size=5000000 -I
"/work/wd/glibc/glibc-many-build/src/binutils/binutils/doc" -I
"/work/wd/glibc/glibc-many-build/src/binutils/binutils/../libiberty" -I
"/work/wd/glibc/glibc-many-build/src/binutils/binutils/../bfd/doc" -I
../../bfd/doc --no-split  -I
/work/wd/glibc/glibc-many-build/src/binutils/binutils/doc \ -o
binutils.info `test -f 'binutils.texi' || echo
'/work/wd/glibc/glibc-many-build/src/binutils/binutils/doc/'`binutils.texi;
\ then \ rc=0; \ else \ rc=$?; \ $restore $backupdir/* `echo
"./binutils.info" | sed 's|[^/]*$||'`; \ fi; \ rm -rf $backupdir; exit
$rc
/work/wd/glibc/glibc-many-build/src/binutils/binutils/doc/binutils.texi:871:
warning: @ref node name should not contain `.'
/work/wd/glibc/glibc-many-build/src/binutils/binutils/doc/binutils.texi:1329:
warning: @xref node name should not contain `.' make[4]: Leaving
directory
'/work/wd/glibc/glibc-many-build/build/compilers/powerpc64-linux-gnu/binutils/binutils/doc'
Making info in po make[4]: Nothing to be done for 'info'. make[4]:
Nothing to be done for 'info-am'. make[2]: *** [Makefile:3701:
all-binutils] Error 2 make[2]: *** Waiting for unfinished jobs.... 


Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma@denx.de
  
Lukasz Majewski Nov. 8, 2021, 7:51 p.m. UTC | #11
Hi Joseph,

> On Mon, 8 Nov 2021, Lukasz Majewski wrote:
> 
> > The 'distro' module is the most elegant (and pythonic) way to get
> > the info about running OS.  
> 
> Florian's suggestion of checking for specific programs, so staying 
> completely independent of the running OS and its version and of
> whether those programs are provided by the OS or not, seems much
> better to me.

Ok.

> 
> > > Why is dnsutils needed?  
> 
> >   File "/usr/lib/python3.7/socket.py", line 748, in getaddrinfo
> >     for res in _socket.getaddrinfo(host, port, family, type, proto,
> > flags): socket.gaierror: [Errno -2] Name or service not known  
> 
> That doesn't answer my question, since getaddrinfo (implemented using
> the C library function of that name, presumably) shouldn't be using
> any of the programs in dnsutils "Transitional package for
> bind9-dnsutils".  "Can do working DNS client lookups from library
> code" is not the same as "has certain executables installed that a
> user might use from the command line to look things up in the DNS".
> 
> If you want to check for working DNS lookups / network access from
> library code, then trying to look up www.gnu.org seems a better
> approach than checking for any installed programs.
> 

Sorry, but I cannot follow your question/explanation above.

The issue is that - without having dnsutils, when trying to run
build-many-glibcs.py the pasted error shows up.

After installing dnsutils (on debian 10) the problem on my setup/machine
is fixed.


Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma@denx.de
  
Adhemerval Zanella Netto Nov. 8, 2021, 8:27 p.m. UTC | #12
On 08/11/2021 16:44, Lukasz Majewski wrote:
> Hi Adhemerval,
> 
>> On 08/11/2021 13:59, Joseph Myers wrote:
>>> On Mon, 8 Nov 2021, Lukasz Majewski wrote:
>>>   
>>>> The same approach (with using the 'distro' python module) can be
>>>> applied to Fedora or Suse.  
>>>
>>> That module isn't part of the Python standard library.  I don't
>>> think we should introduce a dependency on it; rather, any use of it
>>> should be appropriately conditional, so the code still runs
>>> (without these checks) if the module is unavailable (importing
>>> produces an ImportError).
>>>
>>> In particular, even if the OS Python installation includes that
>>> module, the script should work with a separately built copy of
>>> Python without any such modules from the OS.  
>>
>> Maybe add a check without tying to any distribution (tool -v and some
>> version parsing).
> 
> This would require some extra, work but then we would avoid 'distro'
> module as the dependency.

I think the work required is that hard, something like:

--
import shutil
import subprocess

def get_version(progname):
    out = subprocess.run([progname, '--version'], stdout=subprocess.PIPE,
                         check=True, universal_newlines=True).stdout
    return [int(x) for x in out.splitlines()[0].split()[-1].split('.')]

def get_version_awk(progname):
    out = subprocess.run([progname, '--version'], stdout=subprocess.PIPE,
                         check=True, universal_newlines=True).stdout
    version = out.splitlines()[0].split()[2].replace(',','').split('.')
    return [int(x) for x in version]

def check_version(ver, req):
    for v, r in zip(ver, req):
        if v >= r:
            return True
    return False

def version_str(ver):
    return '.'.join([str (x) for x in version])


TOOLS={ 'make'     : (get_version,     (4,0)),
        'makeinfo' : (get_version,     (4,7)),
        'awk'      : (get_version_awk, (3,1,2)),
        'bison'    : (get_version,     (2,7)),
        'sed'      : (get_version,     (3,2)),
        'flex'     : (get_version,     (2,6,0)),
        'git'      : (get_version,     (2,32)),
        'patch'    : (get_version,     (2,7,0)),
        'tar'      : (get_version,     (1,3,4))}

for k, v in TOOLS.items():
    version = v[0](k)
    ok = 'ok' if check_version (version, v[1]) else 'old'
    print('{:9}: {:3} (obtained=\"{}\" required=\"{}\")'.format(k, ok,
        version_str(version), version_str(v[1])))
--

> 
>>
>>>   
>>>> +def check_os_requirements():
>>>> +    if distro.id() == "debian" and distro.version() == "10":
>>>> +        # List 'Debian' specific packages requirements (different
>>>> than
>>>> +        # vanila distro) to run this test without errors.
>>>> +        debian_requirements = ['flex', 'bison', 'dnsutils',
>>>> 'texinfo']  
>>>
>>> Why is dnsutils needed?  
>>
>> Also, strictly to build and check 'texinfo' is not required either.
> 
> if makeinfo --split-size=5000000 --split-size=5000000 -I
> "/work/wd/glibc/glibc-many-build/src/binutils/binutils/doc" -I
> "/work/wd/glibc/glibc-many-build/src/binutils/binutils/../libiberty" -I
> "/work/wd/glibc/glibc-many-build/src/binutils/binutils/../bfd/doc" -I
> ../../bfd/doc --no-split  -I
> /work/wd/glibc/glibc-many-build/src/binutils/binutils/doc \ -o
> binutils.info `test -f 'binutils.texi' || echo
> '/work/wd/glibc/glibc-many-build/src/binutils/binutils/doc/'`binutils.texi;
> \ then \ rc=0; \ else \ rc=$?; \ $restore $backupdir/* `echo
> "./binutils.info" | sed 's|[^/]*$||'`; \ fi; \ rm -rf $backupdir; exit
> $rc
> /work/wd/glibc/glibc-many-build/src/binutils/binutils/doc/binutils.texi:871:
> warning: @ref node name should not contain `.'
> /work/wd/glibc/glibc-many-build/src/binutils/binutils/doc/binutils.texi:1329:
> warning: @xref node name should not contain `.' make[4]: Leaving
> directory
> '/work/wd/glibc/glibc-many-build/build/compilers/powerpc64-linux-gnu/binutils/binutils/doc'
> Making info in po make[4]: Nothing to be done for 'info'. make[4]:
> Nothing to be done for 'info-am'. make[2]: *** [Makefile:3701:
> all-binutils] Error 2 make[2]: *** Waiting for unfinished jobs.... 

So binutils does still requires texinfo. It would be good to get rid of
this requirement, it does help a lot in build/test total wall time.
  
Joseph Myers Nov. 8, 2021, 9:25 p.m. UTC | #13
On Mon, 8 Nov 2021, Lukasz Majewski wrote:

> Sorry, but I cannot follow your question/explanation above.
> 
> The issue is that - without having dnsutils, when trying to run
> build-many-glibcs.py the pasted error shows up.
> 
> After installing dnsutils (on debian 10) the problem on my setup/machine
> is fixed.

Maybe some *dependency* of dnsutils is relevant.

dnsutils itself is a transitional package to install bind9-dnsutils, so 
certainly isn't relevant.  And bind9-dnsutils provides binaries such as 
dig and nslookup.  The exception you quote doesn't involve calls to such 
binaries; it involves a library function.  Whatever the relevant 
difference is between the systems with and without dnsutils installed, 
it's not about whether those binaries are present.
  
Lukasz Majewski Nov. 9, 2021, 1:39 p.m. UTC | #14
Hi Adhemerval,

> On 08/11/2021 16:44, Lukasz Majewski wrote:
> > Hi Adhemerval,
> >   
> >> On 08/11/2021 13:59, Joseph Myers wrote:  
> >>> On Mon, 8 Nov 2021, Lukasz Majewski wrote:
> >>>     
> >>>> The same approach (with using the 'distro' python module) can be
> >>>> applied to Fedora or Suse.    
> >>>
> >>> That module isn't part of the Python standard library.  I don't
> >>> think we should introduce a dependency on it; rather, any use of
> >>> it should be appropriately conditional, so the code still runs
> >>> (without these checks) if the module is unavailable (importing
> >>> produces an ImportError).
> >>>
> >>> In particular, even if the OS Python installation includes that
> >>> module, the script should work with a separately built copy of
> >>> Python without any such modules from the OS.    
> >>
> >> Maybe add a check without tying to any distribution (tool -v and
> >> some version parsing).  
> > 
> > This would require some extra, work but then we would avoid 'distro'
> > module as the dependency.  
> 
> I think the work required is that hard, something like:
> 
> --
> import shutil
> import subprocess
> 
> def get_version(progname):
>     out = subprocess.run([progname, '--version'],
> stdout=subprocess.PIPE, check=True, universal_newlines=True).stdout
>     return [int(x) for x in
> out.splitlines()[0].split()[-1].split('.')]
> 
> def get_version_awk(progname):
>     out = subprocess.run([progname, '--version'],
> stdout=subprocess.PIPE, check=True, universal_newlines=True).stdout
>     version =
> out.splitlines()[0].split()[2].replace(',','').split('.') return
> [int(x) for x in version]
> 
> def check_version(ver, req):
>     for v, r in zip(ver, req):
>         if v >= r:
>             return True
>     return False
> 
> def version_str(ver):
>     return '.'.join([str (x) for x in version])
> 
> 
> TOOLS={ 'make'     : (get_version,     (4,0)),
>         'makeinfo' : (get_version,     (4,7)),
>         'awk'      : (get_version_awk, (3,1,2)),
>         'bison'    : (get_version,     (2,7)),
>         'sed'      : (get_version,     (3,2)),
>         'flex'     : (get_version,     (2,6,0)),
>         'git'      : (get_version,     (2,32)),
>         'patch'    : (get_version,     (2,7,0)),
>         'tar'      : (get_version,     (1,3,4))}
> 
> for k, v in TOOLS.items():
>     version = v[0](k)
>     ok = 'ok' if check_version (version, v[1]) else 'old'
>     print('{:9}: {:3} (obtained=\"{}\" required=\"{}\")'.format(k, ok,
>         version_str(version), version_str(v[1])))
> --

Yes, I think that this approach is the most exhaustive one, so we would
also check the version of required tools.

LGTM :-)

> 
> >   
> >>  
> >>>     
> >>>> +def check_os_requirements():
> >>>> +    if distro.id() == "debian" and distro.version() == "10":
> >>>> +        # List 'Debian' specific packages requirements
> >>>> (different than
> >>>> +        # vanila distro) to run this test without errors.
> >>>> +        debian_requirements = ['flex', 'bison', 'dnsutils',
> >>>> 'texinfo']    
> >>>
> >>> Why is dnsutils needed?    
> >>
> >> Also, strictly to build and check 'texinfo' is not required
> >> either.  
> > 
> > if makeinfo --split-size=5000000 --split-size=5000000 -I
> > "/work/wd/glibc/glibc-many-build/src/binutils/binutils/doc" -I
> > "/work/wd/glibc/glibc-many-build/src/binutils/binutils/../libiberty"
> > -I
> > "/work/wd/glibc/glibc-many-build/src/binutils/binutils/../bfd/doc"
> > -I ../../bfd/doc --no-split  -I
> > /work/wd/glibc/glibc-many-build/src/binutils/binutils/doc \ -o
> > binutils.info `test -f 'binutils.texi' || echo
> > '/work/wd/glibc/glibc-many-build/src/binutils/binutils/doc/'`binutils.texi;
> > \ then \ rc=0; \ else \ rc=$?; \ $restore $backupdir/* `echo
> > "./binutils.info" | sed 's|[^/]*$||'`; \ fi; \ rm -rf $backupdir;
> > exit $rc
> > /work/wd/glibc/glibc-many-build/src/binutils/binutils/doc/binutils.texi:871:
> > warning: @ref node name should not contain `.'
> > /work/wd/glibc/glibc-many-build/src/binutils/binutils/doc/binutils.texi:1329:
> > warning: @xref node name should not contain `.' make[4]: Leaving
> > directory
> > '/work/wd/glibc/glibc-many-build/build/compilers/powerpc64-linux-gnu/binutils/binutils/doc'
> > Making info in po make[4]: Nothing to be done for 'info'. make[4]:
> > Nothing to be done for 'info-am'. make[2]: *** [Makefile:3701:
> > all-binutils] Error 2 make[2]: *** Waiting for unfinished jobs....
> >  
> 
> So binutils does still requires texinfo. It would be good to get rid
> of this requirement, it does help a lot in build/test total wall time.




Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma@denx.de
  
Adhemerval Zanella Netto Nov. 9, 2021, 1:43 p.m. UTC | #15
On 09/11/2021 10:39, Lukasz Majewski wrote:
> Hi Adhemerval,
> 
>> On 08/11/2021 16:44, Lukasz Majewski wrote:
>>> Hi Adhemerval,
>>>   
>>>> On 08/11/2021 13:59, Joseph Myers wrote:  
>>>>> On Mon, 8 Nov 2021, Lukasz Majewski wrote:
>>>>>     
>>>>>> The same approach (with using the 'distro' python module) can be
>>>>>> applied to Fedora or Suse.    
>>>>>
>>>>> That module isn't part of the Python standard library.  I don't
>>>>> think we should introduce a dependency on it; rather, any use of
>>>>> it should be appropriately conditional, so the code still runs
>>>>> (without these checks) if the module is unavailable (importing
>>>>> produces an ImportError).
>>>>>
>>>>> In particular, even if the OS Python installation includes that
>>>>> module, the script should work with a separately built copy of
>>>>> Python without any such modules from the OS.    
>>>>
>>>> Maybe add a check without tying to any distribution (tool -v and
>>>> some version parsing).  
>>>
>>> This would require some extra, work but then we would avoid 'distro'
>>> module as the dependency.  
>>
>> I think the work required is that hard, something like:
>>
>> --
>> import shutil
>> import subprocess
>>
>> def get_version(progname):
>>     out = subprocess.run([progname, '--version'],
>> stdout=subprocess.PIPE, check=True, universal_newlines=True).stdout
>>     return [int(x) for x in
>> out.splitlines()[0].split()[-1].split('.')]
>>
>> def get_version_awk(progname):
>>     out = subprocess.run([progname, '--version'],
>> stdout=subprocess.PIPE, check=True, universal_newlines=True).stdout
>>     version =
>> out.splitlines()[0].split()[2].replace(',','').split('.') return
>> [int(x) for x in version]
>>
>> def check_version(ver, req):
>>     for v, r in zip(ver, req):
>>         if v >= r:
>>             return True
>>     return False
>>
>> def version_str(ver):
>>     return '.'.join([str (x) for x in version])
>>
>>
>> TOOLS={ 'make'     : (get_version,     (4,0)),
>>         'makeinfo' : (get_version,     (4,7)),
>>         'awk'      : (get_version_awk, (3,1,2)),
>>         'bison'    : (get_version,     (2,7)),
>>         'sed'      : (get_version,     (3,2)),
>>         'flex'     : (get_version,     (2,6,0)),
>>         'git'      : (get_version,     (2,32)),
>>         'patch'    : (get_version,     (2,7,0)),
>>         'tar'      : (get_version,     (1,3,4))}
>>
>> for k, v in TOOLS.items():
>>     version = v[0](k)
>>     ok = 'ok' if check_version (version, v[1]) else 'old'
>>     print('{:9}: {:3} (obtained=\"{}\" required=\"{}\")'.format(k, ok,
>>         version_str(version), version_str(v[1])))
>> --
> 
> Yes, I think that this approach is the most exhaustive one, so we would
> also check the version of required tools.
> 
> LGTM :-)

Also, please double check the required version (for instance 'git' and
'patch').  You will also need to add 'perl' and it similar to 'awk',
it emits the version in a GNU manner.
  
Lukasz Majewski Nov. 9, 2021, 3:32 p.m. UTC | #16
On Tue, 9 Nov 2021 10:43:15 -0300
Adhemerval Zanella <adhemerval.zanella@linaro.org> wrote:

> On 09/11/2021 10:39, Lukasz Majewski wrote:
> > Hi Adhemerval,
> >   
> >> On 08/11/2021 16:44, Lukasz Majewski wrote:  
> >>> Hi Adhemerval,
> >>>     
> >>>> On 08/11/2021 13:59, Joseph Myers wrote:    
> >>>>> On Mon, 8 Nov 2021, Lukasz Majewski wrote:
> >>>>>       
> >>>>>> The same approach (with using the 'distro' python module) can
> >>>>>> be applied to Fedora or Suse.      
> >>>>>
> >>>>> That module isn't part of the Python standard library.  I don't
> >>>>> think we should introduce a dependency on it; rather, any use of
> >>>>> it should be appropriately conditional, so the code still runs
> >>>>> (without these checks) if the module is unavailable (importing
> >>>>> produces an ImportError).
> >>>>>
> >>>>> In particular, even if the OS Python installation includes that
> >>>>> module, the script should work with a separately built copy of
> >>>>> Python without any such modules from the OS.      
> >>>>
> >>>> Maybe add a check without tying to any distribution (tool -v and
> >>>> some version parsing).    
> >>>
> >>> This would require some extra, work but then we would avoid
> >>> 'distro' module as the dependency.    
> >>
> >> I think the work required is that hard, something like:
> >>
> >> --
> >> import shutil
> >> import subprocess
> >>
> >> def get_version(progname):
> >>     out = subprocess.run([progname, '--version'],
> >> stdout=subprocess.PIPE, check=True, universal_newlines=True).stdout
> >>     return [int(x) for x in
> >> out.splitlines()[0].split()[-1].split('.')]
> >>
> >> def get_version_awk(progname):
> >>     out = subprocess.run([progname, '--version'],
> >> stdout=subprocess.PIPE, check=True, universal_newlines=True).stdout
> >>     version =
> >> out.splitlines()[0].split()[2].replace(',','').split('.') return
> >> [int(x) for x in version]
> >>
> >> def check_version(ver, req):
> >>     for v, r in zip(ver, req):
> >>         if v >= r:
> >>             return True
> >>     return False
> >>
> >> def version_str(ver):
> >>     return '.'.join([str (x) for x in version])
> >>
> >>
> >> TOOLS={ 'make'     : (get_version,     (4,0)),
> >>         'makeinfo' : (get_version,     (4,7)),
> >>         'awk'      : (get_version_awk, (3,1,2)),
> >>         'bison'    : (get_version,     (2,7)),
> >>         'sed'      : (get_version,     (3,2)),
> >>         'flex'     : (get_version,     (2,6,0)),
> >>         'git'      : (get_version,     (2,32)),
> >>         'patch'    : (get_version,     (2,7,0)),
> >>         'tar'      : (get_version,     (1,3,4))}
> >>
> >> for k, v in TOOLS.items():
> >>     version = v[0](k)
> >>     ok = 'ok' if check_version (version, v[1]) else 'old'
> >>     print('{:9}: {:3} (obtained=\"{}\" required=\"{}\")'.format(k,
> >> ok, version_str(version), version_str(v[1])))
> >> --  
> > 
> > Yes, I think that this approach is the most exhaustive one, so we
> > would also check the version of required tools.
> > 
> > LGTM :-)  
> 
> Also, please double check the required version (for instance 'git' and
> 'patch').  You will also need to add 'perl' and it similar to 'awk',
> it emits the version in a GNU manner.

I thought that the above code is so different from mine, that you would
like to prepare patch for it?


Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma@denx.de
  
DJ Delorie May 26, 2023, 10:12 p.m. UTC | #17
[working my way through old patches ;-]

https://patchwork.sourceware.org/project/glibc/patch/20211108165255.15600-1-lukma@denx.de/
https://patchwork.sourceware.org/project/glibc/patch/20211108201931.17159-1-lukma@denx.de/

Lukasz Majewski <lukma@denx.de> writes:
> I thought that the above code is so different from mine, that you would
> like to prepare patch for it?

How about something like this?  I filled out Adhemerval's sample code
with some error checking, added some user messages, and ended up with
the attached.  I split it up so that the *list* of required tools is at
the beginning of the file (easily findable) but the *code* to handle it
is near the end.

I also thought of adding two more columns to the required_tools table,
one with common RPM package names, and one with common DEB package
names.  This would purely be for convenience, although I assume most bmg
users know how to find which package an executable is in.  Might be
trickier for libraries, if we add "required libraries" to the list
later.

I also considered moving the minimum-version list to the beginning, to
make it easier to find, but decided that should be separate.


diff --git a/scripts/build-many-glibcs.py b/scripts/build-many-glibcs.py
index 95726c4a29..0f35c62b7a 100755
--- a/scripts/build-many-glibcs.py
+++ b/scripts/build-many-glibcs.py
@@ -56,6 +56,20 @@ import sys
 import time
 import urllib.request
 
+def get_list_of_required_tools():
+    global REQUIRED_TOOLS
+    REQUIRED_TOOLS = {
+        'awk'      : (get_version_awk, (3,1,2)),
+        'bison'    : (get_version,     (2,7)),
+        'flex'     : (get_version,     (2,6,0)),
+        'git'      : (get_version,     (2,32)),
+        'make'     : (get_version,     (4,0)),
+        'makeinfo' : (get_version,     (4,7)),
+        'patch'    : (get_version,     (2,7,0)),
+        'sed'      : (get_version,     (3,2)),
+        'tar'      : (get_version,     (1,3,4)),
+    }
+
 try:
     subprocess.run
 except:
@@ -1865,8 +1879,66 @@ def get_parser():
     return parser
 
 
+def get_version_common(progname,line,word,delchars):
+    try:
+        out = subprocess.run([progname, '--version'],
+                             stdout=subprocess.PIPE,
+                             stderr=subprocess.DEVNULL,
+                             check=True, universal_newlines=True).stdout
+        v = out.splitlines()[line].split()[word]
+        if delchars:
+            v = v.replace(delchars,'')
+        return [int(x) for x in v.split('.')]
+    except:
+        return 'missing';
+
+def get_version(progname):
+    return get_version_common (progname, 0, -1, None);
+
+def get_version_awk(progname):
+    return get_version_common (progname, 0, 2, ',');
+
+def check_version(ver, req):
+    for v, r in zip(ver, req):
+        if v > r:
+            return True
+        if v < r:
+            return False
+    return True
+
+def version_str(ver):
+    return '.'.join([str (x) for x in ver])
+
+def check_for_required_tools():
+    get_list_of_required_tools()
+    count_old_tools = 0
+    count_missing_tools = 0
+    
+    for k, v in REQUIRED_TOOLS.items():
+        version = v[0](k)
+        if version == 'missing':
+            ok = 'missing'
+        else:
+            ok = 'ok' if check_version (version, v[1]) else 'old'
+        if ok == 'old':
+            if count_old_tools == 0:
+                print("One or more required tools are too old:")
+            count_old_tools = count_old_tools + 1
+            print('{:9}: {:3} (obtained=\"{}\" required=\"{}\")'.format(k, ok,
+                    version_str(version), version_str(v[1])))
+        if ok == 'missing':
+            if count_missing_tools == 0:
+                print("One or more required tools are missing:")
+            count_missing_tools = count_missing_tools + 1
+            print('{:9}: {:3} (required=\"{}\")'.format(k, ok,
+                    version_str(v[1])))
+    
+    if count_old_tools > 0 or count_missing_tools > 0:
+        exit (1);
+    
 def main(argv):
     """The main entry point."""
+    check_for_required_tools();
     parser = get_parser()
     opts = parser.parse_args(argv)
     topdir = os.path.abspath(opts.topdir)
  
Joseph Myers May 26, 2023, 10:34 p.m. UTC | #18
On Fri, 26 May 2023, DJ Delorie via Libc-alpha wrote:

> +        'git'      : (get_version,     (2,32)),

I don't know where that git version came from, but I'd be very surprised 
if build-many-glibcs.py needs a version anything like that recent.
  
DJ Delorie May 31, 2023, 7:54 p.m. UTC | #19
Joseph Myers <joseph@codesourcery.com> writes:
> On Fri, 26 May 2023, DJ Delorie via Libc-alpha wrote:
>> +        'git'      : (get_version,     (2,32)),
>
> I don't know where that git version came from, but I'd be very surprised 
> if build-many-glibcs.py needs a version anything like that recent.

I tested git 1.8.3 against gitlab and was able to checkout the gcc
sources.  I got git 1.5 from git's github git, and it wouldn't talk to
github.  So I propose listing 1.8.3 until someone complains.

Git 1.8.3 is RHEL 7's git version.  How far back in time do we want to
support building the latest glibc?
  
DJ Delorie May 31, 2023, 8:18 p.m. UTC | #20
I'll also add that we could pre-fill these versions with "version 0"
which would just mean "you have to have this program" until someone
complains ;-)

    REQUIRED_TOOLS = {
        'awk'      : (get_version_awk, (3,1,2)),
        'bison'    : (get_version,     (2,7)),
        'flex'     : (get_version,     (2,6,0)),
        'git'      : (get_version,     (0)),
        'make'     : (get_version,     (4,0)),
        'makeinfo' : (get_version,     (4,7)),
        'patch'    : (get_version,     (2,7,0)),
        'sed'      : (get_version,     (3,2)),
        'tar'      : (get_version,     (1,3,4)),
    }
  
Adhemerval Zanella Netto June 5, 2023, 8:42 p.m. UTC | #21
On 26/05/23 19:12, DJ Delorie wrote:
> 
> [working my way through old patches ;-]
> 
> https://patchwork.sourceware.org/project/glibc/patch/20211108165255.15600-1-lukma@denx.de/
> https://patchwork.sourceware.org/project/glibc/patch/20211108201931.17159-1-lukma@denx.de/
> 
> Lukasz Majewski <lukma@denx.de> writes:
>> I thought that the above code is so different from mine, that you would
>> like to prepare patch for it?
> 
> How about something like this?  I filled out Adhemerval's sample code
> with some error checking, added some user messages, and ended up with
> the attached.  I split it up so that the *list* of required tools is at
> the beginning of the file (easily findable) but the *code* to handle it
> is near the end.
> 
> I also thought of adding two more columns to the required_tools table,
> one with common RPM package names, and one with common DEB package
> names.  This would purely be for convenience, although I assume most bmg
> users know how to find which package an executable is in.  Might be
> trickier for libraries, if we add "required libraries" to the list
> later.
> 
> I also considered moving the minimum-version list to the beginning, to
> make it easier to find, but decided that should be separate.

The patch organization sounds good to me, thanks for working on this.

> 
> 
> diff --git a/scripts/build-many-glibcs.py b/scripts/build-many-glibcs.py
> index 95726c4a29..0f35c62b7a 100755
> --- a/scripts/build-many-glibcs.py
> +++ b/scripts/build-many-glibcs.py
> @@ -56,6 +56,20 @@ import sys
>  import time
>  import urllib.request
>  
> +def get_list_of_required_tools():
> +    global REQUIRED_TOOLS
> +    REQUIRED_TOOLS = {
> +        'awk'      : (get_version_awk, (3,1,2)),
> +        'bison'    : (get_version,     (2,7)),
> +        'flex'     : (get_version,     (2,6,0)),
> +        'git'      : (get_version,     (2,32)),
> +        'make'     : (get_version,     (4,0)),
> +        'makeinfo' : (get_version,     (4,7)),
> +        'patch'    : (get_version,     (2,7,0)),
> +        'sed'      : (get_version,     (3,2)),
> +        'tar'      : (get_version,     (1,3,4)),

The script untar gz, bz2, and xz files, should it check for the associated
tools as well?

> +    }
> +
>  try:
>      subprocess.run
>  except:
> @@ -1865,8 +1879,66 @@ def get_parser():
>      return parser
>  
>  
> +def get_version_common(progname,line,word,delchars):
> +    try:
> +        out = subprocess.run([progname, '--version'],
> +                             stdout=subprocess.PIPE,
> +                             stderr=subprocess.DEVNULL,
> +                             check=True, universal_newlines=True).stdout
> +        v = out.splitlines()[line].split()[word]
> +        if delchars:
> +            v = v.replace(delchars,'')
> +        return [int(x) for x in v.split('.')]
> +    except:
> +        return 'missing';
> +
> +def get_version(progname):
> +    return get_version_common (progname, 0, -1, None);
> +
> +def get_version_awk(progname):
> +    return get_version_common (progname, 0, 2, ',');
> +
> +def check_version(ver, req):
> +    for v, r in zip(ver, req):
> +        if v > r:
> +            return True
> +        if v < r:
> +            return False
> +    return True
> +
> +def version_str(ver):
> +    return '.'.join([str (x) for x in ver])
> +
> +def check_for_required_tools():
> +    get_list_of_required_tools()
> +    count_old_tools = 0
> +    count_missing_tools = 0
> +    
> +    for k, v in REQUIRED_TOOLS.items():
> +        version = v[0](k)
> +        if version == 'missing':
> +            ok = 'missing'
> +        else:
> +            ok = 'ok' if check_version (version, v[1]) else 'old'
> +        if ok == 'old':
> +            if count_old_tools == 0:
> +                print("One or more required tools are too old:")
> +            count_old_tools = count_old_tools + 1
> +            print('{:9}: {:3} (obtained=\"{}\" required=\"{}\")'.format(k, ok,
> +                    version_str(version), version_str(v[1])))
> +        if ok == 'missing':
> +            if count_missing_tools == 0:
> +                print("One or more required tools are missing:")
> +            count_missing_tools = count_missing_tools + 1
> +            print('{:9}: {:3} (required=\"{}\")'.format(k, ok,
> +                    version_str(v[1])))
> +    
> +    if count_old_tools > 0 or count_missing_tools > 0:
> +        exit (1);
> +    
>  def main(argv):
>      """The main entry point."""
> +    check_for_required_tools();
>      parser = get_parser()
>      opts = parser.parse_args(argv)
>      topdir = os.path.abspath(opts.topdir)
>
  
DJ Delorie June 5, 2023, 9:12 p.m. UTC | #22
Adhemerval Zanella Netto <adhemerval.zanella@linaro.org> writes:
>> +        'tar'      : (get_version,     (1,3,4)),
>
> The script untar gz, bz2, and xz files, should it check for the associated
> tools as well?

I'm not opposed, but (1) that assumes tar doesn't have those built in,
and (2) we can figure out what files and versions to put in the table.

I expect that this table will be a bit dynamic as people find versions
that work/don't relative to the table's cutoffs, or as new requirements
are known.  I don't expect it to be perfect the first time around.
Patches to my patches welcome ;-)
  
DJ Delorie Sept. 21, 2023, 9:44 p.m. UTC | #23
Adhemerval Zanella Netto <adhemerval.zanella@linaro.org> writes:
> The script untar gz, bz2, and xz files, should it check for the associated
> tools as well?

Added.  I reset most of the versions to 0,0 and noted in the commit when
to change them; iirc I actually did some research on the git version so
I left that alone (and as an example).

bzip2 was an outlier; it sends its version info to stderr instead of
stdout, and *still* starts a compression stream... sigh.

I added Lukasz and Adhemerval as co-authors as we all contributed code
to this; let me know if you don't want to be listed.

From b83df8bf9c10154be8b21e6a82fff43fcb8ebb3d Mon Sep 17 00:00:00 2001
From: DJ Delorie <dj@redhat.com>
Date: Thu, 21 Sep 2023 17:24:05 -0400
Subject: build-many-glibcs: Check for required system tools

Notes for future devs:

* Add tools as you find they're needed, with version 0,0
* Bump version when you find an old tool that doesn't work
* Don't add a version just because you know it works

Co-authored-by: Lukasz Majewski <lukma@denx.de>
Co-authored-by: Adhemerval Zanella Netto <adhemerval.zanella@linaro.org>

diff --git a/scripts/build-many-glibcs.py b/scripts/build-many-glibcs.py
index 57a5c48b16..edea52bbae 100755
--- a/scripts/build-many-glibcs.py
+++ b/scripts/build-many-glibcs.py
@@ -56,6 +56,26 @@ import sys
 import time
 import urllib.request
 
+# This is a list of system utilities that are expected to be available
+# to this script, and, if a non-zero version is included, the minimum
+# version required to work with this sccript.
+def get_list_of_required_tools():
+    global REQUIRED_TOOLS
+    REQUIRED_TOOLS = {
+        'awk'      : (get_version_awk,   (0,0,0)),
+        'bison'    : (get_version,       (0,0)),
+        'flex'     : (get_version,       (0,0,0)),
+        'git'      : (get_version,       (1,8,3)),
+        'make'     : (get_version,       (4,0)),
+        'makeinfo' : (get_version,       (0,0)),
+        'patch'    : (get_version,       (0,0,0)),
+        'sed'      : (get_version,       (0,0)),
+        'tar'      : (get_version,       (0,0,0)),
+        'gzip'     : (get_version,       (0,0)),
+        'bzip2'    : (get_version_bzip2, (0,0,0)),
+        'xz'       : (get_version,       (0,0,0)),
+    }
+
 try:
     subprocess.run
 except:
@@ -1871,8 +1891,84 @@ def get_parser():
     return parser
 
 
+def get_version_common(progname,line,word,delchars,arg1):
+    try:
+        out = subprocess.run([progname, arg1],
+                             stdout=subprocess.PIPE,
+                             stderr=subprocess.DEVNULL,
+                             stdin=subprocess.DEVNULL,
+                             check=True, universal_newlines=True)
+        v = out.stdout.splitlines()[line].split()[word]
+        if delchars:
+            v = v.replace(delchars,'')
+        return [int(x) for x in v.split('.')]
+    except:
+        return 'missing';
+
+def get_version_common_stderr(progname,line,word,delchars,arg1):
+    try:
+        out = subprocess.run([progname, arg1],
+                             stdout=subprocess.DEVNULL,
+                             stderr=subprocess.PIPE,
+                             stdin=subprocess.DEVNULL,
+                             check=True, universal_newlines=True)
+        v = out.stderr.splitlines()[line].split()[word]
+        if delchars:
+            v = v.replace(delchars,'')
+        return [int(x) for x in v.split('.')]
+    except:
+        return 'missing';
+
+def get_version(progname):
+    return get_version_common (progname, 0, -1, None, '--version');
+
+def get_version_awk(progname):
+    return get_version_common (progname, 0, 2, ',', '--version');
+
+def get_version_bzip2(progname):
+    return get_version_common_stderr (progname, 0, 6, ',', '-h');
+
+def check_version(ver, req):
+    for v, r in zip(ver, req):
+        if v > r:
+            return True
+        if v < r:
+            return False
+    return True
+
+def version_str(ver):
+    return '.'.join([str (x) for x in ver])
+
+def check_for_required_tools():
+    get_list_of_required_tools()
+    count_old_tools = 0
+    count_missing_tools = 0
+    
+    for k, v in REQUIRED_TOOLS.items():
+        version = v[0](k)
+        if version == 'missing':
+            ok = 'missing'
+        else:
+            ok = 'ok' if check_version (version, v[1]) else 'old'
+        if ok == 'old':
+            if count_old_tools == 0:
+                print("One or more required tools are too old:")
+            count_old_tools = count_old_tools + 1
+            print('{:9}: {:3} (obtained=\"{}\" required=\"{}\")'.format(k, ok,
+                    version_str(version), version_str(v[1])))
+        if ok == 'missing':
+            if count_missing_tools == 0:
+                print("One or more required tools are missing:")
+            count_missing_tools = count_missing_tools + 1
+            print('{:9}: {:3} (required=\"{}\")'.format(k, ok,
+                    version_str(v[1])))
+    
+    if count_old_tools > 0 or count_missing_tools > 0:
+        exit (1);
+    
 def main(argv):
     """The main entry point."""
+    check_for_required_tools();
     parser = get_parser()
     opts = parser.parse_args(argv)
     topdir = os.path.abspath(opts.topdir)
  
Adhemerval Zanella Netto Oct. 9, 2023, 6:23 p.m. UTC | #24
On 21/09/23 18:44, DJ Delorie wrote:
> Adhemerval Zanella Netto <adhemerval.zanella@linaro.org> writes:
>> The script untar gz, bz2, and xz files, should it check for the associated
>> tools as well?
> 
> Added.  I reset most of the versions to 0,0 and noted in the commit when
> to change them; iirc I actually did some research on the git version so
> I left that alone (and as an example).
> 
> bzip2 was an outlier; it sends its version info to stderr instead of
> stdout, and *still* starts a compression stream... sigh.
> 
> I added Lukasz and Adhemerval as co-authors as we all contributed code
> to this; let me know if you don't want to be listed.
> 
> From b83df8bf9c10154be8b21e6a82fff43fcb8ebb3d Mon Sep 17 00:00:00 2001
> From: DJ Delorie <dj@redhat.com>
> Date: Thu, 21 Sep 2023 17:24:05 -0400
> Subject: build-many-glibcs: Check for required system tools
> 
> Notes for future devs:
> 
> * Add tools as you find they're needed, with version 0,0
> * Bump version when you find an old tool that doesn't work
> * Don't add a version just because you know it works
> 
> Co-authored-by: Lukasz Majewski <lukma@denx.de>
> Co-authored-by: Adhemerval Zanella Netto <adhemerval.zanella@linaro.org>

LGTM, thanks.

Reviewed-by: Adhemerval Zanella  <adhemerval.zanella@linaro.org>

> 
> diff --git a/scripts/build-many-glibcs.py b/scripts/build-many-glibcs.py
> index 57a5c48b16..edea52bbae 100755
> --- a/scripts/build-many-glibcs.py
> +++ b/scripts/build-many-glibcs.py
> @@ -56,6 +56,26 @@ import sys
>  import time
>  import urllib.request
>  
> +# This is a list of system utilities that are expected to be available
> +# to this script, and, if a non-zero version is included, the minimum
> +# version required to work with this sccript.
> +def get_list_of_required_tools():
> +    global REQUIRED_TOOLS
> +    REQUIRED_TOOLS = {
> +        'awk'      : (get_version_awk,   (0,0,0)),
> +        'bison'    : (get_version,       (0,0)),
> +        'flex'     : (get_version,       (0,0,0)),
> +        'git'      : (get_version,       (1,8,3)),
> +        'make'     : (get_version,       (4,0)),
> +        'makeinfo' : (get_version,       (0,0)),
> +        'patch'    : (get_version,       (0,0,0)),
> +        'sed'      : (get_version,       (0,0)),
> +        'tar'      : (get_version,       (0,0,0)),
> +        'gzip'     : (get_version,       (0,0)),
> +        'bzip2'    : (get_version_bzip2, (0,0,0)),
> +        'xz'       : (get_version,       (0,0,0)),
> +    }
> +
>  try:
>      subprocess.run
>  except:
> @@ -1871,8 +1891,84 @@ def get_parser():
>      return parser
>  
>  
> +def get_version_common(progname,line,word,delchars,arg1):
> +    try:
> +        out = subprocess.run([progname, arg1],
> +                             stdout=subprocess.PIPE,
> +                             stderr=subprocess.DEVNULL,
> +                             stdin=subprocess.DEVNULL,
> +                             check=True, universal_newlines=True)
> +        v = out.stdout.splitlines()[line].split()[word]
> +        if delchars:
> +            v = v.replace(delchars,'')
> +        return [int(x) for x in v.split('.')]
> +    except:
> +        return 'missing';
> +
> +def get_version_common_stderr(progname,line,word,delchars,arg1):
> +    try:
> +        out = subprocess.run([progname, arg1],
> +                             stdout=subprocess.DEVNULL,
> +                             stderr=subprocess.PIPE,
> +                             stdin=subprocess.DEVNULL,
> +                             check=True, universal_newlines=True)
> +        v = out.stderr.splitlines()[line].split()[word]
> +        if delchars:
> +            v = v.replace(delchars,'')
> +        return [int(x) for x in v.split('.')]
> +    except:
> +        return 'missing';
> +
> +def get_version(progname):
> +    return get_version_common (progname, 0, -1, None, '--version');
> +
> +def get_version_awk(progname):
> +    return get_version_common (progname, 0, 2, ',', '--version');
> +
> +def get_version_bzip2(progname):
> +    return get_version_common_stderr (progname, 0, 6, ',', '-h');
> +
> +def check_version(ver, req):
> +    for v, r in zip(ver, req):
> +        if v > r:
> +            return True
> +        if v < r:
> +            return False
> +    return True
> +
> +def version_str(ver):
> +    return '.'.join([str (x) for x in ver])
> +
> +def check_for_required_tools():
> +    get_list_of_required_tools()
> +    count_old_tools = 0
> +    count_missing_tools = 0
> +    
> +    for k, v in REQUIRED_TOOLS.items():
> +        version = v[0](k)
> +        if version == 'missing':
> +            ok = 'missing'
> +        else:
> +            ok = 'ok' if check_version (version, v[1]) else 'old'
> +        if ok == 'old':
> +            if count_old_tools == 0:
> +                print("One or more required tools are too old:")
> +            count_old_tools = count_old_tools + 1
> +            print('{:9}: {:3} (obtained=\"{}\" required=\"{}\")'.format(k, ok,
> +                    version_str(version), version_str(v[1])))
> +        if ok == 'missing':
> +            if count_missing_tools == 0:
> +                print("One or more required tools are missing:")
> +            count_missing_tools = count_missing_tools + 1
> +            print('{:9}: {:3} (required=\"{}\")'.format(k, ok,
> +                    version_str(v[1])))
> +    
> +    if count_old_tools > 0 or count_missing_tools > 0:
> +        exit (1);
> +    
>  def main(argv):
>      """The main entry point."""
> +    check_for_required_tools();
>      parser = get_parser()
>      opts = parser.parse_args(argv)
>      topdir = os.path.abspath(opts.topdir)
>
  
DJ Delorie Oct. 9, 2023, 9:53 p.m. UTC | #25
Thanks!  Pushed.
  

Patch

diff --git a/scripts/build-many-glibcs.py b/scripts/build-many-glibcs.py
index 6046048b75..b7aedf304f 100755
--- a/scripts/build-many-glibcs.py
+++ b/scripts/build-many-glibcs.py
@@ -54,6 +54,7 @@  import subprocess
 import sys
 import time
 import urllib.request
+import distro
 
 try:
     subprocess.run
@@ -1852,9 +1853,24 @@  def get_parser():
                         nargs='*')
     return parser
 
+def check_os_requirements():
+    if distro.id() == "debian" and distro.version() == "10":
+        # List 'Debian' specific packages requirements (different than
+        # vanila distro) to run this test without errors.
+        debian_requirements = ['flex', 'bison', 'dnsutils', 'texinfo']
+        import apt
+        debian_pkgs = apt.Cache()
+        for pkg in debian_requirements:
+            if not debian_pkgs[pkg].is_installed:
+                print(f"error: Debian package: '{pkg}' NOT installed!")
+                return False
+
+    return True
 
 def main(argv):
     """The main entry point."""
+    if not check_os_requirements():
+        sys.exit(0)
     parser = get_parser()
     opts = parser.parse_args(argv)
     topdir = os.path.abspath(opts.topdir)