Re: [OE-Core][PATCH 2/2] oeqa/utils/gitarchive: fix tag computation when creating archive

["Alexis Lothoré" <alexis.lothore@bootlin.com>]

Hello Richard,

On 8/15/23 23:13, Richard Purdie wrote:
> On Fri, 2023-08-11 at 14:55 +0200, Alexis Lothoré via
> lists.openembedded.org wrote:
>> From: Alexis Lothoré <alexis.lothore@bootlin.com>
>>
>> Sporadic errors have been observed in autobuilder when trying to store new
>> tests results:
>>
>> error: failed to push some refs to 'push.yoctoproject.org:yocto-testresults'
>> hint: Updates were rejected because the tag already exists in the remote.
>>
>> The new tag name is generated by gitarchive based on known tags from the
>> repository (learnt with git tag). In autobuilder case, this repository is a
>> shallow clone, so git tag only returns most recent tags, which mean we
>> could miss some older tags which exist in remote but not locally. In this
>> case, gitarchive will likely create a tag which already exists in remote,
>> and so will fail to push
>>
>> Fix this tag duplication by using git ls-remote to learn about existing
>> tags instead of git tag. Two places which wrongly read only local tags has
>> been identified in gitarchive:  expand_tag_strings and get_test_runs
>>
>> Fixes [YOCTO #15140]
>>
>> Signed-off-by: Alexis Lothoré <alexis.lothore@bootlin.com>
>> ---
>>  meta/lib/oeqa/utils/gitarchive.py | 6 ++++--
>>  1 file changed, 4 insertions(+), 2 deletions(-)
>>
>> diff --git a/meta/lib/oeqa/utils/gitarchive.py b/meta/lib/oeqa/utils/gitarchive.py
>> index 6e8040eb5c96..73beafecb5fb 100644
>> --- a/meta/lib/oeqa/utils/gitarchive.py
>> +++ b/meta/lib/oeqa/utils/gitarchive.py
>> @@ -116,7 +116,8 @@ def expand_tag_strings(repo, name_pattern, msg_subj_pattern, msg_body_pattern,
>>          tag_re = tag_re.format(tag_number='(?P<tag_number>[0-9]{1,5})')
>>  
>>          keyws['tag_number'] = 0
>> -        for existing_tag in repo.run_cmd('tag').splitlines():
>> +        tags_refs = repo.run_cmd(['ls-remote', '--refs', '--tags', '-q'])
>> +        for existing_tag in ["".join(d.split()[1].split('/', 2)[2:]) for d in tags_refs.splitlines()]:
>>              match = re.match(tag_re, existing_tag)
>>  
>>              if match and int(match.group('tag_number')) >= keyws['tag_number']:
>> @@ -181,7 +182,8 @@ def get_test_runs(log, repo, tag_name, **kwargs):
>>  
>>      # Get a list of all matching tags
>>      tag_pattern = tag_name.format(**str_fields)
>> -    tags = repo.run_cmd(['tag', '-l', tag_pattern]).splitlines()
>> +    revs = repo.run_cmd(['ls-remote', '--refs', '--tags', 'origin', '-q', tag_pattern]).splitlines()
>> +    tags = ["".join(d.split()[1].split('/', 2)[2:]) for d in revs]
>>      log.debug("Found %d tags matching pattern '%s'", len(tags), tag_pattern)
>>  
>>      # Parse undefined fields from tag names
> 
> 
> I'm worrying a little that this change which recently merged might have
> resulted in:
> 
> https://autobuilder.yoctoproject.org/typhoon/#/builders/133/builds/2099

I may have taken the wrong assumption that directories manipulated with
gitarchive would always have at least one valid remote configured, but this log
tends to prove me wrong :/
Since the patch aimed to fix a sporadic issue but now leads to another
systematic issue, please feel free to revert it while I ensure about the
scenario and find a proper fix

Kind regards,
Alexis

> 
> ?
> 
> I didn't take the test yet since that is causing other issues on the
> autobuilder unrelated to the patch which I'm resolving first.
> 
> Cheers,
> 
> Richard
> 

-- 
Alexis Lothoré, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com