apache / cordova-windows

[DEPRECATED] Apache Cordova Windows
Apache License 2.0
203 stars 171 forks source link

AppVeyor CI is failing because of newer MSBuild getting selected #334

Closed janpio closed 5 years ago

janpio commented 5 years ago

CI started failing when https://github.com/apache/cordova-windows/pull/329 was merged:

https://github.com/apache/cordova-windows/commit/95401b36b20e5c4b37a3253ed0042b98380caf27 https://ci.appveyor.com/project/Humbedooh/cordova-windows/builds/24305931

Example:

.Creating Cordova Windows Project:
    Path: temp\platforms\testcreate 応用
    Namespace: com.test.app
    Name: 応用
Windows project created with cordova-windows@7.1.0-dev
Building project: C:\projects\cordova-windows\temp\platforms\testcreate 応用\CordovaApp.Windows.jsproj
    Configuration : debug
    Platform      : anycpu
    Buildflags    : /p:AppxBundle=Never
    MSBuildTools  : C:\Program Files (x86)\Microsoft Visual Studio\2017\Community\MSBuild\15.0\Bin
buildProject spawn: C:\Program Files (x86)\Microsoft Visual Studio\2017\Community\MSBuild\15.0\Bin\msbuild [ 'C:\\projects\\cordova-windows\\temp\\platforms\\testcreate 応用\\CordovaApp.Windows.jsproj',
  '/clp:NoSummary;NoItemAndPropertyList;Verbosity=minimal',
  '/nologo',
  '/p:Configuration=debug',
  '/p:Platform=anycpu',
  '/p:AppxBundle=Never' ] { stdio: 'inherit' }
  prebuild.js: Patching platform `8.1`
  - Injected `base.js` reference to `www/index.html`
  - Removing /( *)(<script\s+(?:type="text\/javascript"\s+)?src="\/www\/WinJS\/js\/base.js">\s*<\/script>)(\s*)/ from www/index.html
  - Removing /( *)(<script\s+(?:type="text\/javascript"\s+)?src="\/\/Microsoft.Phone.WinJS.2.1\/js\/base.js">\s*<\/script>)(\s*)/ from www/index.html
C:\Program Files (x86)\Microsoft Visual Studio\2017\Community\MSBuild\Microsoft\VisualStudio\v15.0\AppxPackage\Microsoft.AppXPackage.Targets(828,5): error APPX3217: SDK folder containing 'Windows.props' for 'Windows 8.1' cannot be located. See http://go.microsoft.com/fwlink/?LinkID=798187 for more information. [C:\projects\cordova-windows\temp\platforms\testcreate ??\CordovaApp.Windows.jsproj]
C:\projects\cordova-windows\temp\platforms\testcreate 応用\cordova\node_modules\q\q.js:155
                throw e;
                ^
CordovaError: No valid MSBuild was detected for the selected target: Error: C:\Program Files (x86)\Microsoft Visual Studio\2017\Community\MSBuild\15.0\Bin\msbuild: Command failed with exit code 1
    at C:\projects\cordova-windows\temp\platforms\testcreate 応用\cordova\lib\build.js:83:29
    at _rejected (C:\projects\cordova-windows\temp\platforms\testcreate 応用\cordova\node_modules\q\q.js:864:24)
    at C:\projects\cordova-windows\temp\platforms\testcreate 応用\cordova\node_modules\q\q.js:890:30
    at Promise.when (C:\projects\cordova-windows\temp\platforms\testcreate 応用\cordova\node_modules\q\q.js:1142:31)
    at Promise.promise.promiseDispatch (C:\projects\cordova-windows\temp\platforms\testcreate 応用\cordova\node_modules\q\q.js:808:41)
    at C:\projects\cordova-windows\temp\platforms\testcreate 応用\cordova\node_modules\q\q.js:624:44
    at runSingle (C:\projects\cordova-windows\temp\platforms\testcreate 応用\cordova\node_modules\q\q.js:137:13)
    at flush (C:\projects\cordova-windows\temp\platforms\testcreate 応用\cordova\node_modules\q\q.js:125:13)
    at _combinedTickCallback (internal/process/next_tick.js:73:7)
    at process._tickCallback (internal/process/next_tick.js:104:9)
ls: no such file or directory: C:\projects\cordova-windows\temp\platforms\testcreate 応用\AppPackages

(CordovaError: No valid MSBuild was detected for the selected target: is a known red herring here: https://github.com/apache/cordova-windows/issues/266)


Difference between working and broken build is this:

Before:

    MSBuildTools  : C:\Program Files (x86)\MSBuild\14.0\bin\
buildProject spawn: C:\Program Files (x86)\MSBuild\14.0\bin\msbuild [ 'C:\\projects\\cordova-windows\\temp\\platforms\\testcreate 応用\\CordovaApp.Windows10.jsproj',

After:

    MSBuildTools  : C:\Program Files (x86)\Microsoft Visual Studio\2017\Community\MSBuild\15.0\Bin
buildProject spawn: C:\Program Files (x86)\Microsoft Visual Studio\2017\Community\MSBuild\15.0\Bin\msbuild [ 'C:\\projects\\cordova-windows\\temp\\platforms\\testcreate 応用\\CordovaApp.Windows10.jsproj',

So improving the VS detector breaks on AppVeyor :/

janpio commented 5 years ago

Reason behind this is probably that VS17 does not support building for Windows 8.1 any more, so by fixing the VS detector to pick up the correct VS17 install, we broke the 8.1 builds that still are always tested.

Solution might be to separate the Win10 and Win8.1 tests and only run the matching ones?

janpio commented 5 years ago

Another solution might be to use pending similar to https://github.com/apache/cordova-windows/blob/585dc2f4e4c0794b72af6f4b448bb42ffd213800/spec/e2e/endtoend.spec.js#L218-L220 in all the Windows 8.1 tests. Does that make sense @raphinesse?

raphinesse commented 5 years ago

Sure! Seems sensible, judging from what you wrote above. If a fair amount of tests are affected, you might even want to factor above snippet out into a function, so you only have a one-liner in the affected tests. Something like skipIfUsingVS2017().

janpio commented 5 years ago

Currently working on splitting the tests in Win8.1 and Win10 batches, then using pending on them to get rid of the failures. Additionally I will add another build with explicit MSBUILDDIR set to C:\Program Files (x86)\Microsoft Visual Studio\2017\Community\MSBuild\15.0\Bin - so future changes on the discovery logic will pop up earlier.

janpio commented 5 years ago

Build is green :D