1673700 - mach test fails on `security/moz.build` with SyntaxError: EOL while scanning string literal

Status: RESOLVED FIXED

(Firefox Build System :: General, defect)

Description

[Tim Giles [:tgiles]]

On latest mozilla-central, after running a non-artifact build, mach test fails with:

File "<string>", line 11
    return (f, path, (", r, imp.PY_SOURCE))
                                          ^
SyntaxError: EOL while scanning string literal

Pastebin to full error log: https://paste.mozilla.org/7JPi2vm9

Does not seem to be reproducible with artifact builds for additional context

Comment 1

[Ricky Stewart]

I can't repro at HEAD.

That error message seems to be a reference to this line of code, but the line of code looks "wrong" (dirty). Did you update python/mozbuild/mozbuild/util.py locally?

Comment 2

[Tim Giles [:tgiles]]

I can't repro at HEAD

I'm able to reproduce at 6df4ac02675f, but who knows it could be Windows doing its thing yet again sigh

That error message...

I checked 6df4ac02675f's python/mozbuild/mozbuild/util.py and that util.py file is the same as the revision you posted in Comment #1. So I don't know how I've gotten into this state. I've recently (i.e. after pulling 6df4ac02675f) clobbered, bootstrapped, built...and still get the same error log.

Comment 3

[Ricky Stewart]

I can repro on Windows. Let me have a closer look.

Comment 4

[Ricky Stewart]

Okay. I think this is a fallout of bug 1654103. Applying this patch fixes it for me.

My next question is: why is this happening? I can only assume there's some code in mozilla-central that breaks if you use double quotes here...

diff --git a/python/mozbuild/mozbuild/util.py b/python/mozbuild/mozbuild/util.py
--- a/python/mozbuild/mozbuild/util.py
+++ b/python/mozbuild/mozbuild/util.py
@@ -1451,26 +1451,26 @@ def patch_main():
             import sys

             orig_find_module = imp.find_module

             def my_find_module(name, dirs):
                 if name == main_module_name:
                     path = os.path.join(dirs[0], main_file_name)
                     f = open(path)
-                    return (f, path, ("", "r", imp.PY_SOURCE))
+                    return (f, path, ('', 'r', imp.PY_SOURCE))
                 return orig_find_module(name, dirs)

             # Don't allow writing bytecode file for the main module.
             orig_load_module = imp.load_module

             def my_load_module(name, file, path, description):
                 # multiprocess.forking invokes imp.load_module manually and
                 # hard-codes the name __parents_main__ as the module name.
-                if name == "__parents_main__":
+                if name == '__parents_main__':
                     old_bytecode = sys.dont_write_bytecode
                     sys.dont_write_bytecode = True
                     try:
                         return orig_load_module(name, file, path, description)
                     finally:
                         sys.dont_write_bytecode = old_bytecode

                 return orig_load_module(name, file, path, description)

Comment 5

[Ricky Stewart]

Attachment: Bug 1673700 - Don't use double quotes around strings in definition of `fork_interpose`

The definition of patch_main() has behavior that kicks in only on Windows and for Python 2. Unfortunately, not all of our mach commands have been migrated to Python 2, so this still matters.

Bug 1654103 replaced the single-quoted strings in this function with double-quoted strings. This should be fine, except that we call into multiprocessing.forking.main() with some monkey-patching that is meant to fix a Windows-specific bug (see bug 1316140). We don't do any clever serialization or anything here and we end up just passing that source to multiprocessing.forking.main() which aggregates that source code dumbly, wrapping everything in double-quotes again and passing it to _subprocess.CreateProcess(), which ends up failing if the source contains strings formatted with double quotes.

We could revert the changes to the relevant lines and exempt python/mozbuild/mozbuild/util.py from linting via black, but util.py is a large file with a lot of stuff in it, and exempting the entire file from linting for this one legacy use case seems wrong. Instead, we do a code golf-y workaround to avoid literally representing any strings in this block. All of this code will be deleted when we no longer support Python 2, so this is a temporary hack.

Comment 7

[Mathew Hodson]

I think you meant to mark this as regressed by bug 1654103. The Regressions/Regressed By fields are swapped.

Comment 8

[Ricky Stewart]

Thank you.

Comment 9

[Tim Giles [:tgiles]]

Patch in Comment #4 fixes the issue for me. Using as a workaround until the patch lands.

Comment 10

[BugBot [:suhaib / :marco/ :calixte]]

Set release status flags based on info from the regressing bug 1654103

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/48b549b021e1
Don't use double quotes around strings in definition of `fork_interpose` r=firefox-build-system-reviewers,glandium

Comment 12

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/mozilla-central/rev/48b549b021e1