Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Kconfig Module #5031

Merged
merged 3 commits into from
Mar 20, 2019
Merged

Kconfig Module #5031

merged 3 commits into from
Mar 20, 2019

Conversation

bonzini
Copy link
Contributor

@bonzini bonzini commented Mar 7, 2019

Add kconfig integration such that existing projects can use the ".config" file from a kconfig system to conditionally compile in or out features.

The code is an updated version of the original submission in pull request #1617. Compared to the original, it uses the dictionary objects that have since been introduced in meson, and it lets you pass the input as a file object. This makes for a more flexible and easier to use API.

@bonzini bonzini mentioned this pull request Mar 7, 2019
@bonzini bonzini force-pushed the kconfig branch 3 times, most recently from 9a227eb to e6fd635 Compare March 7, 2019 12:10
@dcbaker
Copy link
Member

dcbaker commented Mar 7, 2019

To continue my question from the other thread, the reason I'd suggest a dictionary instead of a configuration_data, is that not all kconfig arguments are going to go into a header, you might, for example want to gate a subdir call based on whether enable_feature is set, and a dictionary is more natural way to do that.

@bonzini
Copy link
Contributor Author

bonzini commented Mar 7, 2019

Yes, this version uses a dictionary (see also the first patch to enable dictionaries in holderify).

@jpakkane
Copy link
Member

There are also questions on how to integrate this with regeneration logic and all that. It would be optimal if we could make this work just by invoking Meson and not having to run kconfig first.

That being the case let's call this unstable-kconfig instead. That way we can change it more easily once the workflow becomes clear.

Copy link
Member

@dcbaker dcbaker left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One small thing, otherwise this looks good to me as long as @jpakkane is happy with it.

test cases/kconfig/2 subdir/dir/meson.build Show resolved Hide resolved
@dcbaker
Copy link
Member

dcbaker commented Mar 14, 2019

looks good to me, I think this just needs a rebase and @jpakkane's sign off.

docs/markdown/Kconfig-module.md Outdated Show resolved Hide resolved

k = import('unstable-kconfig')

conf = k.load(files('config'))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have not used kconfig all that much, but shouldn't we load the output file? Specifically something that will get generated in the build dir rather than reading stuff from the source dir? Loading stuff from source dir means that you can have only one configuration per source dir, not one per build dir as is the Meson standard.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have not used kconfig all that much, but shouldn't we load the output file? Specifically something that will get generated in the build dir rather than reading stuff from the source dir? Loading stuff from source dir means that you can have only one configuration per source dir, not one per build dir as is the Meson standard.

Indeed, since Meson only supports out-of-tree build, it is somewhat unfortunate that the default for kconfig output is .config in the toplevel source directory. However, the output file can be overridden using an environment variable, usually specified like

$ make menuconfig KCONFIG_CONFIG=foo.config

I expect that users of this module will make the location of the configuration file a Meson option too, so that the same file can be specified both as KCONFIG_CONFIG and when invoking Meson. I will add a note to the documentation.

Of course, having to specify the directory twice is ugly; unfortunately on one hand the build directory is not known when you invoke the kconfig frontend, and on the other hand the kconfig frontend must be invoked before Meson. One possible fix is to provide some kind of script that wraps meson setup and invokes kconfig before Meson (removing the need to ship a Makefile), not unlike those Meson users that are providing a simple-minded configure script. That script can then pass a file in the build directory and pass it to both kconfig and Meson.

That said, the placement of the configuration also depends on the usecase. For example, QEMU only uses kconfig as a common format to store pre-defined configuration_data for each binary; it does not employ any front-end, instead all processing of the configuration happens in meson.build. In that case, the configuration files are fixed and committed, and they definitely come from the source directory.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since the module is unstable, we can change it later. We can merge this and change the behaviour as people test it on real world code.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe something like:

k.generate_and_load(init_command: ['kconfig', 'source_file', 'output_file'])

That runs the given command if the output .config file does not exist in the build dir and otherwise just loads it?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems sensible but I would first try and see how people use this. The config frontends usually have their own make-based build system, and invoking make from Meson would be a bit weird...

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe something like:
k.generate_and_load(init_command: ['kconfig', 'source_file', 'output_file'])

If you were referring to: genkconfig .config -> config.h, then yes, I wrote a custom_target that does just that.

If you were referring to generating the default .config automatically with olddefconfig then no, it's very easy but apparently considered bad practice in the (interactive) Kconfig world to automagically build the default configuration. This must be an explicit choice from the user as seen in the Linux kernel example.

bonzini and others added 3 commits March 15, 2019 11:41
This in turn allows modules to return dictionaries, since their return values
is automatically passed through holderify.
Add a kconfig module to allow meson to integrate with existing projects
that use kconfig.
Document best practices for per-builddir config file, and add a test covering
loading a config file from the build directory.

Signed-off-by: Paolo Bonzini <[email protected]>
@jpakkane jpakkane merged commit ed5992a into mesonbuild:master Mar 20, 2019
@bonzini bonzini deleted the kconfig branch March 28, 2019 07:47
@marc-h38
Copy link
Contributor

marc-h38 commented Jan 23, 2020

I would first try and see how people use this.

Sharing some feedback here for the lack of knowing a better place.

First, thank you @bonzini for implementing this, extremely useful! I'm also glad I found these github issues, they add a lot of useful background information to the very... concise https://mesonbuild.com/Kconfig-module.html :-)

It would be optimal if we could make this work just by invoking Meson and not having to run kconfig first.

First, "invokes kconfig" or "runs kconfig" is too vague and has lost me several times. menuconfig and olddefconfig are steps completely different from genconfig. The first two produces a .config whereas genconfig.py produces a config.h from the .config. .config is confusingly both an input and an output as clarified by the recent doc fix zephyrproject-rtos/zephyr#22022

  • I use a custom_target to run genconfig.py. Some critical inputs of genconfig.py ($srctree and $KCONFIG_CONFIG) can only be environment variables (mimicking the C/linux kernel version). However custom_target doesn't support environment variables (yet? Add env: argument to custom_target, run_target, run_command #2723 etc.) so I hacked genconfig.py to add --srctree CLI options.

I'm curious if/how other projects run genconfig.py and solve this.

  • Small issue: I couldn't make /Kconfig a custom_target input: because genconfig.py already searches for it in $srctree, so the ../ added by meson to help find it actually hurts.

  • KCONFIG_CONFIG is not a meson input: either because I want genconfig.py to search for it in both the source and build directories.

not all kconfig arguments are going to go into a header,

As a matter of fact I've run into the biggest issue with this module: the kconfig dictionary produces quoted strings, as they are in the .config file. In other words:

kconfig["CONFIG_QEMU_PARAM1"] == '"some value"'

You can tell Kconfig came from a C world where concatenating strings is as simple as putting them next to one another, however this seems very uncommon outside C. In this case I am (funny enough) configuring a QEMU .cfg file and there you can clearly not do that. BTW C code is most likely using config.h directly and not depending on this module.

As this module is marked as "unstable", would it be acceptable to "unquote" the strings produced by it?

This is my current workaround, any better suggestion welcome:

double_quote = '"' # Makefile memories...

unquoted_qemu_param1 = kconfig['CONFIG_QEMU_PARAM1'].split(double_quote)[1]
kconfig += {'CONFIG_QEMU_PARAM1' : unquoted_qemu_param1 }

Indeed, since Meson only supports out-of-tree build, it is somewhat unfortunate that the default for kconfig output is .config in the toplevel source directory.

BTW genconfig.py can read .config in the source directory and (re-)generate the sanitized version back into a different (build) directory. For (much!) more background see the --sane feature I built on top of that https://github.com/zephyrproject-rtos/zephyr/pull/22022/files#r368187359

One possible fix is to provide some kind of script that wraps meson setup and invokes kconfig before Meson (removing the need to ship a Makefile), not unlike those Meson users that are providing a simple-minded configure script.

Some other root causes and examples of meson wrappers at #6434 (comment)

The config frontends usually have their own make-based build system,

Not sure I fully understand your point here but FYI https://github.com/ulfalizer/Kconfiglib is pure Python

@bonzini
Copy link
Contributor Author

bonzini commented Jan 23, 2020

As this module is marked as "unstable", would it be acceptable to "unquote" the strings produced by it?

Yes, definitely. For example a random Linux .config I have around says

CONFIG_OUTPUT_FORMAT="elf64-x86-64"

I would be happy to review a patch for that.

@bonzini
Copy link
Contributor Author

bonzini commented Feb 25, 2020

QEMU does not support interactive Kconfig at all. It's only used to automatically compute dependencies.

This is not true with kconfiglib.py which conveniently places .config in the current directory.

I think that would be a bug in kconfiglib.py. In general if you use Meson (which doesn't support in-tree builds) you probably want to have a wrapper that invokes both the kconfig front-end and meson setup (it could even compute options based on some kconfig values), or something like that. Perhaps the text should be adjusted to suggest that? You certainly know more than me about this, so I'll let you take care of it. :)

@marc-h38
Copy link
Contributor

marc-h38 commented Feb 25, 2020

I think that would be a bug in kconfiglib.py. In general if you use Meson (which doesn't support in-tree builds)

The main question here is this: is .config a build input or a build output? You can see this pervasive question/confusion all the way from @jpakkane's very first comment in #1617

There is confusion because the answer is: it depends. .config can be seen as either. An interactive user who is interactively (and often: painfully) menuconfiguring a single .config at a time would consider it as a build input and she doesn't want it to be --wiped[*]. For someone or something testing multiple configs, or even someone using a build system generating a single .config from multiple fragments (defconfig, Yocto, chromiumOS, etc.), then .config is indeed a build output that should be --wiped. Last example: if I share a carefully crafted .config and a git SHA1 with you, then you're unlikely to consider the .config to be a build output.

I wouldn't call a "bug" the fact that kconfiglib.py is flexible and convenient enough to automatically support both approaches. But maybe it is ahead of its time and I got carried away by it.

Back to meson.build, maybe the easiest to support both cases is something like:

if found source_root/.config
  configure_file( copy  source_root/.config to build_root/.config )
endif

Thoughts?

PS: for even more confusion, .config is both an input and an output... to itself!
https://github.com/zephyrproject-rtos/zephyr/pull/22022/files

The assignments to hidden symbols are just configuration output, so Kconfig itself ignores assignments to hidden symbols in .config when calculating symbol values.

[*] I started versioning Linux kernel configs after a few too many make distclean mistakes.

EDITED:

you probably want to have a wrapper that invokes both the kconfig front-end and meson setup

meson is a bit of a "wrapper" on top of ninja, meaning users need to invoke and get familiar with two different tools. I'm already using yet another wrapper on top of meson because of #3001 / #4637 but I'd really like meson not to require a 3rd layer eventually.

@bonzini
Copy link
Contributor Author

bonzini commented Feb 25, 2020

is .config a build input or a build output

It is a build input, but it's a build input that can differ for different build roots, hence the original suggestion of placing it (by default) in the build root and configurable with a build option.

Back to meson.build, maybe the easiest to support both cases is something like configure_file( copy source_root/.config to build_root/.config )

(only if the build option is not set?) That can work too. At this point it's a matter of figuring out the best practices and documenting them. Fortunately none of this is affecting the code of the module itself.

@xclaesse
Copy link
Member

With the little knowledge I have on the matter, to me it sounds like .config is an input as far as Meson is concerned. It could be an output of wrapper tools around Meson, but then it's not Meson's issue. For example, a build script could do:

mkdir builddir
cp /path/some_config builddir/.config
meson builddir

Just forget me if I completely missed the point :D

@marc-h38
Copy link
Contributor

Just forget me if I completely missed the point :D

Congratulations: everything you wrote makes sense :-)

This closed PR is really not a great place, so I just filed new #6694. Appreciated if people can find the time to make it there.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants