-
Notifications
You must be signed in to change notification settings - Fork 388
Improve build tasks packaging #299
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
Changes from all commits
c35018a
01c0431
ec01515
77288fb
e4b29b4
ca66f0f
546c274
e3a5191
0aa1891
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
This file was deleted.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,20 +2,62 @@ | |
|
||
<PropertyGroup> | ||
<OutputType>Library</OutputType> | ||
<TargetFramework>netcoreapp2.0</TargetFramework> | ||
<TargetFrameworks>netcoreapp2.0;net472</TargetFrameworks> | ||
sharwell marked this conversation as resolved.
Show resolved
Hide resolved
|
||
<AssemblyVersion>2.4.1</AssemblyVersion> | ||
|
||
<PackageId>coverlet.msbuild</PackageId> | ||
<PackageVersion>2.5.1</PackageVersion> | ||
<Title>coverlet.msbuild</Title> | ||
<Authors>tonerdo</Authors> | ||
<PackageLicenseUrl>https://github.com/tonerdo/coverlet/blob/master/LICENSE</PackageLicenseUrl> | ||
<PackageProjectUrl>http://github.com/tonerdo/coverlet</PackageProjectUrl> | ||
<PackageIconUrl>https://raw.githubusercontent.com/tonerdo/coverlet/master/_assets/coverlet-icon.svg?sanitize=true</PackageIconUrl> | ||
<PackageRequireLicenseAcceptance>false</PackageRequireLicenseAcceptance> | ||
<DevelopmentDependency>true</DevelopmentDependency> | ||
<Description>Coverlet is a cross platform code coverage library for .NET Core, with support for line, branch and method coverage.</Description> | ||
<PackageTags>coverage testing unit-test lcov opencover quality</PackageTags> | ||
<GeneratePackageOnBuild>true</GeneratePackageOnBuild> | ||
|
||
<!-- Items in the Content group are placed in this folder in the NuGet package. --> | ||
<ContentTargetFolders>build</ContentTargetFolders> | ||
|
||
<!-- Build tasks should not be added to the lib folder. --> | ||
<IncludeBuildOutput>false</IncludeBuildOutput> | ||
<IncludeSymbols>true</IncludeSymbols> | ||
<IncludeSources>true</IncludeSources> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Any reason we're including symbols and sources? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It can improve the debugging experience if needed. We can alter the way this is included in a future PR if desired. |
||
|
||
<CopyLocalLockFileAssemblies>true</CopyLocalLockFileAssemblies> | ||
<TargetsForTfmSpecificContentInPackage>$(TargetsForTfmSpecificContentInPackage);PackBuildOutputs</TargetsForTfmSpecificContentInPackage> | ||
</PropertyGroup> | ||
|
||
<ItemGroup> | ||
<PackageReference Include="Microsoft.Build.Utilities.Core" Version="15.5.180" /> | ||
<PackageReference Include="Microsoft.Build.Utilities.Core" Version="15.5.180" PrivateAssets="All" /> | ||
</ItemGroup> | ||
|
||
<ItemGroup> | ||
<ProjectReference Include="$(MSBuildThisFileDirectory)..\coverlet.core\coverlet.core.csproj" /> | ||
<ProjectReference Include="$(MSBuildThisFileDirectory)..\coverlet.core\coverlet.core.csproj" PrivateAssets="All" /> | ||
</ItemGroup> | ||
|
||
<ItemGroup> | ||
<Compile Include="..\coverlet.console\ConsoleTables\ConsoleTable.cs" Link="ConsoleTables\ConsoleTable.cs" /> | ||
</ItemGroup> | ||
|
||
<ItemGroup> | ||
<Content Include="coverlet.msbuild.props" /> | ||
<Content Include="coverlet.msbuild.targets" /> | ||
</ItemGroup> | ||
|
||
<Target Name="PackBuildOutputs" DependsOnTargets="ResolveProjectReferences;SatelliteDllsProjectOutputGroup;DebugSymbolsProjectOutputGroup;SatelliteDllsProjectOutputGroupDependencies;ResolveAssemblyReferences"> | ||
<ItemGroup> | ||
<TfmSpecificPackageFile Include="$(TargetPath)" PackagePath="build\$(TargetFramework)\" /> | ||
<TfmSpecificPackageFile Include="$(DepsFilePath)" PackagePath="build\$(TargetFramework)\" /> | ||
<TfmSpecificPackageFile Include="@(DebugSymbolsProjectOutputGroupOutput)" PackagePath="build\$(TargetFramework)\" /> | ||
<!--<TfmSpecificPackageFile Include="@(SatelliteDllsProjectOutputGroupDependency)" PackagePath="build\$(TargetFramework)\%(SatelliteDllsProjectOutputGroupDependency.DestinationSubDirectory)" />--> | ||
<!--<TfmSpecificPackageFile Include="@(SatelliteDllsProjectOutputGroupOutput->'%(FinalOutputPath)')" PackagePath="build\$(TargetFramework)\%(SatelliteDllsProjectOutputGroupOutput.Culture)\" />--> | ||
<TfmSpecificPackageFile Include="%(_ResolvedProjectReferencePaths.Identity)" PackagePath="build\$(TargetFramework)\" /> | ||
|
||
<TfmSpecificPackageFile Include="@(ReferenceCopyLocalPaths)" Exclude="@(_ResolvedProjectReferencePaths)" PackagePath="build\$(TargetFramework)\%(ReferenceCopyLocalPaths.DestinationSubPath)" /> | ||
</ItemGroup> | ||
</Target> | ||
|
||
</Project> |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,7 +1,7 @@ | ||
<Project Sdk="Microsoft.NET.Sdk"> | ||
|
||
<PropertyGroup> | ||
<TargetFramework>netstandard2.0</TargetFramework> | ||
<TargetFrameworks>netcoreapp2.0;net472</TargetFrameworks> | ||
sharwell marked this conversation as resolved.
Show resolved
Hide resolved
|
||
</PropertyGroup> | ||
|
||
</Project> |
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.
If this is indeed a functional workaround to using
Path.GetRelativePath
then we can makecoverlet.msbuild
be fully compliant withnetcoreapp2.0
andnet472
frameworks by making it targetnetstandard2.0
insteadUh oh!
There was an error while loading. Please reload this page.
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.
To my knowledge Coverlet worked quite well with
net472
when the target wasnetstandard2.0
. Only got issues when I changed it tonetcoreapp2.0
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 is even working with net35
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.
@sharwell are you still able to this? If not, I'll just merge it in and make the necessary changes if that's alright with you
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.
Thanks for your help so far
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.
@tonerdo I wanted to merge this one like this, then update and merge #301, and only then start to focus on enabling .NET Framework scenarios. Currently testing for the latter is completely blocked so I can't say for sure what will or will not work.
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.
I'd prefer if it gets done in this PR. #301 might take a bit to get through and I want the next release (which I want to happen this week) to fix the .NET Framework issue
Uh oh!
There was an error while loading. Please reload this page.
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.
I'm not sure I'll have time to make and test this modification. I'll try to get it done tomorrow morning though (produce one netstandard2.0 assembly, which is included twice in the resulting package under two different paths).
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.
No worries. I'll go ahead and merge this and effect the necessary changes. Thank you so much for all your work on this