-
Notifications
You must be signed in to change notification settings - Fork 519
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
tools/diff-kernel-config: various small improvements #3299
tools/diff-kernel-config: various small improvements #3299
Conversation
In commit 43234d3 - tools/diff-kernel-config: Adjust script to work on variants we switched from specifying kernel versions to specifying variants. The kernel versions are then parsed from the variant definition which has advantages. Properly represent that in the usage message. Signed-off-by: Leonard Foerster <[email protected]>
When saving the full kernel version for later reference in the kernel mapping we may come across situations where we update to a newer version of our base kernel srpm from Amazon Linux which added only added patches from Amazon Linux but stayed at the same upstream base version. The before and after would be indistinguishable when only comparing the version. Add the release number to disambiguate in that situation. Signed-off-by: Leonard Foerster <[email protected]>
Sometimes, the script may fail for a specific variant after we already have spent time successfully building other variants. In order to not let that time go to waste, enable resuming of previous runs of the script. Be aware that the invocation has to be the same as the original invocation, plus the resume flag. Signed-off-by: Leonard Foerster <[email protected]>
@@ -206,7 +206,7 @@ for state in after before; do | |||
;; | |||
esac | |||
|
|||
kver_full=$(rpm --query --queryformat '%{VERSION}' "${kernel_rpm}") | |||
kver_full=$(rpm --query --queryformat '%{VERSION}-%{RELEASE}' "${kernel_rpm}") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This makes sense for completeness. Only wondering out loud: This will report the release field of Bottlerocket's RPM and not the upstream one's, right? We would have to bump the release ourselves. I've seen we've at least done that for the 5.10 series at least once, but probably have not followed it closely (or the case hasn't come up yet for the 5.15 and 6.1 kernel series).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That is correct. It probably does not make sense then as it is right now.
I will probably pull that commit from the PR then and think how we want to do this properly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd still leave it in. We have bumped the release in the past and in the best case we'll be vigilant and do so again in the future when warranted. As an aside, I've been thinking about changes to our kernel version numbering scheme recently. I'll take this case into consideration.
Issue number: -
Description of changes:
Add three improvements (two minor, one bigger) to the diff-kernel-config tool.
The big interesting change enables the script to be resumed, which can save the time one would need for rebuilding things that already had been built successfully. This is particularly helpful when using the script for multiple variants where one variant may not build successfully, but others do. We can fix whatever fails that particular build and resume, keeping the state that finished successful previously.
Testing done:
I have used unpolished versions of these changes for previous kernel updates to cut down on time to redo builds.
Terms of contribution:
By submitting this pull request, I agree that this contribution is dual-licensed under the terms of both the Apache License, version 2.0, and the MIT license.