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

RESOLVED FIXED in Firefox 50

Status

enhancement
RESOLVED FIXED
4 years ago
Last year

People

(Reporter: aryx, Assigned: glandium)

Tracking

unspecified
mozilla50
Dependency tree / graph

Firefox Tracking Flags

(firefox50 fixed)

Details

Attachments

(2 attachments)

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+
Comment on attachment 8775043 [details]
Bug 1195748 - Make mach clobber never auto-log.

https://reviewboard.mozilla.org/r/67388/#review64554
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
https://hg.mozilla.org/mozilla-central/rev/ace86bc6c5b1
https://hg.mozilla.org/mozilla-central/rev/a182c1960415
Status: NEW → RESOLVED
Closed: 3 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.