build: allow using wildcards to disable drivers

Message ID 20200120173725.57529-1-bruce.richardson@intel.com (mailing list archive)
State Superseded, archived
Delegated to: Thomas Monjalon
Headers
Series build: allow using wildcards to disable drivers |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation success Compilation OK
ci/iol-intel-Performance success Performance Testing PASS
ci/travis-robot success Travis build: passed
ci/iol-testing success Testing PASS
ci/iol-mellanox-Performance success Performance Testing PASS
ci/iol-nxp-Performance success Performance Testing PASS

Commit Message

Bruce Richardson Jan. 20, 2020, 5:37 p.m. UTC
  Rather than having to explicitly list each and every driver to disable in a
build, we can use a small python script and the python glob library to
expand out the wildcards. This means that we can configure meson using e.g.

    meson -Ddisable_drivers=crypto/*,event/* build

to do a build omitting all the crypto and event drivers. Explicitly
specified drivers e.g. net/i40e, work as before, and can be mixed with
wildcarded drivers as required.

Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
---
 buildtools/list-dir-globs.sh | 15 +++++++++++++++
 buildtools/meson.build       |  2 +-
 drivers/meson.build          |  3 ++-
 3 files changed, 18 insertions(+), 2 deletions(-)
 create mode 100755 buildtools/list-dir-globs.sh
  

Comments

Thomas Monjalon Jan. 20, 2020, 6:59 p.m. UTC | #1
20/01/2020 18:37, Bruce Richardson:
> Rather than having to explicitly list each and every driver to disable in a
> build, we can use a small python script and the python glob library to
> expand out the wildcards. This means that we can configure meson using e.g.
> 
>     meson -Ddisable_drivers=crypto/*,event/* build
> 
> to do a build omitting all the crypto and event drivers. Explicitly
> specified drivers e.g. net/i40e, work as before, and can be mixed with
> wildcarded drivers as required.
> 
> Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> ---
> --- /dev/null
> +++ b/buildtools/list-dir-globs.sh
> @@ -0,0 +1,15 @@
> +#! /usr/bin/env python

Did you start implementing it as a shell script?
The extension is still .sh though I guess it might be .py.
  
Robin Jarry Jan. 21, 2020, 9:11 a.m. UTC | #2
2020-01-20, Bruce Richardson:
> Rather than having to explicitly list each and every driver to disable in a
> build, we can use a small python script and the python glob library to
> expand out the wildcards. This means that we can configure meson using e.g.
> 
>     meson -Ddisable_drivers=crypto/*,event/* build
> 
> to do a build omitting all the crypto and event drivers. Explicitly
> specified drivers e.g. net/i40e, work as before, and can be mixed with
> wildcarded drivers as required.
> 
> Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> ---
[snip]
> +++ b/buildtools/list-dir-globs.sh
> @@ -0,0 +1,15 @@
> +#! /usr/bin/env python
> +# SPDX-License-Identifier: BSD-3-Clause
> +# Copyright(c) 2020 Intel Corporation
> +
> +from __future__ import print_function
> +from os import chdir, environ
> +from sys import argv
> +from glob import iglob # glob iterator
> +from os.path import isdir, join
> +
> +chdir(join(environ['MESON_SOURCE_ROOT'], environ['MESON_SUBDIR']))
> +dirs = []
> +for path in argv[1].split(','):
> +  dirs.extend([entry for entry in iglob(path) if isdir(entry)])

I do not fancy changing the current directory in scripts. This can lead
to unexpected behavior. You don't need to, you can achieve the same
result with the following code:

    from __future__ import print_function
    import argparse
    import os
    import glob

    parser = argparse.ArgumentParser()
    parser.add_argument('disable_drivers', type=lambda s: s.split(','))
    args = parser.parse_args()
    root = os.path.join(os.environ['MESON_SOURCE_ROOT'],
                        os.environ['MESON_SUBDIR'])
    dirs = []
    for driver in args.disable_drivers:
        driver_path = os.path.join(root, driver)
        for path in glob.iglob(driver_path):
            if os.path.isdir(path):
                dirs.append(os.path.relpath(path, root))
    print(','.join(dirs))

I did not see why you used iglob instead of glob. iglob is case
insensitive. Is that for windows support?

> +++ b/drivers/meson.build
> @@ -17,7 +17,8 @@ dpdk_driver_classes = ['common',
>                'event',   # depends on common, bus, mempool and net.
>                'baseband'] # depends on common and bus.
> 
> -disabled_drivers = get_option('disable_drivers').split(',')
> +disabled_drivers = run_command(list_dir_globs, get_option('disable_drivers'),
> +               ).stdout().strip().split(',')

Why adding commas again? Since you are processing the option value
through a script, you might as well print one folder per line and use
the .splitlines() meson function. It will make the code simpler as you
do not need to use an intermediate list to store the disabled drivers
before printing them.
  
Bruce Richardson Jan. 21, 2020, 10:13 a.m. UTC | #3
On Mon, Jan 20, 2020 at 07:59:13PM +0100, Thomas Monjalon wrote:
> 20/01/2020 18:37, Bruce Richardson:
> > Rather than having to explicitly list each and every driver to disable in a
> > build, we can use a small python script and the python glob library to
> > expand out the wildcards. This means that we can configure meson using e.g.
> > 
> >     meson -Ddisable_drivers=crypto/*,event/* build
> > 
> > to do a build omitting all the crypto and event drivers. Explicitly
> > specified drivers e.g. net/i40e, work as before, and can be mixed with
> > wildcarded drivers as required.
> > 
> > Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> > ---
> > --- /dev/null
> > +++ b/buildtools/list-dir-globs.sh
> > @@ -0,0 +1,15 @@
> > +#! /usr/bin/env python
> 
> Did you start implementing it as a shell script?
> The extension is still .sh though I guess it might be .py.
> 
Yes, good catch, thanks.
  
Bruce Richardson Jan. 21, 2020, 10:17 a.m. UTC | #4
On Tue, Jan 21, 2020 at 10:11:33AM +0100, Robin Jarry wrote:
> 2020-01-20, Bruce Richardson:
> > Rather than having to explicitly list each and every driver to disable in a
> > build, we can use a small python script and the python glob library to
> > expand out the wildcards. This means that we can configure meson using e.g.
> > 
> >     meson -Ddisable_drivers=crypto/*,event/* build
> > 
> > to do a build omitting all the crypto and event drivers. Explicitly
> > specified drivers e.g. net/i40e, work as before, and can be mixed with
> > wildcarded drivers as required.
> > 
> > Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> > ---
> [snip]
> > +++ b/buildtools/list-dir-globs.sh
> > @@ -0,0 +1,15 @@
> > +#! /usr/bin/env python
> > +# SPDX-License-Identifier: BSD-3-Clause
> > +# Copyright(c) 2020 Intel Corporation
> > +
> > +from __future__ import print_function
> > +from os import chdir, environ
> > +from sys import argv
> > +from glob import iglob # glob iterator
> > +from os.path import isdir, join
> > +
> > +chdir(join(environ['MESON_SOURCE_ROOT'], environ['MESON_SUBDIR']))
> > +dirs = []
> > +for path in argv[1].split(','):
> > +  dirs.extend([entry for entry in iglob(path) if isdir(entry)])
> 
> I do not fancy changing the current directory in scripts. This can lead
> to unexpected behavior. You don't need to, you can achieve the same
> result with the following code:

First version indeed did not use chdir, but it seemed simpler to just chdir
and work off that than try and get back to relative paths. However, I'll
fix as you suggest below.

> 
>     from __future__ import print_function
>     import argparse
>     import os
>     import glob
> 
>     parser = argparse.ArgumentParser()
>     parser.add_argument('disable_drivers', type=lambda s: s.split(','))
>     args = parser.parse_args()
>     root = os.path.join(os.environ['MESON_SOURCE_ROOT'],
>                         os.environ['MESON_SUBDIR'])
>     dirs = []
>     for driver in args.disable_drivers:
>         driver_path = os.path.join(root, driver)
>         for path in glob.iglob(driver_path):
>             if os.path.isdir(path):
>                 dirs.append(os.path.relpath(path, root))
>     print(','.join(dirs))
> 
> I did not see why you used iglob instead of glob. iglob is case
> insensitive. Is that for windows support?

No, iglob returns an iterator rather than a full list. It's just slightly
more efficient if there are a large number of items.

> 
> > +++ b/drivers/meson.build
> > @@ -17,7 +17,8 @@ dpdk_driver_classes = ['common',
> >                'event',   # depends on common, bus, mempool and net.
> >                'baseband'] # depends on common and bus.
> > 
> > -disabled_drivers = get_option('disable_drivers').split(',')
> > +disabled_drivers = run_command(list_dir_globs, get_option('disable_drivers'),
> > +               ).stdout().strip().split(',')
> 
> Why adding commas again? Since you are processing the option value
> through a script, you might as well print one folder per line and use
> the .splitlines() meson function. It will make the code simpler as you
> do not need to use an intermediate list to store the disabled drivers
> before printing them.
> 
Good point. Will update.

/Bruce
  

Patch

diff --git a/buildtools/list-dir-globs.sh b/buildtools/list-dir-globs.sh
new file mode 100755
index 000000000..ed5aac41c
--- /dev/null
+++ b/buildtools/list-dir-globs.sh
@@ -0,0 +1,15 @@ 
+#! /usr/bin/env python
+# SPDX-License-Identifier: BSD-3-Clause
+# Copyright(c) 2020 Intel Corporation
+
+from __future__ import print_function
+from os import chdir, environ
+from sys import argv
+from glob import iglob # glob iterator
+from os.path import isdir, join
+
+chdir(join(environ['MESON_SOURCE_ROOT'], environ['MESON_SUBDIR']))
+dirs = []
+for path in argv[1].split(','):
+  dirs.extend([entry for entry in iglob(path) if isdir(entry)])
+print(",".join(dirs))
diff --git a/buildtools/meson.build b/buildtools/meson.build
index cd1d05403..830c391f9 100644
--- a/buildtools/meson.build
+++ b/buildtools/meson.build
@@ -4,7 +4,7 @@ 
 subdir('pmdinfogen')
 
 pmdinfo = find_program('gen-pmdinfo-cfile.sh')
-
+list_dir_globs = find_program('list-dir-globs.sh')
 check_experimental_syms = find_program('check-experimental-syms.sh')
 
 # set up map-to-def script using python, either built-in or external
diff --git a/drivers/meson.build b/drivers/meson.build
index 29708cc2b..292d0f39a 100644
--- a/drivers/meson.build
+++ b/drivers/meson.build
@@ -17,7 +17,8 @@  dpdk_driver_classes = ['common',
 	       'event',   # depends on common, bus, mempool and net.
 	       'baseband'] # depends on common and bus.
 
-disabled_drivers = get_option('disable_drivers').split(',')
+disabled_drivers = run_command(list_dir_globs, get_option('disable_drivers'),
+		).stdout().strip().split(',')
 
 default_cflags = machine_args
 if cc.has_argument('-Wno-format-truncation')