config(8): Add support for implications and specifying multiple files per option.

[• hselasky]

Summary

Implications are solved using a simple 2-SAT solver engine.
The new files syntax is as following:

# this is a comment
include "filename"
option standard { filename(s) }
option [!]dev { filename(s) }
 [ implies { dev(s) } ] |
 [ no-obj ]
 [ compile-with "compile rule" [no-implicit-rule] ]
 [ dependency "dependency-list"] [ before-depend ]
 [ clean "file-list"] [ warning "text warning" ]
 [ obj-prefix "file prefix"]
 [ nowerror ] [ local ]

Using backslash for escaping newline is optional.

MFC after: 1 week
Sponsored by: NVIDIA Networking

[• hselasky]

The changes were mechanically generated by this C-program:

config_compress.c3 KBDownload

[• hselasky]

@manu: I could use whitespace too as a delimiter. Would that help the read impression?

[manu]

In D33224#751020, @hselasky wrote:

@manu: I could use whitespace too as a delimiter. Would that help the read impression?

That would be even worse :)

[allanjude]

I think indenting the line continuations would help with the readability.

[manu]

Also I'm not a fan of the automatic conversion.
I've put some work to sort files.arm64 a while ago and now it's a mess.
For example all coresight files are correctly grouped right now but this change put them in different part because some are standard, some for fdt and some for acpi.

[brooks]

In D33224#751117, @manu wrote:

Also I'm not a fan of the automatic conversion.
I've put some work to sort files.arm64 a while ago and now it's a mess.
For example all coresight files are correctly grouped right now but this change put them in different part because some are standard, some for fdt and some for acpi.

The automated conversion isn't good except as a basis for a hand-curated selection of groupings.

If the parser would permit it, I'd be tempted to make dangling pipes indicate line continuation rather than requiring a ton of \s.

[• hselasky]

In D33224#751147, @brooks wrote:

In D33224#751117, @manu wrote:

Also I'm not a fan of the automatic conversion.
I've put some work to sort files.arm64 a while ago and now it's a mess.
For example all coresight files are correctly grouped right now but this change put them in different part because some are standard, some for fdt and some for acpi.

The automated conversion isn't good except as a basis for a hand-curated selection of groupings.

Yes, exactly. Probably this change will just be an example of what may be done.

If the parser would permit it, I'd be tempted to make dangling pipes indicate line continuation rather than requiring a ton of \s.

It would be easier to user whitespace separation then between the filenames and use the backslash to forward to next line. Whitespace is not allowed in any filenames already.
We know when we have hit the end when "optional" or "standard" is encountered.

Example:

x86/xen/hvm.c \
x86/xen/xen_intr.c \
x86/xen/xen_apic.c		optional	xenhvm

[kevans]

Historically, requiring a newer feature like this right off the bat has been considered suboptimal because folks still use the supported legacy kernel build setup. I don't know how many more folks do, but this was a concern often raised by bde@.

Beyond that, yeah, +1 for not being a fan of automatic conversion. These groups are way too large and actually detrimental for understanding what's happening, and this style of conversion is probably too soon after manu re-alphabetized files.arm64 for the Cheri folks to have forgotten how painful rebasing across changes like this are. :-)

[imp]

Honestly, I'm not a fan. I hate it almost as much as I hate the original files format.

If you're going to go for something better, then you should go for something better, especially if there's some automatic generation involved.

Unfortunately, there's an odd mix of dependencies and hacks to cope with poor dependency expression here.

For example, look at pvclock.c. it depends on kvm_clock or xenhvm. Really, it should depend on pvclock and the former two options should imply the latter. Then we could start to de-tangle the polluted information we have today. It's but one of many.

So if we're going to have such a large change, lets make it count and adopt a more expressive style.

option xenhvm {
x86/xen/hvm.c
x86/xen/xen_intr.c
x86/xen/xen_apic.c
} compile-with {} implies {} requires {} ...

is way more expressive anyway. And there's half a dozen other ways to do that leverages things like UCL / Lua / etc.
But to make much progress down that path, you need to do something about the dependencies. You need to create a cleaner model first and spend time sorting the 'oddballs' correctly.

So I see this as a lot of churn for such tiny gains as to be undesirable.

[kevans]

In D33224#751328, @imp wrote:

Honestly, I'm not a fan. I hate it almost as much as I hate the original files format.

If you're going to go for something better, then you should go for something better, especially if there's some automatic generation involved.

Unfortunately, there's an odd mix of dependencies and hacks to cope with poor dependency expression here.

For example, look at pvclock.c. it depends on kvm_clock or xenhvm. Really, it should depend on pvclock and the former two options should imply the latter. Then we could start to de-tangle the polluted information we have today. It's but one of many.

So if we're going to have such a large change, lets make it count and adopt a more expressive style.

option xenhvm {
x86/xen/hvm.c
x86/xen/xen_intr.c
x86/xen/xen_apic.c
} compile-with {} implies {} requires {} ...

is way more expressive anyway. And there's half a dozen other ways to do that leverages things like UCL / Lua / etc.
But to make much progress down that path, you need to do something about the dependencies. You need to create a cleaner model first and spend time sorting the 'oddballs' correctly.

So I see this as a lot of churn for such tiny gains as to be undesirable.

We should really start formally defining what we think configng should look like and work towards refining that idea into something practical and most universally acceptable. It might be worth setting up a call at some point and hashing out some of the broad design details. I'm OK with incremental improvements, but at some point we should get an idea of what we really want the end-goal to look like and draw the line for a final cutover.

[manu]

In D33224#751336, @kevans wrote:

In D33224#751328, @imp wrote:

Honestly, I'm not a fan. I hate it almost as much as I hate the original files format.

If you're going to go for something better, then you should go for something better, especially if there's some automatic generation involved.

Unfortunately, there's an odd mix of dependencies and hacks to cope with poor dependency expression here.

For example, look at pvclock.c. it depends on kvm_clock or xenhvm. Really, it should depend on pvclock and the former two options should imply the latter. Then we could start to de-tangle the polluted information we have today. It's but one of many.

So if we're going to have such a large change, lets make it count and adopt a more expressive style.

option xenhvm {
x86/xen/hvm.c
x86/xen/xen_intr.c
x86/xen/xen_apic.c
} compile-with {} implies {} requires {} ...

is way more expressive anyway. And there's half a dozen other ways to do that leverages things like UCL / Lua / etc.
But to make much progress down that path, you need to do something about the dependencies. You need to create a cleaner model first and spend time sorting the 'oddballs' correctly.

So I see this as a lot of churn for such tiny gains as to be undesirable.

We should really start formally defining what we think configng should look like and work towards refining that idea into something practical and most universally acceptable. It might be worth setting up a call at some point and hashing out some of the broad design details. I'm OK with incremental improvements, but at some point we should get an idea of what we really want the end-goal to look like and draw the line for a final cutover.

Agreed, count me in for this.

[• hselasky]

Hi Warner,

I like the implies part. That means we can have a quick 2-SAT solver to figure out any conflicts!

Syntax template:

option [!]xenhvm {
x86/xen/hvm.c
x86/xen/xen_intr.c
x86/xen/xen_apic.c
} compile-with {} implies { [!]pci [!]acpi } dependencies {} no-obj ...

standard {
foo/foo.c
} compile-with {} implies { [!]pci [!]acpi } ...

Can I go ahead and implement this?

--HPS

[• hselasky]

Implement @imp suggestion for new files syntax and try to automagically convert the simple cases for a feel and look.

NOTE: The code compiles, but is not fully tested yet.

[• hselasky]

Add some missing newline characters in the files conversion.

[andrew]

sys/conf/files.amd64

100–101

I wonder if we could have something like the following:

option foo {
  group {
    path/to/foo1.S
  } compile-with "${FOO_S}"
  group {
    path/to/foo2.c
  } compile-with "{FOO_C}"
}

This would allow us to keep all files an option enable together and allow custom per-file options.

[• hselasky]

This update brings several bugfixes in new config(8) code.
Cleaned up all config files needed for running new config(8):

cd sys/amd64/conf && config GENERIC

This should give a clear impression what needs to be modified.

Some problem arose, that some device keywords are too generic, so I need to split them, but have not updated any kernel configuration files yet:

Added:
smc_acpi
smc_fdt
sound_isa
spibus_fdt
acpi_isa
compat_linuxkpi_acpi
compat_linuxkpi_pci
compat_linuxkpi_usb
bcmadb
bhnd_cfi
bhnd_slicer
bhnd_spibus
bhnd_pci
bhndb_pci
bhnd_pcib
cfi_fdt
cxgbe_tls
sc_splash
vt_splash
nfscl_common
krpc_kgssapi
ffs_label
teken_state
teken
usb_fdt
dwcotg_fdt
ehci_pci
ohci_pci
uhci_pci
xhci_pci
saf1761otg_fdt
mac_inet
mac_inet6
mac_audit
crypto_common
mrouting6
sctp6
gre6
rss6
netgraph_gif6
fib_algo6
ddb_inet
ddb_wlan
ether_inet
pfil
gcov_fs
geom_uzip_zstd
firewire_pci
syscon_fdt
xenefi
virtio_mmio_acpi
virtio_mmio_fdt

uart_acpi
uart_fdt
uart_isa
uart_pci
uart_puc
uart_scc
uart_quicc
uart_gdb

sdhci_fdt

scc_quicc
ips_pci

ppc_acpi
ppc_isa
ppc_puc
ppc_pci

puc_pci

proto_pci
proto_isa

pwm_fdt

ext_resources_clk
ext_resources_phy
ext_resources_hwreset
ext_resources_nvmem
ext_resources_regulator
ext_resources_syscon
ext_resources_syscon_fdt
ext_resources_syscon_power_fdt

iicoc_fdt
iicoc_pci
iichid_acpi

aacraid_cam
aacraid_pci

pci_isa
cbb_pci

nvme_pci
nvme_scbus
nvme_cam
nvme_ahci

netmap_inet

random_fortuna

gpioled_fdt
gpio_fdt

iicbus_acpi
iicbus_fdt

Removed:
ext_resources
nfs_root
pwm

--HPS

[imp]

sys/conf/files.amd64

52

I don't understand this syntax.

61–82

Can we make { foo } and foo the same here (eg make the {} optional?

[• hselasky]

sys/conf/files.amd64

100–101

I would recommend to wait with more changes. Or have it as a second iteration update feature.

Many of the changes in sys/conf/files* must be done by hand, and are not scriptable! So I want to limit the amount of changes I'm doing for now.

[• hselasky]

sys/conf/files.amd64

61–82

Yes, that's possible. I'll work on that tomorrow.

[• hselasky]

In D33224#751114, @allanjude wrote:

I think indenting the line continuations would help with the readability.

Yes. But maybe after we agree about the format :-) There is a lot of manual work involved here!

[• hselasky]

sys/conf/files.amd64

61–82

Some options can have no filenenames, just to put group rules in there:

option mydriver { } implies { this and that and blah and blah }

If there are no { }'s then you can't so easily tell if there are no files, which is allowed!

How would you solve that? If the filename is magic it is treated specially maybe?

[bz]

I am really not qualified to even remotely look at config(5).

[• hselasky]

Reduced number of new options. I think we should follow the MODULE_DEPEND()'s when generating the implies. For now include more than less.

For example if there is no FDT, ACPI, PCI, ISA then hide that in sys/conf/files.<arch> instead of adding more options to the main sys/conf/files .

The advantages having a plain dependency graph (2-SAT) is that conflict resolution becomes easy and predictable. At the moment you involved multiple ANDs (3-SAT), then it becomes much harder and you will need something like pkg's picosat to solve it.

Made config(8) print full conflict summary. See example below.

# cat GENERIC-TEST 
include "GENERIC"

# We want to remove everything that depends on "pci"
nooptions pci

# config GENERIC-TEST
WARNING: Option 'pci' implies: isa 
WARNING: Option '!pci' implies: !ohci !ocs_fc !oce !nvme !nfsmb !mxge !mwl !mvs !mthca !mpt !mps !mpr !mlx5ib !mlx5en !mlx5 !mlx4ib !mlx4en !mlx4 !mlx !mfi !malo !le !jme !isp !ips !intpm !iicoc !igc !ig4 !iflib !ida !ichsmb !gem !firewire !ehci !dc !cxgbe !cxgb !cpufreq !compat_linuxkpi !cbb !bwn !bwi !bnxt !bhndb_pci !bhndb !bhnd !ath_pci !atavia !atasis !atasiliconimage !ataserverworks !atapromise !atapci !atanvidia !atanetcell !atanational !atamicron !atamarvell !atajmicron !ataite !ataintel !atahighpoint !atacyrix !atacypress !atacenatek !ataati !ataamd !ataacerlabs !ataacard !amdsmb !amdpm !alpm !ale !alc !ahd !ahci !ahc !agp !age !ae !aacraid !aac !pci_iov !pl011 !ppc !proto !pst !puc !ral !rl !rtsx !rtwn_pci !sdhci !sge !siis !sis !sk !snd_ad1816 !snd_als4000 !snd_atiixp !snd_cmi !snd_cs4281 !snd_csa !snd_ds1 !snd_emu10k1 !snd_emu10kx !snd_envy24 !snd_envy24ht !snd_es137x !snd_ess !snd_fm801 !snd_gusc !snd_hda !snd_hdspe !snd_ich !snd_maestro !snd_maestro3 !snd_mss !snd_neomagic !snd_sb16 !snd_sb8 !snd_sbc !snd_solo !snd_spicds !snd_t4dwave !snd_uaudio !snd_via8233 !snd_via82c686 !snd_vibes !sound !ste !tdfx !ti !uart !uhci !viapm !vr !vte !wpi !xhci !xl 
../../conf/files: Option(s) conflict detected!

Here is an updated list of new options:

• bcmadb
• crypto_common
• cxgbe_tls
• ddb_inet
• ddb_wlan
• ether_inet
• ext_resources_clk
• ext_resources_hwreset
• ext_resources_nvmem
• ext_resources_phy
• ext_resources_regulator
• ext_resources_syscon
• ext_resources_syscon_fdt
• ext_resources_syscon_power_fdt
• ffs_label
• fib_algo6
• gcov_fs
• geom_uzip_zstd
• gpioled_fdt
• gre6
• krpc_kgssapi
• mac_audit
• mac_inet
• mac_inet6
• mrouting6
• netgraph_gif6
• netmap_inet
• nfscl_common
• pfil
• random_fortuna
• rss6
• sc_splash
• sctp6
• syscon_fdt
• teken
• teken_state
• vt_splash
• xenefi

Removed options:

• ext_resources
• nfs_root

[• hselasky]

One more example trying to remove ZFS will yield:

# cat GENERIC-TEST 
include "GENERIC"

nooptions zfs

# config GENERIC-TEST
WARNING: Conflict option 'zfs' implies: ufs_acl_nfs4 dtrace_zfs crypto_skein crypto_sha512 crypto_sha256 zlib 
WARNING: Conflict option '!zfs' implies: !geom_uzip_zstd !zstdio 
../../conf/files: Option(s) conflict detected!

[manu]

So if we're going to change the files.XXX format why not going with UCL ?
I'm not a fan of "option standard" too, that doesn't make much sense.
I'm also not a fan of "option !XXX", I know that we have some '!' usage right now but I'm not a fan too.

[• hselasky]

@manu : Can you convert an entry using compile-with into UCL format so that we can see how it would look? Are you sure this won't cause a lot of repeated bloat?

There are two changes that mostly don't have anything to do with eachother:

1. The dependency reorder (implies support) and listing multiple files
2. The format (UCL, XML or how it is now)

Right now minimal changes have been done to the format.

[• hselasky]

Looks like I pressed "accepted" by an accident. Ignore that.

[manu]

In D33224#754126, @hselasky wrote:

@manu : Can you convert an entry using compile-with into UCL format so that we can see how it would look? Are you sure this won't cause a lot of repeated bloat?

There is no "UCL format", we can do whatever we want with it.

There are two changes that mostly don't have anything to do with eachother:

1. The dependency reorder (implies support) and listing multiple files
2. The format (UCL, XML or how it is now)

I don't agree, the format dictate how we express things.

Right now minimal changes have been done to the format.

I think that what I'm trying to say is : like @kevans proposed maybe we should have a call/mail/whatever so we first answer to a few questions like :

• What do we miss with current config ?
• What do we need/want
• etc ...

[imp]

I too would rather see a firm break in the format, despite the PITA this would cause for MFCs.
I also would love to have a call and/or email chain to work on a design.
My other wish list item, in addition to implies is requires which is kinda the opposite: so bar requires foo would turn off bar if foo is also off. This is generally what you want with bus relationships since otherwise if you have driver bar, but bus foo can't be on the system, you'd need to have some kind of hard failure and that's sounds good at first, but is too inflexible for people that want to subset.

[• hselasky]

When are people usually available for a talk?

I see there is a graphics driver meeting december 20th? Should we hijack that one?

[imp]

I don't think I can't do that graphics call, I'll be traveling with the family. I might be able to make it work, but it might be tough.

I started a hackMD that starts with the design notions that I'd like to see, but that we can turn into a requirements and design doc.

https://hackmd.io/w1Tf8mmVQVuZok7-LNEhgw

[manu]

In D33224#754190, @hselasky wrote:

When are people usually available for a talk?

I see there is a graphics driver meeting december 20th? Should we hijack that one?

Mhm no we shouldn't hijack this.

[brooks]

As a downstream consumer with many changes to various config/files* files I'm fine with a break, but would very much like to avoid having more than one round of large-scale changes. @manu's re-sorting of files.amd64 required completely reimplementing our changes as git had not hope of figuring out what to do. I assume the same will be true of other downstreams. If we did go to UCL it might be worth renaming the files (e.g., files-arm64.ucl)