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-tq: make localversion deterministic #6

Closed
wants to merge 2 commits into from
Closed

linux-tq: make localversion deterministic #6

wants to merge 2 commits into from

Conversation

dwagenk
Copy link

@dwagenk dwagenk commented Nov 28, 2023

The localversion string built into the kernel should be deterministic. With OpenEmbedded it is common practice to apply patches to the sources. The commit id of HEAD will differ each time the patching step is rerun. With the previous implementation of the do_set_localversion_tq task this produced non-deterministic commit ids to be used. This is undesireable, because the commit id read from the version string during runtime is not present in the sources. It also caused problems when building out- of-tree kernel-modules if the kernel itself was taken from sstate cache but the kernel-module needed to be rebuild and retriggered the kernels do_patch task (see this issue in meta-freescale, that was due to a similar localversion modification
Freescale/meta-freescale#961).

Rework to produce a deterministic string that will use the commit id given as SRCREV and count the number of patches applied on top.

With e.g. commit id 11aabbcc and 5 patches this will result in a version string
+g11aabbcc+p5

Related: Freescale/meta-freescale#1694

@dwagenk
Copy link
Author

dwagenk commented Nov 30, 2023

I'd suggest not merging this for now, there is some discussion in Freescale/meta-freescale#961 about it breaking AUTOREV.

@dwagenk dwagenk marked this pull request as draft November 30, 2023 13:42
@tq-steina
Copy link
Contributor

Thanks for the Pull Request. But the author is the commit is messed up: kas <kas@example.com>
I can fix it myself if you agree.

Now that the meta-freescale Backport has been merged Freescale/meta-freescale#1695. I will pick this one and add a similar change for sources/meta-tq/meta-tq/recipes-bsp/u-boot/u-boot-tq.inc

@tq-steina
Copy link
Contributor

@dwagenk An additional note: Could you also integrate the change from Freescale/meta-freescale@84c08eb on top?

Daniel Wagenknecht added 2 commits December 6, 2023 22:34
The localversion string built into the kernel should be deterministic.
With OpenEmbedded it is common practice to apply patches to the sources.
The commit id of HEAD will differ each time the patching step is rerun.
With the previous implementation of the do_set_localversion_tq task this
produced non-deterministic commit ids to be used. This is undesireable,
because the commit id read from the version string during runtime is
not present in the sources. It also caused problems when building out-
of-tree kernel-modules if the kernel itself was taken from sstate cache
but the kernel-module needed to be rebuild and retriggered the kernels
do_patch task (see this issue in meta-freescale, that was due to a
similar localversion modification
Freescale/meta-freescale#961).

Rework to produce a deterministic string that will use the commit id
given as SRCREV and count the number of patches applied on top.

With e.g. commit id 11aabbcc and 5 patches this will result in a version
string
  +g11aabbcc+p5

Related: Freescale/meta-freescale#1694

Signed-off-by: Daniel Wagenknecht <dwagenknecht@emlix.com>
Depending on how one uses the Yocto kernel classes the kernels hash
is defined in either "SRCREV_machine" or "SRCREV". If "SRCREV_machine"
is in use, "SRCREV" stays at its bitbake default "INVALID".

If the "SRCREV_machine" or "SRCREV" is set to "AUTOREV" that value is
replaced by "AUTOINC".

If using "SRCREV_machine" and/or "AUTOREV" do_set_localversion_tq fails.

Cope with both use cases.

Adapted from meta-freescale commit 16a356ef0d5d62090e84530e125ea6952994aaeb
by Max Krummenacher <max.krummenacher@toradex.com>

Signed-off-by: Daniel Wagenknecht <dwagenknecht@emlix.com>
@dwagenk dwagenk marked this pull request as ready for review December 6, 2023 21:35
@dwagenk
Copy link
Author

dwagenk commented Dec 6, 2023

@tq-steina addressed the commit author and imported the additional commit for AUTOINC from meta-freescale. Also fixed the task order, so the localversion task runs after the patch task.

Tested with linux-imx-tq_6.1 and with the same recipe but SRCREV set to AUTOINC. Both create the -tq+gbc136cc9c1a1+p0 localversion string. Adding a patch to SRC_URI increases the patch count as expected.

@tq-steina
Copy link
Contributor

Thanks a lot for the contribution. Your patches have passed our internal review and integration and are integrated in kirkstone branch. Once they are publishedon github. I'll close this pull request.

Thanks again

@dwagenk dwagenk closed this by deleting the head repository Jan 29, 2024
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.

None yet

2 participants