[PATCH 0/5] fetch2: Fixes for git and gitsm

[PATCH 0/5] fetch2: Fixes for git and gitsm

[Robert Yang]

The following changes since commit 9818acbb00b6f78dbfcc5796ec78bf25f6535288:

  meta-yocto-bsp: Add the v5.0 kernel support for all the BSPs (2019-03-11 05:43:49 -0700)

are available in the git repository at:

  git://git.pokylinux.org/poky-contrib rbt/fetch2
  http://git.pokylinux.org/cgit.cgi//log/?h=rbt/fetch2

Robert Yang (5):
  fetch2/git: Fix clean to remove clonedir
  fetch2: Add get_mirrorname()
  fetch2: Print SCMs list when SRCREV_FORMAT is not set
  fetch2/gitsm: Add clean function
  fetch2: Add gitsm's function process_submodules()

 bitbake/lib/bb/fetch2/__init__.py | 33 +++++++++++++++++++++++++++++++--
 bitbake/lib/bb/fetch2/git.py      | 16 ++++++++++++----
 bitbake/lib/bb/fetch2/gitsm.py    | 15 +++++++++++++--
 3 files changed, 56 insertions(+), 8 deletions(-)

-- 
2.7.4

[PATCH 1/5] fetch2/git: Fix clean to remove clonedir

[Robert Yang]

The localpath is a symlink to clonedir when it is cloned from a mirror, for
example:
$ bitbake systemtap-native -cfetch
$ ls downloads/git2
sourceware.org.git.systemtap.git -> /path/to/downloads/git2/mirror.path.git.sourceware.org.git.systemtap.git
mirror.path.git.sourceware.org.git.systemtap.git

There are both sourceware.org.git.systemtap.git and
mirror.path.git.sourceware.org.git.systemtap.git in DL_DIR/git2, the symlink
sourceware.org.git.systemtap.git is created by try_mirror_url(), but
do_cleanall" only removed the symlink, didn't remove the real dir
mirror.path.git.sourceware.org.git.systemtap.git, this may cause confusions,
for example, I assumed that do_cleanall removed everything, but it didn't, and
it would the re-used next time when do_fetch. This patch fixes the problem.

Signed-off-by: Robert Yang <liezhi.yang@windriver.com>
---
 bitbake/lib/bb/fetch2/git.py | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/bitbake/lib/bb/fetch2/git.py b/bitbake/lib/bb/fetch2/git.py
index 1a8ebe3..cf8bee7 100644
--- a/bitbake/lib/bb/fetch2/git.py
+++ b/bitbake/lib/bb/fetch2/git.py
@@ -522,9 +522,17 @@ class Git(FetchMethod):
     def clean(self, ud, d):
         """ clean the git directory """
 
-        bb.utils.remove(ud.localpath, True)
-        bb.utils.remove(ud.fullmirror)
-        bb.utils.remove(ud.fullmirror + ".done")
+        to_remove = [ud.localpath, ud.fullmirror, ud.fullmirror + ".done"]
+        # The localpath is a symlink to clonedir when it is cloned from a
+        # mirror, so remove both of them.
+        if os.path.islink(ud.localpath):
+            clonedir = os.path.realpath(ud.localpath)
+            to_remove.append(clonedir)
+
+        for r in to_remove:
+            if os.path.exists(r):
+                bb.note('Removing %s' % r)
+                bb.utils.remove(r, True)
 
     def supports_srcrev(self):
         return True
-- 
2.7.4

[PATCH 2/5] fetch2: Add get_mirrorname()

[Robert Yang]

It can be used by both __init__.py and git.py.

Signed-off-by: Robert Yang <liezhi.yang@windriver.com>
---
 bitbake/lib/bb/fetch2/__init__.py | 5 ++++-
 bitbake/lib/bb/fetch2/git.py      | 2 +-
 2 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/bitbake/lib/bb/fetch2/__init__.py b/bitbake/lib/bb/fetch2/__init__.py
index f112067..5062bb2 100644
--- a/bitbake/lib/bb/fetch2/__init__.py
+++ b/bitbake/lib/bb/fetch2/__init__.py
@@ -889,6 +889,9 @@ def runfetchcmd(cmd, d, quiet=False, cleanup=None, log=None, workdir=None):
 
     return output
 
+def get_mirrorname(ud):
+    return ud.host.replace(':','.') + ud.path.replace('/', '.').replace('*', '.')
+
 def check_network_access(d, info, url):
     """
     log remote network access, and error if BB_NO_NETWORK is set or the given
@@ -910,7 +913,7 @@ def build_mirroruris(origud, mirrors, ld):
     replacements["HOST"] = origud.host
     replacements["PATH"] = origud.path
     replacements["BASENAME"] = origud.path.split("/")[-1]
-    replacements["MIRRORNAME"] = origud.host.replace(':','.') + origud.path.replace('/', '.').replace('*', '.')
+    replacements["MIRRORNAME"] = get_mirrorname(origud)
 
     def adduri(ud, uris, uds, mirrors, tarballs):
         for line in mirrors:
diff --git a/bitbake/lib/bb/fetch2/git.py b/bitbake/lib/bb/fetch2/git.py
index cf8bee7..45d7303 100644
--- a/bitbake/lib/bb/fetch2/git.py
+++ b/bitbake/lib/bb/fetch2/git.py
@@ -248,7 +248,7 @@ class Git(FetchMethod):
                     ud.unresolvedrev[name] = ud.revisions[name]
                 ud.revisions[name] = self.latest_revision(ud, d, name)
 
-        gitsrcname = '%s%s' % (ud.host.replace(':', '.'), ud.path.replace('/', '.').replace('*', '.'))
+        gitsrcname = bb.fetch2.get_mirrorname(ud)
         if gitsrcname.startswith('.'):
             gitsrcname = gitsrcname[1:]
 
-- 
2.7.4

Re: [PATCH 2/5] fetch2: Add get_mirrorname()

[Richard Purdie]

On Thu, 2019-03-14 at 17:28 +0800, Robert Yang wrote:
> It can be used by both __init__.py and git.py.
> 
> Signed-off-by: Robert Yang <liezhi.yang@windriver.com>
> ---
>  bitbake/lib/bb/fetch2/__init__.py | 5 ++++-
>  bitbake/lib/bb/fetch2/git.py      | 2 +-
>  2 files changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/bitbake/lib/bb/fetch2/__init__.py b/bitbake/lib/bb/fetch2/__init__.py
> index f112067..5062bb2 100644
> --- a/bitbake/lib/bb/fetch2/__init__.py
> +++ b/bitbake/lib/bb/fetch2/__init__.py
> @@ -889,6 +889,9 @@ def runfetchcmd(cmd, d, quiet=False, cleanup=None, log=None, workdir=None):
>  
>      return output
>  
> +def get_mirrorname(ud):
> +    return ud.host.replace(':','.') + ud.path.replace('/', '.').replace('*', '.')
> +
>  def check_network_access(d, info, url):
>      """
>      log remote network access, and error if BB_NO_NETWORK is set or the given
> @@ -910,7 +913,7 @@ def build_mirroruris(origud, mirrors, ld):
>      replacements["HOST"] = origud.host
>      replacements["PATH"] = origud.path
>      replacements["BASENAME"] = origud.path.split("/")[-1]
> -    replacements["MIRRORNAME"] = origud.host.replace(':','.') + origud.path.replace('/', '.').replace('*', '.')
> +    replacements["MIRRORNAME"] = get_mirrorname(origud)
>  
>      def adduri(ud, uris, uds, mirrors, tarballs):
>          for line in mirrors:
> diff --git a/bitbake/lib/bb/fetch2/git.py b/bitbake/lib/bb/fetch2/git.py
> index cf8bee7..45d7303 100644
> --- a/bitbake/lib/bb/fetch2/git.py
> +++ b/bitbake/lib/bb/fetch2/git.py
> @@ -248,7 +248,7 @@ class Git(FetchMethod):
>                      ud.unresolvedrev[name] = ud.revisions[name]
>                  ud.revisions[name] = self.latest_revision(ud, d, name)
>  
> -        gitsrcname = '%s%s' % (ud.host.replace(':', '.'), ud.path.replace('/', '.').replace('*', '.'))
> +        gitsrcname = bb.fetch2.get_mirrorname(ud)
>          if gitsrcname.startswith('.'):
>              gitsrcname = gitsrcname[1:]
>  

I think your patch is highlighting a deeper problem here. The mirror
name is something which the given fetcher module should be decoding and
this shouldn't handled in __init__.py unless its some kind of default.

We probably need to look more deeply about why git fetcher knowledge is
being used in the general case and fix that instead...

Cheers,

Richard

Re: [PATCH 2/5] fetch2: Add get_mirrorname()

[Robert Yang]

On 3/22/19 7:42 AM, Richard Purdie wrote:
> On Thu, 2019-03-14 at 17:28 +0800, Robert Yang wrote:
>> It can be used by both __init__.py and git.py.
>>
>> Signed-off-by: Robert Yang <liezhi.yang@windriver.com>
>> ---
>>   bitbake/lib/bb/fetch2/__init__.py | 5 ++++-
>>   bitbake/lib/bb/fetch2/git.py      | 2 +-
>>   2 files changed, 5 insertions(+), 2 deletions(-)
>>
>> diff --git a/bitbake/lib/bb/fetch2/__init__.py b/bitbake/lib/bb/fetch2/__init__.py
>> index f112067..5062bb2 100644
>> --- a/bitbake/lib/bb/fetch2/__init__.py
>> +++ b/bitbake/lib/bb/fetch2/__init__.py
>> @@ -889,6 +889,9 @@ def runfetchcmd(cmd, d, quiet=False, cleanup=None, log=None, workdir=None):
>>   
>>       return output
>>   
>> +def get_mirrorname(ud):
>> +    return ud.host.replace(':','.') + ud.path.replace('/', '.').replace('*', '.')
>> +
>>   def check_network_access(d, info, url):
>>       """
>>       log remote network access, and error if BB_NO_NETWORK is set or the given
>> @@ -910,7 +913,7 @@ def build_mirroruris(origud, mirrors, ld):
>>       replacements["HOST"] = origud.host
>>       replacements["PATH"] = origud.path
>>       replacements["BASENAME"] = origud.path.split("/")[-1]
>> -    replacements["MIRRORNAME"] = origud.host.replace(':','.') + origud.path.replace('/', '.').replace('*', '.')
>> +    replacements["MIRRORNAME"] = get_mirrorname(origud)
>>   
>>       def adduri(ud, uris, uds, mirrors, tarballs):
>>           for line in mirrors:
>> diff --git a/bitbake/lib/bb/fetch2/git.py b/bitbake/lib/bb/fetch2/git.py
>> index cf8bee7..45d7303 100644
>> --- a/bitbake/lib/bb/fetch2/git.py
>> +++ b/bitbake/lib/bb/fetch2/git.py
>> @@ -248,7 +248,7 @@ class Git(FetchMethod):
>>                       ud.unresolvedrev[name] = ud.revisions[name]
>>                   ud.revisions[name] = self.latest_revision(ud, d, name)
>>   
>> -        gitsrcname = '%s%s' % (ud.host.replace(':', '.'), ud.path.replace('/', '.').replace('*', '.'))
>> +        gitsrcname = bb.fetch2.get_mirrorname(ud)
>>           if gitsrcname.startswith('.'):
>>               gitsrcname = gitsrcname[1:]
>>   
> 
> I think your patch is highlighting a deeper problem here. The mirror
> name is something which the given fetcher module should be decoding and
> this shouldn't handled in __init__.py unless its some kind of default.
> 
> We probably need to look more deeply about why git fetcher knowledge is
> being used in the general case and fix that instead...

Make sense, I will work on it.

// Robert

> 
> Cheers,
> 
> Richard
> 
> 

[PATCH 3/5] fetch2: Print SCMs list when SRCREV_FORMAT is not set

[Robert Yang]

This makes it easier to debug, especially when multipe SCMs like gitsm,
otherwise we don't know why there are multiple SCMs.

Signed-off-by: Robert Yang <liezhi.yang@windriver.com>
---
 bitbake/lib/bb/fetch2/__init__.py | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/bitbake/lib/bb/fetch2/__init__.py b/bitbake/lib/bb/fetch2/__init__.py
index 5062bb2..64d7526 100644
--- a/bitbake/lib/bb/fetch2/__init__.py
+++ b/bitbake/lib/bb/fetch2/__init__.py
@@ -777,7 +777,8 @@ def get_srcrev(d, method_name='sortable_revision'):
     #
     format = d.getVar('SRCREV_FORMAT')
     if not format:
-        raise FetchError("The SRCREV_FORMAT variable must be set when multiple SCMs are used.")
+        raise FetchError("The SRCREV_FORMAT variable must be set when multiple SCMs are used.\n"\
+                         "The SCMs are:\n%s" % '\n'.join(scms))
 
     name_to_rev = {}
     seenautoinc = False
-- 
2.7.4

[PATCH 4/5] fetch2/gitsm: Add clean function

[Robert Yang]

The git's clean can only remove parent git repo from DL_DIR, but doesn't remove
submodules, this patch fixes the problem.

Signed-off-by: Robert Yang <liezhi.yang@windriver.com>
---
 bitbake/lib/bb/fetch2/gitsm.py | 15 +++++++++++++--
 1 file changed, 13 insertions(+), 2 deletions(-)

diff --git a/bitbake/lib/bb/fetch2/gitsm.py b/bitbake/lib/bb/fetch2/gitsm.py
index b21fed2..bb61093 100644
--- a/bitbake/lib/bb/fetch2/gitsm.py
+++ b/bitbake/lib/bb/fetch2/gitsm.py
@@ -76,8 +76,8 @@ class GitSM(Git):
         for name in ud.names:
             try:
                 gitmodules = runfetchcmd("%s show %s:.gitmodules" % (ud.basecmd, ud.revisions[name]), d, quiet=True, workdir=workdir)
-            except:
-                # No submodules to update
+            except Exception as esc:
+                logger.info("No submodules found: %s" % workdir)
                 continue
 
             for m, md in parse_gitmodules(gitmodules).items():
@@ -164,6 +164,17 @@ class GitSM(Git):
         Git.download(self, ud, d)
         self.process_submodules(ud, ud.clonedir, download_submodule, d)
 
+    def clean(self, ud, d):
+        def clean_submodule(ud, url, module, modpath, d):
+            try:
+                newfetch = Fetch([url], d, cache=False)
+                newfetch.clean()
+            except Exception as e:
+                logger.warn('gitsm: submodule clean failed: %s %s' % (type(e).__name__, str(e)))
+
+        self.process_submodules(ud, ud.clonedir, clean_submodule, d)
+        Git.clean(self, ud, d)
+
     def unpack(self, ud, destdir, d):
         def unpack_submodules(ud, url, module, modpath, d):
             url += ";bareclone=1;nobranch=1"
-- 
2.7.4

Re: [PATCH 4/5] fetch2/gitsm: Add clean function

[Lisicki, Raphael]

I have not fully understand the logic but I am wondering what happens here is a submodule is located in two different repositories? Wouldn't a clean of one module delete submodules of the other one as well?

Best regards
Raphael

> -----Original Message-----
> From: bitbake-devel-bounces@lists.openembedded.org <bitbake-devel-
> bounces@lists.openembedded.org> On Behalf Of Robert Yang
> Sent: Thursday, March 14, 2019 10:28 AM
> To: bitbake-devel@lists.openembedded.org
> Subject: [bitbake-devel] [PATCH 4/5] fetch2/gitsm: Add clean function
> 
> The git's clean can only remove parent git repo from DL_DIR, but doesn't
> remove
> submodules, this patch fixes the problem.
> 
> Signed-off-by: Robert Yang <liezhi.yang@windriver.com>
> ---
>  bitbake/lib/bb/fetch2/gitsm.py | 15 +++++++++++++--
>  1 file changed, 13 insertions(+), 2 deletions(-)
> 
> diff --git a/bitbake/lib/bb/fetch2/gitsm.py b/bitbake/lib/bb/fetch2/gitsm.py
> index b21fed2..bb61093 100644
> --- a/bitbake/lib/bb/fetch2/gitsm.py
> +++ b/bitbake/lib/bb/fetch2/gitsm.py
> @@ -76,8 +76,8 @@ class GitSM(Git):
>          for name in ud.names:
>              try:
>                  gitmodules = runfetchcmd("%s show %s:.gitmodules" %
> (ud.basecmd, ud.revisions[name]), d, quiet=True, workdir=workdir)
> -            except:
> -                # No submodules to update
> +            except Exception as esc:
> +                logger.info("No submodules found: %s" % workdir)
>                  continue
> 
>              for m, md in parse_gitmodules(gitmodules).items():
> @@ -164,6 +164,17 @@ class GitSM(Git):
>          Git.download(self, ud, d)
>          self.process_submodules(ud, ud.clonedir, download_submodule, d)
> 
> +    def clean(self, ud, d):
> +        def clean_submodule(ud, url, module, modpath, d):
> +            try:
> +                newfetch = Fetch([url], d, cache=False)
> +                newfetch.clean()
> +            except Exception as e:
> +                logger.warn('gitsm: submodule clean failed: %s %s' %
> (type(e).__name__, str(e)))
> +
> +        self.process_submodules(ud, ud.clonedir, clean_submodule, d)
> +        Git.clean(self, ud, d)
> +
>      def unpack(self, ud, destdir, d):
>          def unpack_submodules(ud, url, module, modpath, d):
>              url += ";bareclone=1;nobranch=1"
> --
> 2.7.4
> 
> --
> _______________________________________________
> bitbake-devel mailing list
> bitbake-devel@lists.openembedded.org
> http://lists.openembedded.org/mailman/listinfo/bitbake-devel

Re: [PATCH 4/5] fetch2/gitsm: Add clean function

[Robert Yang]

Hi,

On 3/14/19 8:34 PM, Lisicki, Raphael wrote:
> I have not fully understand the logic but I am wondering what happens here is a submodule is located in two different repositories? Wouldn't a clean of one module delete submodules of the other one as well?

Here are more explanations about the logic, for example, repo_A has two
submodules repo_B and repo_C, then after do_fetch,
they will be"
DL_DIR/repo_A
DL_DIR/repo_B
DL_DIR/repo_C

Without this patch, do_cleanall only removes DL_DIR/repo_A, but leaves
repo_B and repo_C there, this patch makes it remove repo_B and C.


For your question, if repo_B is submodule of both repo_A and repo_D, it
is like foo and foo-native use the same git source, when you run
"bitbake foo -cccleanall", both foo and foo-native's source will be
removed since they use the same source, the similar to gitsm, so it is not
a problem, the source will be re-downloaded automatically when needed.

// Robert

> 
> Best regards
> Raphael
> 
>> -----Original Message-----
>> From: bitbake-devel-bounces@lists.openembedded.org <bitbake-devel-
>> bounces@lists.openembedded.org> On Behalf Of Robert Yang
>> Sent: Thursday, March 14, 2019 10:28 AM
>> To: bitbake-devel@lists.openembedded.org
>> Subject: [bitbake-devel] [PATCH 4/5] fetch2/gitsm: Add clean function
>>
>> The git's clean can only remove parent git repo from DL_DIR, but doesn't
>> remove
>> submodules, this patch fixes the problem.
>>
>> Signed-off-by: Robert Yang <liezhi.yang@windriver.com>
>> ---
>>   bitbake/lib/bb/fetch2/gitsm.py | 15 +++++++++++++--
>>   1 file changed, 13 insertions(+), 2 deletions(-)
>>
>> diff --git a/bitbake/lib/bb/fetch2/gitsm.py b/bitbake/lib/bb/fetch2/gitsm.py
>> index b21fed2..bb61093 100644
>> --- a/bitbake/lib/bb/fetch2/gitsm.py
>> +++ b/bitbake/lib/bb/fetch2/gitsm.py
>> @@ -76,8 +76,8 @@ class GitSM(Git):
>>           for name in ud.names:
>>               try:
>>                   gitmodules = runfetchcmd("%s show %s:.gitmodules" %
>> (ud.basecmd, ud.revisions[name]), d, quiet=True, workdir=workdir)
>> -            except:
>> -                # No submodules to update
>> +            except Exception as esc:
>> +                logger.info("No submodules found: %s" % workdir)
>>                   continue
>>
>>               for m, md in parse_gitmodules(gitmodules).items():
>> @@ -164,6 +164,17 @@ class GitSM(Git):
>>           Git.download(self, ud, d)
>>           self.process_submodules(ud, ud.clonedir, download_submodule, d)
>>
>> +    def clean(self, ud, d):
>> +        def clean_submodule(ud, url, module, modpath, d):
>> +            try:
>> +                newfetch = Fetch([url], d, cache=False)
>> +                newfetch.clean()
>> +            except Exception as e:
>> +                logger.warn('gitsm: submodule clean failed: %s %s' %
>> (type(e).__name__, str(e)))
>> +
>> +        self.process_submodules(ud, ud.clonedir, clean_submodule, d)
>> +        Git.clean(self, ud, d)
>> +
>>       def unpack(self, ud, destdir, d):
>>           def unpack_submodules(ud, url, module, modpath, d):
>>               url += ";bareclone=1;nobranch=1"
>> --
>> 2.7.4
>>
>> --
>> _______________________________________________
>> bitbake-devel mailing list
>> bitbake-devel@lists.openembedded.org
>> http://lists.openembedded.org/mailman/listinfo/bitbake-devel

Re: [PATCH 4/5] fetch2/gitsm: Add clean function

[Mark Hatle]

On 3/14/19 9:50 PM, Robert Yang wrote:
> Hi,
> 
> On 3/14/19 8:34 PM, Lisicki, Raphael wrote:
>> I have not fully understand the logic but I am wondering what happens here is a submodule is located in two different repositories? Wouldn't a clean of one module delete submodules of the other one as well?
> 
> Here are more explanations about the logic, for example, repo_A has two
> submodules repo_B and repo_C, then after do_fetch,
> they will be"
> DL_DIR/repo_A
> DL_DIR/repo_B
> DL_DIR/repo_C
> 
> Without this patch, do_cleanall only removes DL_DIR/repo_A, but leaves
> repo_B and repo_C there, this patch makes it remove repo_B and C.

The issue Raphael is mentioning is when you have multiple projects that use the
same submodules.  I.e.:

recipeA:
 DL_DIR/repo_A
 DL_DIR/repo_B
 DL_DIR/repo_C

recipeB:
 DL_DIR/repo_D
 DL_DIR/repo_C

If you do a clean of A, you will end up removing repo_C which both items end up
using.

I don't know any way to resolve this, since the dependency is implicit, not
explicit -- so the system won't have any idea that the same thing is used in
multiple places.

> 
> For your question, if repo_B is submodule of both repo_A and repo_D, it
> is like foo and foo-native use the same git source, when you run
> "bitbake foo -cccleanall", both foo and foo-native's source will be
> removed since they use the same source, the similar to gitsm, so it is not
> a problem, the source will be re-downloaded automatically when needed.

I'm not sure if this situation will work, since the recipeB will have a stamp
that says the download is complete, even though part of it is now missing.

--Mark

> // Robert
> 
>>
>> Best regards
>> Raphael
>>
>>> -----Original Message-----
>>> From: bitbake-devel-bounces@lists.openembedded.org <bitbake-devel-
>>> bounces@lists.openembedded.org> On Behalf Of Robert Yang
>>> Sent: Thursday, March 14, 2019 10:28 AM
>>> To: bitbake-devel@lists.openembedded.org
>>> Subject: [bitbake-devel] [PATCH 4/5] fetch2/gitsm: Add clean function
>>>
>>> The git's clean can only remove parent git repo from DL_DIR, but doesn't
>>> remove
>>> submodules, this patch fixes the problem.
>>>
>>> Signed-off-by: Robert Yang <liezhi.yang@windriver.com>
>>> ---
>>>   bitbake/lib/bb/fetch2/gitsm.py | 15 +++++++++++++--
>>>   1 file changed, 13 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/bitbake/lib/bb/fetch2/gitsm.py b/bitbake/lib/bb/fetch2/gitsm.py
>>> index b21fed2..bb61093 100644
>>> --- a/bitbake/lib/bb/fetch2/gitsm.py
>>> +++ b/bitbake/lib/bb/fetch2/gitsm.py
>>> @@ -76,8 +76,8 @@ class GitSM(Git):
>>>           for name in ud.names:
>>>               try:
>>>                   gitmodules = runfetchcmd("%s show %s:.gitmodules" %
>>> (ud.basecmd, ud.revisions[name]), d, quiet=True, workdir=workdir)
>>> -            except:
>>> -                # No submodules to update
>>> +            except Exception as esc:
>>> +                logger.info("No submodules found: %s" % workdir)
>>>                   continue
>>>
>>>               for m, md in parse_gitmodules(gitmodules).items():
>>> @@ -164,6 +164,17 @@ class GitSM(Git):
>>>           Git.download(self, ud, d)
>>>           self.process_submodules(ud, ud.clonedir, download_submodule, d)
>>>
>>> +    def clean(self, ud, d):
>>> +        def clean_submodule(ud, url, module, modpath, d):
>>> +            try:
>>> +                newfetch = Fetch([url], d, cache=False)
>>> +                newfetch.clean()
>>> +            except Exception as e:
>>> +                logger.warn('gitsm: submodule clean failed: %s %s' %
>>> (type(e).__name__, str(e)))
>>> +
>>> +        self.process_submodules(ud, ud.clonedir, clean_submodule, d)
>>> +        Git.clean(self, ud, d)
>>> +
>>>       def unpack(self, ud, destdir, d):
>>>           def unpack_submodules(ud, url, module, modpath, d):
>>>               url += ";bareclone=1;nobranch=1"
>>> --
>>> 2.7.4
>>>
>>> --
>>> _______________________________________________
>>> bitbake-devel mailing list
>>> bitbake-devel@lists.openembedded.org
>>> http://lists.openembedded.org/mailman/listinfo/bitbake-devel

Re: [PATCH 4/5] fetch2/gitsm: Add clean function

[Robert Yang]

Hi Mark,

On 3/15/19 11:44 PM, Mark Hatle wrote:
> On 3/14/19 9:50 PM, Robert Yang wrote:
>> Hi,
>>
>> On 3/14/19 8:34 PM, Lisicki, Raphael wrote:
>>> I have not fully understand the logic but I am wondering what happens here is a submodule is located in two different repositories? Wouldn't a clean of one module delete submodules of the other one as well?
>>
>> Here are more explanations about the logic, for example, repo_A has two
>> submodules repo_B and repo_C, then after do_fetch,
>> they will be"
>> DL_DIR/repo_A
>> DL_DIR/repo_B
>> DL_DIR/repo_C
>>
>> Without this patch, do_cleanall only removes DL_DIR/repo_A, but leaves
>> repo_B and repo_C there, this patch makes it remove repo_B and C.
> 
> The issue Raphael is mentioning is when you have multiple projects that use the
> same submodules.  I.e.:
> 
> recipeA:
>   DL_DIR/repo_A
>   DL_DIR/repo_B
>   DL_DIR/repo_C
> 
> recipeB:
>   DL_DIR/repo_D
>   DL_DIR/repo_C
> 
> If you do a clean of A, you will end up removing repo_C which both items end up
> using.
> 
> I don't know any way to resolve this, since the dependency is implicit, not
> explicit -- so the system won't have any idea that the same thing is used in
> multiple places.
> 
>>
>> For your question, if repo_B is submodule of both repo_A and repo_D, it
>> is like foo and foo-native use the same git source, when you run
>> "bitbake foo -cccleanall", both foo and foo-native's source will be
>> removed since they use the same source, the similar to gitsm, so it is not
>> a problem, the source will be re-downloaded automatically when needed.
> 
> I'm not sure if this situation will work, since the recipeB will have a stamp
> that says the download is complete, even though part of it is now missing.


It's a generic issue when the same sources are use across recipes, e.g.:

# Cleanup everything
$ bitbake gnu-config gnu-config-native -ccleanall

# Fetch
$ bitbake gnu-config gnu-config-native -cfetch

# Cleanup the git repo
$ bitbake gnu-config -ccleanall

# Unpack gnu-config-native
$ bitbake gnu-config-native -cunpack

ERROR: gnu-config-native-20181128+gitAUTOINC+058639be22-r0 do_unpack: Unpack 
failure for URL: 'git://git.savannah.gnu.org/config.git'. No up to date source 
found: clone directory not available or not up to date: 
/buildarea1/lyang1/rebase-work/test_fetch/downloads/git2/git.savannah.gnu.org.config.git; 
shallow clone not enabled
ERROR: gnu-config-native-20181128+gitAUTOINC+058639be22-r0 do_unpack:
ERROR: gnu-config-native-20181128+gitAUTOINC+058639be22-r0 do_unpack: Function 
failed: base_do_unpack

So the users should handle it themselves in such a case.

// Robert

> 
> --Mark
> 
>> // Robert
>>
>>>
>>> Best regards
>>> Raphael
>>>
>>>> -----Original Message-----
>>>> From: bitbake-devel-bounces@lists.openembedded.org <bitbake-devel-
>>>> bounces@lists.openembedded.org> On Behalf Of Robert Yang
>>>> Sent: Thursday, March 14, 2019 10:28 AM
>>>> To: bitbake-devel@lists.openembedded.org
>>>> Subject: [bitbake-devel] [PATCH 4/5] fetch2/gitsm: Add clean function
>>>>
>>>> The git's clean can only remove parent git repo from DL_DIR, but doesn't
>>>> remove
>>>> submodules, this patch fixes the problem.
>>>>
>>>> Signed-off-by: Robert Yang <liezhi.yang@windriver.com>
>>>> ---
>>>>    bitbake/lib/bb/fetch2/gitsm.py | 15 +++++++++++++--
>>>>    1 file changed, 13 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/bitbake/lib/bb/fetch2/gitsm.py b/bitbake/lib/bb/fetch2/gitsm.py
>>>> index b21fed2..bb61093 100644
>>>> --- a/bitbake/lib/bb/fetch2/gitsm.py
>>>> +++ b/bitbake/lib/bb/fetch2/gitsm.py
>>>> @@ -76,8 +76,8 @@ class GitSM(Git):
>>>>            for name in ud.names:
>>>>                try:
>>>>                    gitmodules = runfetchcmd("%s show %s:.gitmodules" %
>>>> (ud.basecmd, ud.revisions[name]), d, quiet=True, workdir=workdir)
>>>> -            except:
>>>> -                # No submodules to update
>>>> +            except Exception as esc:
>>>> +                logger.info("No submodules found: %s" % workdir)
>>>>                    continue
>>>>
>>>>                for m, md in parse_gitmodules(gitmodules).items():
>>>> @@ -164,6 +164,17 @@ class GitSM(Git):
>>>>            Git.download(self, ud, d)
>>>>            self.process_submodules(ud, ud.clonedir, download_submodule, d)
>>>>
>>>> +    def clean(self, ud, d):
>>>> +        def clean_submodule(ud, url, module, modpath, d):
>>>> +            try:
>>>> +                newfetch = Fetch([url], d, cache=False)
>>>> +                newfetch.clean()
>>>> +            except Exception as e:
>>>> +                logger.warn('gitsm: submodule clean failed: %s %s' %
>>>> (type(e).__name__, str(e)))
>>>> +
>>>> +        self.process_submodules(ud, ud.clonedir, clean_submodule, d)
>>>> +        Git.clean(self, ud, d)
>>>> +
>>>>        def unpack(self, ud, destdir, d):
>>>>            def unpack_submodules(ud, url, module, modpath, d):
>>>>                url += ";bareclone=1;nobranch=1"
>>>> --
>>>> 2.7.4
>>>>
>>>> --
>>>> _______________________________________________
>>>> bitbake-devel mailing list
>>>> bitbake-devel@lists.openembedded.org
>>>> http://lists.openembedded.org/mailman/listinfo/bitbake-devel
> 
> 

Re: [PATCH 4/5] fetch2/gitsm: Add clean function

[Mark Hatle]

On 3/14/19 4:28 AM, Robert Yang wrote:
> The git's clean can only remove parent git repo from DL_DIR, but doesn't remove
> submodules, this patch fixes the problem.
> 
> Signed-off-by: Robert Yang <liezhi.yang@windriver.com>
> ---
>  bitbake/lib/bb/fetch2/gitsm.py | 15 +++++++++++++--
>  1 file changed, 13 insertions(+), 2 deletions(-)
> 
> diff --git a/bitbake/lib/bb/fetch2/gitsm.py b/bitbake/lib/bb/fetch2/gitsm.py
> index b21fed2..bb61093 100644
> --- a/bitbake/lib/bb/fetch2/gitsm.py
> +++ b/bitbake/lib/bb/fetch2/gitsm.py
> @@ -76,8 +76,8 @@ class GitSM(Git):
>          for name in ud.names:
>              try:
>                  gitmodules = runfetchcmd("%s show %s:.gitmodules" % (ud.basecmd, ud.revisions[name]), d, quiet=True, workdir=workdir)
> -            except:
> -                # No submodules to update
> +            except Exception as esc:
> +                logger.info("No submodules found: %s" % workdir)

We don't want the log message.  git submodules are processed recursively, and
the end of the recursion will -always- have 'gitsm' items that have no
submodules.  This is also why it was a generic except, because we really don't
care WHAT the exception is, only that if one happens -- we know the command
failed and to simply ignore it and finish the iteration.

                  continue
>  
>              for m, md in parse_gitmodules(gitmodules).items():
> @@ -164,6 +164,17 @@ class GitSM(Git):
>          Git.download(self, ud, d)
>          self.process_submodules(ud, ud.clonedir, download_submodule, d)
>  
> +    def clean(self, ud, d):
> +        def clean_submodule(ud, url, module, modpath, d):
> +            try:
> +                newfetch = Fetch([url], d, cache=False)
> +                newfetch.clean()
> +            except Exception as e:
> +                logger.warn('gitsm: submodule clean failed: %s %s' % (type(e).__name__, str(e)))
> +
> +        self.process_submodules(ud, ud.clonedir, clean_submodule, d)
> +        Git.clean(self, ud, d)
> +

I don't see 'clean_submodule' implemented in this patch.  The process_submodules
will simply call that function, if it's not defined it will error or not do
anything if it finds a submodule.

>      def unpack(self, ud, destdir, d):
>          def unpack_submodules(ud, url, module, modpath, d):
>              url += ";bareclone=1;nobranch=1"
> 

Re: [PATCH 4/5] fetch2/gitsm: Add clean function

[Robert Yang]

Hi Mark,

On 3/14/19 10:41 PM, Mark Hatle wrote:
> On 3/14/19 4:28 AM, Robert Yang wrote:
>> The git's clean can only remove parent git repo from DL_DIR, but doesn't remove
>> submodules, this patch fixes the problem.
>>
>> Signed-off-by: Robert Yang <liezhi.yang@windriver.com>
>> ---
>>   bitbake/lib/bb/fetch2/gitsm.py | 15 +++++++++++++--
>>   1 file changed, 13 insertions(+), 2 deletions(-)
>>
>> diff --git a/bitbake/lib/bb/fetch2/gitsm.py b/bitbake/lib/bb/fetch2/gitsm.py
>> index b21fed2..bb61093 100644
>> --- a/bitbake/lib/bb/fetch2/gitsm.py
>> +++ b/bitbake/lib/bb/fetch2/gitsm.py
>> @@ -76,8 +76,8 @@ class GitSM(Git):
>>           for name in ud.names:
>>               try:
>>                   gitmodules = runfetchcmd("%s show %s:.gitmodules" % (ud.basecmd, ud.revisions[name]), d, quiet=True, workdir=workdir)
>> -            except:
>> -                # No submodules to update
>> +            except Exception as esc:
>> +                logger.info("No submodules found: %s" % workdir)
> 
> We don't want the log message.  git submodules are processed recursively, and
> the end of the recursion will -always- have 'gitsm' items that have no
> submodules.  This is also why it was a generic except, because we really don't
> care WHAT the exception is, only that if one happens -- we know the command
> failed and to simply ignore it and finish the iteration.
> 
>                    continue
>>   
>>               for m, md in parse_gitmodules(gitmodules).items():
>> @@ -164,6 +164,17 @@ class GitSM(Git):
>>           Git.download(self, ud, d)
>>           self.process_submodules(ud, ud.clonedir, download_submodule, d)
>>   
>> +    def clean(self, ud, d):
>> +        def clean_submodule(ud, url, module, modpath, d):
>> +            try:
>> +                newfetch = Fetch([url], d, cache=False)
>> +                newfetch.clean()
>> +            except Exception as e:
>> +                logger.warn('gitsm: submodule clean failed: %s %s' % (type(e).__name__, str(e)))
>> +
>> +        self.process_submodules(ud, ud.clonedir, clean_submodule, d)
>> +        Git.clean(self, ud, d)
>> +
> 
> I don't see 'clean_submodule' implemented in this patch.  The process_submodules
> will simply call that function, if it's not defined it will error or not do
> anything if it finds a submodule.


The clean_submodule is inside of 'def clean', right under 'def clean':

+    def clean(self, ud, d):
+        def clean_submodule(ud, url, module, modpath, d):

// Robert

> 
>>       def unpack(self, ud, destdir, d):
>>           def unpack_submodules(ud, url, module, modpath, d):
>>               url += ";bareclone=1;nobranch=1"
>>
> 
> 

Re: [PATCH 4/5] fetch2/gitsm: Add clean function

[Mark Hatle]

On 3/14/19 9:52 PM, Robert Yang wrote:
> Hi Mark,
> 
> On 3/14/19 10:41 PM, Mark Hatle wrote:
>> On 3/14/19 4:28 AM, Robert Yang wrote:
>>> The git's clean can only remove parent git repo from DL_DIR, but doesn't remove
>>> submodules, this patch fixes the problem.
>>>
>>> Signed-off-by: Robert Yang <liezhi.yang@windriver.com>
>>> ---
>>>   bitbake/lib/bb/fetch2/gitsm.py | 15 +++++++++++++--
>>>   1 file changed, 13 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/bitbake/lib/bb/fetch2/gitsm.py b/bitbake/lib/bb/fetch2/gitsm.py
>>> index b21fed2..bb61093 100644
>>> --- a/bitbake/lib/bb/fetch2/gitsm.py
>>> +++ b/bitbake/lib/bb/fetch2/gitsm.py
>>> @@ -76,8 +76,8 @@ class GitSM(Git):
>>>           for name in ud.names:
>>>               try:
>>>                   gitmodules = runfetchcmd("%s show %s:.gitmodules" % (ud.basecmd, ud.revisions[name]), d, quiet=True, workdir=workdir)
>>> -            except:
>>> -                # No submodules to update
>>> +            except Exception as esc:
>>> +                logger.info("No submodules found: %s" % workdir)
>>
>> We don't want the log message.  git submodules are processed recursively, and
>> the end of the recursion will -always- have 'gitsm' items that have no
>> submodules.  This is also why it was a generic except, because we really don't
>> care WHAT the exception is, only that if one happens -- we know the command
>> failed and to simply ignore it and finish the iteration.
>>
>>                    continue
>>>   
>>>               for m, md in parse_gitmodules(gitmodules).items():
>>> @@ -164,6 +164,17 @@ class GitSM(Git):
>>>           Git.download(self, ud, d)
>>>           self.process_submodules(ud, ud.clonedir, download_submodule, d)
>>>   
>>> +    def clean(self, ud, d):
>>> +        def clean_submodule(ud, url, module, modpath, d):
>>> +            try:
>>> +                newfetch = Fetch([url], d, cache=False)
>>> +                newfetch.clean()
>>> +            except Exception as e:
>>> +                logger.warn('gitsm: submodule clean failed: %s %s' % (type(e).__name__, str(e)))
>>> +
>>> +        self.process_submodules(ud, ud.clonedir, clean_submodule, d)
>>> +        Git.clean(self, ud, d)
>>> +
>>
>> I don't see 'clean_submodule' implemented in this patch.  The process_submodules
>> will simply call that function, if it's not defined it will error or not do
>> anything if it finds a submodule.
> 
> 
> The clean_submodule is inside of 'def clean', right under 'def clean':
> 
> +    def clean(self, ud, d):
> +        def clean_submodule(ud, url, module, modpath, d):

Apparently I'm blind.  :)

--Mark

> // Robert
> 
>>
>>>       def unpack(self, ud, destdir, d):
>>>           def unpack_submodules(ud, url, module, modpath, d):
>>>               url += ";bareclone=1;nobranch=1"
>>>
>>
>>

Re: [PATCH 4/5] fetch2/gitsm: Add clean function

[Robert Yang]

Hi Mark,

On 3/14/19 10:41 PM, Mark Hatle wrote:
> On 3/14/19 4:28 AM, Robert Yang wrote:
>> The git's clean can only remove parent git repo from DL_DIR, but doesn't remove
>> submodules, this patch fixes the problem.
>>
>> Signed-off-by: Robert Yang <liezhi.yang@windriver.com>
>> ---
>>   bitbake/lib/bb/fetch2/gitsm.py | 15 +++++++++++++--
>>   1 file changed, 13 insertions(+), 2 deletions(-)
>>
>> diff --git a/bitbake/lib/bb/fetch2/gitsm.py b/bitbake/lib/bb/fetch2/gitsm.py
>> index b21fed2..bb61093 100644
>> --- a/bitbake/lib/bb/fetch2/gitsm.py
>> +++ b/bitbake/lib/bb/fetch2/gitsm.py
>> @@ -76,8 +76,8 @@ class GitSM(Git):
>>           for name in ud.names:
>>               try:
>>                   gitmodules = runfetchcmd("%s show %s:.gitmodules" % (ud.basecmd, ud.revisions[name]), d, quiet=True, workdir=workdir)
>> -            except:
>> -                # No submodules to update
>> +            except Exception as esc:
>> +                logger.info("No submodules found: %s" % workdir)
> 
> We don't want the log message.  git submodules are processed recursively, and
> the end of the recursion will -always- have 'gitsm' items that have no
> submodules.  This is also why it was a generic except, because we really don't
> care WHAT the exception is, only that if one happens -- we know the command
> failed and to simply ignore it and finish the iteration.

Sorry, I forgot to reply on this just now. Submodule is a little different and
complicated then other SCMs, for exmaple, when I worked on gitsm' clean()
function, it always didn't work which puzzled me a lot, at last I found that
I had called Git.clean() before self.process_submodules:

Git.clean(self, ud, d)
self.process_submodules(ud, ud.clonedir, clean_submodule, d)

So that the parent repo had gone before submodules, but there was no message
told me this, it just failed quietly, so I had to debug it line by line.

The logger.info() goes into log.do_fetch which won't affect users by default,
but it would help to understand what happend when they need debug.

// Robert

> 
>                    continue
>>   
>>               for m, md in parse_gitmodules(gitmodules).items():
>> @@ -164,6 +164,17 @@ class GitSM(Git):
>>           Git.download(self, ud, d)
>>           self.process_submodules(ud, ud.clonedir, download_submodule, d)
>>   
>> +    def clean(self, ud, d):
>> +        def clean_submodule(ud, url, module, modpath, d):
>> +            try:
>> +                newfetch = Fetch([url], d, cache=False)
>> +                newfetch.clean()
>> +            except Exception as e:
>> +                logger.warn('gitsm: submodule clean failed: %s %s' % (type(e).__name__, str(e)))
>> +
>> +        self.process_submodules(ud, ud.clonedir, clean_submodule, d)
>> +        Git.clean(self, ud, d)
>> +
> 
> I don't see 'clean_submodule' implemented in this patch.  The process_submodules
> will simply call that function, if it's not defined it will error or not do
> anything if it finds a submodule.
> 
>>       def unpack(self, ud, destdir, d):
>>           def unpack_submodules(ud, url, module, modpath, d):
>>               url += ";bareclone=1;nobranch=1"
>>
> 
> 

[PATCH 5/5] fetch2: Add gitsm's function process_submodules()

[Robert Yang]

So that bbclass can call it as fetcher.process_submodules(), otherwise, the
bbclass has to handle submodules itself.

Signed-off-by: Robert Yang <liezhi.yang@windriver.com>
---
 bitbake/lib/bb/fetch2/__init__.py | 25 +++++++++++++++++++++++++
 1 file changed, 25 insertions(+)

diff --git a/bitbake/lib/bb/fetch2/__init__.py b/bitbake/lib/bb/fetch2/__init__.py
index 64d7526..78bea1f 100644
--- a/bitbake/lib/bb/fetch2/__init__.py
+++ b/bitbake/lib/bb/fetch2/__init__.py
@@ -1807,6 +1807,31 @@ class Fetch(object):
             if ud.lockfile:
                 bb.utils.unlockfile(lf)
 
+    def process_submodules(self, ud, workdir, function, urls=None):
+        if not urls:
+            urls = self.urls
+
+        for url in urls:
+            if url not in self.ud:
+                self.ud[url] = FetchData(url, d)
+            ud = self.ud[url]
+            ud.setup_localpath(self.d)
+
+            if not ud.localfile and ud.localpath is None:
+                continue
+
+
+            if ud.lockfile:
+                lf = bb.utils.lockfile(ud.lockfile)
+
+            ud.method.process_submodules(ud, workdir, function, self.d)
+            if ud.donestamp:
+                bb.utils.remove(ud.donestamp)
+
+            if ud.lockfile:
+                bb.utils.unlockfile(lf)
+
+
 class FetchConnectionCache(object):
     """
         A class which represents an container for socket connections.
-- 
2.7.4

Re: [PATCH 5/5] fetch2: Add gitsm's function process_submodules()

[Mark Hatle]

On 3/14/19 4:28 AM, Robert Yang wrote:
> So that bbclass can call it as fetcher.process_submodules(), otherwise, the
> bbclass has to handle submodules itself.

Are there any general equivalent to submodules in any other SCM?

I think SVN has something similar, but I'm not sure if it requires special
fetching or processing activities.  I don't think other SCMs do.

Why is this needed, for do_clean/do_cleanall?

Theoretically it should be up to the fetch implementation itself to walk
everything and do the clean operation.  (another comment below)

> Signed-off-by: Robert Yang <liezhi.yang@windriver.com>
> ---
>  bitbake/lib/bb/fetch2/__init__.py | 25 +++++++++++++++++++++++++
>  1 file changed, 25 insertions(+)
> 
> diff --git a/bitbake/lib/bb/fetch2/__init__.py b/bitbake/lib/bb/fetch2/__init__.py
> index 64d7526..78bea1f 100644
> --- a/bitbake/lib/bb/fetch2/__init__.py
> +++ b/bitbake/lib/bb/fetch2/__init__.py
> @@ -1807,6 +1807,31 @@ class Fetch(object):
>              if ud.lockfile:
>                  bb.utils.unlockfile(lf)
>  
> +    def process_submodules(self, ud, workdir, function, urls=None):
> +        if not urls:
> +            urls = self.urls
> +
> +        for url in urls:
> +            if url not in self.ud:
> +                self.ud[url] = FetchData(url, d)
> +            ud = self.ud[url]
> +            ud.setup_localpath(self.d)
> +
> +            if not ud.localfile and ud.localpath is None:
> +                continue
> +
> +
> +            if ud.lockfile:
> +                lf = bb.utils.lockfile(ud.lockfile)
> +
> +            ud.method.process_submodules(ud, workdir, function, self.d)

Since most fetchers won't have this implemented, shouldn't you check if the
function is available, and if not simply continue (nothing to do)?

> +            if ud.donestamp:
> +                bb.utils.remove(ud.donestamp)
> +
> +            if ud.lockfile:
> +                bb.utils.unlockfile(lf)
> +
> +
>  class FetchConnectionCache(object):
>      """
>          A class which represents an container for socket connections.
> 

Re: [PATCH 5/5] fetch2: Add gitsm's function process_submodules()

[Robert Yang]

Hi Mark,

On 3/14/19 10:46 PM, Mark Hatle wrote:
> On 3/14/19 4:28 AM, Robert Yang wrote:
>> So that bbclass can call it as fetcher.process_submodules(), otherwise, the
>> bbclass has to handle submodules itself.
> 
> Are there any general equivalent to submodules in any other SCM?
> 
> I think SVN has something similar, but I'm not sure if it requires special
> fetching or processing activities.  I don't think other SCMs do.

Yes, I agree with that.

> 
> Why is this needed, for do_clean/do_cleanall?

We have an internal bbclass (fetcher_helper.bbclass) to collect all SRC_URI, and
add the sources into corresponding download layer. I can get the submodule's
SRC_URI in fetcher_helper.bbclass as I had already done, but I think that
others (e.g., tinfoil.py) may also have the similar requirements, so I think
that we'd better export the API in bitbake/lib/bb/fetch2/__init__.py,
then we can use it as:

fetcher = bb.fetch2.Fetch(src_uri, d)
fetcher.process_submodules(ud, ud.clonedir, customized_function)


Otherwise, we have to handle submodules manually as I had done in 
fetcher_helper.bbclass.


> 
> Theoretically it should be up to the fetch implementation itself to walk
> everything and do the clean operation.  (another comment below)
> 
>> Signed-off-by: Robert Yang <liezhi.yang@windriver.com>
>> ---
>>   bitbake/lib/bb/fetch2/__init__.py | 25 +++++++++++++++++++++++++
>>   1 file changed, 25 insertions(+)
>>
>> diff --git a/bitbake/lib/bb/fetch2/__init__.py b/bitbake/lib/bb/fetch2/__init__.py
>> index 64d7526..78bea1f 100644
>> --- a/bitbake/lib/bb/fetch2/__init__.py
>> +++ b/bitbake/lib/bb/fetch2/__init__.py
>> @@ -1807,6 +1807,31 @@ class Fetch(object):
>>               if ud.lockfile:
>>                   bb.utils.unlockfile(lf)
>>   
>> +    def process_submodules(self, ud, workdir, function, urls=None):
>> +        if not urls:
>> +            urls = self.urls
>> +
>> +        for url in urls:
>> +            if url not in self.ud:
>> +                self.ud[url] = FetchData(url, d)
>> +            ud = self.ud[url]
>> +            ud.setup_localpath(self.d)
>> +
>> +            if not ud.localfile and ud.localpath is None:
>> +                continue
>> +
>> +
>> +            if ud.lockfile:
>> +                lf = bb.utils.lockfile(ud.lockfile)
>> +
>> +            ud.method.process_submodules(ud, workdir, function, self.d)
> 
> Since most fetchers won't have this implemented, shouldn't you check if the
> function is available, and if not simply continue (nothing to do)?

Yes, make sense, fixed it in the pull repo:

+    def process_submodules(self, ud, workdir, function, urls=None):
+        if not hasattr(ud.method, 'process_submodules'):
+            bb.warn('process_submodules() is not implented.')
+            return
[snip]


// Robert


> 
>> +            if ud.donestamp:
>> +                bb.utils.remove(ud.donestamp)
>> +
>> +            if ud.lockfile:
>> +                bb.utils.unlockfile(lf)
>> +
>> +
>>   class FetchConnectionCache(object):
>>       """
>>           A class which represents an container for socket connections.
>>
> 
> 

Re: [PATCH 5/5] fetch2: Add gitsm's function process_submodules()

[Mark Hatle]

On 3/14/19 10:37 PM, Robert Yang wrote:
> Hi Mark,
> 
> On 3/14/19 10:46 PM, Mark Hatle wrote:
>> On 3/14/19 4:28 AM, Robert Yang wrote:
>>> So that bbclass can call it as fetcher.process_submodules(), otherwise, the
>>> bbclass has to handle submodules itself.
>>
>> Are there any general equivalent to submodules in any other SCM?
>>
>> I think SVN has something similar, but I'm not sure if it requires special
>> fetching or processing activities.  I don't think other SCMs do.
> 
> Yes, I agree with that.
> 
>>
>> Why is this needed, for do_clean/do_cleanall?
> 
> We have an internal bbclass (fetcher_helper.bbclass) to collect all SRC_URI, and
> add the sources into corresponding download layer. I can get the submodule's
> SRC_URI in fetcher_helper.bbclass as I had already done, but I think that
> others (e.g., tinfoil.py) may also have the similar requirements, so I think
> that we'd better export the API in bitbake/lib/bb/fetch2/__init__.py,
> then we can use it as:
> 
> fetcher = bb.fetch2.Fetch(src_uri, d)
> fetcher.process_submodules(ud, ud.clonedir, customized_function)
> 
> 
> Otherwise, we have to handle submodules manually as I had done in 
> fetcher_helper.bbclass.
> 
> 
>>
>> Theoretically it should be up to the fetch implementation itself to walk
>> everything and do the clean operation.  (another comment below)
>>
>>> Signed-off-by: Robert Yang <liezhi.yang@windriver.com>
>>> ---
>>>   bitbake/lib/bb/fetch2/__init__.py | 25 +++++++++++++++++++++++++
>>>   1 file changed, 25 insertions(+)
>>>
>>> diff --git a/bitbake/lib/bb/fetch2/__init__.py b/bitbake/lib/bb/fetch2/__init__.py
>>> index 64d7526..78bea1f 100644
>>> --- a/bitbake/lib/bb/fetch2/__init__.py
>>> +++ b/bitbake/lib/bb/fetch2/__init__.py
>>> @@ -1807,6 +1807,31 @@ class Fetch(object):
>>>               if ud.lockfile:
>>>                   bb.utils.unlockfile(lf)
>>>   
>>> +    def process_submodules(self, ud, workdir, function, urls=None):
>>> +        if not urls:
>>> +            urls = self.urls
>>> +
>>> +        for url in urls:
>>> +            if url not in self.ud:
>>> +                self.ud[url] = FetchData(url, d)
>>> +            ud = self.ud[url]
>>> +            ud.setup_localpath(self.d)
>>> +
>>> +            if not ud.localfile and ud.localpath is None:
>>> +                continue
>>> +
>>> +
>>> +            if ud.lockfile:
>>> +                lf = bb.utils.lockfile(ud.lockfile)
>>> +
>>> +            ud.method.process_submodules(ud, workdir, function, self.d)
>>
>> Since most fetchers won't have this implemented, shouldn't you check if the
>> function is available, and if not simply continue (nothing to do)?
> 
> Yes, make sense, fixed it in the pull repo:
> 
> +    def process_submodules(self, ud, workdir, function, urls=None):
> +        if not hasattr(ud.method, 'process_submodules'):
> +            bb.warn('process_submodules() is not implented.')
> +            return
> [snip]

I think it's more simple then that.

def process_submodules(self, ud, workdir, function, urls=None):
    if not hasattr(ud.method, 'process_submodules'):
        return

Since most things won't have a process_submodules, I don't think we want any
sort of warning, it should just return if nothing has happened.

As for the question above if it's a good idea or not, I'll leave that to RP.
I'm not sure either way.

--Mark

> 
> // Robert
> 
> 
>>
>>> +            if ud.donestamp:
>>> +                bb.utils.remove(ud.donestamp)
>>> +
>>> +            if ud.lockfile:
>>> +                bb.utils.unlockfile(lf)
>>> +
>>> +
>>>   class FetchConnectionCache(object):
>>>       """
>>>           A class which represents an container for socket connections.
>>>
>>
>>

Re: [PATCH 5/5] fetch2: Add gitsm's function process_submodules()

[Robert Yang]

On 3/15/19 11:55 PM, Mark Hatle wrote:
> On 3/14/19 10:37 PM, Robert Yang wrote:
>> Hi Mark,
>>
>> On 3/14/19 10:46 PM, Mark Hatle wrote:
>>> On 3/14/19 4:28 AM, Robert Yang wrote:
>>>> So that bbclass can call it as fetcher.process_submodules(), otherwise, the
>>>> bbclass has to handle submodules itself.
>>>
>>> Are there any general equivalent to submodules in any other SCM?
>>>
>>> I think SVN has something similar, but I'm not sure if it requires special
>>> fetching or processing activities.  I don't think other SCMs do.
>>
>> Yes, I agree with that.
>>
>>>
>>> Why is this needed, for do_clean/do_cleanall?
>>
>> We have an internal bbclass (fetcher_helper.bbclass) to collect all SRC_URI, and
>> add the sources into corresponding download layer. I can get the submodule's
>> SRC_URI in fetcher_helper.bbclass as I had already done, but I think that
>> others (e.g., tinfoil.py) may also have the similar requirements, so I think
>> that we'd better export the API in bitbake/lib/bb/fetch2/__init__.py,
>> then we can use it as:
>>
>> fetcher = bb.fetch2.Fetch(src_uri, d)
>> fetcher.process_submodules(ud, ud.clonedir, customized_function)
>>
>>
>> Otherwise, we have to handle submodules manually as I had done in
>> fetcher_helper.bbclass.
>>
>>
>>>
>>> Theoretically it should be up to the fetch implementation itself to walk
>>> everything and do the clean operation.  (another comment below)
>>>
>>>> Signed-off-by: Robert Yang <liezhi.yang@windriver.com>
>>>> ---
>>>>    bitbake/lib/bb/fetch2/__init__.py | 25 +++++++++++++++++++++++++
>>>>    1 file changed, 25 insertions(+)
>>>>
>>>> diff --git a/bitbake/lib/bb/fetch2/__init__.py b/bitbake/lib/bb/fetch2/__init__.py
>>>> index 64d7526..78bea1f 100644
>>>> --- a/bitbake/lib/bb/fetch2/__init__.py
>>>> +++ b/bitbake/lib/bb/fetch2/__init__.py
>>>> @@ -1807,6 +1807,31 @@ class Fetch(object):
>>>>                if ud.lockfile:
>>>>                    bb.utils.unlockfile(lf)
>>>>    
>>>> +    def process_submodules(self, ud, workdir, function, urls=None):
>>>> +        if not urls:
>>>> +            urls = self.urls
>>>> +
>>>> +        for url in urls:
>>>> +            if url not in self.ud:
>>>> +                self.ud[url] = FetchData(url, d)
>>>> +            ud = self.ud[url]
>>>> +            ud.setup_localpath(self.d)
>>>> +
>>>> +            if not ud.localfile and ud.localpath is None:
>>>> +                continue
>>>> +
>>>> +
>>>> +            if ud.lockfile:
>>>> +                lf = bb.utils.lockfile(ud.lockfile)
>>>> +
>>>> +            ud.method.process_submodules(ud, workdir, function, self.d)
>>>
>>> Since most fetchers won't have this implemented, shouldn't you check if the
>>> function is available, and if not simply continue (nothing to do)?
>>
>> Yes, make sense, fixed it in the pull repo:
>>
>> +    def process_submodules(self, ud, workdir, function, urls=None):
>> +        if not hasattr(ud.method, 'process_submodules'):
>> +            bb.warn('process_submodules() is not implented.')
>> +            return
>> [snip]
> 
> I think it's more simple then that.
> 
> def process_submodules(self, ud, workdir, function, urls=None):
>      if not hasattr(ud.method, 'process_submodules'):
>          return
> 
> Since most things won't have a process_submodules, I don't think we want any
> sort of warning, it should just return if nothing has happened.

Thanks, fixed in the repo.

// Robert

> 
> As for the question above if it's a good idea or not, I'll leave that to RP.
> I'm not sure either way.
> 
> --Mark
> 
>>
>> // Robert
>>
>>
>>>
>>>> +            if ud.donestamp:
>>>> +                bb.utils.remove(ud.donestamp)
>>>> +
>>>> +            if ud.lockfile:
>>>> +                bb.utils.unlockfile(lf)
>>>> +
>>>> +
>>>>    class FetchConnectionCache(object):
>>>>        """
>>>>            A class which represents an container for socket connections.
>>>>
>>>
>>>
> 
>