diff mbox series

[2/2] build: fail if explicitly requested driver is unbuildable

Message ID 20230901142332.588856-2-bruce.richardson@intel.com (mailing list archive)
State Accepted, archived
Delegated to: David Marchand
Headers
Series [1/2] build: fail if explicitly requested lib is unbuildable |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/loongarch-compilation success Compilation OK
ci/loongarch-unit-testing success Unit Testing PASS
ci/Intel-compilation success Compilation OK
ci/intel-Testing success Testing PASS
ci/iol-compile-amd64-testing success Testing PASS
ci/iol-sample-apps-testing success Testing PASS
ci/github-robot: build success github build: passed
ci/intel-Functional success Functional PASS
ci/iol-compile-arm64-testing success Testing PASS
ci/iol-mellanox-Performance success Performance Testing PASS
ci/iol-broadcom-Performance success Performance Testing PASS
ci/iol-intel-Performance success Performance Testing PASS
ci/iol-broadcom-Functional success Functional Testing PASS
ci/iol-intel-Functional success Functional Testing PASS
ci/iol-unit-amd64-testing success Testing PASS
ci/iol-unit-arm64-testing success Testing PASS

Commit Message

Bruce Richardson Sept. 1, 2023, 2:23 p.m. UTC 
  When the user passes a list of desired drivers to build via the
"enable_drivers" option, the expectation is that those drivers should be
part of the build. However, if those drivers have either external or
internal dependencies, they still may be silently disabled, for example:
running "meson setup -Denable_drivers=net/iavf build" will successfully
run, but the iavf net driver will not be configured as "common/iavf" is
missing.

We can fix this by setting a flag to indicate when the drivers are
specified via an enable_drivers flag. However, unlike when erroring out
on missing libs, we don't error out if a driver in unbuildable, unless
the driver name explicitly appears in the "enable_drivers" list. This is
implemented this way to ensure that wildcarding still works. For
example: we still want to allow "meson setup -Denable_drivers=net/*" to
work, configuring only the buildable network drivers. While it's true
that this additional restriction may cause some builds to pass when they
should fail, e.g. if the wildcard refers only to a single driver,
implementing things this way avoids massive amounts of complexity, and
is still an improvement on the status-quo.

Suggested-by: Anatoly Burakov <anatoly.burakov@intel.com>
Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
---
 drivers/meson.build | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

Comments

Morten Brørup Sept. 1, 2023, 3:41 p.m. UTC | #1 
> From: Bruce Richardson [mailto:bruce.richardson@intel.com]
> Sent: Friday, 1 September 2023 16.24
> 
> When the user passes a list of desired drivers to build via the
> "enable_drivers" option, the expectation is that those drivers should be
> part of the build. However, if those drivers have either external or
> internal dependencies, they still may be silently disabled, for example:
> running "meson setup -Denable_drivers=net/iavf build" will successfully
> run, but the iavf net driver will not be configured as "common/iavf" is
> missing.
> 
> We can fix this by setting a flag to indicate when the drivers are
> specified via an enable_drivers flag. However, unlike when erroring out
> on missing libs, we don't error out if a driver in unbuildable, unless
> the driver name explicitly appears in the "enable_drivers" list. This is
> implemented this way to ensure that wildcarding still works. For
> example: we still want to allow "meson setup -Denable_drivers=net/*" to
> work, configuring only the buildable network drivers. While it's true
> that this additional restriction may cause some builds to pass when they
> should fail, e.g. if the wildcard refers only to a single driver,
> implementing things this way avoids massive amounts of complexity, and
> is still an improvement on the status-quo.
> 
> Suggested-by: Anatoly Burakov <anatoly.burakov@intel.com>
> Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> ---

Acked-by: Morten Brørup <mb@smartsharesystems.com>
Patrick Robb Sept. 2, 2023, 5:24 p.m. UTC | #2 
ARM Ampere server test fails on this patch are lab infra-failures (I did
some updates on the server yesterday) and they can be ignored.
Bruce Richardson Sept. 14, 2023, 9:42 a.m. UTC | #3 
On Sat, Sep 02, 2023 at 01:24:18PM -0400, Patrick Robb wrote:
>    ARM Ampere server test fails on this patch are lab infra-failures (I
>    did some updates on the server yesterday) and they can be ignored.

Can a retest be run on these now, to clear the errors in patchwork, or have
the issued been fixed?
diff mbox series

Patch

diff --git a/drivers/meson.build b/drivers/meson.build
index 417b64b8fc..8c775bbe62 100644
--- a/drivers/meson.build
+++ b/drivers/meson.build
@@ -41,7 +41,9 @@  disable_drivers = run_command(list_dir_globs, disable_drivers, check: true).stdo
 # add cmdline enabled drivers and meson enabled drivers together
 enable_drivers = ',' + get_option('enable_drivers')
 enable_drivers = run_command(list_dir_globs, enable_drivers, check: true).stdout().split()
+require_drivers = true
 if enable_drivers.length() == 0
+    require_drivers = false
     enable_drivers = run_command(list_dir_globs, '*/*', check: true).stdout().split()
 endif
 
@@ -155,6 +157,12 @@  foreach subpath:subdirs
                 build = false
                 reason = 'requires IOVA in mbuf (set enable_iova_as_pa option)'
             endif
+            # error out if we can't build a driver and that driver was explicitly requested,
+            # i.e. not via wildcard.
+            if not build and require_drivers and get_option('enable_drivers').contains(drv_path)
+                error('Cannot build explicitly requested driver "@0@".\n'.format(drv_path)
+                        +'\tReason: ' + reason)
+            endif
 
             # get dependency objs from strings
             shared_deps = ext_deps
@@ -171,6 +179,12 @@  foreach subpath:subdirs
                     endif
                     message('Disabling @1@ [@2@]: missing internal dependency "@0@"'
                             .format(d, name, 'drivers/' + drv_path))
+                    # error out if we can't build a driver and that driver was explicitly
+                    # requested, i.e. not via wildcard.
+                    if require_drivers and get_option('enable_drivers').contains(drv_path)
+                        error('Cannot build explicitly requested driver "@0@".\n'.format(drv_path)
+                                +'\tPlease enable missing dependency "@0@"'.format(d))
+                    endif
                 else
                     shared_deps += [get_variable('shared_rte_' + d)]
                     static_deps += [get_variable('static_rte_' + d)]