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@lists.fedoraproject.org
To unsubscribe send an email to kernel-le...@lists.fedoraproject.org
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@lists.fedoraproject.org
Do not reply to spam on the list, report it: 
https://pagure.io/fedora-infrastructure

Reply via email to