Page MenuHomeFreeBSD

config: create a 'requires' keyword for files file processing
AbandonedPublic

Authored by imp on Apr 19 2021, 9:21 PM.
Tags
None
Referenced Files
Unknown Object (File)
Thu, Oct 17, 1:26 PM
Unknown Object (File)
Wed, Oct 16, 11:21 PM
Unknown Object (File)
Tue, Oct 15, 5:14 AM
Unknown Object (File)
Oct 13 2024, 10:02 AM
Unknown Object (File)
Oct 12 2024, 8:25 PM
Unknown Object (File)
Oct 12 2024, 2:19 AM
Unknown Object (File)
Oct 10 2024, 6:46 PM
Unknown Object (File)
Oct 10 2024, 9:22 AM
Subscribers

Details

Summary

Add a new, optional, clause for files line processing. 'requires "dev"'
will allow a file to require another device be present. This is a fit
and finish issue to give a failure earlier in the process, hopefully one
that's more actionable than a compile-time error of "foo not found".

This should only be used for those situations where a different device
is required for the file to compile. Since only the first such instance
is reported, devices that have multiple files should only tag the first
one. In cases where device X needs device Y to function, but will
otherwise compile and link, care should be taken to avoid tagging this
because Y might come from a loaded module instead of being statically
compiled into the kernel.

Only one device can be listed in the 'requires' argument. Should we need
more we can make it a space separated list.

Bump config minor version to 600019 to reflect this change.

MFC After: 1 week
Sponsored by: Netflix

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 38700
Build 35589: arc lint + arc unit

Event Timeline

imp requested review of this revision.Apr 19 2021, 9:21 PM
imp created this revision.

Is there an update for config.5?

It would be nice if we could report a few missing required options, although it may be run early enough in buildkernel to not matter.

usr.sbin/config/config.h
61

Maybe change to which other option this requires if we only support one option.

usr.sbin/config/mkmakefile.c
155–156

It looks like this should be errout.

443

Maybe change requires-list if only one option is supported.

What's the difference between this and making an non-optional requirement for the file ?
Is it just to have a nicer error message than a linker error ?
Looking at D29849 it seems that the same thing could be achieve by adding "iflib" in every em(4) related file and since em(4)
needs iflib I think that's the proper way of "fixing" this problem.

In D29848#669964, @manu wrote:

What's the difference between this and making an non-optional requirement for the file ?
Is it just to have a nicer error message than a linker error ?

One of my main annoyances with config(8) is that non-optional requirements require you to actually read and interpret files to understand how to get what you want, and the failure mode sucks- if you get it wrong, you don't even get a linker error IIRC because we simply don't build it unless all the requirements are there.

Is there an update for config.5?

None of this is documented in config(5) nor config(8)

It would be nice if we could report a few missing required options, although it may be run early enough in buildkernel to not matter.

Yea, we report only the first with this code, though I could report them all w/o much work.

usr.sbin/config/config.h
61

It's actually the other device(s) required for this file. Devices and options are closely related but somewhat different.

usr.sbin/config/mkmakefile.c
155–156

Config is massively inconsistent in its useage of that. Other places use printf to stdout for error messages (the place I originally had this code does that). This file uses errout which sends it to stderr.

443

yea, I went back and forth supporting one option or a list. In the end I landed on 1 option only. I'll update.

In D29848#669964, @manu wrote:

What's the difference between this and making an non-optional requirement for the file ?
Is it just to have a nicer error message than a linker error ?

One of my main annoyances with config(8) is that non-optional requirements require you to actually read and interpret files to understand how to get what you want, and the failure mode sucks- if you get it wrong, you don't even get a linker error IIRC because we simply don't build it unless all the requirements are there.

The difference between

a/b/c optional foo bar

and

a/b/c optional foo requires "bar"

is that if bar is not in the kernel in the first case you get no foo. No warning, no nothing, no foo. It is horribly abused to omit, for example, ethernet drivers that can't cope with inet not being in the kernel. The latter emits a nasty error and you don't make it past the config stage.

There's several concepts here that I'd like to explore in the future:
(1) needs X X must be configured, or you don't get this device even if it asked for
(2) requires X if X is not configured, that is an error
(3) implies X Turns X on when this device is present
(4) conflicts-with X Cannot be used at the same time X is present

config(8) only does (1) today. This adds (2) as a stop-gap before we rewrite config(8) entirely, likely in flua. I thought about doing (3) but didn't want to deal with detecting circular implies and such. It's not hard, I just didn't want to spend the time.

In D29848#670260, @imp wrote:
In D29848#669964, @manu wrote:

What's the difference between this and making an non-optional requirement for the file ?
Is it just to have a nicer error message than a linker error ?

One of my main annoyances with config(8) is that non-optional requirements require you to actually read and interpret files to understand how to get what you want, and the failure mode sucks- if you get it wrong, you don't even get a linker error IIRC because we simply don't build it unless all the requirements are there.

The difference between

a/b/c optional foo bar

and

a/b/c optional foo requires "bar"

is that if bar is not in the kernel in the first case you get no foo. No warning, no nothing, no foo. It is horribly abused to omit, for example, ethernet drivers that can't cope with inet not being in the kernel. The latter emits a nasty error and you don't make it past the config stage.

There's several concepts here that I'd like to explore in the future:
(1) needs X X must be configured, or you don't get this device even if it asked for
(2) requires X if X is not configured, that is an error
(3) implies X Turns X on when this device is present
(4) conflicts-with X Cannot be used at the same time X is present

config(8) only does (1) today. This adds (2) as a stop-gap before we rewrite config(8) entirely, likely in flua. I thought about doing (3) but didn't want to deal with detecting circular implies and such. It's not hard, I just didn't want to spend the time.

Yeah, I think this is fine for now (and this review specifically looks OK, modulo what's already been pointed out inline). For something like a NIC driver, it's nearly critical that an end-user be kept apprised of such omissions as early as we can manage.

I'm no longer sure this is a good idea based on this and other feedback. Punting for now since in our current system, it's unclear when to use this.