Message ID | 20230811125532.9427-3-alexis.lothore@bootlin.com |
---|---|
State | New |
Headers | show |
Series | oeqa/utils/gitarchive: fix tag name computation | expand |
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
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 >
On Wed, 2023-08-16 at 08:36 +0200, Alexis Lothoré wrote: > 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 Thanks, I was wondering why one performance worker was affected and the other was not. I guess one has the remote, maybe from when we've tried to fix something and the other doesn't. I've reverted for now until we get to the bottom of it as the stream of failures would be a pain. Cheers, Richard
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