794729 - mach fails to build on Windows: WindowsError: [Error 193] %1 is not a valid Win32 application

Status: RESOLVED FIXED

(Firefox Build System :: Mach Core, enhancement)

Description

[Gregory Szorc [:gps]]

$ ./mach build
Traceback (most recent call last):
  File "./mach", line 48, in <module>
    mach.run(sys.argv[1:])
  File "d:\src\mozilla-central\python/mach\mach\main.py", line 133, in run
    fn(**stripped)
  File "d:\src\mozilla-central\python/mach\mach\build.py", line 23, in build
    warnings_path = self._get_state_filename('warnings.json')
  File "d:\src\mozilla-central\python/mozbuild\mozbuild\base.py", line 170, in _get_state_filename
    path = self.statedir
  File "d:\src\mozilla-central\python/mozbuild\mozbuild\base.py", line 89, in statedir
    return os.path.join(self.topobjdir, '.mozbuild')
  File "d:\src\mozilla-central\python/mozbuild\mozbuild\base.py", line 72, in topobjdir
    self._load_mozconfig()
  File "d:\src\mozilla-central\python/mozbuild\mozbuild\base.py", line 110, in _load_mozconfig
    env['CONFIG_GUESS'] = self._config_guess
  File "d:\src\mozilla-central\python/mozbuild\mozbuild\base.py", line 151, in _config_guess
    cwd=self.topsrcdir).strip()
  File "c:\dev\mozilla-build\python\lib\subprocess.py", line 537, in check_output
    process = Popen(stdout=PIPE, *popenargs, **kwargs)
  File "c:\dev\mozilla-build\python\lib\subprocess.py", line 679, in __init__
    errread, errwrite)
  File "c:\dev\mozilla-build\python\lib\subprocess.py", line 893, in _execute_child
    startupinfo)
WindowsError: [Error 193] %1 is not a valid Win32 application


I reckon we need to execute config.guess within a shell. Yay Windows.

Comment 1

[Gregory Szorc [:gps]]

Attachment: Run config.guess and mozconfig in shells, v1

Ugh, Windows and shells. I should have tested the "mach build" patch on Windows.

Comment 2

[Gregory Szorc [:gps]]

Comment on attachment 665249 [details] [diff] [review]
Run config.guess and mozconfig in shells, v1

glandium: if you get to this while California is sleeping, that would rock. Feel free to check in to m-c (don't forget DONTBUILD) if you are so inclined. If not, don't worry about it.

Comment 3

[Mike Hommey [:glandium]]

Comment on attachment 665249 [details] [diff] [review]
Run config.guess and mozconfig in shells, v1

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

::: python/mozbuild/mozbuild/base.py
@@ +273,5 @@
>  
>      def _run_command_in_objdir(self, **args):
>          self._run_command(cwd=self.topobjdir, **args)
>  
> +    def _run_command(self, args=None, cwd=None, stderr=None,

You're not doing anything with stderr.

Comment 4

[Gregory Szorc [:gps]]

(In reply to Mike Hommey [:glandium] from comment #3)
> ::: python/mozbuild/mozbuild/base.py
> @@ +273,5 @@
> >  
> >      def _run_command_in_objdir(self, **args):
> >          self._run_command(cwd=self.topobjdir, **args)
> >  
> > +    def _run_command(self, args=None, cwd=None, stderr=None,
> 
> You're not doing anything with stderr.

And that's why we have code review :/

Turns out that plugging stderr in to ProcessHandlerMixin breaks things. And, we can't use ProcessHandlerMixin for this usage because... ProcessHandlerMixin force redirects stderr to stdout!

So, I'll throw together a new patch that doesn't use mozprocess. (We also need to change mozconfig processing to go through shells and those executed scripts rely on stderr and stdout being different streams.)

Long term it would be great if mozprocess didn't cross the streams.

Comment 5

[Gregory Szorc [:gps]]

Attachment: Run config.guess and mozconfig in shells, v2

First to get to it "wins."

The environment variable key type foo makes me feel dirty.

Comment 6

[Mike Hommey [:glandium]]

(In reply to Gregory Szorc [:gps] from comment #5)
> Created attachment 665516 [details] [diff] [review]
> Run config.guess and mozconfig in shells, v2
> 
> First to get to it "wins."
> 
> The environment variable key type foo makes me feel dirty.

It makes you feel dirty, but why is it needed?

Comment 7

[Mike Hommey [:glandium]]

(In reply to Mike Hommey [:glandium] from comment #6)
> It makes you feel dirty, but why is it needed?

Forget it, missed the comment.

Comment 8

[Mike Hommey [:glandium]]

Comment on attachment 665516 [details] [diff] [review]
Run config.guess and mozconfig in shells, v2

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

::: python/mozbuild/mozbuild/base.py
@@ +344,5 @@
> +        if not require_unix_environment or not _in_msys:
> +            return args
> +
> +        # Always munge Windows-style into Unix style for the command.
> +        prog = args[0].replace('\\', '/')

You should only do that on windows.

Comment 9

[Gregory Szorc [:gps]]

(In reply to Mike Hommey [:glandium] from comment #8)
> ::: python/mozbuild/mozbuild/base.py
> @@ +344,5 @@
> > +        if not require_unix_environment or not _in_msys:
> > +            return args
> > +
> > +        # Always munge Windows-style into Unix style for the command.
> > +        prog = args[0].replace('\\', '/')
> 
> You should only do that on windows.

We only get here if _in_msys and require_unix_environment. _in_msys implies Windows. So, no change is necessary.

Comment 10

[Gregory Szorc [:gps]]

https://hg.mozilla.org/mozilla-central/rev/74522aafbcdc