Added support to download nuget and added python code to automate the… #23961

angelortiz1007 · 2019-08-13T16:44:16Z

… x64 and arm64 mach build -r/-d --uwp build process.

Modified/added python code to:

Download nuget which is required for building support\HoloLens
added python code to perform a nuget install/restore ServoApp.sln which causes the Angle libraries to be installed so that mach build will succeed.
added python code to perform a MsBuild of the support\HoloLens\ServoApp.sln solution so that mach build will build ServoApp and create a ServoApp .appx install file.

Note: The Msbuild path must be present otherwise the msbuild command will fail.

./mach build -d does not report any errors
./mach test-tidy does not report any errors
These changes fix Support building UWP app from mach #23753 (GitHub issue number if applicable)

There are tests for these changes OR
These changes do not require tests because by performing python mach build -r/-d --uwp ---win-arm64 or mach build -r/-d --uwp

This change is

angelortiz1007 · 2019-08-13T16:50:51Z

Important note.
When building, the msbuild.exe must be within the %PATH%.
If you open a x86_64 VS2017 command prompt msbuild will be found.

if you perform a cross platform build by opening a regular command prompt, you must set the path to where msbuild.exe is prior to:

set PKG_CONFIG_ALLOW_CROSS=1
python mach build -r --uwp --win-arm64

Here is an attempt to determine the msbuild.exe path at build time.
VSINSTALLDIR=C:\Program Files (x86)\Microsoft Visual Studio\2017\Professional\

location of Msbuild.exe => MSBuild\15.0\Bin

VisualStudioVersion =15.0

set MSBUILDDIR=%VSINSTALLDIR%MsBuild%VisualStudioVersion%\bin

nirbheek · 2019-08-13T17:11:55Z

Here is an attempt to determine the msbuild.exe path at build time.

The approach used in GStreamer's Cerbero build system is to extract PATH from the environment set by vcvarsall.bat: https://gitlab.freedesktop.org/gstreamer/cerbero/blob/master/cerbero/ide/vs/env.py#L106

Might be worth trying to do the same in Servo too.

angelortiz1007 · 2019-08-13T17:14:33Z

Here is an attempt to determine the msbuild.exe path at build time.

The approach used in GStreamer's Cerbero build system is to extract PATH from the environment set by vcvarsall.bat: https://gitlab.freedesktop.org/gstreamer/cerbero/blob/master/cerbero/ide/vs/env.py#L106
Might be worth trying to do the same in Servo too.

Thank you for looking at the pull request and providing this suggestion. I will review your suggestion!

Thank you,
Angel

jdm

Looks like a good start! I've left some comments for ways to make the code a bit more idiomatic and integrate better with the rest of the build system.

python/servo/build_commands.py

jdm · 2019-08-15T16:37:55Z

@bors-servo try=windows

bors-servo · 2019-08-15T16:38:00Z

⌛ Trying commit d28df22 with merge 55e471f...

Added support to download nuget and added python code to automate the… … x64 and arm64 mach build -r/-d --uwp build process.  Modified/added python code to: 1. Download nuget which is required for building support\HoloLens 2. added python code to perform a nuget install/restore ServoApp.sln which causes the Angle libraries to be installed so that mach build will succeed. 3. added python code to perform a MsBuild of the support\HoloLens\ServoApp.sln solution so that mach build will build ServoApp and create a ServoApp .appx install file. Note: The Msbuild path must be present otherwise the msbuild command will fail. ---  - [x] `./mach build -d` does not report any errors - [x] `./mach test-tidy` does not report any errors - [ ] These changes fix #_23753__ (GitHub issue number if applicable)  - [x] There are tests for these changes OR - [x] These changes do not require tests because by performing python mach build -r/-d --uwp ---win-arm64 or mach build -r/-d --uwp    --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/23961)

bors-servo · 2019-08-15T19:35:11Z

💔 Test failed - status-taskcluster

jdm · 2019-08-15T19:50:03Z

@bors-servo retry

bors-servo · 2019-08-15T19:50:09Z

⌛ Trying commit d28df22 with merge 227ad85...

Added support to download nuget and added python code to automate the… … x64 and arm64 mach build -r/-d --uwp build process.  Modified/added python code to: 1. Download nuget which is required for building support\HoloLens 2. added python code to perform a nuget install/restore ServoApp.sln which causes the Angle libraries to be installed so that mach build will succeed. 3. added python code to perform a MsBuild of the support\HoloLens\ServoApp.sln solution so that mach build will build ServoApp and create a ServoApp .appx install file. Note: The Msbuild path must be present otherwise the msbuild command will fail. ---  - [x] `./mach build -d` does not report any errors - [x] `./mach test-tidy` does not report any errors - [ ] These changes fix #_23753__ (GitHub issue number if applicable)  - [x] There are tests for these changes OR - [x] These changes do not require tests because by performing python mach build -r/-d --uwp ---win-arm64 or mach build -r/-d --uwp    --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/23961)

angelortiz1007 · 2019-08-15T20:50:19Z

Implemented changes requested by JDM and performed the below tests. Rebased changes and performed a pull request for testing.

Ran tests to verify the following passed:

mach build -r/-d --uwp
python mach build -r --uwp --win-arm64
Tested return value from build_uwp_hololens() which calls check_call() by modifying msbuild parameter .\support\hololens\servoapp.sln to servoapp.sln2.
Upon performing a build the logic introduced to set status = 1was tested. Output from test is shown below:
MSBUILD : error MSB1009: Project file does not exist.
Switch: .\support\hololens\servoapp.sln2
Error running mach:

['build', '-r', '--uwp']

The error occurred in code that was called by the mach command. This is either
a bug in the called code itself or in the way that mach is calling it.

You should 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:

CalledProcessError: Command 'm s b u i l d / m / p : p r o j e c t = S e r v o A p p . \ s u p p o r t \ h o l o l e n s \ s e r v o a p p . s l n 2 / p : S o l u t i o n D i r = . \ s u p p o r t \ h o l o l e n s / p : C o n f i g u r a t i o n = R e l e a s e / p : P l a t f o r m = x 6 4 / p : A p p x B u n d l e = A l w a y s ; A p p x B u n d l e P l a t f o r m s = x 6 4' returned non-zero exit status 1

File "C:\git-mozilla\angelortiz1007-2\python\servo\build_commands.py", line 702, in build
if build_uwp_hololens(target_triple, dev):
File "C:\git-mozilla\angelortiz1007-2\python\servo\build_commands.py", line 962, in build_uwp_hololens
check_call(msbuild_cmd + msbuild_conf)
File "C:\git-mozilla\angelortiz1007-2\python\servo\command_base.py", line 195, in check_call
raise subprocess.CalledProcessError(status, ' '.join(*args))

python/servo/build_commands.py

jdm · 2019-08-15T20:45:01Z

python/servo/build_commands.py

+        print("Angle Found...")
+    else:
+        print("Angle not found.  Performing nuget to restore Angle package")
+        check_call(nuget_command)


Instead of passing a single string, we should use a list:

check_call(["nuget", "restore", nuget_app])

python/servo/build_commands.py

jdm · 2019-08-19T18:51:38Z

@bors-servo try=windows

bors-servo · 2019-08-19T18:51:43Z

⌛ Trying commit cd6b048 with merge cad7e83...

Added support to download nuget and added python code to automate the… … x64 and arm64 mach build -r/-d --uwp build process.  Modified/added python code to: 1. Download nuget which is required for building support\HoloLens 2. added python code to perform a nuget install/restore ServoApp.sln which causes the Angle libraries to be installed so that mach build will succeed. 3. added python code to perform a MsBuild of the support\HoloLens\ServoApp.sln solution so that mach build will build ServoApp and create a ServoApp .appx install file. Note: The Msbuild path must be present otherwise the msbuild command will fail. ---  - [x] `./mach build -d` does not report any errors - [x] `./mach test-tidy` does not report any errors - [ ] These changes fix #23753 (GitHub issue number if applicable)  - [x] There are tests for these changes OR - [x] These changes do not require tests because by performing python mach build -r/-d --uwp ---win-arm64 or mach build -r/-d --uwp    --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/23961)

bors-servo · 2019-08-19T19:56:59Z

💔 Test failed - status-taskcluster

jdm · 2019-08-19T20:03:03Z

Error running mach:

    ['build', '--dev', '--uwp', '--win-arm64']

The error occurred in code that was called by the mach command. This is either
a bug in the called code itself or in the way that mach is calling it.

You should 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:

CalledProcessError: Command 'msbuild /m /p:project=ServoApp .\support\hololens\ServoApp.sln /p:SolutionDir=.\support\hololens /p:Configuration=Debug /p:Platform=arm64 /p:AppxBundle=Always;AppxBundlePlatforms=arm64' returned non-zero exit status 1

  File "C:\Users\task_1566239357\repo\python\servo\build_commands.py", line 705, in build
    build_uwp_hololens(target_triple, dev)
  File "C:\Users\task_1566239357\repo\python\servo\build_commands.py", line 976, in build_uwp_hololens
    "/p:AppxBundle=Always;AppxBundlePlatforms=" + vs_platform])
  File "C:\Users\task_1566239357\repo\python\servo\command_base.py", line 195, in check_call
    raise subprocess.CalledProcessError(status, ' '.join(*args))

Let's try using check_output instead of check_call, which should give us any output from the failing msbuild command.

CYBAI · 2019-08-22T05:49:06Z

Build started 8/22/2019 3:41:12 AM.
     1>Project "C:\Users\task_1566430230\repo\support\hololens\ServoApp.sln" on node 1 (default targets).
     1>ValidateSolutionConfiguration:
         Building solution configuration "Debug|x64".
     1>Project "C:\Users\task_1566430230\repo\support\hololens\ServoApp.sln" (1) is building "C:\Users\task_1566430230\repo\support\hololens\ServoApp\ServoApp.vcxproj" (2) on node 1 (default targets).
     2>C:\Users\task_1566430230\repo\support\hololens\ServoApp\ServoApp.vcxproj(323,5): error : This project references NuGet package(s) that are missing on this computer. Use NuGet Package Restore to download them.  For more information, see http://go.microsoft.com/fwlink/?LinkID=322105. The missing file is ..\packages\Microsoft.Windows.CppWinRT.2.0.190620.2\build\native\Microsoft.Windows.CppWinRT.props.
     2>Done Building Project "C:\Users\task_1566430230\repo\support\hololens\ServoApp\ServoApp.vcxproj" (default targets) -- FAILED.
     1>Done Building Project "C:\Users\task_1566430230\repo\support\hololens\ServoApp.sln" (default targets) -- FAILED.

Build FAILED.

       "C:\Users\task_1566430230\repo\support\hololens\ServoApp.sln" (default target) (1) ->
       "C:\Users\task_1566430230\repo\support\hololens\ServoApp\ServoApp.vcxproj" (default target) (2) ->
       (EnsureNuGetPackageBuildImports target) -> 
         C:\Users\task_1566430230\repo\support\hololens\ServoApp\ServoApp.vcxproj(323,5): error : This project references NuGet package(s) that are missing on this computer. Use NuGet Package Restore to download them.  For more information, see http://go.microsoft.com/fwlink/?LinkID=322105. The missing file is ..\packages\Microsoft.Windows.CppWinRT.2.0.190620.2\build\native\Microsoft.Windows.CppWinRT.props.

    0 Warning(s)
    1 Error(s)

jdm · 2019-08-22T05:56:21Z

@bors-servo try=windows

bors-servo · 2019-08-22T05:56:25Z

⌛ Trying commit 57adaaa with merge 0d7280a...

Added support to download nuget and added python code to automate the… … x64 and arm64 mach build -r/-d --uwp build process.  Modified/added python code to: 1. Download nuget which is required for building support\HoloLens 2. added python code to perform a nuget install/restore ServoApp.sln which causes the Angle libraries to be installed so that mach build will succeed. 3. added python code to perform a MsBuild of the support\HoloLens\ServoApp.sln solution so that mach build will build ServoApp and create a ServoApp .appx install file. Note: The Msbuild path must be present otherwise the msbuild command will fail. ---  - [x] `./mach build -d` does not report any errors - [x] `./mach test-tidy` does not report any errors - [ ] These changes fix #23753 (GitHub issue number if applicable)  - [x] There are tests for these changes OR - [x] These changes do not require tests because by performing python mach build -r/-d --uwp ---win-arm64 or mach build -r/-d --uwp    --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/23961)

bors-servo · 2019-08-22T07:00:04Z

☀️ Test successful - status-taskcluster
State: approved= try=True

jdm · 2019-08-22T13:36:11Z

@bors-servo r+

bors-servo · 2019-08-22T13:36:12Z

📌 Commit 57adaaa has been approved by jdm

bors-servo · 2019-08-22T14:46:22Z

⌛ Testing commit 57adaaa with merge 382ebcc...

Added support to download nuget and added python code to automate the… … x64 and arm64 mach build -r/-d --uwp build process.  Modified/added python code to: 1. Download nuget which is required for building support\HoloLens 2. added python code to perform a nuget install/restore ServoApp.sln which causes the Angle libraries to be installed so that mach build will succeed. 3. added python code to perform a MsBuild of the support\HoloLens\ServoApp.sln solution so that mach build will build ServoApp and create a ServoApp .appx install file. Note: The Msbuild path must be present otherwise the msbuild command will fail. ---  - [x] `./mach build -d` does not report any errors - [x] `./mach test-tidy` does not report any errors - [ ] These changes fix #23753 (GitHub issue number if applicable)  - [x] There are tests for these changes OR - [x] These changes do not require tests because by performing python mach build -r/-d --uwp ---win-arm64 or mach build -r/-d --uwp    --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/23961)

bors-servo · 2019-08-22T16:41:10Z

💔 Test failed - status-taskcluster

jdm · 2019-08-22T19:24:12Z

@bors-servo retry

bors-servo · 2019-08-22T20:54:10Z

💣 Failed to start rebuilding: Unknown error

bors-servo · 2019-08-22T20:54:15Z

⌛ Testing commit 57adaaa with merge ee9b3b3...

Added support to download nuget and added python code to automate the… … x64 and arm64 mach build -r/-d --uwp build process.  Modified/added python code to: 1. Download nuget which is required for building support\HoloLens 2. added python code to perform a nuget install/restore ServoApp.sln which causes the Angle libraries to be installed so that mach build will succeed. 3. added python code to perform a MsBuild of the support\HoloLens\ServoApp.sln solution so that mach build will build ServoApp and create a ServoApp .appx install file. Note: The Msbuild path must be present otherwise the msbuild command will fail. ---  - [x] `./mach build -d` does not report any errors - [x] `./mach test-tidy` does not report any errors - [ ] These changes fix #23753 (GitHub issue number if applicable)  - [x] There are tests for these changes OR - [x] These changes do not require tests because by performing python mach build -r/-d --uwp ---win-arm64 or mach build -r/-d --uwp    --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/23961)

bors-servo · 2019-08-22T21:45:42Z

☀️ Test successful - linux-rel-css, linux-rel-wpt, status-taskcluster
Approved by: jdm
Pushing ee9b3b3 to master...

highfive added the S-awaiting-review There is new code that needs to be reviewed. label Aug 13, 2019

jdm requested changes Aug 15, 2019

View reviewed changes

jdm added S-needs-code-changes Changes have not yet been made that were requested by a reviewer. and removed S-awaiting-review There is new code that needs to be reviewed. labels Aug 15, 2019

highfive added the S-tests-failed The changes caused existing tests to fail. label Aug 15, 2019

highfive added S-awaiting-review There is new code that needs to be reviewed. and removed S-tests-failed The changes caused existing tests to fail. S-needs-code-changes Changes have not yet been made that were requested by a reviewer. labels Aug 15, 2019

jdm requested changes Aug 15, 2019

View reviewed changes

jdm added S-needs-code-changes Changes have not yet been made that were requested by a reviewer. and removed S-awaiting-review There is new code that needs to be reviewed. labels Aug 15, 2019

highfive added S-awaiting-review There is new code that needs to be reviewed. and removed S-needs-code-changes Changes have not yet been made that were requested by a reviewer. labels Aug 19, 2019

highfive added the S-tests-failed The changes caused existing tests to fail. label Aug 19, 2019

highfive added S-tests-failed The changes caused existing tests to fail. and removed S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. labels Aug 22, 2019

Remove explicit nuget setup in CI.

57adaaa

highfive added S-awaiting-review There is new code that needs to be reviewed. and removed S-tests-failed The changes caused existing tests to fail. labels Aug 22, 2019

highfive added S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. and removed S-awaiting-review There is new code that needs to be reviewed. labels Aug 22, 2019

highfive added S-tests-failed The changes caused existing tests to fail. and removed S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. labels Aug 22, 2019

highfive added S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. and removed S-tests-failed The changes caused existing tests to fail. labels Aug 22, 2019

bors-servo merged commit 57adaaa into servo:master Aug 22, 2019

highfive removed the S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. label Aug 22, 2019

bors-servo mentioned this pull request Aug 22, 2019

Enable GStreamer on Windows. #24004

Merged

3 tasks

angelortiz1007 deleted the servo-issue branch September 11, 2019 23:28

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added support to download nuget and added python code to automate the… #23961

Added support to download nuget and added python code to automate the… #23961

angelortiz1007 commented Aug 13, 2019 •

edited by CYBAI

angelortiz1007 commented Aug 13, 2019

nirbheek commented Aug 13, 2019

angelortiz1007 commented Aug 13, 2019

jdm left a comment

jdm commented Aug 15, 2019

bors-servo commented Aug 15, 2019

bors-servo commented Aug 15, 2019

jdm commented Aug 15, 2019

bors-servo commented Aug 15, 2019

angelortiz1007 commented Aug 15, 2019

jdm Aug 15, 2019

jdm commented Aug 19, 2019

bors-servo commented Aug 19, 2019

bors-servo commented Aug 19, 2019

jdm commented Aug 19, 2019

CYBAI commented Aug 22, 2019

jdm commented Aug 22, 2019

bors-servo commented Aug 22, 2019

bors-servo commented Aug 22, 2019

jdm commented Aug 22, 2019

bors-servo commented Aug 22, 2019

bors-servo commented Aug 22, 2019

bors-servo commented Aug 22, 2019

jdm commented Aug 22, 2019

bors-servo commented Aug 22, 2019

bors-servo commented Aug 22, 2019

bors-servo commented Aug 22, 2019

Added support to download nuget and added python code to automate the… #23961

Added support to download nuget and added python code to automate the… #23961

Conversation

angelortiz1007 commented Aug 13, 2019 • edited by CYBAI

angelortiz1007 commented Aug 13, 2019

nirbheek commented Aug 13, 2019

angelortiz1007 commented Aug 13, 2019

jdm left a comment

Choose a reason for hiding this comment

jdm commented Aug 15, 2019

bors-servo commented Aug 15, 2019

bors-servo commented Aug 15, 2019

jdm commented Aug 15, 2019

bors-servo commented Aug 15, 2019

angelortiz1007 commented Aug 15, 2019

jdm Aug 15, 2019

Choose a reason for hiding this comment

jdm commented Aug 19, 2019

bors-servo commented Aug 19, 2019

bors-servo commented Aug 19, 2019

jdm commented Aug 19, 2019

CYBAI commented Aug 22, 2019

jdm commented Aug 22, 2019

bors-servo commented Aug 22, 2019

bors-servo commented Aug 22, 2019

jdm commented Aug 22, 2019

bors-servo commented Aug 22, 2019

bors-servo commented Aug 22, 2019

bors-servo commented Aug 22, 2019

jdm commented Aug 22, 2019

bors-servo commented Aug 22, 2019

bors-servo commented Aug 22, 2019

bors-servo commented Aug 22, 2019

angelortiz1007 commented Aug 13, 2019 •

edited by CYBAI