Closed Bug 1316140 Opened 8 years ago Closed 8 years ago

Move our windows multiprocessing hacks to a central location so we can use them for config.status as well

Categories

(Firefox Build System :: General, defect)

defect
Not set
normal

Tracking

(firefox53 fixed)

RESOLVED FIXED
mozilla53
Tracking Status
firefox53 --- fixed

People

(Reporter: chmanchester, Assigned: chmanchester)

References

Details

Attachments

(1 file)

I found we hit bug 914563 as seen as we try to use multiprocessing as a part of build-backend, because we can end up with "config.status" as the file for our main module. This bug is about moving the monkey patching we currently do for "mach" to a central location so we can use it for "config.status" as well.
I'm not a fan of the multiprocessing module because it is brittle [on Windows]. If the opportunity is there, converting to concurrent.futures.ProcessPoolExecutor is one way to solve the problem.
I should have said I'm already using ProcessPoolExecutor. I mention multiprocessing because concurrent.futures is implemented with multiprocessing, and that's where the error is.
(In reply to Chris Manchester (:chmanchester) from comment #2) > I should have said I'm already using ProcessPoolExecutor. I mention > multiprocessing because concurrent.futures is implemented with > multiprocessing, and that's where the error is. TIL. FML. Unless you absolutely need the "pool" bits, ThreadPoolExecutor + subprocess.call() might even be preferable. I've just had so many bad experiences with multiprocessing over the years that I try to avoid it.
Comment on attachment 8809095 [details] Bug 1316140 - Allow use of multiprocessing from config.status on windows. https://reviewboard.mozilla.org/r/91746/#review92366 Looks good! I think patch_main() should probably have a different home before landing, though. ::: configure.py:82 (Diff revision 1) > "'non_global_defines', 'substs', 'mozconfig']") > > if config.get('MOZ_BUILD_APP') != 'js' or config.get('JS_STANDALONE'): > fh.write(textwrap.dedent(''' > if __name__ == '__main__': > + from mozbuild.action.patch_main import patch_main I think mozbuild.action is probably the wrong place for this. That's for python utilities called during build time, like buildlist or the preprocessor: https://dxr.mozilla.org/mozilla-central/rev/d38d06f85ef59c5dbb5d4a1a8d895957a78714de/config/rules.mk#1425 https://dxr.mozilla.org/mozilla-central/rev/d38d06f85ef59c5dbb5d4a1a8d895957a78714de/config/makefiles/xpidl/Makefile.in#56 I don't think we'd want to $(call py_action,patch_main) anywhere, right? Maybe mozbuild/util.py is a better fit.
Attachment #8809095 - Flags: review?(mshal) → review+
Pushed by cmanchester@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/1f85e7bfe97d Allow use of multiprocessing from config.status on windows. r=mshal
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla53
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: