Tooling migration #432

josephwoodward · 2017-04-10T23:31:48Z

Initial PR for project.json > .csproj migration

slang25 · 2017-04-11T23:13:20Z

@josephwoodward <DebugType>full</DebugType> Should fix most failing tests

josephwoodward · 2017-06-29T10:40:24Z

Hmm, still really stuck with this error, anyone seen it before? Google's not showing much for it!

jeffkl · 2017-06-29T14:56:59Z

@josephwoodward I work on the MSBuild team at Microsoft. Now that you're switching to the MSBuild-based projects and away from xproj, the Cake script probably doesn't need to run DotNetCorePack. You can have a NuGet package created at build time by setting an MSBuild property GeneratePackageOnBuild to true. Here's an example: http://blog.myget.org/post/2017/03/15/visual-studio-2017-and-net-core-support-on-myget.aspx

If the package is created during build, you won't need to run pack as a separate phase but if you do want to, you would now run msbuild /Target:Pack. I'm not very familiar with Cake but I'm sure there's a way to do that.

josephwoodward · 2017-06-30T13:31:30Z

Thanks a bunch @jeffkl, I'd missed that part! I'm still getting the SatelliteDllsProjectOutputGroupWithTargetFramework error so I'll keep digging.

"C:\projects\shouldly\src\Shouldly.sln" (Build target) (1) ->
"C:\projects\shouldly\src\Shouldly\Shouldly.csproj" (default target) (2) ->
"C:\projects\shouldly\src\Shouldly\Shouldly.csproj" (SatelliteDllsProjectOutputGroupWithTargetFramework target) (2:5) ->
  C:\projects\shouldly\src\Shouldly\Shouldly.csproj : error MSB4057: The target "SatelliteDllsProjectOutputGroupWithTargetFramework" does not exist in the project.

jeffkl · 2017-06-30T16:33:17Z

Alright I checked out your branch and figured out what's wrong. You're using MSBuild.Sdk.Extras which defines this target SatelliteDllsProjectOutputGroupWithTargetFramework. At restore time, NuGet creates obj\Shouldly.csproj.nuget.g.targets that gets imported and it contains the following:

<Project ToolsVersion="14.0" xmlns="http://schemas.microsoft.com/developer/msbuild/2003">
  <PropertyGroup>
    <MSBuildAllProjects>$(MSBuildAllProjects);$(MSBuildThisFileFullPath)</MSBuildAllProjects>
  </PropertyGroup>
  <ImportGroup Condition=" '$(TargetFramework)' == '' AND '$(ExcludeRestorePackageImports)' != 'true' ">
    <Import Project="$(NuGetPackageRoot)msbuild.sdk.extras\1.0.0\buildMultiTargeting\MSBuild.Sdk.Extras.targets" Condition="Exists('$(NuGetPackageRoot)msbuild.sdk.extras\1.0.0\buildMultiTargeting\MSBuild.Sdk.Extras.targets')" />
  </ImportGroup>
  <ImportGroup Condition=" '$(TargetFramework)' == 'netstandard1.3' AND '$(ExcludeRestorePackageImports)' != 'true' ">
    <Import Project="$(NuGetPackageRoot)msbuild.sdk.extras\1.0.0\build\netstandard1.0\MSBuild.Sdk.Extras.targets" Condition="Exists('$(NuGetPackageRoot)msbuild.sdk.extras\1.0.0\build\netstandard1.0\MSBuild.Sdk.Extras.targets')" />
  </ImportGroup>
</Project>

The initial build of a project with TargetFrameworks set launches N number of builds with TargetFramework set. For the first build, it imports buildMultiTargeting\MSBuild.Sdk.Extras.targets which tells MSBuild to build a target. However, when building the project targeting a single framework, net35 and net451 aren't importing the file that has the necessary target.

The package supports netstandard1.0 and portable-net4+wp7+sl4+win8+wpa81 which I believe should cover all frameworks. But NuGet doesn't appear to map portable-net4+wp7+sl4+win8+wpa81 to net35 or net461. @onovotny might know why that's happening or if we just need to add a net35 folder to the package with the appropriate imports.

As a workaround, you can define an empty target in Shouldly.csproj which is overridden when targeting netstandard1.0. I'm not sure if that breaks things but I don't believe you're using satellite assemblies anyway.

  <Target Name="SatelliteDllsProjectOutputGroupWithTargetFramework" />

  <Import Project="$(MSBuildSDKExtrasTargets)" Condition="Exists('$(MSBuildSDKExtrasTargets)')" />

clairernovotny

I would make a few changes based on what I've suggested

clairernovotny · 2017-06-30T20:14:29Z

global.json

 {
-    "projects": [ "src" ],
-    "sdk": { "version": "1.0.0-preview2-003131" }
+    "sdk": { "version": "1.0.0" }


I would not use 1.0.0. Use 1.0.4 at the very least. Many bugs fixed.

clairernovotny · 2017-06-30T20:14:48Z

NuGet.config

    <!-- add key="CoreCLR Xunit" value="https://www.myget.org/F/coreclr-xunit/api/v3/index.json" / -->
    <!-- add key="AspNet CI Feed" value="https://www.myget.org/F/aspnetcirelease/api/v3/index.json" /> -->
    <add key="myget.org xunit" value="https://www.myget.org/F/xunit/api/v3/index.json" />
+    <add key="myget.org msbuilsdkdextras" value="https://www.myget.org/F/msbuildsdkextras/api/v3/index.json" />


No need for this. NuGet.org has the latest version.

clairernovotny · 2017-06-30T20:15:39Z

src/Shouldly.Tests/Shouldly.Tests.csproj

+    <TargetFramework>net452</TargetFramework>
+    <AssemblyName>Shouldly.Tests</AssemblyName>
+    <PackageId>Shouldly.Tests</PackageId>
+    <GenerateRuntimeConfigurationFiles>true</GenerateRuntimeConfigurationFiles>


I don't think this is required

clairernovotny · 2017-06-30T20:16:14Z

src/Shouldly.Tests/Shouldly.Tests.csproj

+  </ItemGroup>
+
+  <ItemGroup>
+    <PackageReference Include="Microsoft.NET.Test.Sdk" Version="15.0.0-preview-20170106-08" />


I'd use either the stable version or the latest 15.3 preview test sdk version.

clairernovotny · 2017-06-30T20:18:12Z

src/Shouldly/Shouldly.csproj

+    <PackageIconUrl>https://raw.github.com/shouldly/shouldly/master/Icons/package_icon.png</PackageIconUrl>
+    <PackageProjectUrl>http://shouldly.github.com</PackageProjectUrl>
+    <PackageLicenseUrl>https://github.com/shouldly/shouldly/blob/master/LICENSE.txt</PackageLicenseUrl>
+    <NetStandardImplicitPackageVersion Condition=" '$(TargetFramework)' == 'netstandard1.3' ">1.6.0</NetStandardImplicitPackageVersion>


Don't do this -- Remove this line and enable the default implicit ref or 1.6.1 for the netstandard 1.3 tfm. 1.6.0 contains many bugs, esp with Xamarin.

clairernovotny · 2017-06-30T20:18:44Z

src/Shouldly/Shouldly.csproj

+    <IncludeSource>True</IncludeSource>
+
+    <ComVisible>false</ComVisible>
+    <IncludeProjectPriFile>false</IncludeProjectPriFile>


Not sure why you need this? There shouldn't be one generated anyway

clairernovotny · 2017-06-30T20:20:21Z

src/Shouldly/Shouldly.csproj

+    <DefineConstants>$(DefineConstants);Async;Dynamic;NewReflection;ExpressionTrees</DefineConstants>
+  </PropertyGroup>
+
+  <ItemGroup Condition=" '$(TargetFramework)' == 'netstandard1.3' ">


Don't do this....this is considered bad practice now. @terrajobst can explain in detail if you want. Remove this item group and leave the default netstandard.library 1.6.1 if the tfm is a netstandard one.

clairernovotny · 2017-06-30T20:21:02Z

src/Shouldly/Shouldly.csproj

+    <PackageReference Include="System.Reflection.TypeExtensions" Version="4.3.0" />
+    <PackageReference Include="System.Threading.Timer" Version="4.3.0" />
+    <PackageReference Include="Microsoft.CSharp" Version="4.3.0" />
+    <PackageReference Include="MSBuild.Sdk.Extras" Version="1.0.0" PrivateAssets="all" />


Not sure why you need the sdk extras at all? It's really only needed to build project types other than net/netstandard/netcoreapp (like Xamarin, UWP, etc).

clairernovotny · 2017-06-30T20:21:20Z

src/Shouldly/Shouldly.csproj

+    <PackageReference Include="MSBuild.Sdk.Extras" Version="1.0.0" PrivateAssets="all" />
+  </ItemGroup>
+
+  <Import Project="$(MSBuildSDKExtrasTargets)" Condition="Exists('$(MSBuildSDKExtrasTargets)')" />


Can be removed, the extras package shouldn't be needed here.

From what I saw, this is a bug you hit in 15.2, should be fine in 15.2.2.

clairernovotny · 2017-06-30T20:26:02Z

Also, with your net35 target, you may not be able to use dotnet build. You might need msbuild /t:build, msbuild /t:pack, etc.

josephwoodward · 2017-06-30T22:15:53Z

@jeffkl @onovotny You guys are amazing, thanks so much for your help! I ran the migration tool so wasn't sure what I needed vs didn't. I'll give your suggestions a try and see how I get on.

jeffkl · 2017-06-30T23:30:06Z

Glad to help. I'll keep an eye on this PR.

FYI dotnet/msbuild#2250

clairernovotny · 2017-07-04T19:00:18Z

src/Shouldly/Shouldly.csproj

  <PropertyGroup>
    <Description>Shouldly - Assertion framework for .NET. The way asserting *Should* be</Description>
-    <TargetFrameworks>net451;netstandard1.3</TargetFrameworks>
+    <TargetFrameworks>net451;net452;netstandard1.3</TargetFrameworks>


Is there a particular reason you need to support net451 and net452? there's virtually no API surface area different between them; in general, I suggest targeting the fewest frameworks you need to support.

Agreed, I presume this was temporary in an effort to troubleshoot the ci build.

@josephwoodward I've PRed a working build. If you are happy to merge it would be good to get some more eyes on this.

Hi @onovotny
It was a desperate attempt to get the build working before popping out, given the tests were running against 452. Thanks for the PR @slang25, that's all green now - good job! 👍

…et/Home#5021

josephwoodward mentioned this pull request Apr 10, 2017

Migrate from project.json to .csproj #429

Closed

josephwoodward mentioned this pull request May 30, 2017

Object Graph Comparison - ShouldBeEquivalentTo(...) #411

Merged

clairernovotny suggested changes Jun 30, 2017

View reviewed changes

clairernovotny reviewed Jul 4, 2017

View reviewed changes

slang25 mentioned this pull request Jul 4, 2017

Tooling migration continued #441

Merged

josephwoodward mentioned this pull request Jul 4, 2017

Drop support for .NET 3.5? #442

Closed

Joseph Woodward and others added 5 commits July 5, 2017 15:21

Initial tooling migration commit

a5162ed

Add FrameworkPathOverride for net35 to workaround dotnet/msbuild#1333

58f8268

Added <IncludeProjectPriFile>false</IncludeProjectPriFile> to fix NuG…

28ab5ee

…et/Home#5021

Added MSBuild.Sdk.Extras as suggested in NuGet/Home#5021

207837f

Fix build

6b08b7a

josephwoodward force-pushed the ToolingMigration branch from 4188009 to 6b08b7a Compare July 5, 2017 14:25

josephwoodward merged commit c2354fa into master Jul 5, 2017

slang25 mentioned this pull request Aug 1, 2017

Update cake build script for new tooling #452

Merged

SimonCropp deleted the ToolingMigration branch April 1, 2023 05:41

Uh oh!

Tooling migration #432

Tooling migration #432

Uh oh!

Conversation

josephwoodward commented Apr 10, 2017

Uh oh!

slang25 commented Apr 11, 2017

Uh oh!

josephwoodward commented Jun 29, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jeffkl commented Jun 29, 2017

Uh oh!

josephwoodward commented Jun 30, 2017

Uh oh!

jeffkl commented Jun 30, 2017

Uh oh!

clairernovotny left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

clairernovotny commented Jun 30, 2017

Uh oh!

josephwoodward commented Jun 30, 2017

Uh oh!

jeffkl commented Jun 30, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

josephwoodward commented Jun 29, 2017 •

edited

Loading