[1/4] recipetool: Set master branch only as fallback

Message ID 20211206080406.3249-1-stefan.herbrechtsmeier-oss@weidmueller.com
State Accepted, archived
Commit 34ece8030e7a6a100b5e3e7b94e6c786c0e199a6
Headers show
Series [1/4] recipetool: Set master branch only as fallback | expand

Commit Message

Stefan Herbrechtsmeier Dec. 6, 2021, 8:04 a.m. UTC
From: Stefan Herbrechtsmeier <stefan.herbrechtsmeier@weidmueller.com>

The commit 'meta/scripts: Manual git url branch additions (dc53fe75cc)'
sets the branch= parameter too early to master and thereby breaks the
-B/--srcbranch option.

ERROR: branch= parameter and -B/--srcbranch option cannot both be specified - use one or the other

Signed-off-by: Stefan Herbrechtsmeier <stefan.herbrechtsmeier@weidmueller.com>
---

 scripts/lib/recipetool/create.py | 15 +++++++--------
 1 file changed, 7 insertions(+), 8 deletions(-)

Comments

Richard Purdie Dec. 7, 2021, 3:45 p.m. UTC | #1
On Mon, 2021-12-06 at 09:04 +0100, Stefan Herbrechtsmeier wrote:
> From: Stefan Herbrechtsmeier <stefan.herbrechtsmeier@weidmueller.com>
> 
> The commit 'meta/scripts: Manual git url branch additions (dc53fe75cc)'
> sets the branch= parameter too early to master and thereby breaks the
> -B/--srcbranch option.
> 
> ERROR: branch= parameter and -B/--srcbranch option cannot both be specified - use one or the other
> 
> Signed-off-by: Stefan Herbrechtsmeier <stefan.herbrechtsmeier@weidmueller.com>
> ---
> 
>  scripts/lib/recipetool/create.py | 15 +++++++--------
>  1 file changed, 7 insertions(+), 8 deletions(-)

I think something in this series is causing:

https://autobuilder.yoctoproject.org/typhoon/#/builders/79/builds/2926/steps/14/logs/stdio

(the other selftest builds also failed)

Cheers,

Richard
Stefan Herbrechtsmeier Dec. 8, 2021, 7:45 a.m. UTC | #2
Hi Richard,

Am 07.12.2021 um 16:45 schrieb Richard Purdie:
> On Mon, 2021-12-06 at 09:04 +0100, Stefan Herbrechtsmeier wrote:
>> From: Stefan Herbrechtsmeier <stefan.herbrechtsmeier@weidmueller.com>
>>
>> The commit 'meta/scripts: Manual git url branch additions (dc53fe75cc)'
>> sets the branch= parameter too early to master and thereby breaks the
>> -B/--srcbranch option.
>>
>> ERROR: branch= parameter and -B/--srcbranch option cannot both be specified - use one or the other
>>
>> Signed-off-by: Stefan Herbrechtsmeier <stefan.herbrechtsmeier@weidmueller.com>
>> ---
>>
>>   scripts/lib/recipetool/create.py | 15 +++++++--------
>>   1 file changed, 7 insertions(+), 8 deletions(-)
> 
> I think something in this series is causing:
> 
> https://autobuilder.yoctoproject.org/typhoon/#/builders/79/builds/2926/steps/14/logs/stdio

Sorry I missed the adaption of the test test_devtool_add_fetch_git. It 
doesn't expect a "branch=master" which is wrong.

- gitsm://git.yoctoproject.org/mraa;branch=master
?                                  --------------
+ gitsm://git.yoctoproject.org/mraa

I will update my patch series but need some time to test it locally. The 
devtool test build a lot of recipe on every run because it use a private 
sstate cache. How does the autobuilder overcome this problem?

> 
> (the other selftest builds also failed)
What do you mean by this?

Regards
   Stefan
Alexander Kanavin Dec. 8, 2021, 8:10 a.m. UTC | #3
On Wed, 8 Dec 2021 at 08:45, Stefan Herbrechtsmeier <
stefan.herbrechtsmeier-oss@weidmueller.com> wrote:

> I will update my patch series but need some time to test it locally. The
> devtool test build a lot of recipe on every run because it use a private
> sstate cache. How does the autobuilder overcome this problem?
>

If you don't rebase the branch used for testing, and set the cache to a
directory outside of build directory (e.g. /home/stefan/sstate ) it will
reuse that cache between selftest invocations.

The autobuilder uses a single writeable cache for all builds and builders,
mounted as read-write NFS partition.

Alex
Stefan Herbrechtsmeier Dec. 8, 2021, 8:27 a.m. UTC | #4
Am 08.12.2021 um 09:10 schrieb Alexander Kanavin:
> On Wed, 8 Dec 2021 at 08:45, Stefan Herbrechtsmeier 
> <stefan.herbrechtsmeier-oss@weidmueller.com 
> <mailto:stefan.herbrechtsmeier-oss@weidmueller.com>> wrote:
> 
>     I will update my patch series but need some time to test it locally.
>     The
>     devtool test build a lot of recipe on every run because it use a
>     private
>     sstate cache. How does the autobuilder overcome this problem?
> 
> 
> If you don't rebase the branch used for testing, and set the cache to a 
> directory outside of build directory (e.g. /home/stefan/sstate ) it will 
> reuse that cache between selftest invocations.

The devtool test use its own PRIVATE sstate cache:

def setUpClass(cls):
...
cls.original_sstate = bb_vars['SSTATE_DIR']
cls.devtool_sstate = os.path.join(bb_vars['TOPDIR'], 'sstate_devtool')
cls.sstate_conf  = 'SSTATE_DIR = "%s"\n' % cls.devtool_sstate
cls.sstate_conf += ('SSTATE_MIRRORS += "file://.* file:///%s/PATH"\n'
                     % cls.original_sstate)

And remove it after the test run:

def tearDownClass(cls):
...
runCmd('rm -rf %s' % cls.devtool_sstate)

Even a keep builddir doesn't help to keep the private sstate cache.

> The autobuilder uses a single writeable cache for all builds and 
> builders, mounted as read-write NFS partition.

This doesn't help in this case. The devtool test clean parts of the 
sstate cache and therefore use its private sstate cache and the original 
sstate cache as mirror. This is a problem if a test triggers a build of 
dependencies.

Regards
   Stefan
Alexander Kanavin Dec. 8, 2021, 8:32 a.m. UTC | #5
I haven't looked at this, but probably the test could be adjusted to first
build the needed parts with only the original cache?

Alex

On Wed, 8 Dec 2021 at 09:27, Stefan Herbrechtsmeier <
stefan.herbrechtsmeier-oss@weidmueller.com> wrote:

> Am 08.12.2021 um 09:10 schrieb Alexander Kanavin:
> > On Wed, 8 Dec 2021 at 08:45, Stefan Herbrechtsmeier
> > <stefan.herbrechtsmeier-oss@weidmueller.com
> > <mailto:stefan.herbrechtsmeier-oss@weidmueller.com>> wrote:
> >
> >     I will update my patch series but need some time to test it locally.
> >     The
> >     devtool test build a lot of recipe on every run because it use a
> >     private
> >     sstate cache. How does the autobuilder overcome this problem?
> >
> >
> > If you don't rebase the branch used for testing, and set the cache to a
> > directory outside of build directory (e.g. /home/stefan/sstate ) it will
> > reuse that cache between selftest invocations.
>
> The devtool test use its own PRIVATE sstate cache:
>
> def setUpClass(cls):
> ...
> cls.original_sstate = bb_vars['SSTATE_DIR']
> cls.devtool_sstate = os.path.join(bb_vars['TOPDIR'], 'sstate_devtool')
> cls.sstate_conf  = 'SSTATE_DIR = "%s"\n' % cls.devtool_sstate
> cls.sstate_conf += ('SSTATE_MIRRORS += "file://.* file:///%s/PATH"\n'
>                      % cls.original_sstate)
>
> And remove it after the test run:
>
> def tearDownClass(cls):
> ...
> runCmd('rm -rf %s' % cls.devtool_sstate)
>
> Even a keep builddir doesn't help to keep the private sstate cache.
>
> > The autobuilder uses a single writeable cache for all builds and
> > builders, mounted as read-write NFS partition.
>
> This doesn't help in this case. The devtool test clean parts of the
> sstate cache and therefore use its private sstate cache and the original
> sstate cache as mirror. This is a problem if a test triggers a build of
> dependencies.
>
> Regards
>    Stefan
>
Stefan Herbrechtsmeier Dec. 8, 2021, 8:48 a.m. UTC | #6
Am 08.12.2021 um 09:32 schrieb Alexander Kanavin via lists.openembedded.org:
> I haven't looked at this, but probably the test could be adjusted to 
> first build the needed parts with only the original cache?

This needs a rework of the tests because the private sstate cache is set 
in the test setup and we need to know the dependencies of the test to 
build those manual.

> On Wed, 8 Dec 2021 at 09:27, Stefan Herbrechtsmeier 
> <stefan.herbrechtsmeier-oss@weidmueller.com 
> <mailto:stefan.herbrechtsmeier-oss@weidmueller.com>> wrote:
> 
>     Am 08.12.2021 um 09:10 schrieb Alexander Kanavin:
>      > On Wed, 8 Dec 2021 at 08:45, Stefan Herbrechtsmeier
>      > <stefan.herbrechtsmeier-oss@weidmueller.com
>     <mailto:stefan.herbrechtsmeier-oss@weidmueller.com>
>      > <mailto:stefan.herbrechtsmeier-oss@weidmueller.com
>     <mailto:stefan.herbrechtsmeier-oss@weidmueller.com>>> wrote:
>      >
>      >     I will update my patch series but need some time to test it
>     locally.
>      >     The
>      >     devtool test build a lot of recipe on every run because it use a
>      >     private
>      >     sstate cache. How does the autobuilder overcome this problem?
>      >
>      >
>      > If you don't rebase the branch used for testing, and set the
>     cache to a
>      > directory outside of build directory (e.g. /home/stefan/sstate )
>     it will
>      > reuse that cache between selftest invocations.
> 
>     The devtool test use its own PRIVATE sstate cache:
> 
>     def setUpClass(cls):
>     ...
>     cls.original_sstate = bb_vars['SSTATE_DIR']
>     cls.devtool_sstate = os.path.join(bb_vars['TOPDIR'], 'sstate_devtool')
>     cls.sstate_conf  = 'SSTATE_DIR = "%s"\n' % cls.devtool_sstate
>     cls.sstate_conf += ('SSTATE_MIRRORS += "file://.* file:///%s/PATH"\n'
>                           % cls.original_sstate)
> 
>     And remove it after the test run:
> 
>     def tearDownClass(cls):
>     ...
>     runCmd('rm -rf %s' % cls.devtool_sstate)
> 
>     Even a keep builddir doesn't help to keep the private sstate cache.
> 
>      > The autobuilder uses a single writeable cache for all builds and
>      > builders, mounted as read-write NFS partition.
> 
>     This doesn't help in this case. The devtool test clean parts of the
>     sstate cache and therefore use its private sstate cache and the
>     original
>     sstate cache as mirror. This is a problem if a test triggers a build of
>     dependencies.
> 
>     Regards
>         Stefan
Richard Purdie Dec. 8, 2021, 9:12 a.m. UTC | #7
On Wed, 2021-12-08 at 08:45 +0100, Stefan Herbrechtsmeier wrote:
> Hi Richard,
> 
> Am 07.12.2021 um 16:45 schrieb Richard Purdie:
> > On Mon, 2021-12-06 at 09:04 +0100, Stefan Herbrechtsmeier wrote:
> > > From: Stefan Herbrechtsmeier <stefan.herbrechtsmeier@weidmueller.com>
> > > 
> > > The commit 'meta/scripts: Manual git url branch additions (dc53fe75cc)'
> > > sets the branch= parameter too early to master and thereby breaks the
> > > -B/--srcbranch option.
> > > 
> > > ERROR: branch= parameter and -B/--srcbranch option cannot both be specified - use one or the other
> > > 
> > > Signed-off-by: Stefan Herbrechtsmeier <stefan.herbrechtsmeier@weidmueller.com>
> > > ---
> > > 
> > >   scripts/lib/recipetool/create.py | 15 +++++++--------
> > >   1 file changed, 7 insertions(+), 8 deletions(-)
> > 
> > I think something in this series is causing:
> > 
> > https://autobuilder.yoctoproject.org/typhoon/#/builders/79/builds/2926/steps/14/logs/stdio
> 
> Sorry I missed the adaption of the test test_devtool_add_fetch_git. It 
> doesn't expect a "branch=master" which is wrong.
> 
> - gitsm://git.yoctoproject.org/mraa;branch=master
> ?                                  --------------
> + gitsm://git.yoctoproject.org/mraa

I suspected something like that, thanks.

> I will update my patch series but need some time to test it locally. The 
> devtool test build a lot of recipe on every run because it use a private 
> sstate cache. How does the autobuilder overcome this problem?

The autobuilder doesn't. We stopped devtool using the public shared cache as it
is shared between autobuilder workers and one worker could delete artefacts
whilst another is using them. This caused all kinds of weird races and there
isn't much extra build time from the autobuilder perspective. The autobuilder
never deletes things from the shared cache outside of the timeout cron job which
removes objects that haven't been accessed in a month or so.

> > 
> > (the other selftest builds also failed)
> What do you mean by this?

I mean that we run oe-selftest on 4 different distros and they all failed the
same way so there were four failures in the build and I just pasted a link to
one of them.

Cheers,

Richard
Richard Purdie Dec. 8, 2021, 9:13 a.m. UTC | #8
On Wed, 2021-12-08 at 09:10 +0100, Alexander Kanavin wrote:
> On Wed, 8 Dec 2021 at 08:45, Stefan Herbrechtsmeier
> <stefan.herbrechtsmeier-oss@weidmueller.com> wrote:
> > I will update my patch series but need some time to test it locally. The 
> > devtool test build a lot of recipe on every run because it use a private 
> > sstate cache. How does the autobuilder overcome this problem?
> > 
> 
> 
> If you don't rebase the branch used for testing, and set the cache to a
> directory outside of build directory (e.g. /home/stefan/sstate ) it will reuse
> that cache between selftest invocations.
> 
> The autobuilder uses a single writeable cache for all builds and builders,
> mounted as read-write NFS partition.

Stefan is correct here, it deliberately avoids the main cache. It does this as
it deletes things from it to run the test and we cannot do that on the public
cache.

Cheers,

Richard

Patch

diff --git a/scripts/lib/recipetool/create.py b/scripts/lib/recipetool/create.py
index 4f6e01c639..406c97f1c5 100644
--- a/scripts/lib/recipetool/create.py
+++ b/scripts/lib/recipetool/create.py
@@ -389,9 +389,6 @@  def reformat_git_uri(uri):
                 parms.update({('protocol', 'ssh')})
         elif (scheme == "http" or scheme == 'https' or scheme == 'ssh') and not ('protocol' in parms):
             parms.update({('protocol', scheme)})
-        # We assume 'master' branch if not set
-        if not 'branch' in parms:
-            parms.update({('branch', 'master')})
         # Always append 'git://'
         fUrl = bb.fetch2.encodeurl(('git', host, path, user, pswd, parms))
         return fUrl
@@ -481,6 +478,9 @@  def create_recipe(args):
             storeTagName = params['tag']
             params['nobranch'] = '1'
             del params['tag']
+        # Assume 'master' branch if not set
+        if scheme in ['git', 'gitsm'] and 'branch' not in params and 'nobranch' not in params:
+            params['branch'] = 'master'
         fetchuri = bb.fetch2.encodeurl((scheme, network, path, user, passwd, params))
 
         tmpparent = tinfoil.config_data.getVar('BASE_WORKDIR')
@@ -530,10 +530,9 @@  def create_recipe(args):
             # Remove HEAD reference point and drop remote prefix
             get_branch = [x.split('/', 1)[1] for x in get_branch if not x.startswith('origin/HEAD')]
             if 'master' in get_branch:
-                # If it is master, we do not need to append 'branch=master' as this is default.
                 # Even with the case where get_branch has multiple objects, if 'master' is one
                 # of them, we should default take from 'master'
-                srcbranch = ''
+                srcbranch = 'master'
             elif len(get_branch) == 1:
                 # If 'master' isn't in get_branch and get_branch contains only ONE object, then store result into 'srcbranch'
                 srcbranch = get_branch[0]
@@ -546,8 +545,8 @@  def create_recipe(args):
         # Since we might have a value in srcbranch, we need to
         # recontruct the srcuri to include 'branch' in params.
         scheme, network, path, user, passwd, params = bb.fetch2.decodeurl(srcuri)
-        if srcbranch:
-            params['branch'] = srcbranch
+        if scheme in ['git', 'gitsm']:
+            params['branch'] = srcbranch or 'master'
 
         if storeTagName and scheme in ['git', 'gitsm']:
             # Check srcrev using tag and check validity of the tag
@@ -606,7 +605,7 @@  def create_recipe(args):
                     splitline = line.split()
                     if len(splitline) > 1:
                         if splitline[0] == 'origin' and scriptutils.is_src_url(splitline[1]):
-                            srcuri = reformat_git_uri(splitline[1])
+                            srcuri = reformat_git_uri(splitline[1]) + ';branch=master'
                             srcsubdir = 'git'
                             break