Skip to content

linux-common-config: Refactor, clean up#27949

Merged
NeQuissimus merged 1 commit intoNixOS:masterfrom
NeQuissimus:common_kernel_config
Aug 6, 2017
Merged

linux-common-config: Refactor, clean up#27949
NeQuissimus merged 1 commit intoNixOS:masterfrom
NeQuissimus:common_kernel_config

Conversation

@NeQuissimus
Copy link
Member

@NeQuissimus NeQuissimus commented Aug 5, 2017

Motivation for this change

Refactor and clean up common kernel configuration.
Some of our kernels were throwing out plenty of warnings and this gets rid of them.
Also, it is now much easier to find out what options are valid for which kernel versions and feature toggles.

Things done
  • Tested using sandboxing
    (nix.useSandbox on NixOS,
    or option build-use-sandbox in nix.conf
    on non-NixOS)
  • Built on platform(s)
    • NixOS
    • macOS
    • Linux
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nox --run "nox-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Fits CONTRIBUTING.md.

I built nix-build -A <PKG>.configfile for

  • linux_3_10
  • linux_4_4
  • linux_4_9
  • linux_4_12
  • linux_hardened_copperhead
  • linux_samus_4_12
  • linux_testing
  • linux_chromiumos_3_18
  • linux_rpi
  • linux_mptcp
  • linux_minipli (from linux-minipli: Init at 4.9.40 #27935)

None of the above report any warnings (or errors) anymore.

@NeQuissimus NeQuissimus added 6.topic: developer experience nixpkgs development workflow 6.topic: hygiene Cleaning up and removing cruft 6.topic: kernel The Linux kernel 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: clean-up This PR removes packages or removes other cruft 8.has: package (update) This PR updates a package to a newer version labels Aug 5, 2017
@mention-bot
Copy link

@NeQuissimus, thanks for your PR! By analyzing the history of the files in this pull request, we identified @edolstra, @wkennington and @dezgeg to be potential reviewers.

@NeQuissimus NeQuissimus force-pushed the common_kernel_config branch 2 times, most recently from 87c7223 to af01a8b Compare August 6, 2017 23:15
@NeQuissimus NeQuissimus force-pushed the common_kernel_config branch from af01a8b to 0cf0d71 Compare August 6, 2017 23:17
@NeQuissimus NeQuissimus merged commit ed0fce4 into NixOS:master Aug 6, 2017
@NeQuissimus NeQuissimus deleted the common_kernel_config branch August 6, 2017 23:17
@dezgeg
Copy link
Contributor

dezgeg commented Aug 7, 2017

Err, why? The categories were definitely meaningful, in fact even more categorized was planned & would be much better: https://github.com/NixOS/nixpkgs/pull/12158/files. After this change it's now completely impossible to see the reasons of why e.g. ATOMIC64_SELFTEST or DEVKMEM are set like they are,
or which options should those NixOS users disable who don't want wireless support.

And why painstakingly split these by kernel version instead of just letting the question mark do its thing? E.g.:

# Disable various self-test modules that have no use in a production system
 -  ASYNC_RAID6_TEST? n
 -  ATOMIC64_SELFTEST? n
 -  BACKTRACE_SELF_TEST? n
....

The only purpose of these is to disable unnecessary stuff, and if the symbol doesn't exist, then there is simply nothing to do, the end result is the same either way! The version of the kernel didn't matter a single bit to NixOS.

This file is completely unmaintainable right now.

@joachifm
Copy link
Contributor

joachifm commented Aug 7, 2017

Can we revisit this? I agree organizing entirely around feature flags/versions is harder to follow than organizing by purpose/topic (fuzzy as that can be). Is it possible to improve things along the lines in this proposal without completely changing the primary organizing principle?

Of more immediate concern is that the config no longer builds on i686, some previously optional but now required configs are evidently platform dependent (e.g., NUMA and KEXEC_FILE).

@NeQuissimus
Copy link
Member Author

Definitely, let me revert the change... The common config needs refactoring for sure, though. All those optionals floating around everywhere does not help its readability.

In the end, I would love to see #12158 revived and our builds adapted to it, let's ping @copumpkin

NeQuissimus added a commit that referenced this pull request Aug 7, 2017
Order common kernel config by functionality
See #27949
@joachifm
Copy link
Contributor

joachifm commented Aug 7, 2017

@NeQuissimus thank you

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

6.topic: developer experience nixpkgs development workflow 6.topic: hygiene Cleaning up and removing cruft 6.topic: kernel The Linux kernel 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: clean-up This PR removes packages or removes other cruft 8.has: package (update) This PR updates a package to a newer version

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants