diff mbox series

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

Message ID 20211108165255.15600-1-lukma@denx.de
State New
Headers show
Series ci: Check for necessary Debian packages when running build-many-glibcs.py | expand

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 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 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 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 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
diff mbox series

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)