1287633 - Default environment in MSYS2 does not allow for building

Status: RESOLVED INVALID

(Firefox Build System :: General, defect)

Comment 1

[Nathan Hakkakzadeh [:Nat]]

Attachment: Bug 1287633 - Added build/mozconfig.msys2 which sets the environment variables necessary to build under MSYS2.

Review commit: https://reviewboard.mozilla.org/r/65062/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/65062/

Comment 2

[Nathan Hakkakzadeh [:Nat]]

Attachment: Bug 1287633 - Have ./mach bootstrap provide instructions of how to add the environment setup script to the user's .mozconfig

Review commit: https://reviewboard.mozilla.org/r/65068/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/65068/

Comment 3

[Gregory Szorc [:gps]]

https://reviewboard.mozilla.org/r/65062/#review62100

::: build/mozconfig.msys2:3
(Diff revision 1)
> +
> +# These lines will be installation-dependent.
> +export VSINSTALLDIR='C:\Program Files (x86)\Microsoft Visual Studio 14.0\'

On my machine, VCDIR and VCINSTALLDIR are defined globally and both point to C:\Program Files (x86)\Microsoft Visual Studio 14\VC\. Could we try setting these variables conditionally?

Also, WINDOWSSDKDIR is defined globally on my machine too.

We should at least construct paths from PROGRAMFILES and WINDIR and verify paths exist instead of blindly setting. Fail fast!

::: build/mozconfig.msys2:16
(Diff revision 1)
> +export WindowsSDK_ExecutablePath_x86='C:\Program Files (x86)\Microsoft SDKs\Windows\v10.0A\bin\NETFX 4.6.1 Tools\'
> +export WindowsSDK_ExecutablePath="${WindowsSDK_ExecutablePath_x64}"
> +
> +# The following should be largely installation-independent.
> +export VCINSTALLDIR="$VSINSTALLDIR"'VC\'
> +export DevEnvDir="$VSINSTALLDIR"'Common7\IDE\'

Does this need to be set? I don't think it does.

::: build/mozconfig.msys2:58
(Diff revision 1)
> +export PATH="${c_FrameworkDir}${FrameworkVersion}:$PATH"
> +export PATH="${c_VSINSTALLDIR}VC/bin/amd64:$PATH"
> +export PATH="${c_MSBUILDDIR}bin/amd64:$PATH"
> +
> +# not related to VS, but to make things easier for configure
> +export PATH="/c/mozilla-build/moztools-x64/bin:/c/mozilla-build/nsis-3.0b1:$PATH"

This is specific to your system. Please remove.

Comment 4

[Gregory Szorc [:gps]]

Comment on attachment 8772200 [details]
Bug 1287633 - Added build/mozconfig.msys2 which sets the environment variables necessary to build under MSYS2.

https://reviewboard.mozilla.org/r/65062/#review62102

Comment 5

[Nathan Hakkakzadeh [:Nat]]

Comment on attachment 8772200 [details]
Bug 1287633 - Added build/mozconfig.msys2 which sets the environment variables necessary to build under MSYS2.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/65062/diff/1-2/

Comment 6

[Nathan Hakkakzadeh [:Nat]]

Comment on attachment 8772203 [details]
Bug 1287633 - Have ./mach bootstrap provide instructions of how to add the environment setup script to the user's .mozconfig

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/65068/diff/1-2/

Comment 7

[Gregory Szorc [:gps]]

Comment on attachment 8772200 [details]
Bug 1287633 - Added build/mozconfig.msys2 which sets the environment variables necessary to build under MSYS2.

https://reviewboard.mozilla.org/r/65062/#review62646

::: build/mozconfig.msys2:4
(Diff revision 2)
> +# We need this because MSYS2 doesn't provide a variable to the 32-bit Program Files, so we assume a sane Windows path structure
> +PROGRAMFILES32=$(cd "$PROGRAMFILES/../Program Files (x86)" && pwd)
> +# These lines will be installation-dependent.
> +if test -z "$VSINSTALLDIR"; then 

Nit: trailing whitespace here and on the next 2 if lines.

::: build/mozconfig.msys2:5
(Diff revision 2)
> +# We need this because MSYS2 doesn't provide a variable to the 32-bit Program Files, so we assume a sane Windows path structure
> +PROGRAMFILES32=$(cd "$PROGRAMFILES/../Program Files (x86)" && pwd)
> +# These lines will be installation-dependent.
> +if test -z "$VSINSTALLDIR"; then 
> +    export VSINSTALLDIR='$PROGRAMFILES32\Microsoft Visual Studio 14.0\'

Does this work with single quotes? Single quotes don't expand variables...

Comment 8

[Gregory Szorc [:gps]]

Comment on attachment 8772203 [details]
Bug 1287633 - Have ./mach bootstrap provide instructions of how to add the environment setup script to the user's .mozconfig

https://reviewboard.mozilla.org/r/65068/#review62652

Comment 9

[Nathan Hakkakzadeh [:Nat]]

https://reviewboard.mozilla.org/r/65062/#review62646

> Does this work with single quotes? Single quotes don't expand variables...

It doesn't, there's actually a few problems I just uncovered, update to this review request should be out soon...

Comment 10

[(not currently active) Ted Mielczarek]

We're not going to support msys2.