[master][zeus][PATCH 1/3] knotty: Make the bb.command.CommandExit event terminate bitbake

[master][zeus][PATCH 1/3] knotty: Make the bb.command.CommandExit event terminate bitbake

[Peter Kjellerstedt]

This matches the other bb.command.Command* events and without it,
running `bitbake --revisions-changed` will hang indefinitely if there
are changed revisions.

Signed-off-by: Peter Kjellerstedt <peter.kjellerstedt@axis.com>
---
 bitbake/lib/bb/ui/knotty.py | 1 +
 1 file changed, 1 insertion(+)

diff --git a/bitbake/lib/bb/ui/knotty.py b/bitbake/lib/bb/ui/knotty.py
index a0340dfc20..cbb289b05f 100644
--- a/bitbake/lib/bb/ui/knotty.py
+++ b/bitbake/lib/bb/ui/knotty.py
@@ -590,6 +590,7 @@ def main(server, eventHandler, params, tf = TerminalFilter):
             if isinstance(event, bb.command.CommandExit):
                 if not return_value:
                     return_value = event.exitcode
+                main.shutdown = 2
                 continue
             if isinstance(event, (bb.command.CommandCompleted, bb.cooker.CookerExit)):
                 main.shutdown = 2
-- 
2.21.1

[master][zeus][PATCH 2/3] fetch2: Make fetcher_compare_revisions() work

[Peter Kjellerstedt]

This seems to have been broken for a very long time. Now it also works
regardless of BB_SRCREV_POLICY.

Signed-off-by: Peter Kjellerstedt <peter.kjellerstedt@axis.com>
---
 bitbake/lib/bb/fetch2/__init__.py | 33 +++++++++++++------------------
 1 file changed, 14 insertions(+), 19 deletions(-)

diff --git a/bitbake/lib/bb/fetch2/__init__.py b/bitbake/lib/bb/fetch2/__init__.py
index 1b7f573642..56462767e5 100644
--- a/bitbake/lib/bb/fetch2/__init__.py
+++ b/bitbake/lib/bb/fetch2/__init__.py
@@ -485,17 +485,22 @@ def fetcher_init(d):
     Called to initialize the fetchers once the configuration data is known.
     Calls before this must not hit the cache.
     """
+
+    revs = bb.persist_data.persist('BB_URI_HEADREVS', d)
+    try:
+        # fetcher_init is called multiple times, so make sure we only save the
+        # revs the first time it is called.
+        if not bb.fetch2.saved_headrevs:
+            bb.fetch2.saved_headrevs = dict(revs)
+    except:
+        pass
+
     # When to drop SCM head revisions controlled by user policy
     srcrev_policy = d.getVar('BB_SRCREV_POLICY') or "clear"
     if srcrev_policy == "cache":
         logger.debug(1, "Keeping SRCREV cache due to cache policy of: %s", srcrev_policy)
     elif srcrev_policy == "clear":
         logger.debug(1, "Clearing SRCREV cache due to cache policy of: %s", srcrev_policy)
-        revs = bb.persist_data.persist('BB_URI_HEADREVS', d)
-        try:
-            bb.fetch2.saved_headrevs = revs.items()
-        except:
-            pass
         revs.clear()
     else:
         raise FetchError("Invalid SRCREV cache policy of: %s" % srcrev_policy)
@@ -514,22 +519,12 @@ def fetcher_parse_done():
 
 def fetcher_compare_revisions(d):
     """
-    Compare the revisions in the persistant cache with current values and
-    return true/false on whether they've changed.
+    Compare the revisions in the persistent cache with the saved values from
+    when bitbake was started and return true if they have changed.
     """
 
-    data = bb.persist_data.persist('BB_URI_HEADREVS', d).items()
-    data2 = bb.fetch2.saved_headrevs
-
-    changed = False
-    for key in data:
-        if key not in data2 or data2[key] != data[key]:
-            logger.debug(1, "%s changed", key)
-            changed = True
-            return True
-        else:
-            logger.debug(2, "%s did not change", key)
-    return False
+    headrevs = dict(bb.persist_data.persist('BB_URI_HEADREVS', d))
+    return headrevs != bb.fetch2.saved_headrevs
 
 def mirror_from_string(data):
     mirrors = (data or "").replace('\\n',' ').split()
-- 
2.21.1

[master][zeus][PATCH 3/3] fetch2: Allow ${AUTOREV} to be used when BB_SRCREV_POLICY is "cache"

[Peter Kjellerstedt]

Mark any keys used to cache the srcrevs for a recipe as "dontcache" if
BB_DONT_CACHE is set for the recipe. Remove any such keys upon the
next bitbake run even if BB_SRCREV_POLICY is set to "cache". This will
make sure the srcrev is updated as expected if ${AUTOREV} is used.

Signed-off-by: Peter Kjellerstedt <peter.kjellerstedt@axis.com>
---
 bitbake/lib/bb/fetch2/__init__.py | 18 ++++++++++++++----
 1 file changed, 14 insertions(+), 4 deletions(-)

diff --git a/bitbake/lib/bb/fetch2/__init__.py b/bitbake/lib/bb/fetch2/__init__.py
index 56462767e5..b83347ad90 100644
--- a/bitbake/lib/bb/fetch2/__init__.py
+++ b/bitbake/lib/bb/fetch2/__init__.py
@@ -499,6 +499,11 @@ def fetcher_init(d):
     srcrev_policy = d.getVar('BB_SRCREV_POLICY') or "clear"
     if srcrev_policy == "cache":
         logger.debug(1, "Keeping SRCREV cache due to cache policy of: %s", srcrev_policy)
+        # We still need to remove keys that are marked with "dontcache".
+        for key in list(revs.keys()):
+            if key.startswith("dontcache-"):
+                logger.debug(1, "Removing SRCREV key: %s" % key)
+                revs.pop(key)
     elif srcrev_policy == "clear":
         logger.debug(1, "Clearing SRCREV cache due to cache policy of: %s", srcrev_policy)
         revs.clear()
@@ -729,9 +734,8 @@ def subprocess_setup():
     signal.signal(signal.SIGPIPE, signal.SIG_DFL)
 
 def get_autorev(d):
-    #  only not cache src rev in autorev case
-    if d.getVar('BB_SRCREV_POLICY') != "cache":
-        d.setVar('BB_DONT_CACHE', '1')
+    # Do not cache the srcrev in the autorev case
+    d.setVar('BB_DONT_CACHE', '1')
     return "AUTOINC"
 
 def get_srcrev(d, method_name='sortable_revision'):
@@ -1594,7 +1598,13 @@ class FetchMethod(object):
         return True, str(latest_rev)
 
     def generate_revision_key(self, ud, d, name):
-        return self._revision_key(ud, d, name)
+        key = self._revision_key(ud, d, name)
+        if d.getVar('BB_DONT_CACHE'):
+            # Mark the key so it can be removed on the next bitbake run even if
+            # BB_SRCREV_POLICY is "cache".
+            return "dontcache-%s" % key
+        else:
+            return key
 
     def latest_versionstring(self, ud, d):
         """
-- 
2.21.1

Re: [master][zeus][PATCH 1/3] knotty: Make the bb.command.CommandExit event terminate bitbake

[Peter Kjellerstedt]

> -----Original Message-----
> From: bitbake-devel-bounces@lists.openembedded.org <bitbake-devel-
> bounces@lists.openembedded.org> On Behalf Of Peter Kjellerstedt
> Sent: den 15 februari 2020 04:30
> To: bitbake-devel@lists.openembedded.org
> Subject: [bitbake-devel] [master][zeus][PATCH 1/3] knotty: Make the
> bb.command.CommandExit event terminate bitbake
> 
> This matches the other bb.command.Command* events and without it,
> running `bitbake --revisions-changed` will hang indefinitely if there
> are changed revisions.
> 
> Signed-off-by: Peter Kjellerstedt <peter.kjellerstedt@axis.com>
> ---
>  bitbake/lib/bb/ui/knotty.py | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/bitbake/lib/bb/ui/knotty.py b/bitbake/lib/bb/ui/knotty.py
> index a0340dfc20..cbb289b05f 100644
> --- a/bitbake/lib/bb/ui/knotty.py
> +++ b/bitbake/lib/bb/ui/knotty.py
> @@ -590,6 +590,7 @@ def main(server, eventHandler, params, tf = TerminalFilter):
>              if isinstance(event, bb.command.CommandExit):
>                  if not return_value:
>                      return_value = event.exitcode
> +                main.shutdown = 2
>                  continue
>              if isinstance(event, (bb.command.CommandCompleted, bb.cooker.CookerExit)):
>                  main.shutdown = 2
> --
> 2.21.1

Please backport the three patches in this set to Zeus now that they have 
been integrated to master.

//Peter

Re: [master][zeus][PATCH 1/3] knotty: Make the bb.command.CommandExit event terminate bitbake

[akuster808]

got them.

had to cherry pick as the patches did not apply.

- armin

On 3/16/20 4:19 AM, Peter Kjellerstedt wrote:
>> -----Original Message-----
>> From: bitbake-devel-bounces@lists.openembedded.org <bitbake-devel-
>> bounces@lists.openembedded.org> On Behalf Of Peter Kjellerstedt
>> Sent: den 15 februari 2020 04:30
>> To: bitbake-devel@lists.openembedded.org
>> Subject: [bitbake-devel] [master][zeus][PATCH 1/3] knotty: Make the
>> bb.command.CommandExit event terminate bitbake
>>
>> This matches the other bb.command.Command* events and without it,
>> running `bitbake --revisions-changed` will hang indefinitely if there
>> are changed revisions.
>>
>> Signed-off-by: Peter Kjellerstedt <peter.kjellerstedt@axis.com>
>> ---
>>  bitbake/lib/bb/ui/knotty.py | 1 +
>>  1 file changed, 1 insertion(+)
>>
>> diff --git a/bitbake/lib/bb/ui/knotty.py b/bitbake/lib/bb/ui/knotty.py
>> index a0340dfc20..cbb289b05f 100644
>> --- a/bitbake/lib/bb/ui/knotty.py
>> +++ b/bitbake/lib/bb/ui/knotty.py
>> @@ -590,6 +590,7 @@ def main(server, eventHandler, params, tf = TerminalFilter):
>>              if isinstance(event, bb.command.CommandExit):
>>                  if not return_value:
>>                      return_value = event.exitcode
>> +                main.shutdown = 2
>>                  continue
>>              if isinstance(event, (bb.command.CommandCompleted, bb.cooker.CookerExit)):
>>                  main.shutdown = 2
>> --
>> 2.21.1
> Please backport the three patches in this set to Zeus now that they have 
> been integrated to master.
>
> //Peter
>

Re: [master][zeus][PATCH 3/3] fetch2: Allow ${AUTOREV} to be used when BB_SRCREV_POLICY is "cache"

[Khem Raj]

[-- Attachment #1: Type: text/plain, Size: 407 bytes --]

removing

if d.getVar('BB_SRCREV_POLICY') != "cache":

Will cause autorevs to be excluded from policy and its not documented

https://www.yoctoproject.org/docs/2.1/bitbake-user-manual/bitbake-user-manual.html#var-BB_SRCREV_POLICY

did not state that AUTOREVs plus BB_SRCREV_POLICY behavior, this patch is regressing the case
where we rely on this behavior to use externalsrc on AUTOREVed recipes.

[-- Attachment #2: Type: text/html, Size: 594 bytes --]

Re: [bitbake-devel] [master][zeus][PATCH 3/3] fetch2: Allow ${AUTOREV} to be used when BB_SRCREV_POLICY is "cache"

[Peter Kjellerstedt]

[-- Attachment #1: Type: text/plain, Size: 1026 bytes --]

Rather than blindly reverting, can’t we update the documentation instead? If you use ${AUTOREV} you obviously do it because you always want the latest revision.

Before this change, using ${AUTOREV} in combination with BB_SRCREV_POLICY = “cache” didn’t work, which also wasn’t documented.

//Peter

From: bitbake-devel@lists.openembedded.org <bitbake-devel@lists.openembedded.org> On Behalf Of Khem Raj
Sent: den 21 mars 2020 22:44
To: bitbake-devel@lists.openembedded.org
Subject: Re: [bitbake-devel] [master][zeus][PATCH 3/3] fetch2: Allow ${AUTOREV} to be used when BB_SRCREV_POLICY is "cache"

removing

if d.getVar('BB_SRCREV_POLICY') != "cache":

Will cause autorevs to be excluded from policy and its not documented

https://www.yoctoproject.org/docs/2.1/bitbake-user-manual/bitbake-user-manual.html#var-BB_SRCREV_POLICY

did not state that AUTOREVs plus BB_SRCREV_POLICY behavior, this patch is regressing the case
where we rely on this behavior to use externalsrc on AUTOREVed recipes.

[-- Attachment #2: Type: text/html, Size: 4184 bytes --]

Re: [bitbake-devel] [master][zeus][PATCH 3/3] fetch2: Allow ${AUTOREV} to be used when BB_SRCREV_POLICY is "cache"

[Khem Raj]

On Sat, Mar 21, 2020 at 4:50 PM Peter Kjellerstedt
<peter.kjellerstedt@axis.com> wrote:
>
> Rather than blindly reverting, can’t we update the documentation instead? If you use ${AUTOREV} you obviously do it because you always want the latest revision.
>

Autorev + clear srcrev policy if you want that

>
>
> Before this change, using ${AUTOREV} in combination with BB_SRCREV_POLICY = “cache” didn’t work, which also wasn’t documented.
>
It could be interpreted also that cache also caches AUTOREVs
this will break downstream projects. Its equivalent of ABI change,
that perhaps will do more harm then what it fixes.

>
>
> //Peter
>
>
>
> From: bitbake-devel@lists.openembedded.org <bitbake-devel@lists.openembedded.org> On Behalf Of Khem Raj
> Sent: den 21 mars 2020 22:44
> To: bitbake-devel@lists.openembedded.org
> Subject: Re: [bitbake-devel] [master][zeus][PATCH 3/3] fetch2: Allow ${AUTOREV} to be used when BB_SRCREV_POLICY is "cache"
>
>
>
> removing
>
> if d.getVar('BB_SRCREV_POLICY') != "cache":
>
> Will cause autorevs to be excluded from policy and its not documented
>
> https://www.yoctoproject.org/docs/2.1/bitbake-user-manual/bitbake-user-manual.html#var-BB_SRCREV_POLICY
>
> did not state that AUTOREVs plus BB_SRCREV_POLICY behavior, this patch is regressing the case
> where we rely on this behavior to use externalsrc on AUTOREVed recipes.

Re: [bitbake-devel] [master][zeus][PATCH 3/3] fetch2: Allow ${AUTOREV} to be used when BB_SRCREV_POLICY is "cache"

[Peter Kjellerstedt]

> -----Original Message-----
> From: Khem Raj <raj.khem@gmail.com>
> Sent: den 22 mars 2020 00:59
> To: Peter Kjellerstedt <peter.kjellerstedt@axis.com>
> Cc: bitbake-devel@lists.openembedded.org
> Subject: Re: [bitbake-devel] [master][zeus][PATCH 3/3] fetch2: Allow
> ${AUTOREV} to be used when BB_SRCREV_POLICY is "cache"
> 
> On Sat, Mar 21, 2020 at 4:50 PM Peter Kjellerstedt
> <peter.kjellerstedt@axis.com> wrote:
> >
> > Rather than blindly reverting, can’t we update the documentation
> > instead? If you use ${AUTOREV} you obviously do it because you always want
> > the latest revision.
>  
> Autorev + clear srcrev policy if you want that

Well, obviously ${AUTOREV} works with BB_SRCREV_POLICY = "clear". But 
BB_SRCREV_POLICY is a distro configuration and it is not possible to override 
it per recipe. Which means that if it is set to "cache", it is not possible 
to use ${AUTOREV}. This is not documented, and to the developers it will look 
as if it is working, since it fetches the very first time the recipe is built.
But there will be no updates, so it is as useful as setting SRCREV = "master" 
(which some of our developers also have tried before being educated to how 
BitBake works)...

> > Before this change, using ${AUTOREV} in combination with
> > BB_SRCREV_POLICY = "cache" didn’t work, which also wasn’t documented.
> 
> It could be interpreted also that cache also caches AUTOREVs
> this will break downstream projects. Its equivalent of ABI change,
> that perhaps will do more harm then what it fixes.

I fail to see how it will break stuff as it was not possible to use ${AUTOREV} 
before in a meaningful way together with BB_SRCREV_POLICY = "cache".

> > //Peter

//Peter

> > From: bitbake-devel@lists.openembedded.org <bitbake-
> devel@lists.openembedded.org> On Behalf Of Khem Raj
> > Sent: den 21 mars 2020 22:44
> > To: bitbake-devel@lists.openembedded.org
> > Subject: Re: [bitbake-devel] [master][zeus][PATCH 3/3] fetch2: Allow
> ${AUTOREV} to be used when BB_SRCREV_POLICY is "cache"
> >
> >
> >
> > removing
> >
> > if d.getVar('BB_SRCREV_POLICY') != "cache":
> >
> > Will cause autorevs to be excluded from policy and its not documented
> >
> > https://www.yoctoproject.org/docs/2.1/bitbake-user-manual/bitbake-user-
> manual.html#var-BB_SRCREV_POLICY
> >
> > did not state that AUTOREVs plus BB_SRCREV_POLICY behavior, this patch
> is regressing the case
> > where we rely on this behavior to use externalsrc on AUTOREVed recipes.

Re: [bitbake-devel] [master][zeus][PATCH 3/3] fetch2: Allow ${AUTOREV} to be used when BB_SRCREV_POLICY is "cache"

[Khem Raj]

[-- Attachment #1: Type: text/plain, Size: 2836 bytes --]

On Sat, Mar 21, 2020 at 5:38 PM Peter Kjellerstedt <
peter.kjellerstedt@axis.com> wrote:

> > -----Original Message-----
> > From: Khem Raj <raj.khem@gmail.com>
> > Sent: den 22 mars 2020 00:59
> > To: Peter Kjellerstedt <peter.kjellerstedt@axis.com>
> > Cc: bitbake-devel@lists.openembedded.org
> > Subject: Re: [bitbake-devel] [master][zeus][PATCH 3/3] fetch2: Allow
> > ${AUTOREV} to be used when BB_SRCREV_POLICY is "cache"
> >
> > On Sat, Mar 21, 2020 at 4:50 PM Peter Kjellerstedt
> > <peter.kjellerstedt@axis.com> wrote:
> > >
> > > Rather than blindly reverting, can’t we update the documentation
> > > instead? If you use ${AUTOREV} you obviously do it because you always
> want
> > > the latest revision.
> >
> > Autorev + clear srcrev policy if you want that
>
> Well, obviously ${AUTOREV} works with BB_SRCREV_POLICY = "clear". But
> BB_SRCREV_POLICY is a distro configuration and it is not possible to
> override
> it per recipe. Which means that if it is set to "cache", it is not
> possible
> to use ${AUTOREV}. This is not documented, and to the developers it will
> look
> as if it is working, since it fetches the very first time the recipe is
> built.
> But there will be no updates, so it is as useful as setting SRCREV =
> "master"
> (which some of our developers also have tried before being educated to how
> BitBake works)...
>
> > > Before this change, using ${AUTOREV} in combination with
> > > BB_SRCREV_POLICY = "cache" didn’t work, which also wasn’t documented.
> >
> > It could be interpreted also that cache also caches AUTOREVs
> > this will break downstream projects. Its equivalent of ABI change,
> > that perhaps will do more harm then what it fixes.
>
> I fail to see how it will break stuff as it was not possible to use
> ${AUTOREV}
> before in a meaningful way together with BB_SRCREV_POLICY = "cache".


There are workflows depending on this behavior unfortunately


>
> > > //Peter
>
> //Peter
>
> > > From: bitbake-devel@lists.openembedded.org <bitbake-
> > devel@lists.openembedded.org> On Behalf Of Khem Raj
> > > Sent: den 21 mars 2020 22:44
> > > To: bitbake-devel@lists.openembedded.org
> > > Subject: Re: [bitbake-devel] [master][zeus][PATCH 3/3] fetch2: Allow
> > ${AUTOREV} to be used when BB_SRCREV_POLICY is "cache"
> > >
> > >
> > >
> > > removing
> > >
> > > if d.getVar('BB_SRCREV_POLICY') != "cache":
> > >
> > > Will cause autorevs to be excluded from policy and its not documented
> > >
> > >
> https://www.yoctoproject.org/docs/2.1/bitbake-user-manual/bitbake-user-
> > manual.html#var-BB_SRCREV_POLICY
> > >
> > > did not state that AUTOREVs plus BB_SRCREV_POLICY behavior, this patch
> > is regressing the case
> > > where we rely on this behavior to use externalsrc on AUTOREVed recipes.
>

[-- Attachment #2: Type: text/html, Size: 4310 bytes --]

Re: [bitbake-devel] [master][zeus][PATCH 3/3] fetch2: Allow ${AUTOREV} to be used when BB_SRCREV_POLICY is "cache"

[Peter Kjellerstedt]

[-- Attachment #1: Type: text/plain, Size: 3463 bytes --]

Care to elaborate? Because I obviously do not see the usefulness in combining BB_SRCREV_POLICY = "cache" with ${AUTOREV} as it was implemented before my change.

//Peter

From: Khem Raj <raj.khem@gmail.com>
Sent: den 22 mars 2020 02:27
To: Peter Kjellerstedt <peter.kjellerstedt@axis.com>
Cc: bitbake-devel@lists.openembedded.org
Subject: Re: [bitbake-devel] [master][zeus][PATCH 3/3] fetch2: Allow ${AUTOREV} to be used when BB_SRCREV_POLICY is "cache"



On Sat, Mar 21, 2020 at 5:38 PM Peter Kjellerstedt <peter.kjellerstedt@axis.com<mailto:peter.kjellerstedt@axis.com>> wrote:
> -----Original Message-----
> From: Khem Raj <raj.khem@gmail.com<mailto:raj.khem@gmail.com>>
> Sent: den 22 mars 2020 00:59
> To: Peter Kjellerstedt <peter.kjellerstedt@axis.com<mailto:peter.kjellerstedt@axis.com>>
> Cc: bitbake-devel@lists.openembedded.org<mailto:bitbake-devel@lists.openembedded.org>
> Subject: Re: [bitbake-devel] [master][zeus][PATCH 3/3] fetch2: Allow
> ${AUTOREV} to be used when BB_SRCREV_POLICY is "cache"
>
> On Sat, Mar 21, 2020 at 4:50 PM Peter Kjellerstedt
> <peter.kjellerstedt@axis.com<mailto:peter.kjellerstedt@axis.com>> wrote:
> >
> > Rather than blindly reverting, can’t we update the documentation
> > instead? If you use ${AUTOREV} you obviously do it because you always want
> > the latest revision.
>
> Autorev + clear srcrev policy if you want that

Well, obviously ${AUTOREV} works with BB_SRCREV_POLICY = "clear". But
BB_SRCREV_POLICY is a distro configuration and it is not possible to override
it per recipe. Which means that if it is set to "cache", it is not possible
to use ${AUTOREV}. This is not documented, and to the developers it will look
as if it is working, since it fetches the very first time the recipe is built.
But there will be no updates, so it is as useful as setting SRCREV = "master"
(which some of our developers also have tried before being educated to how
BitBake works)...

> > Before this change, using ${AUTOREV} in combination with
> > BB_SRCREV_POLICY = "cache" didn’t work, which also wasn’t documented.
>
> It could be interpreted also that cache also caches AUTOREVs
> this will break downstream projects. Its equivalent of ABI change,
> that perhaps will do more harm then what it fixes.

I fail to see how it will break stuff as it was not possible to use ${AUTOREV}
before in a meaningful way together with BB_SRCREV_POLICY = "cache".

There are workflows depending on this behavior unfortunately



> > //Peter

//Peter

> > From: bitbake-devel@lists.openembedded.org<mailto:bitbake-devel@lists.openembedded.org> <bitbake-
> devel@lists.openembedded.org<mailto:devel@lists.openembedded.org>> On Behalf Of Khem Raj
> > Sent: den 21 mars 2020 22:44
> > To: bitbake-devel@lists.openembedded.org<mailto:bitbake-devel@lists.openembedded.org>
> > Subject: Re: [bitbake-devel] [master][zeus][PATCH 3/3] fetch2: Allow
> ${AUTOREV} to be used when BB_SRCREV_POLICY is "cache"
> >
> >
> >
> > removing
> >
> > if d.getVar('BB_SRCREV_POLICY') != "cache":
> >
> > Will cause autorevs to be excluded from policy and its not documented
> >
> > https://www.yoctoproject.org/docs/2.1/bitbake-user-manual/bitbake-user-
> manual.html#var-BB_SRCREV_POLICY
> >
> > did not state that AUTOREVs plus BB_SRCREV_POLICY behavior, this patch
> is regressing the case
> > where we rely on this behavior to use externalsrc on AUTOREVed recipes.

[-- Attachment #2: Type: text/html, Size: 7838 bytes --]

Re: [bitbake-devel] [master][zeus][PATCH 3/3] fetch2: Allow ${AUTOREV} to be used when BB_SRCREV_POLICY is "cache"

[Richard Purdie]

On Sat, 2020-03-21 at 23:50 +0000, Peter Kjellerstedt wrote:
> Rather than blindly reverting, can’t we update the documentation
> instead? If you use ${AUTOREV} you obviously do it because you always
> want the latest revision. 
>  
> Before this change, using ${AUTOREV} in combination with
> BB_SRCREV_POLICY = “cache” didn’t work, which also wasn’t documented.

I can confirm that the design of "cache" with AUTOREV was only to
update the first time.

This differentiates it from the other option or clear where you'd get
the behaviour you'd describe.

Cheers,

Richard

Re: [bitbake-devel] [master][zeus][PATCH 3/3] fetch2: Allow ${AUTOREV} to be used when BB_SRCREV_POLICY is "cache"

[Richard Purdie]

On Sun, 2020-03-22 at 00:38 +0000, Peter Kjellerstedt wrote:
> > -----Original Message-----
> > From: Khem Raj <raj.khem@gmail.com>
> > Sent: den 22 mars 2020 00:59
> > To: Peter Kjellerstedt <peter.kjellerstedt@axis.com>
> > Cc: bitbake-devel@lists.openembedded.org
> > Subject: Re: [bitbake-devel] [master][zeus][PATCH 3/3] fetch2: Allow
> > ${AUTOREV} to be used when BB_SRCREV_POLICY is "cache"
> > 
> > On Sat, Mar 21, 2020 at 4:50 PM Peter Kjellerstedt
> > <peter.kjellerstedt@axis.com> wrote:
> > > Rather than blindly reverting, can’t we update the documentation
> > > instead? If you use ${AUTOREV} you obviously do it because you always want
> > > the latest revision.
> >  
> > Autorev + clear srcrev policy if you want that
> 
> Well, obviously ${AUTOREV} works with BB_SRCREV_POLICY = "clear". But 
> BB_SRCREV_POLICY is a distro configuration and it is not possible to override 
> it per recipe.

BB_SRCREV_POLICY is quite an old piece of code and its design has
issues. It should really be changed to work per recipe (and use
something other than persist db but that is another story).
 
>  Which means that if it is set to "cache", it is not possible 
> to use ${AUTOREV}. This is not documented, and to the developers it will look 
> as if it is working, since it fetches the very first time the recipe is built.
> But there will be no updates, so it is as useful as setting SRCREV = "master" 
> (which some of our developers also have tried before being educated to how 
> BitBake works)...
> 
> > > Before this change, using ${AUTOREV} in combination with
> > > BB_SRCREV_POLICY = "cache" didn’t work, which also wasn’t documented.
> > 
> > It could be interpreted also that cache also caches AUTOREVs
> > this will break downstream projects. Its equivalent of ABI change,
> > that perhaps will do more harm then what it fixes.
> 
> I fail to see how it will break stuff as it was not possible to use ${AUTOREV} 
> before in a meaningful way together with BB_SRCREV_POLICY = "cache".

It gives a behaviour people asked for and need under certain scenarios.
I appreciate that isn't yours. I should not have merged the patch as it
breaks behaviour that people do use and I misunderstood what the patch
was doing :(.

The question is where from here, technically a revert is justified but
I do understand the per recipe configuration issue.

Cheers,

Richard

Re: [bitbake-devel] [master][zeus][PATCH 3/3] fetch2: Allow ${AUTOREV} to be used when BB_SRCREV_POLICY is "cache"

[Richard Purdie]

On Sun, 2020-03-22 at 06:19 +0000, Peter Kjellerstedt wrote:
> Care to elaborate? Because I obviously do not see the usefulness in
> combining BB_SRCREV_POLICY = "cache" with ${AUTOREV} as it was
> implemented before my change.

One of the use cases this was used for was in an autobuilder
environment where a given build directory could have multiple bitbake
invocations against it and have the same src revs. Something like:

MACHINE=A bitbake X
MACHINE=B bitbake X
MACHINE=C bitbake X

and have the same revision of some AUTOREV recipe built for this
session/build directory.

Cheers,

Richard

Re: [bitbake-devel] [master][zeus][PATCH 3/3] fetch2: Allow ${AUTOREV} to be used when BB_SRCREV_POLICY is "cache"

[Khem Raj]

On Sun, Mar 22, 2020 at 1:56 AM Richard Purdie
<richard.purdie@linuxfoundation.org> wrote:
>
> On Sun, 2020-03-22 at 00:38 +0000, Peter Kjellerstedt wrote:
> > > -----Original Message-----
> > > From: Khem Raj <raj.khem@gmail.com>
> > > Sent: den 22 mars 2020 00:59
> > > To: Peter Kjellerstedt <peter.kjellerstedt@axis.com>
> > > Cc: bitbake-devel@lists.openembedded.org
> > > Subject: Re: [bitbake-devel] [master][zeus][PATCH 3/3] fetch2: Allow
> > > ${AUTOREV} to be used when BB_SRCREV_POLICY is "cache"
> > >
> > > On Sat, Mar 21, 2020 at 4:50 PM Peter Kjellerstedt
> > > <peter.kjellerstedt@axis.com> wrote:
> > > > Rather than blindly reverting, can’t we update the documentation
> > > > instead? If you use ${AUTOREV} you obviously do it because you always want
> > > > the latest revision.
> > >
> > > Autorev + clear srcrev policy if you want that
> >
> > Well, obviously ${AUTOREV} works with BB_SRCREV_POLICY = "clear". But
> > BB_SRCREV_POLICY is a distro configuration and it is not possible to override
> > it per recipe.
>
> BB_SRCREV_POLICY is quite an old piece of code and its design has
> issues. It should really be changed to work per recipe (and use
> something other than persist db but that is another story).
>

Right, I agree lets keep it as it is.

> >  Which means that if it is set to "cache", it is not possible
> > to use ${AUTOREV}. This is not documented, and to the developers it will look
> > as if it is working, since it fetches the very first time the recipe is built.
> > But there will be no updates, so it is as useful as setting SRCREV = "master"
> > (which some of our developers also have tried before being educated to how
> > BitBake works)...
> >
> > > > Before this change, using ${AUTOREV} in combination with
> > > > BB_SRCREV_POLICY = "cache" didn’t work, which also wasn’t documented.
> > >
> > > It could be interpreted also that cache also caches AUTOREVs
> > > this will break downstream projects. Its equivalent of ABI change,
> > > that perhaps will do more harm then what it fixes.
> >
> > I fail to see how it will break stuff as it was not possible to use ${AUTOREV}
> > before in a meaningful way together with BB_SRCREV_POLICY = "cache".
>
> It gives a behaviour people asked for and need under certain scenarios.
> I appreciate that isn't yours. I should not have merged the patch as it
> breaks behaviour that people do use and I misunderstood what the patch
> was doing :(.
>
> The question is where from here, technically a revert is justified but
> I do understand the per recipe configuration issue.

I think there are large usecases depending on current behaviour. While
RDK will migrate away
from AUTOREVs but this is not happening in 3.1 time frame and there
are other projects perhaps
who are in same boat.

>
> Cheers,
>
> Richard
>