From: Thorsten Leemhuis on gitlab.com https://gitlab.com/cki-project/kernel-ark/-/merge_requests/1757#note_944982234 I quickly took a look at the state of things after this patchset and noticed a few odd things I thought I mention; some or all of them are from before this patchset, but with all the changes you are doing maybe you are interested in these comments. I can file separate issues for any items raised here if you are interested, too. * is there a specific reason why kernel-ark is using the first 15 characters of the commit-id for snapshot naming instead of just 12 characters, as typically used by kernel developers upstream? I ask as I ran into failures like `5.18.0-0.rc0.20220401gite8b767f5e04097a.15.vanilla.1.fc36.aarch64 exceeds 64 characters` with my vanilla builds. Seems the `20220401git` part is gone after these patches which would solve this for me, but I'd say being more in line with what upstream typically does would be nice. I also looked at the generated kernel.spec for fedora and noticed a few things that sill look odd, so I'll quickly mention them as well: * Is a `%define kversion 5` worth it if `kversion` is later just used once? * This whole section looks odd too me: ``` %define specversion 5.18.0 %define patchversion 5.18 %define pkgrelease 0.rc6.47.test %define kversion 5 %define tarfile_release 5.18-rc6 # This is needed to do merge window version magic %define patchlevel 18 # allow pkg_release to have configurable %%{?dist} tag %define specrelease 0.rc6.47%{?buildid}%{?dist} # # End of genspec.sh variables # %define pkg_release %{specrelease} ``` Having both `pkgrelease` and `pkg_release` with slightly different contents for example is confusing. And why are we defining `specrelease` just to assign it to `pkg_release` a little later? And there is so much redundancy here, if someone wants to change the spec file directly downstream (say from a SRPM) this gets hard. To illustrate what I mean: shouldn't the section above maybe look more like this (untested! just for illustration!)? ``` %define kversion 5 %define patchlevel 18 %define patchversion %{kversion}.%{patchlevel} %define specversion %{patchversion}.0 %define pkgrelease 0.rc6.47.test %define tarfile_release %{patchversion}-rc6 # # End of genspec.sh variables # %define pkg_release 0.rc6.47%{?buildid}%{?dist} ``` Still not nice and definitely won't cover quite a few use cases, but as I said: I just wanted to illustrate how things could look a bit easier to understand in the resulting spec file. _______________________________________________ kernel mailing list -- kernel@xxxxxxxxxxxxxxxxxxxxxxx To unsubscribe send an email to kernel-leave@xxxxxxxxxxxxxxxxxxxxxxx Fedora Code of Conduct: https://docs.fedoraproject.org/en-US/project/code-of-conduct/ List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedoraproject.org/archives/list/kernel@xxxxxxxxxxxxxxxxxxxxxxx Do not reply to spam on the list, report it: https://pagure.io/fedora-infrastructure
