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

linux: allow disabling common-config #106756

Closed

Conversation

lheckemann
Copy link
Member

Motivation for this change

This is useful for building kernels for embedded systems with the
nixpkgs kernel config tools.

Things done

This is useful for building kernels for embedded systems with the
nixpkgs kernel config tools.
@teto
Copy link
Member

teto commented Dec 12, 2020

may be more elegant to have something like #106395 but my PR kinda breaks nixos :(

@lheckemann
Copy link
Member Author

lheckemann commented Dec 12, 2020

With your approach:

  • Merging behaviour can be obtained by setting extraStructuredConfig = lib.mkDefault { SOME_OPTION = yes; }
  • Setting extraStructuredConfig without mkDefault will completely disable common-config. This runs a bit counter to my interpretation of "extra" config.

With my approach:

  • Merging behaviour remains default
  • Individual settings from common-config can be overridden with mkForce, as in extraStructuredConfig = { SOME_OPTION = lib.mkForce yes; }
  • All of common-config can be disabled using useCommonConfig = false;


and I just noticed that without any changes, one can already override all of common-config using extraStructuredConfig = lib.mkForce {…}

IMHO your approach is very likely to result in surprises: setting extraStructuredConfig for just one option will suddenly disable a bunch of others.

@lheckemann
Copy link
Member Author

Closing in favour of #90065 and use of extraStructuredConfig = lib.mkForce {…} to override all of common-config.

@lheckemann lheckemann closed this Dec 12, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants