DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev] [PATCH] build: allow using wildcards to disable drivers
@ 2020-01-20 17:37 Bruce Richardson
  2020-01-20 18:59 ` Thomas Monjalon
                   ` (4 more replies)
  0 siblings, 5 replies; 19+ messages in thread
From: Bruce Richardson @ 2020-01-20 17:37 UTC (permalink / raw)
  To: dev; +Cc: 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>
---
 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

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')
-- 
2.20.1


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

* Re: [dpdk-dev] [PATCH] build: allow using wildcards to disable drivers
  2020-01-20 17:37 [dpdk-dev] [PATCH] build: allow using wildcards to disable drivers Bruce Richardson
@ 2020-01-20 18:59 ` Thomas Monjalon
  2020-01-21  9:11   ` Robin Jarry
  2020-01-21 10:13   ` Bruce Richardson
  2020-01-21 11:12 ` [dpdk-dev] [PATCH v2] " Bruce Richardson
                   ` (3 subsequent siblings)
  4 siblings, 2 replies; 19+ messages in thread
From: Thomas Monjalon @ 2020-01-20 18:59 UTC (permalink / raw)
  To: Bruce Richardson; +Cc: dev, robin.jarry

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.




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

* Re: [dpdk-dev] [PATCH] build: allow using wildcards to disable drivers
  2020-01-20 18:59 ` Thomas Monjalon
@ 2020-01-21  9:11   ` Robin Jarry
  2020-01-21 10:17     ` Bruce Richardson
  2020-01-21 10:13   ` Bruce Richardson
  1 sibling, 1 reply; 19+ messages in thread
From: Robin Jarry @ 2020-01-21  9:11 UTC (permalink / raw)
  To: Thomas Monjalon; +Cc: Bruce Richardson, dev

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.

-- 
Robin

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

* Re: [dpdk-dev] [PATCH] build: allow using wildcards to disable drivers
  2020-01-20 18:59 ` Thomas Monjalon
  2020-01-21  9:11   ` Robin Jarry
@ 2020-01-21 10:13   ` Bruce Richardson
  1 sibling, 0 replies; 19+ messages in thread
From: Bruce Richardson @ 2020-01-21 10:13 UTC (permalink / raw)
  To: Thomas Monjalon; +Cc: dev, robin.jarry

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.

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

* Re: [dpdk-dev] [PATCH] build: allow using wildcards to disable drivers
  2020-01-21  9:11   ` Robin Jarry
@ 2020-01-21 10:17     ` Bruce Richardson
  0 siblings, 0 replies; 19+ messages in thread
From: Bruce Richardson @ 2020-01-21 10:17 UTC (permalink / raw)
  To: Robin Jarry; +Cc: Thomas Monjalon, dev

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

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

* [dpdk-dev] [PATCH v2] build: allow using wildcards to disable drivers
  2020-01-20 17:37 [dpdk-dev] [PATCH] build: allow using wildcards to disable drivers Bruce Richardson
  2020-01-20 18:59 ` Thomas Monjalon
@ 2020-01-21 11:12 ` Bruce Richardson
  2020-01-21 13:22   ` Robin Jarry
  2020-01-24 10:32 ` [dpdk-dev] [PATCH v3] " Bruce Richardson
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 19+ messages in thread
From: Bruce Richardson @ 2020-01-21 11:12 UTC (permalink / raw)
  To: dev; +Cc: thomas, robin.jarry, 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>
---

V2:
- fixed file suffix
- since it's being called from meson, make this python3 only
- remove use of chdir()
- use '\n' rather than ',' between entries
---
 buildtools/list-dir-globs.py | 13 +++++++++++++
 buildtools/meson.build       |  2 +-
 drivers/meson.build          |  3 ++-
 3 files changed, 16 insertions(+), 2 deletions(-)
 create mode 100755 buildtools/list-dir-globs.py

diff --git a/buildtools/list-dir-globs.py b/buildtools/list-dir-globs.py
new file mode 100755
index 000000000..3019f5999
--- /dev/null
+++ b/buildtools/list-dir-globs.py
@@ -0,0 +1,13 @@
+#! /usr/bin/env python3
+# SPDX-License-Identifier: BSD-3-Clause
+# Copyright(c) 2020 Intel Corporation
+
+import sys
+import os
+from glob import iglob # glob iterator
+from os.path import join, relpath, isdir
+
+root = join(os.environ['MESON_SOURCE_ROOT'], os.environ['MESON_SUBDIR'])
+for path in sys.argv[1].split(','):
+  relpaths = [relpath(p, root) for p in iglob(join(root, path)) if isdir(p)]
+  print("\n".join(relpaths))
diff --git a/buildtools/meson.build b/buildtools/meson.build
index cd1d05403..0f563d89a 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.py')
 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..3ee998d80 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().split()
 
 default_cflags = machine_args
 if cc.has_argument('-Wno-format-truncation')
-- 
2.20.1


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

* Re: [dpdk-dev] [PATCH v2] build: allow using wildcards to disable drivers
  2020-01-21 11:12 ` [dpdk-dev] [PATCH v2] " Bruce Richardson
@ 2020-01-21 13:22   ` Robin Jarry
  2020-01-21 13:37     ` Bruce Richardson
  0 siblings, 1 reply; 19+ messages in thread
From: Robin Jarry @ 2020-01-21 13:22 UTC (permalink / raw)
  To: Bruce Richardson; +Cc: dev, thomas

2020-01-21, 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.py
> @@ -0,0 +1,13 @@
> +#! /usr/bin/env python3
> +# SPDX-License-Identifier: BSD-3-Clause
> +# Copyright(c) 2020 Intel Corporation
> +
> +import sys
> +import os
> +from glob import iglob # glob iterator

No need to make it explicit. People can read the description in the
official docs.

> +from os.path import join, relpath, isdir

You already imported 'os'. These functions are available under the
'os.path' namespace. No need to import them again.

> +root = join(os.environ['MESON_SOURCE_ROOT'], os.environ['MESON_SUBDIR'])
> +for path in sys.argv[1].split(','):

Directly accessing sys.argv exposes you to ugly errors when the script
is called with the wrong number of arguments. It would be better to use
argparse which will handle the error reporting for you.

> +  relpaths = [relpath(p, root) for p in iglob(join(root, path)) if isdir(p)]
> +  print("\n".join(relpaths))

Using one-liner syntax like these really makes the code hard to
understand. Explicit for loops with explicit if tests would be a lot
nicer.

Also, why use an intermediate variable to then, join each element with
'\n' and print that? You can print the matching dirs as you iterate over
them.

Have a look at my previous reply for a complete example of what I mean.

-- 
Robin

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

* Re: [dpdk-dev] [PATCH v2] build: allow using wildcards to disable drivers
  2020-01-21 13:22   ` Robin Jarry
@ 2020-01-21 13:37     ` Bruce Richardson
  2020-01-21 13:48       ` Robin Jarry
  0 siblings, 1 reply; 19+ messages in thread
From: Bruce Richardson @ 2020-01-21 13:37 UTC (permalink / raw)
  To: Robin Jarry; +Cc: dev, thomas

On Tue, Jan 21, 2020 at 02:22:38PM +0100, Robin Jarry wrote:
> 2020-01-21, 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.py
> > @@ -0,0 +1,13 @@
> > +#! /usr/bin/env python3
> > +# SPDX-License-Identifier: BSD-3-Clause
> > +# Copyright(c) 2020 Intel Corporation
> > +
> > +import sys
> > +import os
> > +from glob import iglob # glob iterator
> 
> No need to make it explicit. People can read the description in the
> official docs.

Except the fact that you yourself mistook it for a case-insensitive glob
implies that a comment is needed! :-)

> 
> > +from os.path import join, relpath, isdir
> 
> You already imported 'os'. These functions are available under the
> 'os.path' namespace. No need to import them again.
> 

Yes, using more words. Simpler syntax to call join() and isdir() rather
than os.path.join(), os.path.isdir()

> > +root = join(os.environ['MESON_SOURCE_ROOT'], os.environ['MESON_SUBDIR'])
> > +for path in sys.argv[1].split(','):
> 
> Directly accessing sys.argv exposes you to ugly errors when the script
> is called with the wrong number of arguments. It would be better to use
> argparse which will handle the error reporting for you.

This is script is to be called from the build system. I'm not worried about
incorrect numbers of args, and argparse itself seems overkill. I'll add a
check to print an error if len(argv) != 1.

> 
> > +  relpaths = [relpath(p, root) for p in iglob(join(root, path)) if isdir(p)]
> > +  print("\n".join(relpaths))
> 
> Using one-liner syntax like these really makes the code hard to
> understand. Explicit for loops with explicit if tests would be a lot
> nicer.
> 
> Also, why use an intermediate variable to then, join each element with
> '\n' and print that? You can print the matching dirs as you iterate over
> them.
> 
> Have a look at my previous reply for a complete example of what I mean.
> 

Just keeping the code short. I actually had originally got the print join
in the same line as the list comprehension but that even I felt was a bit
unreadable - even if it did leave a nice short script!
I'll expand to explicit for loop in V3.

/Bruce

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

* Re: [dpdk-dev] [PATCH v2] build: allow using wildcards to disable drivers
  2020-01-21 13:37     ` Bruce Richardson
@ 2020-01-21 13:48       ` Robin Jarry
  0 siblings, 0 replies; 19+ messages in thread
From: Robin Jarry @ 2020-01-21 13:48 UTC (permalink / raw)
  To: Bruce Richardson; +Cc: dev, thomas

2020-01-21, Bruce Richardson:
> > > +from glob import iglob # glob iterator
> > 
> > No need to make it explicit. People can read the description in the
> > official docs.
> 
> Except the fact that you yourself mistook it for a case-insensitive glob
> implies that a comment is needed! :-)

The fact that I should check things before I make comments should not
force people to add comments next to imports :-)

> This is script is to be called from the build system. I'm not worried about
> incorrect numbers of args, and argparse itself seems overkill. I'll add a
> check to print an error if len(argv) != 1.

Fair enough. However, argparse is never overkill. Using it is not going
to slow down your script.

> Just keeping the code short. I actually had originally got the print join
> in the same line as the list comprehension but that even I felt was a bit
> unreadable - even if it did leave a nice short script!

This is not perl^Wa competition :-)

-- 
Robin

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

* [dpdk-dev] [PATCH v3] build: allow using wildcards to disable drivers
  2020-01-20 17:37 [dpdk-dev] [PATCH] build: allow using wildcards to disable drivers Bruce Richardson
  2020-01-20 18:59 ` Thomas Monjalon
  2020-01-21 11:12 ` [dpdk-dev] [PATCH v2] " Bruce Richardson
@ 2020-01-24 10:32 ` Bruce Richardson
  2020-01-24 12:28   ` Robin Jarry
  2020-01-24 15:10 ` [dpdk-dev] [PATCH v4] " Bruce Richardson
  2020-01-27 14:28 ` [dpdk-dev] [PATCH v5] " Bruce Richardson
  4 siblings, 1 reply; 19+ messages in thread
From: Bruce Richardson @ 2020-01-24 10:32 UTC (permalink / raw)
  To: dev; +Cc: thomas, robin.jarry, 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>
---

V3:
- added check for correct number of params
- replaced list comprehension with loops for simplicity
- allow running without meson environmental vars set (for easier testing)

V2:
- fixed file suffix
- since it's being called from meson, make this python3 only
- remove use of chdir()
- use '\n' rather than ',' between entries
---
 buildtools/list-dir-globs.py | 21 +++++++++++++++++++++
 buildtools/meson.build       |  2 +-
 drivers/meson.build          |  3 ++-
 3 files changed, 24 insertions(+), 2 deletions(-)
 create mode 100755 buildtools/list-dir-globs.py

diff --git a/buildtools/list-dir-globs.py b/buildtools/list-dir-globs.py
new file mode 100755
index 000000000..b9a536869
--- /dev/null
+++ b/buildtools/list-dir-globs.py
@@ -0,0 +1,21 @@
+#! /usr/bin/env python3
+# SPDX-License-Identifier: BSD-3-Clause
+# Copyright(c) 2020 Intel Corporation
+
+import sys
+import os
+from glob import iglob
+from os.path import join, relpath, isdir
+
+if len(sys.argv) != 2:
+  print("Usage: {0} <path-glob>[,<path-glob>[,...]]".format(sys.argv[0]))
+  sys.exit(1)
+
+root = '.'
+if 'MESON_SOURCE_ROOT' in os.environ and 'MESON_SUBDIR' in os.environ:
+  root = join(os.environ['MESON_SOURCE_ROOT'], os.environ['MESON_SUBDIR'])
+
+for path in sys.argv[1].split(','):
+  for p in iglob(join(root, path)):
+    if isdir(p):
+      print(relpath(p))
diff --git a/buildtools/meson.build b/buildtools/meson.build
index cd1d05403..0f563d89a 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.py')
 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..3ee998d80 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().split()
 
 default_cflags = machine_args
 if cc.has_argument('-Wno-format-truncation')
-- 
2.20.1


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

* Re: [dpdk-dev] [PATCH v3] build: allow using wildcards to disable drivers
  2020-01-24 10:32 ` [dpdk-dev] [PATCH v3] " Bruce Richardson
@ 2020-01-24 12:28   ` Robin Jarry
  2020-01-24 14:57     ` Richardson, Bruce
  0 siblings, 1 reply; 19+ messages in thread
From: Robin Jarry @ 2020-01-24 12:28 UTC (permalink / raw)
  To: Bruce Richardson; +Cc: dev, thomas

2020-01-24, 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]
> +from os.path import join, relpath, isdir

As a general rule, it is better to only import one symbol per line. This
makes subsequent patches easier to read.

> +if len(sys.argv) != 2:
> +  print("Usage: {0} <path-glob>[,<path-glob>[,...]]".format(sys.argv[0]))
> +  sys.exit(1)

PEP8 advises to use 4 spaces per indentation level. This is the
indentation style adopted by all other python scripts in dpdk (see
doc/guides/contributing/coding_style.rst). Could you fix that?

> +root = '.'
> +if 'MESON_SOURCE_ROOT' in os.environ and 'MESON_SUBDIR' in os.environ:
> +  root = join(os.environ['MESON_SOURCE_ROOT'], os.environ['MESON_SUBDIR'])

You can do simpler and shorter:

  root = join(os.getenv('MESON_SOURCE_ROOT', ''),
              os.getenv('MESON_SUBDIR', ''), '.')

Sorry to pester you with all this, but python code in DPDK really needs
some loving :-)

-- 
Robin

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

* Re: [dpdk-dev] [PATCH v3] build: allow using wildcards to disable drivers
  2020-01-24 12:28   ` Robin Jarry
@ 2020-01-24 14:57     ` Richardson, Bruce
  0 siblings, 0 replies; 19+ messages in thread
From: Richardson, Bruce @ 2020-01-24 14:57 UTC (permalink / raw)
  To: Robin Jarry; +Cc: dev, thomas



> -----Original Message-----
> From: Robin Jarry <robin.jarry@6wind.com>
> Sent: Friday, January 24, 2020 12:28 PM
> To: Richardson, Bruce <bruce.richardson@intel.com>
> Cc: dev@dpdk.org; thomas@monjalon.net
> Subject: Re: [PATCH v3] build: allow using wildcards to disable drivers
> 
> 2020-01-24, 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]
> > +from os.path import join, relpath, isdir
> 
> As a general rule, it is better to only import one symbol per line. This
> makes subsequent patches easier to read.
> 
> > +if len(sys.argv) != 2:
> > +  print("Usage: {0}
> > +<path-glob>[,<path-glob>[,...]]".format(sys.argv[0]))
> > +  sys.exit(1)
> 
> PEP8 advises to use 4 spaces per indentation level. This is the
> indentation style adopted by all other python scripts in dpdk (see
> doc/guides/contributing/coding_style.rst). Could you fix that?
> 
> > +root = '.'
> > +if 'MESON_SOURCE_ROOT' in os.environ and 'MESON_SUBDIR' in os.environ:
> > +  root = join(os.environ['MESON_SOURCE_ROOT'],
> > +os.environ['MESON_SUBDIR'])
> 
> You can do simpler and shorter:
> 
>   root = join(os.getenv('MESON_SOURCE_ROOT', ''),
>               os.getenv('MESON_SUBDIR', ''), '.')

I think a little shorter again by putting the '.' as the default value for one of the env vars.


> 
> Sorry to pester you with all this, but python code in DPDK really needs
> some loving :-)
> 
> --
> Robin

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

* [dpdk-dev] [PATCH v4] build: allow using wildcards to disable drivers
  2020-01-20 17:37 [dpdk-dev] [PATCH] build: allow using wildcards to disable drivers Bruce Richardson
                   ` (2 preceding siblings ...)
  2020-01-24 10:32 ` [dpdk-dev] [PATCH v3] " Bruce Richardson
@ 2020-01-24 15:10 ` Bruce Richardson
  2020-01-24 15:34   ` Robin Jarry
                     ` (2 more replies)
  2020-01-27 14:28 ` [dpdk-dev] [PATCH v5] " Bruce Richardson
  4 siblings, 3 replies; 19+ messages in thread
From: Bruce Richardson @ 2020-01-24 15:10 UTC (permalink / raw)
  To: dev; +Cc: thomas, robin.jarry, 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>
---

V4:
- get pep8/pycodestyle compliance
- simplify getting the root directory

V3:
- added check for correct number of params
- replaced list comprehension with loops for simplicity
- allow running without meson environmental vars set (for easier testing)

V2:
- fixed file suffix
- since it's being called from meson, make this python3 only
- remove use of chdir()
- use '\n' rather than ',' between entries
---
 buildtools/list-dir-globs.py | 19 +++++++++++++++++++
 buildtools/meson.build       |  2 +-
 drivers/meson.build          |  3 ++-
 3 files changed, 22 insertions(+), 2 deletions(-)
 create mode 100755 buildtools/list-dir-globs.py

diff --git a/buildtools/list-dir-globs.py b/buildtools/list-dir-globs.py
new file mode 100755
index 000000000..80b5e801f
--- /dev/null
+++ b/buildtools/list-dir-globs.py
@@ -0,0 +1,19 @@
+#! /usr/bin/env python3
+# SPDX-License-Identifier: BSD-3-Clause
+# Copyright(c) 2020 Intel Corporation
+
+import sys
+import os
+from glob import iglob
+
+if len(sys.argv) != 2:
+    print("Usage: {0} <path-glob>[,<path-glob>[,...]]".format(sys.argv[0]))
+    sys.exit(1)
+
+root = os.path.join(os.getenv('MESON_SOURCE_ROOT', '.'),
+                    os.getenv('MESON_SUBDIR', '.'))
+
+for path in sys.argv[1].split(','):
+    for p in iglob(os.path.join(root, path)):
+        if os.path.isdir(p):
+            print(os.path.relpath(p))
diff --git a/buildtools/meson.build b/buildtools/meson.build
index cd1d05403..0f563d89a 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.py')
 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..3ee998d80 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().split()
 
 default_cflags = machine_args
 if cc.has_argument('-Wno-format-truncation')
-- 
2.20.1


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

* Re: [dpdk-dev] [PATCH v4] build: allow using wildcards to disable drivers
  2020-01-24 15:10 ` [dpdk-dev] [PATCH v4] " Bruce Richardson
@ 2020-01-24 15:34   ` Robin Jarry
  2020-01-27 11:26   ` Andrzej Ostruszka
  2020-01-27 12:12   ` Luca Boccassi
  2 siblings, 0 replies; 19+ messages in thread
From: Robin Jarry @ 2020-01-24 15:34 UTC (permalink / raw)
  To: Bruce Richardson; +Cc: dev, thomas

2020-01-24, 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>

Looks good to me.

Reviewed-by: Robin Jarry <robin.jarry@6wind.com>

-- 
Robin

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

* Re: [dpdk-dev] [PATCH v4] build: allow using wildcards to disable drivers
  2020-01-24 15:10 ` [dpdk-dev] [PATCH v4] " Bruce Richardson
  2020-01-24 15:34   ` Robin Jarry
@ 2020-01-27 11:26   ` Andrzej Ostruszka
  2020-01-27 12:15     ` Bruce Richardson
  2020-01-27 12:12   ` Luca Boccassi
  2 siblings, 1 reply; 19+ messages in thread
From: Andrzej Ostruszka @ 2020-01-27 11:26 UTC (permalink / raw)
  To: dev

On 1/24/20 4:10 PM, Bruce Richardson wrote:
> 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

Maybe documenting that in 'build-sdk-meson.txt' would be beneficial?

With regards
Andrzej Ostruszka

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

* Re: [dpdk-dev] [PATCH v4] build: allow using wildcards to disable drivers
  2020-01-24 15:10 ` [dpdk-dev] [PATCH v4] " Bruce Richardson
  2020-01-24 15:34   ` Robin Jarry
  2020-01-27 11:26   ` Andrzej Ostruszka
@ 2020-01-27 12:12   ` Luca Boccassi
  2 siblings, 0 replies; 19+ messages in thread
From: Luca Boccassi @ 2020-01-27 12:12 UTC (permalink / raw)
  To: Bruce Richardson, dev

On Fri, 2020-01-24 at 15:10 +0000, Bruce Richardson wrote:
> 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
> >
> ---
> 
> V4:
> - get pep8/pycodestyle compliance
> - simplify getting the root directory
> 
> V3:
> - added check for correct number of params
> - replaced list comprehension with loops for simplicity
> - allow running without meson environmental vars set (for easier
> testing)
> 
> V2:
> - fixed file suffix
> - since it's being called from meson, make this python3 only
> - remove use of chdir()
> - use '\n' rather than ',' between entries

Acked-by: Luca Boccassi <bluca@debian.org>

-- 
Kind regards,
Luca Boccassi

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

* Re: [dpdk-dev] [PATCH v4] build: allow using wildcards to disable drivers
  2020-01-27 11:26   ` Andrzej Ostruszka
@ 2020-01-27 12:15     ` Bruce Richardson
  0 siblings, 0 replies; 19+ messages in thread
From: Bruce Richardson @ 2020-01-27 12:15 UTC (permalink / raw)
  To: Andrzej Ostruszka; +Cc: dev

On Mon, Jan 27, 2020 at 12:26:53PM +0100, Andrzej Ostruszka wrote:
> On 1/24/20 4:10 PM, Bruce Richardson wrote:
> > 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
> 
> Maybe documenting that in 'build-sdk-meson.txt' would be beneficial?
> 
Yes, yes it would. Thanks for the suggestion.

/Bruce

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

* [dpdk-dev] [PATCH v5] build: allow using wildcards to disable drivers
  2020-01-20 17:37 [dpdk-dev] [PATCH] build: allow using wildcards to disable drivers Bruce Richardson
                   ` (3 preceding siblings ...)
  2020-01-24 15:10 ` [dpdk-dev] [PATCH v4] " Bruce Richardson
@ 2020-01-27 14:28 ` Bruce Richardson
  2020-02-06  2:26   ` Thomas Monjalon
  4 siblings, 1 reply; 19+ messages in thread
From: Bruce Richardson @ 2020-01-27 14:28 UTC (permalink / raw)
  To: dev; +Cc: thomas, robin.jarry, amo, bluca, 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>
Reviewed-by: Robin Jarry <robin.jarry@6wind.com>
Acked-by: Luca Boccassi <bluca@debian.org>
---
V5:
- add a couple of lines to the doc as an example of using this

V4:
- get pep8/pycodestyle compliance
- simplify getting the root directory

V3:
- added check for correct number of params
- replaced list comprehension with loops for simplicity
- allow running without meson environmental vars set (for easier testing)

V2:
- fixed file suffix
- since it's being called from meson, make this python3 only
- remove use of chdir()
- use '\n' rather than ',' between entries
---
 buildtools/list-dir-globs.py | 19 +++++++++++++++++++
 buildtools/meson.build       |  2 +-
 doc/build-sdk-meson.txt      |  5 ++++-
 drivers/meson.build          |  3 ++-
 4 files changed, 26 insertions(+), 3 deletions(-)
 create mode 100755 buildtools/list-dir-globs.py

diff --git a/buildtools/list-dir-globs.py b/buildtools/list-dir-globs.py
new file mode 100755
index 000000000..80b5e801f
--- /dev/null
+++ b/buildtools/list-dir-globs.py
@@ -0,0 +1,19 @@
+#! /usr/bin/env python3
+# SPDX-License-Identifier: BSD-3-Clause
+# Copyright(c) 2020 Intel Corporation
+
+import sys
+import os
+from glob import iglob
+
+if len(sys.argv) != 2:
+    print("Usage: {0} <path-glob>[,<path-glob>[,...]]".format(sys.argv[0]))
+    sys.exit(1)
+
+root = os.path.join(os.getenv('MESON_SOURCE_ROOT', '.'),
+                    os.getenv('MESON_SUBDIR', '.'))
+
+for path in sys.argv[1].split(','):
+    for p in iglob(os.path.join(root, path)):
+        if os.path.isdir(p):
+            print(os.path.relpath(p))
diff --git a/buildtools/meson.build b/buildtools/meson.build
index cd1d05403..0f563d89a 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.py')
 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/doc/build-sdk-meson.txt b/doc/build-sdk-meson.txt
index fc7fe37b5..319a19ef6 100644
--- a/doc/build-sdk-meson.txt
+++ b/doc/build-sdk-meson.txt
@@ -84,7 +84,10 @@ Project-specific options are passed used -Doption=value::
 
 	meson -Dmachine=default  # use builder-independent baseline -march
 
-Examples of setting the same options using meson configure::
+	meson -Ddisable_drivers=event/*,net/tap  # disable tap driver and all
+					# eventdev PMDs for a smaller build
+
+Examples of setting some of the same options using meson configure::
 
 	meson configure -Dwerror=true
 
diff --git a/drivers/meson.build b/drivers/meson.build
index 29708cc2b..3ee998d80 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().split()
 
 default_cflags = machine_args
 if cc.has_argument('-Wno-format-truncation')
-- 
2.20.1


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

* Re: [dpdk-dev] [PATCH v5] build: allow using wildcards to disable drivers
  2020-01-27 14:28 ` [dpdk-dev] [PATCH v5] " Bruce Richardson
@ 2020-02-06  2:26   ` Thomas Monjalon
  0 siblings, 0 replies; 19+ messages in thread
From: Thomas Monjalon @ 2020-02-06  2:26 UTC (permalink / raw)
  To: Bruce Richardson; +Cc: dev, robin.jarry, amo, bluca

27/01/2020 15:28, 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>
> Reviewed-by: Robin Jarry <robin.jarry@6wind.com>
> Acked-by: Luca Boccassi <bluca@debian.org>

Applied, thanks




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

end of thread, other threads:[~2020-02-06  2:26 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-01-20 17:37 [dpdk-dev] [PATCH] build: allow using wildcards to disable drivers Bruce Richardson
2020-01-20 18:59 ` Thomas Monjalon
2020-01-21  9:11   ` Robin Jarry
2020-01-21 10:17     ` Bruce Richardson
2020-01-21 10:13   ` Bruce Richardson
2020-01-21 11:12 ` [dpdk-dev] [PATCH v2] " Bruce Richardson
2020-01-21 13:22   ` Robin Jarry
2020-01-21 13:37     ` Bruce Richardson
2020-01-21 13:48       ` Robin Jarry
2020-01-24 10:32 ` [dpdk-dev] [PATCH v3] " Bruce Richardson
2020-01-24 12:28   ` Robin Jarry
2020-01-24 14:57     ` Richardson, Bruce
2020-01-24 15:10 ` [dpdk-dev] [PATCH v4] " Bruce Richardson
2020-01-24 15:34   ` Robin Jarry
2020-01-27 11:26   ` Andrzej Ostruszka
2020-01-27 12:15     ` Bruce Richardson
2020-01-27 12:12   ` Luca Boccassi
2020-01-27 14:28 ` [dpdk-dev] [PATCH v5] " Bruce Richardson
2020-02-06  2:26   ` Thomas Monjalon

DPDK patches and discussions

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://inbox.dpdk.org/dev/0 dev/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 dev dev/ https://inbox.dpdk.org/dev \
		dev@dpdk.org
	public-inbox-index dev

Example config snippet for mirrors.
Newsgroup available over NNTP:
	nntp://inbox.dpdk.org/inbox.dpdk.dev


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git