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