[bitbake-devel,5/5] fetch2: Add gitsm's function process_submodules()

Submitted by Robert Yang on March 14, 2019, 9:28 a.m. | Patch ID: 159568

Details

Message ID d73ce5c9d372d09ff28654f55640611fe0b36e3c.1552555629.git.liezhi.yang@windriver.com
State New
Headers show

Commit Message

Robert Yang March 14, 2019, 9:28 a.m.
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(+)

Patch hide | download patch | download mbox

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.

Comments

Mark Hatle March 14, 2019, 2:46 p.m.
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.
>
Robert Yang March 15, 2019, 3:37 a.m.
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.
>>
> 
>
Mark Hatle March 15, 2019, 3:55 p.m.
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.
>>>
>>
>>
Robert Yang March 19, 2019, 3:47 a.m.
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.
>>>>
>>>
>>>
> 
>