Closed Bug 1316140 Opened 4 years ago Closed 4 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
https://hg.mozilla.org/mozilla-central/rev/1f85e7bfe97d
Status: NEW → RESOLVED
Closed: 4 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.