Re: [Openembedded-architecture] Workflow improvements - incremental source updates?

2023-08-21 Thread Richard Purdie
On Fri, 2023-08-18 at 16:01 +, Peter Kjellerstedt wrote:
> 
> I think we can live without it. If I am not mistaken we can always 
> inject our extra information by doing something like 
> PKGV:append = "${MAINTPV}" in recipes where we need it and then rely on 
> your code to add the SHA-1 if the output from ${MAINTPV} includes a "+".
> 
> > 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.
> 
> I am a bit unsure about the trying to avoid calls to the get_pkgv_string() 
> function during parsing, because it seems to happen with the current 
> code. When I first added your changes, I got errors for a number of 
> recipes in meta-oe that failed to set SRCREV_FORMAT (Khem has since 
> corrected those recipes). E.g., if you add meta-oe to the build and 
> revert commit 4228656bae, then you will get the following error if 
> you run `bitbake -p`:
> 
> WARNING: .../meta-oe/recipes-extended/sysdig/sysdig_0.28.0.bb: Exception 
> during build_dependencies for fetcher_hashes_dummyfunc
> WARNING: .../meta-oe/recipes-extended/sysdig/sysdig_0.28.0.bb: Error during 
> finalise of 
> /home/pkj/dists/poky-master/meta-oe/recipes-extended/sysdig/sysig_0.28.0.bb
> ERROR: ExpansionError during parsing 
> .../meta-oe/recipes-extended/sysdig/sysdig_0.28.0.bb

It will happen, we're just making it happen at a point of our choosing
rather than the current situation where it happens when PV happens to
be expanded, which could be anywhere during parsing.

> This is causing problems for me in our code base because we have a 
> number of recipes that fetch from Git repositories with restricted 
> access that I cannot fetch (we have alternative recipes that install 
> prebuilt binaries for those of us without access to the source code).
> Now all those recipes fail during parsing because bitbake cannot run 
> `git ls-remote`, even if they will never be built in my builds.

I suspect that isn't from get_pkgv_string but from get_hashvalue?

If so, I'd suggest your "internal only" recipes need to raise
SkipRecipe() if they can't function and that should avoid the issue.


Cheers,

Richard

-=-=-=-=-=-=-=-=-=-=-=-
Links: You receive all messages sent to this group.
View/Reply Online (#1739): 
https://lists.openembedded.org/g/openembedded-architecture/message/1739
Mute This Topic: https://lists.openembedded.org/mt/100618420/21656
Group Owner: openembedded-architecture+ow...@lists.openembedded.org
Unsubscribe: https://lists.openembedded.org/g/openembedded-architecture/unsub 
[arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-



Re: [Openembedded-architecture] Workflow improvements - incremental source updates?

2023-08-18 Thread Peter Kjellerstedt
> -Original Message-
> From: Richard Purdie 
> Sent: den 16 augusti 2023 13:13
> To: Peter Kjellerstedt ; 
> openembedded-architecture 
> Subject: Re: [Openembedded-architecture] Workflow improvements - incremental 
> source updates?
> 
> On Tue, 2023-08-15 at 21:27 +, 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

[cut]

> > 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.

[cut]

> I was thinking more about the "in a variable" request and there is a
> good reason for not doing it.

I think we can live without it. If I am not mistaken we can always 
inject our extra information by doing something like 
PKGV:append = "${MAINTPV}" in recipes where we need it and then rely on 
your code to add the SHA-1 if the output from ${MAINTPV} includes a "+".

> 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.

I am a bit unsure about the trying to avoid calls to the get_pkgv_string() 
function during parsing, because it seems to happen with the current 
code. When I first added your changes, I got errors for a number of 
recipes in meta-oe that failed to set SRCREV_FORMAT (Khem has since 
corrected those recipes). E.g., if you add meta-oe to the build and 
revert commit 4228656bae, then you will get the following error if 
you run `bitbake -p`:

WARNING: .../meta-oe/recipes-extended/sysdig/sysdig_0.28.0.bb: Exception during 
build_dependencies for fetcher_hashes_dummyfunc
WARNING: .../meta-oe/recipes-extended/sysdig/sysdig_0.28.0.bb: Error during 
finalise of 
/home/pkj/dists/poky-master/meta-oe/recipes-extended/sysdig/sysig_0.28.0.bb
ERROR: ExpansionError during parsing 
.../meta-oe/recipes-extended/sysdig/sysdig_0.28.0.bb

This is causing problems for me in our code base because we have a 
number of recipes that fetch from Git repositories with restricted 
access that I cannot fetch (we have alternative recipes that install 
prebuilt binaries for those of us without access to the source code). 
Now all those recipes fail during parsing because bitbake cannot run 
`git ls-remote`, even if they will never be built in my builds.

> 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

//Peter


-=-=-=-=-=-=-=-=-=-=-=-
Links: You receive all messages sent to this group.
View/Reply Online (#1738): 
https://lists.openembedded.org/g/openembedded-architecture/message/1738
Mute This Topic: https://lists.openembedded.org/mt/100618420/21656
Group Owner: openembedded-architecture+ow...@lists.openembedded.org
Unsubscribe: https://lists.openembedded.org/g/openembedded-architecture/unsub 
[arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-



Re: [Openembedded-architecture] Workflow improvements - incremental source updates?

2023-08-16 Thread Richard Purdie
On Tue, 2023-08-15 at 15:11 +, chris.lapla...@agilent.com wrote:
> Hi Richard,
> 
> > I tried some experiments and this doesn't actually turn out to be so 
> > difficult
> > to do. I've included a patch below which basically drops the SRCPV pieces
> > into PKGV, the conditional can probably be simplified further.
> > 
> > This patch isn't quite right in that we need to split get_srcrev into two
> > versions, one which returns the SRCREV_FORMAT string and one which
> > returns the full hashes for taskhash purposes.
> > 
> > The nice thing about this approach is that SRCPV can simply be removed over
> > time, it no longer does anything. That means compatibility is retained with
> > existing recipes.
> > 
> > I've not tested this extensively yet with AUTOREV but normal builds appear
> > to work ok with it and the WORKDIR directory naming simplification is
> > probably a win.
> 
> This sounds interesting; thank you for your tiresome work! I will try
> to find some time to play with the patch.
> 
> Just to add another dimension to the discussion, I wonder how (if at
> all) this could affect how cve-check.bbclass works? One thing I don't
> like about that class is that the CVE checking happens after
> do_fetch, when really all do_cve_check cares about is the resolved
> revision. Is there an opportunity here to optimize do_cve_check?
> 
> Over the past few months (maybe even longer), I've had an idea in my
> head for a task that happens before do_fetch that actually performs
> revision resolution. Maybe do_resolve_autorev or something. But I
> think your recent work obsoletes that idea. 

FWIW I suspect there are ways to make this work without a task at all,
yes. Definitely worth exploring. 

I can try and provide help if needed if someone wants to experiment
with it but I'd note the PKGV idea still isn't quite right or merged
yet!

Cheers,

Richard

-=-=-=-=-=-=-=-=-=-=-=-
Links: You receive all messages sent to this group.
View/Reply Online (#1737): 
https://lists.openembedded.org/g/openembedded-architecture/message/1737
Mute This Topic: https://lists.openembedded.org/mt/100618420/21656
Group Owner: openembedded-architecture+ow...@lists.openembedded.org
Unsubscribe: https://lists.openembedded.org/g/openembedded-architecture/unsub 
[arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-



Re: [Openembedded-architecture] Workflow improvements - incremental source updates?

2023-08-16 Thread Richard Purdie
On Tue, 2023-08-15 at 21:27 +, 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+" instead of the current 
> "AUTOINC+".
> 
> 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)
 

Re: [Openembedded-architecture] Workflow improvements - incremental source updates?

2023-08-15 Thread Richard Purdie
On Tue, 2023-08-15 at 21:27 +, Peter Kjellerstedt wrote:
> > -Original Message-
> > From: openembedded-architecture@lists.openembedded.org
> >  On Behalf Of
> > Richard Purdie
> > Sent: den 10 augusti 2023 18:53
> > To: openembedded-architecture
> > 
> > Subject: Re: [Openembedded-architecture] Workflow improvements -
> > incremental source updates?
> > 
> > On Tue, 2023-08-08 at 10:10 +0100, Richard Purdie via
> > lists.openembedded.org 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.

Thanks, I'm happy someone else is looking at this and you raise some
issues I hadn't thought about as I've been too busy trying to even get
the test suites and devtool to work correctly with the changes.

> 
> 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


I've definitely been focusing on the latter, to the expense of the
former.

> 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'):

Thanks, I'll have to look into the ordering of this. The original code
changes were in this function but that didn't work for other reasons.

> 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+" instead of the current 
> "AUTOINC+".

That would then add a separator to strings where there currently isn't
one. The intent wasn't to add to fixed versions but I've over
simplified one of the patches.

> 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.

I think we can probably come up with a heuristic like a "+" in PV
triggers the addition which should work 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 
> SRC

Re: [Openembedded-architecture] Workflow improvements - incremental source updates?

2023-08-15 Thread Peter Kjellerstedt
> -Original Message-
> From: openembedded-architecture@lists.openembedded.org 
>  On Behalf Of Richard Purdie
> Sent: den 10 augusti 2023 18:53
> To: openembedded-architecture 
> 
> Subject: Re: [Openembedded-architecture] Workflow improvements - incremental 
> source updates?
> 
> On Tue, 2023-08-08 at 10:10 +0100, Richard Purdie via lists.openembedded.org 
> wrote:
> > There is a recent discussion about AUTOREV which has got me thinking a
> > bit about the big picture and incremental development.
> >
> > Currently AUTOREV injects into PV. The reasons for that are historical,
> > when OE was not taskhash based we had to do that to trigger updates.
> > There are now different ways it could be done with the revision just
> > needing injection into PKGPV for packaging purposes.
> >
> > I've also been looking at externalsrc bugs.
> >
> > This got me thinking, if we did make AUTOREV work without messing with
> > PV, we could probably go a step further and allow the fetcher to
> > incrementally update an existing git checkout rather than remove and
> > start again from scratch. Patches may or may not be an issue, we could
> > mandate a clean single git:// srcuri to start with although the patch
> > code can remove the patch stack already. This would need to hook in to
> > disable some of the cleanup/removal code.
> >
> > The usecase would be to allow the developer to pull updates from a
> > remote repo, or even commit changes locally and then have the system
> > "flow" the changes through from there. This would remove the annoying
> > clean and re-checkout process which drives some developers crazy. It
> > might also allow externalsrc to change to a new model with less
> > deviation from the normal workflow.
> 
> I tried some experiments and this doesn't actually turn out to be so
> difficult to do. I've included a patch below which basically drops the
> SRCPV pieces into PKGV, the conditional can probably be simplified
> further.
> 
> This patch isn't quite right in that we need to split get_srcrev into
> two versions, one which returns the SRCREV_FORMAT string and one which
> returns the full hashes for taskhash purposes.
> 
> The nice thing about this approach is that SRCPV can simply be removed
> over time, it no longer does anything. That means compatibility is
> retained with existing recipes.
> 
> I've not tested this extensively yet with AUTOREV but normal builds
> appear to work ok with it and the WORKDIR directory naming
> simplification is probably a win.
> 
> Cheers,
> 
> Richard

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, 
wh

Re: [Openembedded-architecture] Workflow improvements - incremental source updates?

2023-08-10 Thread Richard Purdie
On Tue, 2023-08-08 at 10:10 +0100, Richard Purdie via lists.openembedded.org 
wrote:
> There is a recent discussion about AUTOREV which has got me thinking a
> bit about the big picture and incremental development.
> 
> Currently AUTOREV injects into PV. The reasons for that are historical,
> when OE was not taskhash based we had to do that to trigger updates.
> There are now different ways it could be done with the revision just
> needing injection into PKGPV for packaging purposes.
> 
> I've also been looking at externalsrc bugs.
> 
> This got me thinking, if we did make AUTOREV work without messing with
> PV, we could probably go a step further and allow the fetcher to
> incrementally update an existing git checkout rather than remove and
> start again from scratch. Patches may or may not be an issue, we could
> mandate a clean single git:// srcuri to start with although the patch
> code can remove the patch stack already. This would need to hook in to
> disable some of the cleanup/removal code.
> 
> The usecase would be to allow the developer to pull updates from a
> remote repo, or even commit changes locally and then have the system
> "flow" the changes through from there. This would remove the annoying
> clean and re-checkout process which drives some developers crazy. It
> might also allow externalsrc to change to a new model with less
> deviation from the normal workflow.

I tried some experiments and this doesn't actually turn out to be so
difficult to do. I've included a patch below which basically drops the
SRCPV pieces into PKGV, the conditional can probably be simplified
further.

This patch isn't quite right in that we need to split get_srcrev into
two versions, one which returns the SRCREV_FORMAT string and one which
returns the full hashes for taskhash purposes.

The nice thing about this approach is that SRCPV can simply be removed
over time, it no longer does anything. That means compatibility is
retained with existing recipes.

I've not tested this extensively yet with AUTOREV but normal builds
appear to work ok with it and the WORKDIR directory naming
simplification is probably a win.

Cheers,

Richard

From: Richard Purdie 
Subject: fetch: Move SRCPV from PV to PKGV

Signed-off-by: Richard Purdie 
---
 bitbake/lib/bb/fetch2/__init__.py   |  5 -
 meta/classes-global/base.bbclass| 26 +-
 meta/classes-global/package.bbclass |  6 +-
 meta/conf/bitbake.conf  |  5 +
 4 files changed, 11 insertions(+), 31 deletions(-)

diff --git a/bitbake/lib/bb/fetch2/__init__.py 
b/bitbake/lib/bb/fetch2/__init__.py
index ad9d8a31a7e..ea51a009e1d 100644
--- a/bitbake/lib/bb/fetch2/__init__.py
+++ b/bitbake/lib/bb/fetch2/__init__.py
@@ -753,7 +753,7 @@ def get_autorev(d):
 d.setVar("__BBAUTOREV_SEEN", True)
 return "AUTOINC"
 
-def get_srcrev(d, method_name='sortable_revision'):
+def get_srcrev(d, method_name='sortable_revision', allow_none=False):
 """
 Return the revision string, usually for use in the version string (PV) of 
the current package
 Most packages usually only have one SCM so we just pass on the call.
@@ -781,6 +781,9 @@ def get_srcrev(d, method_name='sortable_revision'):
 scms.append(u)
 
 if not scms:
+if allow_none:
+d.delVar("__BBINSRCREV")
+return ""
 raise FetchError("SRCREV was used yet no valid SCM was found in 
SRC_URI")
 
 if len(scms) == 1 and len(urldata[scms[0]].names) == 1:
diff --git a/meta/classes-global/base.bbclass b/meta/classes-global/base.bbclass
index cbda8d12f09..2567b7d24be 100644
--- a/meta/classes-global/base.bbclass
+++ b/meta/classes-global/base.bbclass
@@ -130,7 +130,7 @@ addtask fetch
 do_fetch[dirs] = "${DL_DIR}"
 do_fetch[file-checksums] = "${@bb.fetch.get_checksum_file_list(d)}"
 do_fetch[file-checksums] += " ${@get_lic_checksum_file_list(d)}"
-do_fetch[vardeps] += "SRCREV"
+do_fetch[vardepvalue] += "${@bb.fetch.get_srcrev(d, allow_none=True)}"
 do_fetch[network] = "1"
 python base_do_fetch() {
 
@@ -606,7 +606,6 @@ python () {
 bb.debug(1, "Skipping recipe %s because of incompatible 
license(s): %s" % (pn, ' '.join(incompatible_lic)))
 raise bb.parse.SkipRecipe("it has incompatible license(s): 
%s" % ' '.join(incompatible_lic))
 
-needsrcrev = False
 srcuri = d.getVar('SRC_URI')
 for uri_string in srcuri.split():
 uri = bb.fetch.URI(uri_string)
@@ -619,24 +618,17 @@ python () {
 
 # Svn packages should DEPEND on subversion-native
 if uri.scheme == "svn":
-needsrcrev = True
 d.appendVarFlag('do_fetch', 'depends', ' 
subversion-native:do_populate_sysroot')
 
 # Git packages should DEPEND on git-native
 elif uri.scheme in ("git", "gitsm"):
-needsrcrev = True
 d.appendVarFlag('do_fetch', 'depends', ' 
git-native:do_populate_sysroot')
 
 # Mercurial packages should DEPEND on 

Re: [Openembedded-architecture] Workflow improvements - incremental source updates?

2023-08-09 Thread Richard Purdie
On Wed, 2023-08-09 at 22:49 +0100, Richard Purdie via
lists.openembedded.org wrote:
> On Wed, 2023-08-09 at 14:03 +, Peter Kjellerstedt wrote:
> > 
> > One question though: would any of this make it possible to use 
> > ${AUTOREV} together with BB_SRCREV_POLICY = "cache"?
> 
> No, since those two things are mutually exclusive.

Let me put this another way. I don't think those two options can ever
work together. 

It may be possible to add a different policy to do what you're thinking
of. The main issue with that is documentation, automated tests and
making it easy for users to know which they want. That is probably
possible but not easy.

Cheers,

Richard



-=-=-=-=-=-=-=-=-=-=-=-
Links: You receive all messages sent to this group.
View/Reply Online (#1730): 
https://lists.openembedded.org/g/openembedded-architecture/message/1730
Mute This Topic: https://lists.openembedded.org/mt/100618420/21656
Group Owner: openembedded-architecture+ow...@lists.openembedded.org
Unsubscribe: https://lists.openembedded.org/g/openembedded-architecture/unsub 
[arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-



Re: [Openembedded-architecture] Workflow improvements - incremental source updates?

2023-08-09 Thread Richard Purdie
On Wed, 2023-08-09 at 14:03 +, Peter Kjellerstedt wrote:
> > -Original Message-
> > From: openembedded-architecture@lists.openembedded.org
> >  > architect...@lists.openembedded.org> On Behalf Of Richard Purdie
> > Sent: den 8 augusti 2023 11:11
> > To: openembedded-architecture  > architect...@lists.openembedded.org>
> > Subject: [Openembedded-architecture] Workflow improvements -
> > incremental
> > source updates?
> > 
> > There is a recent discussion about AUTOREV which has got me
> > thinking a
> > bit about the big picture and incremental development.
> > 
> > Currently AUTOREV injects into PV. The reasons for that are
> > historical,
> > when OE was not taskhash based we had to do that to trigger
> > updates.
> > There are now different ways it could be done with the revision
> > just
> > needing injection into PKGPV for packaging purposes.
> > 
> > I've also been looking at externalsrc bugs.
> > 
> > This got me thinking, if we did make AUTOREV work without messing
> > with
> > PV, we could probably go a step further and allow the fetcher to
> > incrementally update an existing git checkout rather than remove
> > and
> > start again from scratch. Patches may or may not be an issue, we
> > could
> > mandate a clean single git:// srcuri to start with although the
> > patch
> > code can remove the patch stack already. This would need to hook in
> > to
> > disable some of the cleanup/removal code.
> > 
> > The usecase would be to allow the developer to pull updates from a
> > remote repo, or even commit changes locally and then have the
> > system
> > "flow" the changes through from there. This would remove the
> > annoying
> > clean and re-checkout process which drives some developers crazy.
> > It
> > might also allow externalsrc to change to a new model with less
> > deviation from the normal workflow.
> > 
> > Whilst on the subject, I have also been wondering about a new image
> > target, effectively one that would build a new image but then allow
> > an
> > rsync over to a running target. Obviously this may or may not be
> > advisable depending on what was actually running but for some
> > development, it could be useful.
> > 
> > Anyway, just thinking out loud but wanted to document the idea. I
> > might
> > look into at least improving the way AUTOREV works...
> > 
> > Cheers,
> > 
> > Richard
> 
> It sounds like there are a lot of good possibilities here.
> 
> One question though: would any of this make it possible to use 
> ${AUTOREV} together with BB_SRCREV_POLICY = "cache"?

No, since those two things are mutually exclusive.

Cheers,

Richard

-=-=-=-=-=-=-=-=-=-=-=-
Links: You receive all messages sent to this group.
View/Reply Online (#1729): 
https://lists.openembedded.org/g/openembedded-architecture/message/1729
Mute This Topic: https://lists.openembedded.org/mt/100618420/21656
Group Owner: openembedded-architecture+ow...@lists.openembedded.org
Unsubscribe: https://lists.openembedded.org/g/openembedded-architecture/unsub 
[arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-



Re: [Openembedded-architecture] Workflow improvements - incremental source updates?

2023-08-09 Thread Peter Kjellerstedt
> -Original Message-
> From: openembedded-architecture@lists.openembedded.org  architect...@lists.openembedded.org> On Behalf Of Richard Purdie
> Sent: den 8 augusti 2023 11:11
> To: openembedded-architecture  architect...@lists.openembedded.org>
> Subject: [Openembedded-architecture] Workflow improvements - incremental
> source updates?
> 
> There is a recent discussion about AUTOREV which has got me thinking a
> bit about the big picture and incremental development.
> 
> Currently AUTOREV injects into PV. The reasons for that are historical,
> when OE was not taskhash based we had to do that to trigger updates.
> There are now different ways it could be done with the revision just
> needing injection into PKGPV for packaging purposes.
> 
> I've also been looking at externalsrc bugs.
> 
> This got me thinking, if we did make AUTOREV work without messing with
> PV, we could probably go a step further and allow the fetcher to
> incrementally update an existing git checkout rather than remove and
> start again from scratch. Patches may or may not be an issue, we could
> mandate a clean single git:// srcuri to start with although the patch
> code can remove the patch stack already. This would need to hook in to
> disable some of the cleanup/removal code.
> 
> The usecase would be to allow the developer to pull updates from a
> remote repo, or even commit changes locally and then have the system
> "flow" the changes through from there. This would remove the annoying
> clean and re-checkout process which drives some developers crazy. It
> might also allow externalsrc to change to a new model with less
> deviation from the normal workflow.
> 
> Whilst on the subject, I have also been wondering about a new image
> target, effectively one that would build a new image but then allow an
> rsync over to a running target. Obviously this may or may not be
> advisable depending on what was actually running but for some
> development, it could be useful.
> 
> Anyway, just thinking out loud but wanted to document the idea. I might
> look into at least improving the way AUTOREV works...
> 
> Cheers,
> 
> Richard

It sounds like there are a lot of good possibilities here.

One question though: would any of this make it possible to use 
${AUTOREV} together with BB_SRCREV_POLICY = "cache"?

//Peter


-=-=-=-=-=-=-=-=-=-=-=-
Links: You receive all messages sent to this group.
View/Reply Online (#1728): 
https://lists.openembedded.org/g/openembedded-architecture/message/1728
Mute This Topic: https://lists.openembedded.org/mt/100618420/21656
Group Owner: openembedded-architecture+ow...@lists.openembedded.org
Unsubscribe: https://lists.openembedded.org/g/openembedded-architecture/unsub 
[arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-