Default environment in MSYS2 does not allow for building

NEW
Assigned to

Status

()

Core
Build Config
a year ago
a year ago

People

(Reporter: Nat, Assigned: Nat)

Tracking

(Blocks: 1 bug)

Firefox Tracking Flags

(Not tracked)

Details

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(2 attachments)

Comment hidden (empty)
(Assignee)

Comment 1

a year ago
Created attachment 8772200 [details]
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/
Attachment #8772200 - Flags: review?(gps)
(Assignee)

Comment 2

a year ago
Created attachment 8772203 [details]
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/
Attachment #8772203 - Flags: review?(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 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
Attachment #8772200 - Flags: review?(gps) → review-
(Assignee)

Updated

a year ago
Depends on: 1287943
(Assignee)

Comment 5

a year ago
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/
Attachment #8772200 - Flags: review- → review?(gps)
(Assignee)

Comment 6

a year ago
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 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...
Attachment #8772200 - Flags: review?(gps) → review-
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
Attachment #8772203 - Flags: review?(gps) → review+
(Assignee)

Comment 9

a year ago
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...
Depends on: 1289949
You need to log in before you can comment on or make changes to this bug.