[1/2] build: fail if explicitly requested lib is unbuildable
Checks
Commit Message
When the user passes a list of desired libraries to build via the
"enable_libs" option, the expectation is that those libraries should be
part of the build. However, if those libs have either external or
internal dependencies, they still may be silently disabled, for example:
running "meson setup -Denable_libs=security build" will successfully
run, but the security lib will not be configured as "cryptodev" is
missing.
We can fix this by setting a flag to indicate when the libraries are
specified via an enable_libs flag. If so, then we error out when a
library is unbuildable, giving a suitable error message. For the above
example case, the "meson setup" run fails with:
Message: Disabling security [lib/security]: missing internal dependency "cryptodev"
lib/meson.build:218:16: ERROR: Problem encountered: Cannot build explicitly requested lib "security".
Please add missing dependency "cryptodev" to "enable_libs" option
Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
---
lib/meson.build | 10 ++++++++++
1 file changed, 10 insertions(+)
Comments
On Fri, Sep 1, 2023 at 4:23 PM Bruce Richardson
<bruce.richardson@intel.com> wrote:
>
> When the user passes a list of desired libraries to build via the
> "enable_libs" option, the expectation is that those libraries should be
> part of the build. However, if those libs have either external or
> internal dependencies, they still may be silently disabled, for example:
> running "meson setup -Denable_libs=security build" will successfully
> run, but the security lib will not be configured as "cryptodev" is
> missing.
>
> We can fix this by setting a flag to indicate when the libraries are
> specified via an enable_libs flag. If so, then we error out when a
> library is unbuildable, giving a suitable error message. For the above
> example case, the "meson setup" run fails with:
>
> Message: Disabling security [lib/security]: missing internal dependency "cryptodev"
>
> lib/meson.build:218:16: ERROR: Problem encountered: Cannot build explicitly requested lib "security".
> Please add missing dependency "cryptodev" to "enable_libs" option
>
> Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
I remember setting a enable_drivers=net/af_xdp on a system lacking
libbpf-devel and not getting warned about its absence.
So I suspect enable_drivers is affected by the same issue.
On Fri, Sep 1, 2023 at 4:29 PM David Marchand <david.marchand@redhat.com> wrote:
>
> On Fri, Sep 1, 2023 at 4:23 PM Bruce Richardson
> <bruce.richardson@intel.com> wrote:
> >
> > When the user passes a list of desired libraries to build via the
> > "enable_libs" option, the expectation is that those libraries should be
> > part of the build. However, if those libs have either external or
> > internal dependencies, they still may be silently disabled, for example:
> > running "meson setup -Denable_libs=security build" will successfully
> > run, but the security lib will not be configured as "cryptodev" is
> > missing.
> >
> > We can fix this by setting a flag to indicate when the libraries are
> > specified via an enable_libs flag. If so, then we error out when a
> > library is unbuildable, giving a suitable error message. For the above
> > example case, the "meson setup" run fails with:
> >
> > Message: Disabling security [lib/security]: missing internal dependency "cryptodev"
> >
> > lib/meson.build:218:16: ERROR: Problem encountered: Cannot build explicitly requested lib "security".
> > Please add missing dependency "cryptodev" to "enable_libs" option
> >
> > Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
>
> I remember setting a enable_drivers=net/af_xdp on a system lacking
> libbpf-devel and not getting warned about its absence.
> So I suspect enable_drivers is affected by the same issue.
Arf sorry, did not see it was a two patch series :-).
On Fri, Sep 01, 2023 at 04:30:56PM +0200, David Marchand wrote:
> On Fri, Sep 1, 2023 at 4:29 PM David Marchand <david.marchand@redhat.com> wrote:
> >
> > On Fri, Sep 1, 2023 at 4:23 PM Bruce Richardson
> > <bruce.richardson@intel.com> wrote:
> > >
> > > When the user passes a list of desired libraries to build via the
> > > "enable_libs" option, the expectation is that those libraries should be
> > > part of the build. However, if those libs have either external or
> > > internal dependencies, they still may be silently disabled, for example:
> > > running "meson setup -Denable_libs=security build" will successfully
> > > run, but the security lib will not be configured as "cryptodev" is
> > > missing.
> > >
> > > We can fix this by setting a flag to indicate when the libraries are
> > > specified via an enable_libs flag. If so, then we error out when a
> > > library is unbuildable, giving a suitable error message. For the above
> > > example case, the "meson setup" run fails with:
> > >
> > > Message: Disabling security [lib/security]: missing internal dependency "cryptodev"
> > >
> > > lib/meson.build:218:16: ERROR: Problem encountered: Cannot build explicitly requested lib "security".
> > > Please add missing dependency "cryptodev" to "enable_libs" option
> > >
> > > Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> >
> > I remember setting a enable_drivers=net/af_xdp on a system lacking
> > libbpf-devel and not getting warned about its absence.
> > So I suspect enable_drivers is affected by the same issue.
>
> Arf sorry, did not see it was a two patch series :-).
>
No problem.
At least your response confirms to me that this is indeed a common issue,
that could do with a solution. As I explain in patch 2, fixing for drivers
is a little trickier than for libs, as for drivers we really need to take
account of wildcards to avoid breaking people's existing builds. [Or, at
minimum, my own builds! :-)].
BTW: while we could look to handle wildcards for libs, I think their use is
probably much more limited there, and I decided to go with the simplest
possible solution instead.
/Bruce
> From: Bruce Richardson [mailto:bruce.richardson@intel.com]
> Sent: Friday, 1 September 2023 16.24
>
> When the user passes a list of desired libraries to build via the
> "enable_libs" option, the expectation is that those libraries should be
> part of the build. However, if those libs have either external or
> internal dependencies, they still may be silently disabled, for example:
> running "meson setup -Denable_libs=security build" will successfully
> run, but the security lib will not be configured as "cryptodev" is
> missing.
>
> We can fix this by setting a flag to indicate when the libraries are
> specified via an enable_libs flag. If so, then we error out when a
> library is unbuildable, giving a suitable error message. For the above
> example case, the "meson setup" run fails with:
>
> Message: Disabling security [lib/security]: missing internal dependency
> "cryptodev"
>
> lib/meson.build:218:16: ERROR: Problem encountered: Cannot build explicitly
> requested lib "security".
> Please add missing dependency "cryptodev" to "enable_libs" option
>
> Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
Acked-by: Morten Brørup <mb@smartsharesystems.com>
On Fri, Sep 1, 2023 at 4:44 PM Bruce Richardson
<bruce.richardson@intel.com> wrote:
>
> On Fri, Sep 01, 2023 at 04:30:56PM +0200, David Marchand wrote:
> > On Fri, Sep 1, 2023 at 4:29 PM David Marchand <david.marchand@redhat.com> wrote:
> > >
> > > On Fri, Sep 1, 2023 at 4:23 PM Bruce Richardson
> > > <bruce.richardson@intel.com> wrote:
> > > >
> > > > When the user passes a list of desired libraries to build via the
> > > > "enable_libs" option, the expectation is that those libraries should be
> > > > part of the build. However, if those libs have either external or
> > > > internal dependencies, they still may be silently disabled, for example:
> > > > running "meson setup -Denable_libs=security build" will successfully
> > > > run, but the security lib will not be configured as "cryptodev" is
> > > > missing.
> > > >
> > > > We can fix this by setting a flag to indicate when the libraries are
> > > > specified via an enable_libs flag. If so, then we error out when a
> > > > library is unbuildable, giving a suitable error message. For the above
> > > > example case, the "meson setup" run fails with:
> > > >
> > > > Message: Disabling security [lib/security]: missing internal dependency "cryptodev"
> > > >
> > > > lib/meson.build:218:16: ERROR: Problem encountered: Cannot build explicitly requested lib "security".
> > > > Please add missing dependency "cryptodev" to "enable_libs" option
> > > >
> > > > Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> > >
> > > I remember setting a enable_drivers=net/af_xdp on a system lacking
> > > libbpf-devel and not getting warned about its absence.
> > > So I suspect enable_drivers is affected by the same issue.
> >
> > Arf sorry, did not see it was a two patch series :-).
> >
> No problem.
>
> At least your response confirms to me that this is indeed a common issue,
> that could do with a solution. As I explain in patch 2, fixing for drivers
> is a little trickier than for libs, as for drivers we really need to take
> account of wildcards to avoid breaking people's existing builds. [Or, at
> minimum, my own builds! :-)].
>
> BTW: while we could look to handle wildcards for libs, I think their use is
> probably much more limited there, and I decided to go with the simplest
> possible solution instead.
We have categories (classes) of drivers, and for them, I can imagine
people using wildcards.
But for libraries, I don't see how wildcards could be used, so I think
we can ignore them for now (until proven wrong ;-).
Thanks Bruce, this series is also enhancing the situation for a user.
Like when wanting to enable explicity a driver or a library: the
explicit error message on the missing dependency is easier to catch
than scrolling back/parsing meson output.
I did not review in detail, but it lgtm for my few tests.
For the series,
Acked-by: David Marchand <david.marchand@redhat.com>
Recheck-request: iol-unit-arm64-testing, iol-intel-Performance
On Fri, Sep 01, 2023 at 03:23:31PM +0100, Bruce Richardson wrote:
> When the user passes a list of desired libraries to build via the
> "enable_libs" option, the expectation is that those libraries should be
> part of the build. However, if those libs have either external or
> internal dependencies, they still may be silently disabled, for example:
> running "meson setup -Denable_libs=security build" will successfully
> run, but the security lib will not be configured as "cryptodev" is
> missing.
>
> We can fix this by setting a flag to indicate when the libraries are
> specified via an enable_libs flag. If so, then we error out when a
> library is unbuildable, giving a suitable error message. For the above
> example case, the "meson setup" run fails with:
>
> Message: Disabling security [lib/security]: missing internal dependency "cryptodev"
>
> lib/meson.build:218:16: ERROR: Problem encountered: Cannot build explicitly requested lib "security".
> Please add missing dependency "cryptodev" to "enable_libs" option
>
> Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> ---
> lib/meson.build | 10 ++++++++++
> 1 file changed, 10 insertions(+)
>
Recheck-request: iol-unit-arm64-testing, iol-intel-Performance
I'm informed now that there were infra issues which should now have been fixed,
so requesting a second re-check.
/Bruce
On Fri, Sep 01, 2023 at 03:23:31PM +0100, Bruce Richardson wrote:
> When the user passes a list of desired libraries to build via the
> "enable_libs" option, the expectation is that those libraries should be
> part of the build. However, if those libs have either external or
> internal dependencies, they still may be silently disabled, for example:
> running "meson setup -Denable_libs=security build" will successfully
> run, but the security lib will not be configured as "cryptodev" is
> missing.
>
> We can fix this by setting a flag to indicate when the libraries are
> specified via an enable_libs flag. If so, then we error out when a
> library is unbuildable, giving a suitable error message. For the above
> example case, the "meson setup" run fails with:
>
> Message: Disabling security [lib/security]: missing internal dependency "cryptodev"
>
> lib/meson.build:218:16: ERROR: Problem encountered: Cannot build explicitly requested lib "security".
> Please add missing dependency "cryptodev" to "enable_libs" option
>
> Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> ---
> lib/meson.build | 10 ++++++++++
> 1 file changed, 10 insertions(+)
>
> diff --git a/lib/meson.build b/lib/meson.build
> index 099b0ed18a..cf4aa63630 100644
> --- a/lib/meson.build
> +++ b/lib/meson.build
> @@ -131,7 +131,9 @@ endforeach
> disable_libs = run_command(list_dir_globs, get_option('disable_libs'), check: true).stdout().split()
>
> enable_libs = run_command(list_dir_globs, get_option('enable_libs'), check: true).stdout().split()
> +require_libs = true
> if enable_libs.length() == 0
> + require_libs = false
> enable_libs += optional_libs
> endif
> enable_libs += always_enable
> @@ -189,6 +191,10 @@ foreach l:libraries
>
> if build
> subdir(l)
> + if not build and require_libs
> + error('Cannot build explicitly requested lib "@0@".\n'.format(name)
> + +'\tReason: ' + reason)
> + endif
> endif
> if name != l
> warning('Library name, "@0@", and directory name, "@1@", do not match'.format(name, l))
> @@ -208,6 +214,10 @@ foreach l:libraries
> endif
> message('Disabling @1@ [@2@]: missing internal dependency "@0@"'
> .format(d, name, 'lib/' + l))
> + if require_libs
> + error('Cannot build explicitly requested lib "@0@".\n'.format(name)
> + + '\tPlease add missing dependency "@0@" to "enable_libs" option'.format(d))
> + endif
> else
> shared_deps += [get_variable('shared_rte_' + d)]
> static_deps += [get_variable('static_rte_' + d)]
> --
> 2.39.2
>
On Fri, Sep 1, 2023 at 4:23 PM Bruce Richardson
<bruce.richardson@intel.com> wrote:
>
> When the user passes a list of desired libraries to build via the
> "enable_libs" option, the expectation is that those libraries should be
> part of the build. However, if those libs have either external or
> internal dependencies, they still may be silently disabled, for example:
> running "meson setup -Denable_libs=security build" will successfully
> run, but the security lib will not be configured as "cryptodev" is
> missing.
>
> We can fix this by setting a flag to indicate when the libraries are
> specified via an enable_libs flag. If so, then we error out when a
> library is unbuildable, giving a suitable error message. For the above
> example case, the "meson setup" run fails with:
>
> Message: Disabling security [lib/security]: missing internal dependency "cryptodev"
>
> lib/meson.build:218:16: ERROR: Problem encountered: Cannot build explicitly requested lib "security".
> Please add missing dependency "cryptodev" to "enable_libs" option
>
> Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
CI now passes fine after a retest.
Series applied, thanks Bruce.
@@ -131,7 +131,9 @@ endforeach
disable_libs = run_command(list_dir_globs, get_option('disable_libs'), check: true).stdout().split()
enable_libs = run_command(list_dir_globs, get_option('enable_libs'), check: true).stdout().split()
+require_libs = true
if enable_libs.length() == 0
+ require_libs = false
enable_libs += optional_libs
endif
enable_libs += always_enable
@@ -189,6 +191,10 @@ foreach l:libraries
if build
subdir(l)
+ if not build and require_libs
+ error('Cannot build explicitly requested lib "@0@".\n'.format(name)
+ +'\tReason: ' + reason)
+ endif
endif
if name != l
warning('Library name, "@0@", and directory name, "@1@", do not match'.format(name, l))
@@ -208,6 +214,10 @@ foreach l:libraries
endif
message('Disabling @1@ [@2@]: missing internal dependency "@0@"'
.format(d, name, 'lib/' + l))
+ if require_libs
+ error('Cannot build explicitly requested lib "@0@".\n'.format(name)
+ + '\tPlease add missing dependency "@0@" to "enable_libs" option'.format(d))
+ endif
else
shared_deps += [get_variable('shared_rte_' + d)]
static_deps += [get_variable('static_rte_' + d)]