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

[Robert Yang <liezhi.yang@windriver.com>]

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