Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

actions: Add muliprocessing start-method to matrix #1255

Conversation

zmedico
Copy link
Member

@zmedico zmedico commented Feb 6, 2024

Add multiprocessing start-method to matrix so that we are prepared for when python changes to default to spawn. Exclude all python versions except 3.12-dev for now.

This adds a conditional ci step that patches the sources just for the spawn start-method. It patches all bin/* scripts with python shebangs, and also patches the test command in meson.build.

Bug: https://bugs.gentoo.org/914876

@thesamesam this does something like what you suggested in #1249 (comment). This implementation is kind of ugly, but maybe good enough for the first iteration. I've also added this to #1251.

Add multiprocessing start-method to matrix so that we are prepared
for when python changes to default to spawn. Exclude all python
versions except 3.12-dev for now.

This adds a conditional ci step that patches the sources just for
the spawn start-method. It patches all bin/* scripts with python
shebangs, and also patches the test command in meson.build.

Bug: https://bugs.gentoo.org/914876
Signed-off-by: Zac Medico <zmedico@gentoo.org>
Copy link
Member

@thesamesam thesamesam left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's gnarly but disappointingly, there's no way to control it from the command line (was really hoping for something like the new var for CPU cores). Thank you!

@gentoo-bot gentoo-bot merged commit b419577 into gentoo:master Feb 7, 2024
12 checks passed
@zmedico zmedico deleted the bug_914876_github_actions_matrix_multiprocessing_start_method_spawn branch February 7, 2024 00:34
@zmedico
Copy link
Member Author

zmedico commented Feb 12, 2024

I temporarily added a multiprocessing.get_start_method() == "fork" to one of the tests, and inadvertently found out that somehow multiprocessing.set_start_method("spawn", force=True) in the pytest command is practically ignored when the pytest-xdist plugin is used. We still get a lot of useful coverage anyway from the patched bin/* python scripts, but it will be nice if we can fix the pytest-xdist plugin somehow. Seems related to execnet usage, which has support form threading models but seemingly not multiprocessing models.

Maybe we just need to patch the portage.tests.TestCase setUp method to call multiprocessing.set_start_method("spawn", force=True). Yeah, this seems to work:

--- a/lib/portage/tests/__init__.py
+++ b/lib/portage/tests/__init__.py
@@ -1,3 +1,3 @@
 # tests/__init__.py -- Portage Unit Test functionality
-# Copyright 2006-2023 Gentoo Authors
+# Copyright 2006-2024 Gentoo Authors
 # Distributed under the terms of the GNU General Public License v2
@@ -5,2 +5,3 @@
 import argparse
+import multiprocessing
 import sys
@@ -81,2 +82,6 @@ class TestCase(unittest.TestCase):
 
+    def setUp(self):
+        if os.environ.get("PORTAGE_MULTIPROCESSING_START_METHOD") == "spawn":
+            multiprocessing.set_start_method("spawn", force=True)
+
     def assertRaisesMsg(self, msg, excClass, callableObj, *args, **kwargs):

Sent #1265.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants