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)
Tracking
(Not tracked)
RESOLVED
FIXED
mozilla34
People
(Reporter: mayhemer, Assigned: mshal)
References
Details
(Whiteboard: [mach])
Attachments
(5 files)
1.26 KB,
patch
|
mshal
:
review+
|
Details | Diff | Splinter Review |
5.21 KB,
patch
|
gps
:
review-
|
Details | Diff | Splinter Review |
1.03 KB,
patch
|
Details | Diff | Splinter Review | |
1.70 KB,
patch
|
glandium
:
review+
|
Details | Diff | Splinter Review |
4.09 KB,
patch
|
gps
:
review+
|
Details | Diff | Splinter Review |
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
Updated•11 years ago
|
Component: mach → Build Config
Whiteboard: [mach]
Comment 2•11 years ago
|
||
I'm also experiencing this today on normal builds on win7 64.
Comment 4•11 years ago
|
||
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
Comment 5•11 years ago
|
||
Attachment #809476 -
Flags: review?(mshal)
Updated•11 years ago
|
Assignee: nobody → gps
Assignee | ||
Comment 6•11 years ago
|
||
Comment on attachment 809476 [details] [diff] [review] Temporarily disable build resource recording on Windows Looks good to me!
Attachment #809476 -
Flags: review?(mshal) → review+
Comment 7•11 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/e4983ed4903d Will keep this bug open to track fixing this.
Whiteboard: [mach] → [mach][leave open]
Comment 8•11 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/e4983ed4903d
Comment 9•10 years ago
|
||
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.
Comment 10•10 years ago
|
||
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)
Assignee | ||
Comment 11•10 years ago
|
||
Can you provide me with some steps to reproduce the error you're seeing?
Flags: needinfo?(james)
Comment 12•10 years ago
|
||
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)
Assignee | ||
Comment 13•10 years ago
|
||
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)
Assignee | ||
Comment 14•10 years ago
|
||
Hacked up mach for module importing, for reference.
Assignee | ||
Comment 16•10 years ago
|
||
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
Comment 17•10 years ago
|
||
*sigh* Let's go with the hack, then.
Comment 18•10 years ago
|
||
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.
Updated•10 years ago
|
Assignee: gps → nobody
Comment 19•10 years ago
|
||
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-
Comment 20•10 years ago
|
||
sure. I don't care what the solution is as long as we have one.
Blocks: 945222
Assignee | ||
Comment 21•10 years ago
|
||
I think this works. I've incorporated your earlier review feedback.
Assignee: nobody → mshal
Attachment #8443026 -
Flags: review?(mh+mozilla)
Comment 22•10 years ago
|
||
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+
Assignee | ||
Comment 23•10 years ago
|
||
try: https://tbpl.mozilla.org/?tree=Try&rev=d9699e0a1672 inbound: https://hg.mozilla.org/integration/mozilla-inbound/rev/a6b5b5388155
Assignee | ||
Updated•10 years ago
|
Whiteboard: [mach][leave open] → [mach]
Assignee | ||
Comment 25•10 years ago
|
||
Do we want to undo the disable-resource-monitoring-in-Windows patch from this bug now?
Flags: needinfo?(gps)
Comment 26•10 years ago
|
||
(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)
Assignee | ||
Comment 27•10 years ago
|
||
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)
Updated•10 years ago
|
Attachment #8456321 -
Flags: review?(gps) → review+
Assignee | ||
Comment 28•10 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/ade872c15c02
https://hg.mozilla.org/mozilla-central/rev/ade872c15c02
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla34
Updated•10 years ago
|
QA Whiteboard: [qa-]
Updated•6 years ago
|
Product: Core → Firefox Build System
You need to log in
before you can comment on or make changes to this bug.
Description
•