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

[Richard Purdie <richard.purdie@linuxfoundation.org>]

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