On Tue, 2023-08-15 at 21:27 +0000, Peter Kjellerstedt wrote:
>
> I decided to give the related changes you have in master-next a try. I
> cherry-picked the following three commits:
>
> bitbake: fetch2: Add new srcrev fetcher API
> recipes/classes/scripts: Drop SRCPV usage in OE-Core
> base/package: Move source revision information from PV to PKGV
>
> I am of course aware that the above changes are not final, but I have
> some concerns.
>
> I tested building "iso-codes" and "xcursor-transparent-theme". They are
> both allarch recipes with few dependencies. The former has a SRCREV that
> matches PV, and the latter does not (and thus uses PV = "0.1.1+git${SRCPV}").
>
> Here are the names of the resulting RPM packages before the changes are
> applied:
>
> iso-codes-4.15.0-r0.4.noarch.rpm
> xcursor-transparent-theme-0.1.1+git0+23c8af5ba4-r0.0.noarch.rpm
>
> And here they are afterwards:
>
> iso-codes-4.15.0AUTOINC+69ba16daef-r0.0.noarch.rpm
> xcursor-transparent-theme-0.1.1+gitAUTOINC+23c8af5ba4-r0.0.noarch.rpm
>
> The first problem is of course that AUTOINC is not replaced as intended
> when a PR server is active. This is due to the code in package_get_auto_pr()
> that should update PRSERV_PV_AUTOINC not being triggered and thus it
> retains its default value "AUTOINC". The code isn't triggered because it
> looks for "AUTOINC" in PKGV, but it isn't there (yet) since ${SRCPV} is
> no longer used. The following patch solves it:
>
> diff --git a/meta/classes-global/package.bbclass
> b/meta/classes-global/package.bbclass
> index 7f55b123c4..bf824b0b83 100644
> --- a/meta/classes-global/package.bbclass
> +++ b/meta/classes-global/package.bbclass
> @@ -266,6 +266,9 @@ python package_get_auto_pr() {
> d.setVar("PRSERV_HOST", host)
>
> pkgv = d.getVar("PKGV")
> + srcpv = bb.fetch.get_pkgv_string(d)
> + if srcpv:
> + pkgv += srcpv
>
> # PR Server not active, handle AUTOINC
> if not d.getVar('PRSERV_HOST'):
>
> The second problem is that the value from bb.fetch.get_pkgv_string() is
> concatenated to the version without any separator. So if the AUTOINC
> part had been replaced as expected, the RPM name for the iso-codes
> package would have become iso-codes-4.15.00+69ba16daef-r0.0.noarch.rpm,
> which I doubt was intended... The easiest solution should be to make
> get_pkgv_string() return "+AUTOINC+<ID>" instead of the current
> "AUTOINC+<ID>".
>
> Third, I am not sure if it is a good idea to always include the SHA-1
> in the package name. Before it was typically only done when the SRCREV
> did not match PV. For most packages it is redundant. It also means that
> the PR server needs to keep track of the version for all Git recipes,
> not just the ones where ${SRCPV} is used today. However, I understand
> that it is probably not easily solvable to automatically detect when
> it is needed and when it is not. So it is either do it manually as
> today and risk missing to do it when it is needed, or always do it and
> have it be redundant in most cases.
>
> One thing that I would prefer is if there is a variable, similar to
> the old SRCPV, that contains "${@bb.fetch.get_pkgv_string(d)}" by
> default and is used from package_convert_pr_autoinc() instead of
> calling the method directly. The reason for that is to make it
> possible to use a different method to determine the value. E.g., in
> our case we have a variable called MAINTPV, which is similar to
> SRCPV. The reason for it is that it handles Gerrit references and
> expands them in a more suitable way than just a SHA-1. Thus I would
> like to be able to use its underlying function instead of
> bb.fetch.get_pkgv_string(d) to generate the string for PKGV.
I think this patch might address some of the issues:
diff --git a/meta/classes-global/package.bbclass
b/meta/classes-global/package.bbclass
index 7f55b123c4..0338a5c690 100644
--- a/meta/classes-global/package.bbclass
+++ b/meta/classes-global/package.bbclass
@@ -315,17 +315,21 @@ python package_get_auto_pr() {
# Package functions suitable for inclusion in PACKAGEFUNCS
#
-python package_convert_pr_autoinc() {
+python package_setup_pkgv() {
+ pkgv = d.getVar("PKGV")
# Expand SRCPV into PKGV if not present
srcpv = bb.fetch.get_pkgv_string(d)
- if srcpv:
+ if srcpv and "+" in pkgv:
d.appendVar("PKGV", srcpv)
+ pkgv = d.getVar("PKGV")
- pkgv = d.getVar("PKGV")
# Adjust pkgv as necessary...
if 'AUTOINC' in pkgv:
d.setVar("PKGV", pkgv.replace("AUTOINC", "${PRSERV_PV_AUTOINC}"))
+}
+
+python package_convert_pr_autoinc() {
# Change PRSERV_PV_AUTOINC and EXTENDPRAUTO usage to special values
d.setVar('PRSERV_PV_AUTOINC', '@PRSERV_PV_AUTOINC@')
d.setVar('EXTENDPRAUTO', '@EXTENDPRAUTO@')
@@ -498,6 +502,7 @@ python do_package () {
oe.qa.handle_error("var-undefined", msg, d)
return
+ bb.build.exec_func("package_setup_pkgv", d)
bb.build.exec_func("package_convert_pr_autoinc", d)
# Check for conflict between renamed packages and existing ones
@@ -581,6 +586,7 @@ addtask do_package_setscene
# Copy from PKGDESTWORK to tempdirectory as tempdirectory can be cleaned at
both
# do_package_setscene and do_packagedata_setscene leading to races
python do_packagedata () {
+ bb.build.exec_func("package_setup_pkgv", d)
bb.build.exec_func("package_get_auto_pr", d)
src = d.expand("${PKGDESTWORK}")
I was thinking more about the "in a variable" request and there is a
good reason for not doing it. I'm deliberately trying to avoid putting
the function call into a variable as it then becomes part of the task
hash computations. This means there are calls into it during parsing
rather than at runtime. That was part of the pain in the previous
approach and something I'm keen to avoid this time around to simplify
things if we can. It does get included in the taskhashes after the
changes but at points we control (vardepvalue) rather than 'random'
expansion points of a common variable like PV during parsing.
If we did need control the use of it, we'd have to use some other
variable to control it rather than the older approach. We then come
back to my other feeling of wanting to get it right so people don't
need to change it.
Cheers,
Richard
-=-=-=-=-=-=-=-=-=-=-=-
Links: You receive all messages sent to this group.
View/Reply Online (#1736):
https://lists.openembedded.org/g/openembedded-architecture/message/1736
Mute This Topic: https://lists.openembedded.org/mt/100618420/21656
Group Owner: [email protected]
Unsubscribe: https://lists.openembedded.org/g/openembedded-architecture/unsub
[[email protected]]
-=-=-=-=-=-=-=-=-=-=-=-
