-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
resolve red builds on the develop branch after VS2017 15.3 #1428
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
@olevett, It will cover your contributions to all .NET Foundation-managed open source projects. |
|
Note, this shouldn't be merged till Cake supports pre-release |
|
@olevett, thanks for signing the contribution license agreement. We will now validate the agreement and then the pull request. |
|
Changes Unknown when pulling 1b2fda1 on vs-2017-preview into ** on develop**. |
|
I've hijacked this PR to get us to green builds on the develop branch. Changes:
|
| } | ||
| .WithTarget("restore;pack") | ||
| .WithTarget("restore;build;pack") | ||
| .WithProperty("PackageOutputPath", MakeAbsolute(Directory(artifactDirectory)).ToString()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Had to add build here to generate our unit test assemblies (ReactiveUI.Tests/bin/**/). Before VS2017 15.3 they were generated automatically via the pack step so we never specified it. I guess we were relying on a toolchain quirk which should have never worked this way in the first place.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it could be that IsPackable has been set to false in Test project sdk targets , and that's why pack doesn't do anything here.
|
|
||
| MSBuild(solution, new MSBuildSettings() | ||
| MSBuild(solution, new MSBuildSettings() { | ||
| ToolPath = msBuildPath, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Had to specify the location of msbuild (an oversight, we do this as part of the buildreactiveui step and forgot to add it in here)
| <ItemGroup> | ||
| <PackageReference Include="MSBuild.Sdk.Extras" Version="1.0.1" PrivateAssets="All" /> | ||
| <PackageReference Include="MSBuild.Sdk.Extras" Version="1.0.6" PrivateAssets="All" /> | ||
| </ItemGroup> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
dis got bumped; it's very important but I don't have the backstory.
What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)
Fix for #1420
What is the current behavior? (You can also link to an open issue here)
Appveyor is old version
What is the new behavior (if this is a feature change)?
Appveyor is preview builds
What might this PR break?
Hopefully nothing
Please check if the PR fulfills these requirements
Other information:
Have raised a ticket on Cake, as the VSWhere tool we're using doesn't currently include pre-release builds, so VS can't find what it's doing. The build is falling back to MSBuild 14, which doesn't support new csprojs, hence breaking the build.
Cake ticket cake-build/cake#1754, notes on vswhere prerelease flag https://blogs.msdn.microsoft.com/heaths/2017/06/24/vswhere-version-2-0-released/