Newly-installed FC18 machine. I expect that there are all kinds of packages missing in my build environment, but I wanted to start a build and see what failed. I didn't expect mach to be the failure point, though ;-) [bsmedberg@stravinsky git-mozilla]$ MOZCONFIG=~/mozconfig-debug mach configure Error running mach: ['configure'] The error occurred in mach itself. This is likely a bug in mach itself or a fundamental problem with a loaded module. Please consider filing a bug against mach by going to the URL: https://bugzilla.mozilla.org/enter_bug.cgi?product=Core&component=mach 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 "/home/bsmedberg/git-mozilla/python/mach/mach/main.py", line 231, in run return self._run(argv) File "/home/bsmedberg/git-mozilla/python/mach/mach/main.py", line 310, in _run instance = cls(context) File "/home/bsmedberg/git-mozilla/python/mozbuild/mozbuild/base.py", line 454, in __init__ self.mozconfig File "/home/bsmedberg/git-mozilla/python/mozbuild/mozbuild/base.py", line 190, in mozconfig self._mozconfig = loader.read_mozconfig() File "/home/bsmedberg/git-mozilla/python/mozbuild/mozbuild/mozconfig.py", line 198, in read_mozconfig parsed = self._parse_loader_output(output) File "/home/bsmedberg/git-mozilla/python/mozbuild/mozbuild/mozconfig.py", line 306, in _parse_loader_output assert current_type is not None Contents of mozconfig: ac_add_options --enable-debug mk_add_options MOZ_OBJDIR=$(topsrcdir)/obj-debug

Note: I got this to work by replace $(topsrcdir) with @TOPSRCDIR@

It's unfortunate we see an AssertionError here. We should throw a MozconfigValidationError or similar, catch it appropriately in mach land, and display a helpful message to the user.

Is this still an issue? I know we've touched some of this code since this was filed...

Yes, this is still an issue.

I'm able to repro this with a test. Thank you for the detailed test case!

The parser is getting confused on the following line of output: /home/gps/src/firefox/python/mozbuild/mozbuild/mozconfig_loader: 1: /tmp/tmpusr0bK: topsrcdir: not found (That /tmp path is the mozconfig used in the test.) Improper variable references should cause an assertion error.

What's happening here is the mozconfig was invalid because |$(topsrcdir)| is a command reference to a command that (obviously) doesn't exist. This is a run-time execution error, so we want invalid command references to trigger early program termination. Unfortunately, I'm not sure if we can catch these errors in regular shell. We are already executing with set -e. However, this type of expansion is a "simple command" in POSIX shell and a "command substitution" in BASH. Both are special and not impacted by set -e. I'm not a shell wizard, but I don't believe it's possible to trap "simple command" failures in POSIX shell. Or, at least I couldn't find anything to do this. Bash, however, does seem to support it. We could use -E/-o errtrace combined with a trap on the special ERR signal to catch these types of failures. However, switching to bash may have unintended consequences. I'm not sure if that's something we can do easily. glandium: You know more shell than me. Any ideas?

I think the simpler thing to do is to just not redirect stderr in stdout for mozconfig_loader, and consider that if there's something showing up on stderr, we should error out. As well as if the exit code is not 0, which, when using $(topsrcdir), is the case. (BTW, local testing indicates the error *is* caught by set -e, we're just ignoring it)

(In reply to Mike Hommey [:glandium] from comment #8) > (BTW, local testing indicates the error *is* caught by set -e, we're just > ignoring it) Where are we ignoring it? I've executed the mozconfig_loader script manually through my shell and $? is 0. I believe I also tried capturing the result of |. $2| from inside mozconfig_loader and it too reported 0 unless errtrace was set.

(In reply to Gregory Szorc [:gps] from comment #9) > (In reply to Mike Hommey [:glandium] from comment #8) > > (BTW, local testing indicates the error *is* caught by set -e, we're just > > ignoring it) > > Where are we ignoring it? Where are we checking the return code of mozconfig_loader? > I've executed the mozconfig_loader script manually > through my shell and $? is 0. I believe I also tried capturing the result of > |. $2| from inside mozconfig_loader and it too reported 0 unless errtrace > was set. Both bash and dash error out for me on running mozconfig_loader manually with a mozconfig containing $(topsrcdir). Did you single quote it?

We check the return code in mozbuild.mozconfig:MozconfigLoader.read_mozconfig(). We're calling subprocess.check_output, which should raise on non-0 exit codes. The code looks good to me. And I'm pretty sure we have test coverage of it. The problem (at least for me) is that the shell is not exiting with a non-0 exit code! On OS X (and presumably the Linux machine I initially tested this on): gps@gps-mbp:~/src/firefox$ pwd /Users/gps/src/firefox gps@gps-mbp:~/src/firefox$ cat test-mozconfig mk_add_options MOZ_OBJDIR=$(topsrcdir)/obj-debug gps@gps-mbp:~/src/firefox$ sh python/mozbuild/mozbuild/mozconfig_loader ~/src/firefox test-mozconfig ------BEGIN_BEFORE_SOURCE Apple_PubSub_Socket_Render=/tmp/launch-9OQjDD/Render BASH=/bin/sh BASH_ARGC=([0]="2") BASH_ARGV=([0]="test-mozconfig" [1]="/Users/gps/src/firefox") BASH_LINENO=([0]="0") BASH_SOURCE=([0]="python/mozbuild/mozbuild/mozconfig_loader") BASH_VERSINFO=([0]="3" [1]="2" [2]="51" [3]="1" [4]="release" [5]="x86_64-apple-darwin13") BASH_VERSION='3.2.51(1)-release' CLICOLOR=1 COLORFGBG='7;0' COMMAND_MODE=unix2003 DIRSTACK=() DISPLAY=/tmp/launch-KaPNTW/org.macosforge.xquartz:0 EUID=501 GROUPS=() HOME=/Users/gps HOMEBREW_CC=clang HOSTNAME=gps-mbp HOSTTYPE=x86_64 IFS=' ' ITERM_PROFILE=Default ITERM_SESSION_ID=w0t0p0 JAVA_HOME= LANG=en_US.UTF-8 LOGNAME=gps LSCOLORS=gxfxcxdxbxegedabagacad MACHTYPE=x86_64-apple-darwin13 MOZ_QUIET=1 OPTERR=1 OPTIND=1 OSTYPE=darwin13 PATH=/Users/gps/src/phabricator/arcanist/bin:/opt/local/bin:/opt/local/sbin:/Users/gps/bin:/Users/gps/src/moz-git-tools:/Users/gps/.cabal/bin:/usr/local/sbin:/usr/local/bin:/usr/local/Cellar/coreutils/8.17/libexec/gnubin:/usr/bin:/bin:/usr/sbin:/sbin:/usr/local/bin:/opt/X11/bin PIPESTATUS=([0]="0") PIP_DOWNLOAD_CACHE=/Users/gps/Library/Caches/pip-downloads POSIXLY_CORRECT=y PPID=533 PS4='+ ' PWD=/Users/gps/src/firefox SHELL=/bin/bash SHELLOPTS=braceexpand:errexit:hashall:interactive-comments:posix SHLVL=2 SSH_AUTH_SOCK=/tmp/launch-u8iCdh/Listeners TERM=xterm-256color TERM_PROGRAM=iTerm.app TMPDIR=/var/folders/hl/c8xf3l7j4tz0t4fyvsgn8_dc0000gn/T/ UID=501 USER=gps _=------BEGIN_BEFORE_SOURCE __CF_USER_TEXT_ENCODING=0x1F5:0:0 __CHECKFIX1436934=1 ------END_BEFORE_SOURCE test-mozconfig: line 1: topsrcdir: command not found ------BEGIN_MK_OPTION MOZ_OBJDIR=/obj-debug ------END_MK_OPTION ------BEGIN_AFTER_SOURCE Apple_PubSub_Socket_Render=/tmp/launch-9OQjDD/Render BASH=/bin/sh BASH_ARGC=([0]="2") BASH_ARGV=([0]="test-mozconfig" [1]="/Users/gps/src/firefox") BASH_LINENO=([0]="0") BASH_SOURCE=([0]="python/mozbuild/mozbuild/mozconfig_loader") BASH_VERSINFO=([0]="3" [1]="2" [2]="51" [3]="1" [4]="release" [5]="x86_64-apple-darwin13") BASH_VERSION='3.2.51(1)-release' CLICOLOR=1 COLORFGBG='7;0' COMMAND_MODE=unix2003 DIRSTACK=() DISPLAY=/tmp/launch-KaPNTW/org.macosforge.xquartz:0 EUID=501 GROUPS=() HOME=/Users/gps HOMEBREW_CC=clang HOSTNAME=gps-mbp HOSTTYPE=x86_64 IFS=' ' ITERM_PROFILE=Default ITERM_SESSION_ID=w0t0p0 JAVA_HOME= LANG=en_US.UTF-8 LOGNAME=gps LSCOLORS=gxfxcxdxbxegedabagacad MACHTYPE=x86_64-apple-darwin13 MOZ_QUIET=1 OPTERR=1 OPTIND=1 OSTYPE=darwin13 PATH=/Users/gps/src/phabricator/arcanist/bin:/opt/local/bin:/opt/local/sbin:/Users/gps/bin:/Users/gps/src/moz-git-tools:/Users/gps/.cabal/bin:/usr/local/sbin:/usr/local/bin:/usr/local/Cellar/coreutils/8.17/libexec/gnubin:/usr/bin:/bin:/usr/sbin:/sbin:/usr/local/bin:/opt/X11/bin PIPESTATUS=([0]="0") PIP_DOWNLOAD_CACHE=/Users/gps/Library/Caches/pip-downloads POSIXLY_CORRECT=y PPID=533 PS4='+ ' PWD=/Users/gps/src/firefox SHELL=/bin/bash SHELLOPTS=braceexpand:errexit:hashall:interactive-comments:posix SHLVL=2 SSH_AUTH_SOCK=/tmp/launch-u8iCdh/Listeners TERM=xterm-256color TERM_PROGRAM=iTerm.app TMPDIR=/var/folders/hl/c8xf3l7j4tz0t4fyvsgn8_dc0000gn/T/ UID=501 USER=gps _=------BEGIN_AFTER_SOURCE __CF_USER_TEXT_ENCODING=0x1F5:0:0 __CHECKFIX1436934=1 ------END_AFTER_SOURCE gps@gps-mbp:~/src/firefox$ echo $? 0 ps@gps-mbp:~/src/firefox$ /bin/sh --version GNU bash, version 3.2.51(1)-release (x86_64-apple-darwin13) Copyright (C) 2007 Free Software Foundation, Inc.

I think I can get behind the plan to error if stderr contains output. Might make a few people complain. But it trends us towards a more strict mozconfig implementation, which I think is a good thing.