Closed Bug 914563 Opened 11 years ago Closed 10 years ago

(Not fatal) Error running mach: ['build'] AssertionError mach_commands.py", line 376, in build, monitor.finish(record_usage=status==0) after full build is done or ImportError: No module named mach

Categories

(Firefox Build System :: General, defect)

x86_64
Windows 7
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
mozilla34

People

(Reporter: mayhemer, Assigned: mshal)

References

Details

(Whiteboard: [mach])

Attachments

(5 files)

Error running mach:

    ['build']

The error occurred in code that was called by the mach command. This is either
a bug in the called code itself or in the way that mach is calling it.

You should consider filing a bug for this issue.

If filing a bug, please include the full output of mach, including this error
message.

The details of the failure are as follows:

AssertionError

  File "c:\Mozilla\src\gum\python/mozbuild/mozbuild/mach_commands.py", line 376, in build
    monitor.finish(record_usage=status==0)
  File "c:\Mozilla\src\gum\python/mozbuild\mozbuild\controller\building.py", line 370, in finish
    self.resources.stop()
  File "c:\Mozilla\src\gum\testing/mozbase/mozsystemmonitor\mozsystemmonitor\resourcemonitor.py", line 256, in stop
    assert done




It was a clobbered run of './mach build' on projects/gum @ c158106395f2 (= m-c @ 218d4334d29e) with locally fixed build issues (refer [1]).

The build has actually finished (I can run produced firefox).


[1] https://tbpl.mozilla.org/?tree=Gum&rev=c158106395f2
Component: mach → Build Config
Whiteboard: [mach]
I'm also experiencing this today on normal builds on win7 64.
Both these errors are related to the system resource monitoring using multiprocessing to spin up a new process. IMO we should disable resource monitoring on Windows until the issue is resolved.
Summary: (Not fatal) Error running mach: ['build'] AssertionError mach_commands.py", line 376, in build, monitor.finish(record_usage=status==0) after full build is done → (Not fatal) Error running mach: ['build'] AssertionError mach_commands.py", line 376, in build, monitor.finish(record_usage=status==0) after full build is done or ImportError: No module named mach
Assignee: nobody → gps
Comment on attachment 809476 [details] [diff] [review]
Temporarily disable build resource recording on Windows

Looks good to me!
Attachment #809476 - Flags: review?(mshal) → review+
https://hg.mozilla.org/integration/mozilla-inbound/rev/e4983ed4903d

Will keep this bug open to track fixing this.
Whiteboard: [mach] → [mach][leave open]
So this bug with mach + multiprocessing + windows has hit my work with the web-platform-tests runner; I really need to get this fixed to land that so that people can actually run the tests on Windows.

It feels like it should be possible to get to a solution by having a mach.py file which is importable and have the "mach" file simply be a frontend to that. However that doesn't work too well at present because the mach library is also called "mach" so we end up with the wrong module being used in mach_bootstrap.
Attached patch bug-914563.diffSplinter Review
Kind of hacky solution that seems to work for me. Basically splits mach into mach and mach.py and adjusts the path/modules in mach.py so that importing mach in mach_bootstrap.py imports the library, not the wrapper.
Attachment #8422373 - Flags: review?(gps)
Can you provide me with some steps to reproduce the error you're seeing?
Flags: needinfo?(james)
My use case is the new mach command "web-platform-tests" which currently only works on Cedar. Cedar also has my patch here applied, so you will need to revert that if you want to get it to fail.

So the steps are

[1] hg clone https://hg.mozilla.org/projects/cedar
[2] cd cedar
[3] hg backout -r da2bb9fdcd04
[4] ./mach web-platform-tests
Flags: needinfo?(james)
So I was hoping to review this for gps since he isn't back yet, but it looks like it's a bit more complicated than it would first appear. I think having another top-level file for mach is not something we should add lightly, so I was trying to find a way around that. Unfortunately the issue in python isn't going to be released until 3.5 (according to http://bugs.python.org/issue19946), so we can't rely on that.

glandium had the idea of patching the module importer so that when it looks for 'mach', we can open the correct file rather than go looking for 'mach.py'. That seems to work as well, but it is also quite ugly, so I'm not sure what the best way forward here is.

glandium, can you make a call here? I don't think I can make a proper decision here.
Flags: needinfo?(mh+mozilla)
Hacked up mach for module importing, for reference.
Did you try the symlink thing?
Flags: needinfo?(mh+mozilla)
If you commit a symlink on a unix system and check out the repo on a windows system, you'll get a file that just contains a string of the filename. So mach.py would be a file containing the string "mach" instead of any sort of link to the file. It looks like the bug to treat those as junction points is WONTFIX'd: http://bz.selenic.com/show_bug.cgi?id=1825
*sigh*

Let's go with the hack, then.
Comment on attachment 8425538 [details] [diff] [review]
mach-multiprocessing.patch

Review of attachment 8425538 [details] [diff] [review]:
-----------------------------------------------------------------

::: mach
@@ +64,5 @@
> +    return orig_find_module(name, dirs)
> +
> +imp.find_module = my_find_module
> +from multiprocessing.forking import main; main()
> +'''

For more readability, it would be better if the fork_code was indented. You can then remove the indentation with replace.

Or, you can define that code in a function, and use inspect.getsourcelines(function)

Or you can define it in a separate file, add the directory of that file to sys.path and import it.

Anyways, that needs to be made windows-only.
Assignee: gps → nobody
Comment on attachment 8422373 [details] [diff] [review]
bug-914563.diff

Review of attachment 8422373 [details] [diff] [review]:
-----------------------------------------------------------------

I *really* don't like creating a top-level mach.py as it just serves to create confusion and doubt. Let's try one of the other (hacky) solutions.

FWIW, it's only a matter of time before we implement a custom importer to handle .pyc files inside the objdir. So don't feel put off about writing a custom importer.
Attachment #8422373 - Flags: review?(gps) → review-
sure. I don't care what the solution is as long as we have one.
Blocks: 945222
Attached patch mach.patchSplinter Review
I think this works. I've incorporated your earlier review feedback.
Assignee: nobody → mshal
Attachment #8443026 - Flags: review?(mh+mozilla)
Comment on attachment 8443026 [details] [diff] [review]
mach.patch

Review of attachment 8443026 [details] [diff] [review]:
-----------------------------------------------------------------

::: mach
@@ +86,5 @@
> +        def my_get_command_line():
> +            fork_code, lineno = inspect.getsourcelines(fork_interpose)
> +            # Remove the first line (for 'def fork_interpose():') and the three
> +            # levels of indentation.
> +            fork_string = ''.join(x.replace('    ', '', 3) for x in fork_code[1:])

x.replace() can be written as x[12:]

@@ +88,5 @@
> +            # Remove the first line (for 'def fork_interpose():') and the three
> +            # levels of indentation.
> +            fork_string = ''.join(x.replace('    ', '', 3) for x in fork_code[1:])
> +            cmdline = orig_command_line()
> +            cmdline[2] = fork_string

Please add a comment describing roughly what the hack consists of.
Attachment #8443026 - Flags: review?(mh+mozilla) → review+
Whiteboard: [mach][leave open] → [mach]
Do we want to undo the disable-resource-monitoring-in-Windows patch from this bug now?
Flags: needinfo?(gps)
(In reply to Michael Shal [:mshal] from comment #25)
> Do we want to undo the disable-resource-monitoring-in-Windows patch from
> this bug now?

If it works, definitely!
Flags: needinfo?(gps)
For some reason I can't reproduce the original error, even after backing out both patches. So, I'm not totally sure if this now works. I also converted my tabs to spaces, which I snuck in there since vim thought 'mach' was a shell script :)
Attachment #8456321 - Flags: review?(gps)
Attachment #8456321 - Flags: review?(gps) → review+
https://hg.mozilla.org/mozilla-central/rev/ade872c15c02
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla34
QA Whiteboard: [qa-]
Product: Core → Firefox Build System
See Also: → 1720591
You need to log in before you can comment on or make changes to this bug.