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

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

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