[PATCH v2 0/2] oeqa/gitarchive: Push tag before copying log files

[PATCH v2 0/2] oeqa/gitarchive: Push tag before copying log files

[Mathieu Dubois-Briand]

Signed-off-by: Mathieu Dubois-Briand <mathieu.dubois-briand@bootlin.com>
---
Changes in v2:
- Split the `push` parameter of gitarchive in two parameters, instead of
  trying to parse the multiple possible types.
- Add a --push parameter instead of always pushing when log archiving is
  requested.
- Link to v1: https://lore.kernel.org/r/20251230-mathieu-fix-15696-v1-0-b4de13efebc0@bootlin.com

---
Mathieu Dubois-Briand (2):
      oeqa/gitarchive: Fix git push URL parameter
      oeqa/gitarchive: Push tag before copying log files

 meta/lib/oeqa/selftest/cases/gitarchivetests.py | 4 ++--
 meta/lib/oeqa/utils/gitarchive.py               | 8 ++++----
 scripts/lib/resulttool/store.py                 | 9 ++++++++-
 3 files changed, 14 insertions(+), 7 deletions(-)
---
base-commit: 235e6d49e5888ad04416219e10b6df91a738661a
change-id: 20251230-mathieu-fix-15696-32be41851aca

Best regards,
-- 
Mathieu Dubois-Briand <mathieu.dubois-briand@bootlin.com>

[PATCH v2 1/2] oeqa/gitarchive: Fix git push URL parameter

[Mathieu Dubois-Briand]

The gitarchive() function takes a `push` parameter than can be either a
boolean or a string. But this parameter is then passed to
expand_tag_strings(), which clearly expect it to be a string if it is
defined. Split this in two arguments: a `push` boolean value and a
`push_remote` optional string.

Signed-off-by: Mathieu Dubois-Briand <mathieu.dubois-briand@bootlin.com>
---
 meta/lib/oeqa/selftest/cases/gitarchivetests.py | 4 ++--
 meta/lib/oeqa/utils/gitarchive.py               | 8 ++++----
 scripts/lib/resulttool/store.py                 | 2 +-
 3 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/meta/lib/oeqa/selftest/cases/gitarchivetests.py b/meta/lib/oeqa/selftest/cases/gitarchivetests.py
index 71382089c124..dcf0eb3be569 100644
--- a/meta/lib/oeqa/selftest/cases/gitarchivetests.py
+++ b/meta/lib/oeqa/selftest/cases/gitarchivetests.py
@@ -74,7 +74,7 @@ class GitArchiveTests(OESelftestTestCase):
                               "Results of {branch}:{commit}", "branch: {branch}\ncommit: {commit}", "{branch}",
                               False, "{branch}/{commit_count}-g{commit}/{tag_number}",
                               'Test run #{tag_number} of {branch}:{commit}', '',
-                              [], [], False, keywords, logger)
+                              [], [], False, None, keywords, logger)
         self.assertTrue(tag_exists(git_obj, target_tag), msg=f"Tag {target_tag} has not been created")
         delete_fake_repository(path)
 
@@ -88,7 +88,7 @@ class GitArchiveTests(OESelftestTestCase):
                               "Results of {branch}:{commit}", "branch: {branch}\ncommit: {commit}", "{branch}",
                               False, "{branch}/{commit_count}-g{commit}/{tag_number}",
                               'Test run #{tag_number} of {branch}:{commit}', '',
-                              [], [], False, keywords, logger)
+                              [], [], False, None, keywords, logger)
         self.assertTrue(tag_exists(git_obj, second_tag), msg=f"Second tag {second_tag} has not been created")
         delete_fake_repository(path)
 
diff --git a/meta/lib/oeqa/utils/gitarchive.py b/meta/lib/oeqa/utils/gitarchive.py
index 7e1d5057482f..6ec17d36958c 100644
--- a/meta/lib/oeqa/utils/gitarchive.py
+++ b/meta/lib/oeqa/utils/gitarchive.py
@@ -162,7 +162,7 @@ def expand_tag_strings(repo, name_pattern, msg_subj_pattern, msg_body_pattern,
     msg_body = format_str(msg_body_pattern, keyws)
     return tag_name, msg_subj + '\n\n' + msg_body
 
-def gitarchive(data_dir, git_dir, no_create, bare, commit_msg_subject, commit_msg_body, branch_name, no_tag, tagname, tag_msg_subject, tag_msg_body, exclude, notes, push, keywords, log):
+def gitarchive(data_dir, git_dir, no_create, bare, commit_msg_subject, commit_msg_body, branch_name, no_tag, tagname, tag_msg_subject, tag_msg_body, exclude, notes, push, push_remote, keywords, log):
 
     if not os.path.isdir(data_dir):
         raise ArchiveError("Not a directory: {}".format(data_dir))
@@ -179,7 +179,7 @@ def gitarchive(data_dir, git_dir, no_create, bare, commit_msg_subject, commit_ms
         tag_name, tag_msg = expand_tag_strings(data_repo, tagname,
                                                tag_msg_subject,
                                                tag_msg_body,
-                                               push, log, keywords)
+                                               push_remote, log, keywords)
 
     # Commit data
     commit = git_commit_data(data_repo, data_dir, branch_name,
@@ -195,10 +195,10 @@ def gitarchive(data_dir, git_dir, no_create, bare, commit_msg_subject, commit_ms
         cmd = ['push', '--tags']
         # If no remote is given we push with the default settings from
         # gitconfig
-        if push is not True:
+        if push_remote is not None:
             notes_refs = ['refs/notes/' + ref.format(branch_name=branch_name)
                            for ref, _ in notes]
-            cmd.extend([push, branch_name] + notes_refs)
+            cmd.extend([push_remote, branch_name] + notes_refs)
         log.info("Pushing data to remote")
         data_repo.run_cmd(cmd)
 
diff --git a/scripts/lib/resulttool/store.py b/scripts/lib/resulttool/store.py
index b143334e699d..f3caafaff822 100644
--- a/scripts/lib/resulttool/store.py
+++ b/scripts/lib/resulttool/store.py
@@ -82,7 +82,7 @@ def store(args, logger):
                                   "Results of {branch}:{commit}", "branch: {branch}\ncommit: {commit}", "{branch}",
                                   False, "{branch}/{commit_count}-g{commit}/{tag_number}",
                                   'Test run #{tag_number} of {branch}:{commit}', '',
-                                  excludes, [], False, keywords, logger)
+                                  excludes, [], False, None, keywords, logger)
 
             if args.logfile_archive:
                 logdir = args.logfile_archive + "/" + tagname

-- 
2.47.3

[PATCH v2 2/2] oeqa/gitarchive: Push tag before copying log files

[Mathieu Dubois-Briand]

Resulttool creates a git tag in the yocto-testresults git and then
copies log files to a newly created folder on the NFS share, whose name
is controlled by the name of this git tag. As tags are unique, the
folder name is also unique, preventing any clash between different
builds.

Today, the tag is pushed from the calling script, so after the folder is
copied. This can lead to some issues if for any reason the tag is not
pushed. This might also lead to some race condition. Allow to push the
tag before coying data, in order to prevent these issues, and add a
warning if the calling script choose to not push the tag but still copy
the log files on the NFS share.

Fixes [YOCTO #15696]

Signed-off-by: Mathieu Dubois-Briand <mathieu.dubois-briand@bootlin.com>
---
 scripts/lib/resulttool/store.py | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/scripts/lib/resulttool/store.py b/scripts/lib/resulttool/store.py
index f3caafaff822..dc2c259331b2 100644
--- a/scripts/lib/resulttool/store.py
+++ b/scripts/lib/resulttool/store.py
@@ -82,9 +82,14 @@ def store(args, logger):
                                   "Results of {branch}:{commit}", "branch: {branch}\ncommit: {commit}", "{branch}",
                                   False, "{branch}/{commit_count}-g{commit}/{tag_number}",
                                   'Test run #{tag_number} of {branch}:{commit}', '',
-                                  excludes, [], False, None, keywords, logger)
+                                  excludes, [], args.push_tags, None, keywords, logger)
 
             if args.logfile_archive:
+                if not args.push_tags:
+                    # As no tag was pushed, we can't guarantee there "tagname"
+                    # is uniq and so we might have several builds trying to use
+                    # the same "logdir" target.
+                    logger.warning("Archiving log files but the %s tag was not pushed: this may result in target folder conflicts")
                 logdir = args.logfile_archive + "/" + tagname
                 shutil.copytree(tempdir, logdir)
                 os.chmod(logdir, 0o755)
@@ -123,3 +128,5 @@ def register_commands(subparsers):
                               help='only store data for the specified revision')
     parser_build.add_argument('-l', '--logfile-archive', default='',
                               help='directory to separately archive log files along with a copy of the results')
+    parser_build.add_argument('-p', '--push-tags', action='store_true',
+                              help='push created tags to remote git')

-- 
2.47.3