Message ID | 20240705144638.441976-3-alexis.lothore@bootlin.com |
---|---|
State | Accepted, archived |
Commit | 4aeb10aa38efc6768928fbb74985e36e972b8e46 |
Headers | show |
Series | oeqa/utils/postactions: fix disk storage consumption | expand |
Thank you for your submission. Patchtest identified one or more issues with the patch. Please see the log below for more information: --- Testing patch /home/patchtest/share/mboxes/2-2-oeqa-utils-postactions-transfer-whole-archive-over-ssh-instead-of-doing-individual-copies.patch FAIL: test bugzilla entry format: Bugzilla issue ID is not correctly formatted - specify it with format: "[YOCTO #<bugzilla ID>]" (test_mbox.TestMbox.test_bugzilla_entry_format) PASS: pretest pylint (test_python_pylint.PyLint.pretest_pylint) PASS: test Signed-off-by presence (test_mbox.TestMbox.test_signed_off_by_presence) PASS: test author valid (test_mbox.TestMbox.test_author_valid) PASS: test commit message presence (test_mbox.TestMbox.test_commit_message_presence) PASS: test max line length (test_metadata.TestMetadata.test_max_line_length) PASS: test mbox format (test_mbox.TestMbox.test_mbox_format) PASS: test non-AUH upgrade (test_mbox.TestMbox.test_non_auh_upgrade) PASS: test pylint (test_python_pylint.PyLint.test_pylint) PASS: test shortlog format (test_mbox.TestMbox.test_shortlog_format) PASS: test shortlog length (test_mbox.TestMbox.test_shortlog_length) SKIP: pretest src uri left files: No modified recipes, skipping pretest (test_metadata.TestMetadata.pretest_src_uri_left_files) SKIP: test CVE check ignore: No modified recipes or older target branch, skipping test (test_metadata.TestMetadata.test_cve_check_ignore) SKIP: test CVE tag format: No new CVE patches introduced (test_patch.TestPatch.test_cve_tag_format) SKIP: test Signed-off-by presence: No new CVE patches introduced (test_patch.TestPatch.test_signed_off_by_presence) SKIP: test Upstream-Status presence: No new CVE patches introduced (test_patch.TestPatch.test_upstream_status_presence_format) SKIP: test lic files chksum modified not mentioned: No modified recipes, skipping test (test_metadata.TestMetadata.test_lic_files_chksum_modified_not_mentioned) SKIP: test lic files chksum presence: No added recipes, skipping test (test_metadata.TestMetadata.test_lic_files_chksum_presence) SKIP: test license presence: No added recipes, skipping test (test_metadata.TestMetadata.test_license_presence) SKIP: test series merge on head: Merge test is disabled for now (test_mbox.TestMbox.test_series_merge_on_head) SKIP: test src uri left files: No modified recipes, skipping pretest (test_metadata.TestMetadata.test_src_uri_left_files) SKIP: test summary presence: No added recipes, skipping test (test_metadata.TestMetadata.test_summary_presence) SKIP: test target mailing list: Series merged, no reason to check other mailing lists (test_mbox.TestMbox.test_target_mailing_list) --- Please address the issues identified and submit a new revision of the patch, or alternatively, reply to this email with an explanation of why the patch should be accepted. If you believe these results are due to an error in patchtest, please submit a bug at https://bugzilla.yoctoproject.org/ (use the 'Patchtest' category under 'Yocto Project Subprojects'). For more information on specific failures, see: https://wiki.yoctoproject.org/wiki/Patchtest. Thank you!
On Fri, 2024-07-05 at 16:46 +0200, Alexis Lothoré via lists.openembedded.org wrote: > From: Alexis Lothoré <alexis.lothore@bootlin.com> > > Fixes [YOCTO 15536] > > The postactions retrieval actions currently rely on scp executed > individually on any file or directory expanded from > TESTIMAGE_FAILED_QA_ARTIFACTS. Unfortunately, symlinks are not preserved > with this mechanism, which lead to big storage space consumption. Things > may go even worse if those symlinks create some circular chains. This > mechanism then needs to be updated to preserve symlinks instead of > following them during copy. There are multiple ways to do it: > - create a local archive on the target and execute scp on this file > - use rsync instead of scp for all files > - create an archive and pipe it to ssh instead of storing it onto the > target > > The first solution may create pressure on targets storage space, while the > second assumes that rsync is installed on the target, which may not be > true. So the third one is a compromise: tar is very likely present, at > least through busybox, and no disk space is used on the target. > > Replace the current per-file scp call by a single call to tar run on the > target. Retrieve the generated compressed archive directly from SSH output, > and feed it to another tar process but on host, to uncompress and extract > it at the same place as before. > > Signed-off-by: Alexis Lothoré <alexis.lothore@bootlin.com> > --- > meta/lib/oeqa/utils/postactions.py | 19 +++++++++---------- > 1 file changed, 9 insertions(+), 10 deletions(-) > > diff --git a/meta/lib/oeqa/utils/postactions.py b/meta/lib/oeqa/utils/postactions.py > index ecdddd2d40e3..f7ce25f2af8e 100644 > --- a/meta/lib/oeqa/utils/postactions.py > +++ b/meta/lib/oeqa/utils/postactions.py > @@ -62,17 +62,16 @@ def get_artifacts_list(target, raw_list): > return result > > def retrieve_test_artifacts(target, artifacts_list, target_dir): > + import io, subprocess > local_artifacts_dir = os.path.join(target_dir, "artifacts") > - for artifact_path in artifacts_list: > - if not os.path.isabs(artifact_path): > - bb.warn(f"{artifact_path} is not an absolute path") > - continue > - try: > - dest_dir = os.path.join(local_artifacts_dir, os.path.dirname(artifact_path[1:])) > - os.makedirs(dest_dir, exist_ok=True) > - target.copyFrom(artifact_path, dest_dir) > - except Exception as e: > - bb.warn(f"Can not retrieve {artifact_path} from test target: {e}") > + try: > + cmd = f"tar zcf - {" ".join(artifacts_list)}" Thanks for the patch. The syntax above causes failures: https://valkyrie.yoctoproject.org/#/builders/95/builds/59/steps/14/logs/stdio I've put a fix on master-next which I can squash in but it does make me wonder how it was tested. Cheers, Richard > + (status, output) = target.run(cmd, raw = True) > + if status != 0 or not output: > + raise Exception("Error while fetching compressed artifacts") > + p = subprocess.run(["tar", "zxf", "-", "-C", local_artifacts_dir], input=output) > + except Exception as e: > + bb.warn(f"Can not retrieve {artifact_path} from test target: {e}") > > def list_and_fetch_failed_tests_artifacts(d, tc): > artifacts_list = get_artifacts_list(tc.target, d.getVar("TESTIMAGE_FAILED_QA_ARTIFACTS")) > > -=-=-=-=-=-=-=-=-=-=-=- > Links: You receive all messages sent to this group. > View/Reply Online (#201610): https://lists.openembedded.org/g/openembedded-core/message/201610 > Mute This Topic: https://lists.openembedded.org/mt/107054834/1686473 > Group Owner: openembedded-core+owner@lists.openembedded.org > Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [richard.purdie@linuxfoundation.org] > -=-=-=-=-=-=-=-=-=-=-=- >
Hello Richard, On 7/7/24 10:10, Richard Purdie wrote: > On Fri, 2024-07-05 at 16:46 +0200, Alexis Lothoré via lists.openembedded.org wrote: >> From: Alexis Lothoré <alexis.lothore@bootlin.com> >> + try: >> + cmd = f"tar zcf - {" ".join(artifacts_list)}" > > Thanks for the patch. The syntax above causes failures: > > https://valkyrie.yoctoproject.org/#/builders/95/builds/59/steps/14/logs/stdio > > I've put a fix on master-next which I can squash in but it does make me > wonder how it was tested. Sorry about that. This error made me doubt about my testing setup, so I ran new tests, and I confirm that this line is being executed and returns a correct tar command on my machine, which confused me even more because it indeed looks broken. My best guess about those differences is a mismatch on python versions between the autobuilder and my machine. I am running python 3.12, and it appears that it has brought many improvements to python f-strings, especially "quote reuse", which makes python not raise an exception anymore when the expression inside an f-string reuses the same kind of quotes as for the parent f-string (see [1]). I ran a quick test with Python 3.11 instead of 3.12, and I now I see the exception. I guess it would be better for me to keep working with the same python version as in the autobuilder. I see mentions of python 3.6 in README-WALKTHROUGHS.md from the autobuilder2 repository, but I am not sure how up-to-date it is. Could you please confirm whether this is the correct version or not? Thanks, Alexis [1] https://docs.python.org/3/whatsnew/3.12.html#pep-701-syntactic-formalization-of-f-strings
On Mon, 2024-07-08 at 23:10 +0200, Alexis Lothoré wrote: > Hello Richard, > > On 7/7/24 10:10, Richard Purdie wrote: > > On Fri, 2024-07-05 at 16:46 +0200, Alexis Lothoré via lists.openembedded.org wrote: > > > From: Alexis Lothoré <alexis.lothore@bootlin.com> > > > + try: > > > + cmd = f"tar zcf - {" ".join(artifacts_list)}" > > > > Thanks for the patch. The syntax above causes failures: > > > > https://valkyrie.yoctoproject.org/#/builders/95/builds/59/steps/14/logs/stdio > > > > I've put a fix on master-next which I can squash in but it does make me > > wonder how it was tested. > > Sorry about that. This error made me doubt about my testing setup, so I ran new > tests, and I confirm that this line is being executed and returns a correct tar > command on my machine, which confused me even more because it indeed looks broken. > > My best guess about those differences is a mismatch on python versions between > the autobuilder and my machine. I am running python 3.12, and it appears that it > has brought many improvements to python f-strings, especially "quote reuse", > which makes python not raise an exception anymore when the expression inside an > f-string reuses the same kind of quotes as for the parent f-string (see [1]). I > ran a quick test with Python 3.11 instead of 3.12, and I now I see the exception. That explains things. I'm very surprised any version of python accepts that formatting to be honest! > I guess it would be better for me to keep working with the same python version > as in the autobuilder. I see mentions of python 3.6 in README-WALKTHROUGHS.md > from the autobuilder2 repository, but I am not sure how up-to-date it is. Could > you please confirm whether this is the correct version or not? The docs are out of date and the definitive version is the one checked in bitbake: https://git.openembedded.org/bitbake/tree/lib/bb/__init__.py#n15 "Sorry, python 3.8.0 or later is required for this version of bitbake" so 3.8+ is our current requirement. Cheers, Richard
On 7/8/24 23:37, Richard Purdie wrote: > On Mon, 2024-07-08 at 23:10 +0200, Alexis Lothoré wrote: >> Hello Richard, [...] >> My best guess about those differences is a mismatch on python versions between >> the autobuilder and my machine. I am running python 3.12, and it appears that it >> has brought many improvements to python f-strings, especially "quote reuse", >> which makes python not raise an exception anymore when the expression inside an >> f-string reuses the same kind of quotes as for the parent f-string (see [1]). I >> ran a quick test with Python 3.11 instead of 3.12, and I now I see the exception. > > That explains things. I'm very surprised any version of python accepts > that formatting to be honest! > >> I guess it would be better for me to keep working with the same python version >> as in the autobuilder. I see mentions of python 3.6 in README-WALKTHROUGHS.md >> from the autobuilder2 repository, but I am not sure how up-to-date it is. Could >> you please confirm whether this is the correct version or not? > > The docs are out of date and the definitive version is the one checked in bitbake: > > https://git.openembedded.org/bitbake/tree/lib/bb/__init__.py#n15 > > "Sorry, python 3.8.0 or later is required for this version of bitbake" > > so 3.8+ is our current requirement. ACK, I'll then align with this version for any future development. Thanks, Alexis
diff --git a/meta/lib/oeqa/utils/postactions.py b/meta/lib/oeqa/utils/postactions.py index ecdddd2d40e3..f7ce25f2af8e 100644 --- a/meta/lib/oeqa/utils/postactions.py +++ b/meta/lib/oeqa/utils/postactions.py @@ -62,17 +62,16 @@ def get_artifacts_list(target, raw_list): return result def retrieve_test_artifacts(target, artifacts_list, target_dir): + import io, subprocess local_artifacts_dir = os.path.join(target_dir, "artifacts") - for artifact_path in artifacts_list: - if not os.path.isabs(artifact_path): - bb.warn(f"{artifact_path} is not an absolute path") - continue - try: - dest_dir = os.path.join(local_artifacts_dir, os.path.dirname(artifact_path[1:])) - os.makedirs(dest_dir, exist_ok=True) - target.copyFrom(artifact_path, dest_dir) - except Exception as e: - bb.warn(f"Can not retrieve {artifact_path} from test target: {e}") + try: + cmd = f"tar zcf - {" ".join(artifacts_list)}" + (status, output) = target.run(cmd, raw = True) + if status != 0 or not output: + raise Exception("Error while fetching compressed artifacts") + p = subprocess.run(["tar", "zxf", "-", "-C", local_artifacts_dir], input=output) + except Exception as e: + bb.warn(f"Can not retrieve {artifact_path} from test target: {e}") def list_and_fetch_failed_tests_artifacts(d, tc): artifacts_list = get_artifacts_list(tc.target, d.getVar("TESTIMAGE_FAILED_QA_ARTIFACTS"))