This repository has been archived by the owner. It is now read-only.

kernel config checking #589

Merged
merged 7 commits into from Sep 18, 2017

Conversation

Projects
None yet
2 participants
@ata2001
Member

ata2001 commented Sep 16, 2017

#554

ata2001 added some commits Sep 16, 2017

@ata2001

This comment has been minimized.

Show comment
Hide comment
@ata2001

ata2001 Sep 16, 2017

Member

Could easily be extended to support string or integer options.

Member

ata2001 commented Sep 16, 2017

Could easily be extended to support string or integer options.

ollieparanoid added some commits Sep 18, 2017

Refactor code to be more like the rest of the codebase
* move is_set() function out of check()
* use a dictionary in the config instead of tuples, so we have
  descriptive variable names instead of option[0] and option[1]
* merge duplicate warning code
* raise exception, when the value is not True/False
Allow checking multiple kernels and configs at once, add to travis
* Check all configs in an aport, otherwise the selected device's
  arch gets used - which may not make sense when the selected
  device is qemu-amd64 for example.
* Allow specifying multiple kernel packages, and also no packages
  which defaults to scanning all kernel configs (it is super fast
  anyway)
* Add the check to Travis CI
@ollieparanoid

This comment has been minimized.

Show comment
Hide comment
@ollieparanoid

ollieparanoid Sep 18, 2017

Member

I have refactored some details to match the rest of the code-base better, and applied the following functional changes:

  • Moved is_set to an own function, and return True/False if it was found or not (the regex function would return None or a regex object otherwise, which means we can't use it in simple True/False checks directly)
  • Raise a meaningful exception, when the config lists something other than True/False as value
  • Return True/False to indicate if the check was successful or not
  • Use that information to raise an exception on failure, when called with "pmbootstrap kconfig_check"
  • Always check the configs of all architectures, not just the current device architecture (otherwise, when qemu-amd64 is the current device, you can't check the other device configs for example)
  • Allow to specify multiple packages/kernels
  • Allow to leave out the packages/kernels argument, check all kernels in that case (it is super fast anyway)
  • Add the check for all kernels to the Travis CI config

I hope that the changes are okay with you - please tell me if you don't like any of this and I'll revert it. Thanks a lot for making this PR, I'm sure this will prevent a lot of frustration caused by wrong kernel configs.

Member

ollieparanoid commented Sep 18, 2017

I have refactored some details to match the rest of the code-base better, and applied the following functional changes:

  • Moved is_set to an own function, and return True/False if it was found or not (the regex function would return None or a regex object otherwise, which means we can't use it in simple True/False checks directly)
  • Raise a meaningful exception, when the config lists something other than True/False as value
  • Return True/False to indicate if the check was successful or not
  • Use that information to raise an exception on failure, when called with "pmbootstrap kconfig_check"
  • Always check the configs of all architectures, not just the current device architecture (otherwise, when qemu-amd64 is the current device, you can't check the other device configs for example)
  • Allow to specify multiple packages/kernels
  • Allow to leave out the packages/kernels argument, check all kernels in that case (it is super fast anyway)
  • Add the check for all kernels to the Travis CI config

I hope that the changes are okay with you - please tell me if you don't like any of this and I'll revert it. Thanks a lot for making this PR, I'm sure this will prevent a lot of frustration caused by wrong kernel configs.

@ata2001

This comment has been minimized.

Show comment
Hide comment
@ata2001

ata2001 Sep 18, 2017

Member

It's much better now.

Member

ata2001 commented Sep 18, 2017

It's much better now.

ollieparanoid added some commits Sep 18, 2017

Adjust kernel configs, so they pass the kconfig_check.
(I've had to put in a lot of defaults in the aarch64
linux-postmarketos configs, that's why the diff is a bit unclean.)

@ollieparanoid ollieparanoid merged commit ae6a58b into master Sep 18, 2017

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details

@ollieparanoid ollieparanoid deleted the kconfig_check branch Sep 18, 2017

PureTryOut added a commit that referenced this pull request Feb 21, 2018

Close #554: kernel config checking (#589)
* Check kernel config
* Allow specifying multiple kernel packages, and also no packages
  which defaults to scanning all kernel configs (it is super fast
  anyway)
* Add the check to Travis CI
* Adjust existing kernel configs, so they pass the kconfig_check.
(We've had to put in a lot of defaults in the aarch64
linux-postmarketos configs, that's why the diff is a bit unclean.)
* Increase modified kernel pkgrels
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.