Closed Bug 1195748 Opened 9 years ago Closed 8 years ago

|mach build| doesn't create last_log.json (which means |mach show-log| fails)

Categories

(Firefox Build System :: Mach Core, enhancement)

enhancement
Not set
normal

Tracking

(firefox50 fixed)

RESOLVED FIXED
mozilla50
Tracking Status
firefox50 --- fixed

People

(Reporter: aryx, Assigned: glandium)

References

Details

Attachments

(2 files)

Windows 8.1 64 bit, MozillaBuild 2.0, mozilla-central tip Bug 985857 added automatic logging of the build output. Unfortunately, the object directory's .mozbuild folder doesn't contain a last_log.json. The .mozconfig used: mk_add_options MOZ_OBJDIR=@TOPSRCDIR@/../objdir-firefox-central ac_add_options --with-l10n-base=../l10n-central ac_add_options --enable-application=browser
This bug means "mach show-log" fails clumsily with output like the following: { $ ./mach show-log Error running mach: ['show-log'] The error occurred in the implementation of the invoked mach command. This should never occur and is likely a bug in the implementation of that command. 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: IOError: [Errno 2] No such file or directory: u'/scratch/work/builds/mozilla-inbound/obj/.mozbuild/last_log.json' File "/scratch/work/builds/mozilla-inbound/mozilla/python/mozbuild/mozbuild/mach_commands.py", line 631, in show_log log_file = open(path, 'rb') } glandium, is this supposed to work?
Flags: needinfo?(mh+mozilla)
Summary: |mach build| doesn't create last_log.json → |mach build| doesn't create last_log.json (which means |mach show-log| fails)
(Note: in my case, the /scratch/work/builds/mozilla-inbound/obj/.mozbuild/ directory exists, and contains 2 files: build_resources.json and warnings.json. There's no last_log.json, though.) My mozconfig is pretty minimal, like Aryx's: mk_add_options MOZ_OBJDIR=@TOPSRCDIR@/../obj ac_add_options --enable-debug --disable-optimize mk_add_options AUTOCLOBBER=1 ac_add_options --enable-warnings-as-errors ac_add_options --enable-profiling
(In reply to Daniel Holbert [:dholbert] from comment #1) > glandium, is this supposed to work? Yes. And it does work for me, so I can't tell much besides suggesting that you look into the code added in bug 985857 and see why it's not triggered for you.
Flags: needinfo?(mh+mozilla)
Flags: needinfo?(dholbert)
So self.log_manager.terminal is None for me at https://hg.mozilla.org/mozilla-central/rev/632d018e4f26#l1.15 so nothing gets logged. logging.py fails silently if setting up the terminal fails (added in bug 878089): https://dxr.mozilla.org/mozilla-central/rev/388bdc46ba51ee31da8b8abe977e0ca38d117434/python/mach/mach/logging.py#167 The is_a_tty check is implemented at https://github.com/erikrose/blessings/blob/master/blessings/__init__.py#L87 and works for me if I launch the python console in the MozillaBuild console and run that code.
This actually seems to be WORKSFORME now. At least: - In a current mozilla-central build, ./mach show-log worked just fine. - In a current mozilla-inbound build, ./mach show-log complained about missing last_log.json, but then after a clobber + rebuild it was fine. (so I'm guessing I had some broken state before I clobbered maybe.) So I think this may have been fixed elsewhere. aryx, is it worksforyou?
Flags: needinfo?(dholbert) → needinfo?(aryx.bugmail)
No, after a clobber and |mach build| with latest tip, I still get an error for |mach show-log| and the object directory's .mozbuild folder only contains the file warnings.json.
Flags: needinfo?(aryx.bugmail)
So, as I just experienced the problem first hand on Windows, I looked at it, and it roots to the blessings module failing to be imported here: https://dxr.mozilla.org/mozilla-central/rev/ceb63dec9267e9bb62f5e5e1f4c9d32d3ac1fbac/python/mach/mach/logging.py#12 which leads blessings to being None for this test: https://dxr.mozilla.org/mozilla-central/rev/ceb63dec9267e9bb62f5e5e1f4c9d32d3ac1fbac/python/mach/mach/logging.py#168 The reason importing blessings fails is this: >>> import blessings Traceback (most recent call last): File "<stdin>", line 1, in <module> File "z:\mozilla-central\build\mach_bootstrap.py", line 369, in __call__ module = self._original_import(name, globals, locals, fromlist, level) File "z:\mozilla-central\python\blessings\blessings\__init__.py", line 2, in <module> import curses File "z:\mozilla-central\build\mach_bootstrap.py", line 369, in __call__ module = self._original_import(name, globals, locals, fromlist, level) File "c:\mozilla-build\python\Lib\curses\__init__.py", line 15, in <module> from _curses import * File "z:\mozilla-central\build\mach_bootstrap.py", line 369, in __call__ module = self._original_import(name, globals, locals, fromlist, level) ImportError: No module named _curses
... which means the curses module is not properly shipped in mozilla-build's python.
Component: mach → MozillaBuild
Product: Core → mozilla.org
Version: Trunk → other
Mmmm in fact, the module simply doesn't exist on windows. http://stackoverflow.com/questions/35850362/importerror-no-module-named-curses-when-trying-to-import-blessings My first reading of the quoted paragraph in that page, when I read it on the python site, made me think it is supported. But installing plain python showed that it's not. I don't know why we're using blessings just to know whether sys.stdout is a terminal, which os.isatty can tell us...
Component: MozillaBuild → mach
Product: mozilla.org → Core
Version: other → unspecified
Ah, the function is used as os.isatty for the last_log thing, but is used for other reasons in other places.
We were relying on the log manager's terminal to know whether we're running on a terminal to turn on auto-logging, but the log manager's terminal is always None on Windows because blessings imports curses, which doesn't work on Windows. So instead, just use a plain os.isatty. Review commit: https://reviewboard.mozilla.org/r/67352/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/67352/
Attachment #8775013 - Flags: review?(gps)
Comment on attachment 8775013 [details] Bug 1195748 - Fix mach auto-logging on Windows. Review request updated; see interdiff: https://reviewboard.mozilla.org/r/67352/diff/1-2/
Attachment #8775013 - Attachment description: Bug 1195748 - Fix mach auto-logging on Windows → Bug 1195748 - Fix mach auto-logging on Windows.
This never was a problem since auto-logging was broken on Windows, but having mach clobber auto-log is a problem on Windows since it tries to remove the log file while itself has it open, which fails. Review commit: https://reviewboard.mozilla.org/r/67388/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/67388/
Attachment #8775043 - Flags: review?(gps)
https://reviewboard.mozilla.org/r/67352/#review64410 ::: python/mozbuild/mozbuild/mach_commands.py:677 (Diff revision 2) > path = self._get_state_filename('last_log.json') > log_file = open(path, 'rb') > > - if self.log_manager.terminal: > + if os.isatty(sys.stdout.fileno()): > env = dict(os.environ) > if 'LESS' not in env: Should this be `b'LESS'` too?
(In reply to :Ms2ger (⌚ UTC+1/+2) from comment #14) > https://reviewboard.mozilla.org/r/67352/#review64410 > > ::: python/mozbuild/mozbuild/mach_commands.py:677 > (Diff revision 2) > > path = self._get_state_filename('last_log.json') > > log_file = open(path, 'rb') > > > > - if self.log_manager.terminal: > > + if os.isatty(sys.stdout.fileno()): > > env = dict(os.environ) > > if 'LESS' not in env: > > Should this be `b'LESS'` too? python doesn't care whether strings given to __contains__() are unicode or str, even on Windows, and matches properly. What it does care about, though, is that values used when setting the environment are str.
Comment on attachment 8775013 [details] Bug 1195748 - Fix mach auto-logging on Windows. https://reviewboard.mozilla.org/r/67352/#review64546 Please address the other LESS variable.
Attachment #8775013 - Flags: review?(gps) → review+
Attachment #8775043 - Flags: review?(gps) → review+
(In reply to Gregory Szorc [:gps] from comment #16) > Comment on attachment 8775013 [details] > Bug 1195748 - Fix mach auto-logging on Windows. Note: I was hitting this on Linux, so if the fix here is windows-specific, there be another way of triggering this lurking around. I can file a new bug if I notice this happening again (on Linux).
(In reply to Gregory Szorc [:gps] from comment #16) > Comment on attachment 8775013 [details] > Bug 1195748 - Fix mach auto-logging on Windows. > > https://reviewboard.mozilla.org/r/67352/#review64546 > > Please address the other LESS variable. The one from comment 14? There's arguably nothing to fix there, since it works, as opposed to the other cases, where the script raises an exception if we don't use a byte literal.
Pushed by mh@glandium.org: https://hg.mozilla.org/integration/autoland/rev/ace86bc6c5b1 Fix mach auto-logging on Windows. r=gps https://hg.mozilla.org/integration/autoland/rev/a182c1960415 Make mach clobber never auto-log. r=gps
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla50
Assignee: nobody → mh+mozilla
Depends on: 1290201
(In reply to Daniel Holbert [:dholbert] from comment #18) > (In reply to Gregory Szorc [:gps] from comment #16) > > Comment on attachment 8775013 [details] > > Bug 1195748 - Fix mach auto-logging on Windows. > > Note: I was hitting this on Linux, so if the fix here is windows-specific, > there be another way of triggering this lurking around. I can file a new > bug if I notice this happening again (on Linux). There's a scenario where I now know this can happen: autoclobber. (and it now fails on windows because of fixing this bug ; on linux and mac, it just makes the log disappear)
Depends on: 1298210
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: