Skip to content

Commit

Permalink
Package linker for SDK (dotnet/linker#532)
Browse files Browse the repository at this point in the history
For dotnet/sdk#3125.

This most notably adds a net472 build of ILLink.Tasks, and modifies Sdk.props to import the correct build of the tasks, depending on the MSBuild runtime type. This makes it possible to use the linker tasks on desktop MSBuild once again. Regardless of the MSBuild runtime, illink.dll will run on .NET Core. We set 'rollForwardOnNoCandidateFx' to allow running on .NET Core 3 (even though illink.dl is built targeting netcoreapp2.0).

This change removes many workarounds from the old packaging logic, as we can now use NuGet extension points and avoid using a custom .nuspec.

Commit migrated from dotnet/linker@010f98e
  • Loading branch information
sbomer authored and marek-safar committed Apr 17, 2019
1 parent 3e2e67f commit a8d99d4
Show file tree
Hide file tree
Showing 7 changed files with 78 additions and 210 deletions.
37 changes: 0 additions & 37 deletions src/tools/illink/src/ILLink.Tasks/AdapterLogger.cs

This file was deleted.

207 changes: 63 additions & 144 deletions src/tools/illink/src/ILLink.Tasks/ILLink.Tasks.csproj
Original file line number Diff line number Diff line change
@@ -1,103 +1,30 @@
<Project Sdk="Microsoft.NET.Sdk">
<Import Project="$(MSBuildThisFileDirectory)../../eng/Versions.props" Condition=" '$(ArcadeBuild)' != 'true' " />
<PropertyGroup>
<!-- net46 build is disabled until cecil uses SDK-style projects. -->
<TargetFrameworks>netcoreapp2.0</TargetFrameworks>
<TargetFrameworks>netcoreapp2.0;net472</TargetFrameworks>
<TargetFrameworks Condition=" '$(OS)' != 'Windows_NT' ">netcoreapp2.0</TargetFrameworks>
<EnableDefaultCompileItems>false</EnableDefaultCompileItems>
<BaseOutputPath>bin/</BaseOutputPath>
<PackageOutputPath>$(BaseOutputPath)nupkgs</PackageOutputPath>
<IsPackable>true</IsPackable>
<!-- IsTool true causes the build output to be placed in the
package's tools folder. This allows projects to reference the
tasks package without including the tasks dll in their
output. -->
<!-- TODO: This has no effect currently, because we are using a
custom .nuspec with the tools path hardcoded. Uncomment this
once we are able to remove the custom .nuspec workaround. -->
<!-- <IsTool>true</IsTool> -->

<!-- We want to package the tasks package together with its
package dependencies, the linker, and the linker's
dependencies, in order to prevent projects that consume the
tasks package from pulling in the linker. To do this, we need
to include project references and package references in the
package, and prevent any of these references from being
marked as dependencies in the tasks package.
To include the linker in the package, we want to package the
tasks project together with its project references. This is
not supported by the pack targets
(https://github.com/dotnet/cli/issues/1290,
https://github.com/dotnet/cli/issues/3959), so we work around
this by explicitly setting the package path to include the
build output. Using the publish directory will also cause
dependencies from package references to be packaged.
To prevent the linker from being marked as a package
dependency, we can't use PrivateAssets="All", because this
removes it from the publish output as well, due to an issue
in the SDK (https://github.com/dotnet/sdk/issues/952). To
work around this, we use a custom .nuspec that doesn't
declare any dependencies. This also prevents package
references from being marked as dependencies. -->
<!-- TODO: Remove the custom .nuspec once the P2P PrivateAssets
issue is fixed. -->
<PackageLicenseExpression>MIT</PackageLicenseExpression>
<NuspecFileName>ILLink.Tasks.nuspec</NuspecFileName>
<NuspecFile>$(BaseOutputPath)$(NuspecFileName)</NuspecFile>
<NuspecProperties>id=$(AssemblyName);authors=$(AssemblyName);description=linker tasks;</NuspecProperties>
</PropertyGroup>

<!-- TODO: Remove this workaround once we're able to avoid using a
custom .nuspec. We may still need a similar workaround to
dynamically include the publish output in the package contents.
We can't specify the output path to package in the static
nuspec properties, because the project's output path gets set
at a later point. To work around this, we add the output path
to the nuspec properties dynamically as part of the pack
target. We use the same workaround to set the version in the
.nuspec file.
We can't insert this into the pack target by modifying
PackDependsOn, since GenerateNuspec is always prepended to
PackDependsOn, which would cause the nuspec file to be
generated before our extra properties are added.
Instead, we use GenerateNuspecDependsOn. We could probably also
use BeforeTargets="GenerateNuspec". -->
<PropertyGroup>
<GenerateNuspecDependsOn>SetDynamicNuspecProperties;BinPlacePackageDeps;$(GenerateNuspecDependsOn)</GenerateNuspecDependsOn>
<Description>MSBuild tasks for running the IL Linker</Description>
<Authors>$(AssemblyName)</Authors>
<!-- Don't include the build output. Instead, we want to include
the TargetFramework-specific publish output. -->
<IncludeBuildOutput>false</IncludeBuildOutput>
<!-- We want to package the tasks package together with its
transitive dependencies and the linker, without marking them
as dependencies in the tasks package.
Pack doesn't support including project references
(https://github.com/NuGet/Home/issues/3891), so we work
around this by explicitly including the publish output in the
package. -->
<TargetsForTfmSpecificContentInPackage>$(TargetsForTfmSpecificContentInPackage);_LayoutPackage</TargetsForTfmSpecificContentInPackage>
</PropertyGroup>
<Target Name="SetDynamicNuspecProperties"
DependsOnTargets="LayoutPackage">
<PropertyGroup>
<NuspecProperties>$(NuspecProperties)version=$(Version);</NuspecProperties>
</PropertyGroup>
</Target>

<!-- This target is necessary because the .nuspec includes the full
path of the selected dlls in the package layout. We want the
assets to be included in the package without the bin prefix, so
we place the .nuspec and the included targets alongside the
publish directories in the bin directory. -->
<Target Name="BinPlacePackageDeps">
<Copy SourceFiles="$(NuspecFileName)" DestinationFolder="$(BaseOutputPath)" />
<Copy SourceFiles="ILLink.Tasks.targets" DestinationFolder="$(BaseOutputPath)" />
</Target>

<Target Name="LayoutPackage">
<ItemGroup>
<TFMsToPublish Include="$(TargetFrameworks)" />
<ProjectsToPublish Include="$(MSBuildProjectFile)">
<AdditionalProperties>TargetFramework=%(TFMsToPublish.Identity);PublishDir=$(BaseOutputPath)%(TFMsToPublish.Identity)</AdditionalProperties>
</ProjectsToPublish>
</ItemGroup>
<MSBuild Projects="@(ProjectsToPublish)" Targets="Publish" />
</Target>

<ItemGroup>
<Compile Include="AdapterLogger.cs" />
<Compile Include="LinkTask.cs" />
<Compile Include="CheckEmbeddedRootDescriptor.cs" />
<Compile Include="CompareSizes.cs" />
Expand All @@ -110,59 +37,53 @@
<Compile Include="FindNativeDeps.cs" />
<Compile Include="SetAssemblyActions.cs" />
<Compile Include="Utils.cs" />
<Content Include="Sdk\Sdk.props">
<CopyToOutputDirectory>Always</CopyToOutputDirectory>
</ItemGroup>

<ItemGroup>
<Content Include="ILLink.Tasks.targets">
<PackagePath>build</PackagePath>
</Content>
<Content Include="Sdk/Sdk.props">
<PackagePath>Sdk</PackagePath>
</Content>
</ItemGroup>

<!-- TODO: Uncomment this once we can avoid hard-coding this in a
custom .nuspec. -->
<!-- Targets under the build directory of the package automatically
get included in the consumer's build.
-->
<!--
<ItemGroup>
<Content Include="ILLink.Tasks.targets">
<PackagePath>build</PackagePath>
</Content>
</ItemGroup>
-->

<!-- TODO: Use this to set the package contents once we can avoid
using a custom .nuspec. -->
<!-- We can't glob everything in the output path for two reasons:
1. Content gets expanded before "Build" is called during
"Pack". We could work around this by creating our own target to
call build and then pack, but it would be nice to avoid
changing the build/package pipeline.
2. We'll try to include the tasks dll twice. This only causes a
warning during pack, but should be avoided.
<Content Include="$(OutputPath)*.dll;$(OutputPath)*.json">
<PackagePath>tools</PackagePath>
</Content>
There may also be a better ItemGroup to use than
Content. Content semantics mean to output with the build, which
isn't our goal. Instead, we want to include these dependencies
in the package without necessarily including them in the build.
-->
<!--
<ItemGroup>
<Content Include="$(OutputPath)illink.dll;$(OutputPath)Mono.Cecil.dll">
<PackagePath>tools</PackagePath>
</Content>
</ItemGroup>
-->
<Target Name="_LayoutPackage">

<PropertyGroup>
<PublishDir>$(BaseOutputPath)$(TargetFramework)</PublishDir>
</PropertyGroup>

<ItemGroup>
<ProjectsToPublish Include="$(MSBuildProjectFile)">
<AdditionalProperties>TargetFramework=$(TargetFramework);PublishDir=$(PublishDir)</AdditionalProperties>
</ProjectsToPublish>
</ItemGroup>

<!-- Clean the publish directory in case there are any left-over
artifacts (publish does not work incrementally). -->
<ItemGroup>
<_FilesToDelete Remove="@(_FilesToDelete)" />
<_FilesToDelete Include="$(PublishDir)/*.dll" />
<_FilesToDelete Include="$(PublishDir)/*.json" />
</ItemGroup>
<Delete Files="@(_FilesToDelete)" />

<MSBuild Projects="@(ProjectsToPublish)" Targets="Publish" />
<ItemGroup>
<TfmSpecificPackageFile Include="$(PublishDir)/ILLink.*.dll" PackagePath="tools\$(TargetFramework)" />
<TfmSpecificPackageFile Include="$(PublishDir)/Mono.Cecil.*.dll" PackagePath="tools\$(TargetFramework)" />
<TfmSpecificPackageFile Include="$(PublishDir)/illink.dll" PackagePath="tools\$(TargetFramework)"
Condition=" '$(TargetFramework)' == 'netcoreapp2.0' " />
<TfmSpecificPackageFile Include="$(PublishDir)/*.json" PackagePath="tools\$(TargetFramework)" />
</ItemGroup>

</Target>

<ItemGroup>
<!-- TODO: Once https://github.com/dotnet/sdk/issues/952 is fixed,
use PrivateAssets="All" to prevent this project reference
from being marked as a dependency of the tasks package (while
still including it in the publish output). -->
<ProjectReference Include="../linker/Mono.Linker.csproj">
<ProjectReference Include="../linker/Mono.Linker.csproj"
PrivateAssets="All"
Condition=" '$(TargetFramework)' == 'netcoreapp2.0' ">
<!-- SetConfiguration isn't required when the configuration is
already set in the solution. Setting it here allows packing
the tasks csproj on its own. This lets us avoid some of the
Expand All @@ -185,9 +106,12 @@
-->
<SetConfiguration>Configuration=illink_$(Configuration)</SetConfiguration>
</ProjectReference>
<ProjectReference Include="../../external/cecil/Mono.Cecil.csproj" />
<ProjectReference Include="../../external/cecil/Mono.Cecil.csproj"
PrivateAssets="All" />

<ProjectReference Include="../ILLink.CustomSteps/ILLink.CustomSteps.csproj">
<ProjectReference Include="../ILLink.CustomSteps/ILLink.CustomSteps.csproj"
PrivateAssets="All"
Condition=" '$(TargetFramework)' == 'netcoreapp2.0' ">
<SetConfiguration>Configuration=illink_$(Configuration)</SetConfiguration>
</ProjectReference>
</ItemGroup>
Expand All @@ -205,11 +129,6 @@

<ItemGroup>

<!-- TODO: Once we can avoid using a custom .nuspec, we should be
able to set PrivateAssets="All" on these packages to prevent
them from becoming package dependencies, and use an msbuild
itemgroup to include their assets in the package instead of
passing the publish path to the .nuspec. -->
<!-- We use private assets for the Microsoft.Build packages to
prevent them from being published with the tasks dll, because
these are already a part of the SDK. -->
Expand Down
16 changes: 0 additions & 16 deletions src/tools/illink/src/ILLink.Tasks/ILLink.Tasks.nuspec

This file was deleted.

3 changes: 2 additions & 1 deletion src/tools/illink/src/ILLink.Tasks/LinkTask.cs
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,8 @@ public string ILLinkPath {
return _illinkPath;
}
var taskDirectory = Path.GetDirectoryName (Assembly.GetExecutingAssembly ().Location);
_illinkPath = Path.Combine (taskDirectory, "illink.dll");
// The linker always runs on .NET Core, even when using desktop MSBuild to host ILLink.Tasks.
_illinkPath = Path.Combine (Path.GetDirectoryName (taskDirectory), "netcoreapp2.0", "illink.dll");
return _illinkPath;
}
set => _illinkPath = value;
Expand Down
11 changes: 10 additions & 1 deletion src/tools/illink/src/ILLink.Tasks/Sdk/Sdk.props
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,15 @@ Copyright (c) .NET Foundation. All rights reserved.
-->
<Project ToolsVersion="14.0">

<UsingTask TaskName="ILLink.Tasks.ILLink" AssemblyFile="$(MSBuildThisFileDirectory)..\tools\netcoreapp2.0\ILLink.Tasks.dll" />
<PropertyGroup>
<_ILLinkTasksDirectoryRoot Condition=" '$(_ILLinkTasksDirectoryRoot)' == '' ">$(MSBuildThisFileDirectory)../tools/</_ILLinkTasksDirectoryRoot>
<_ILLinkTasksTFM Condition=" '$(MSBuildRuntimeType)' == 'Core' ">netcoreapp2.0</_ILLinkTasksTFM>
<_ILLinkTasksTFM Condition=" '$(_ILLinkTasksTFM)' == '' ">net472</_ILLinkTasksTFM>
<_ILLinkTasksDirectory>$(_ILLinkTasksDirectoryRoot)$(_ILLinkTasksTFM)/</_ILLinkTasksDirectory>
<ILLinkTasksAssembly Condition=" '$(ILLinkTasksAssembly)' == '' ">$(_ILLinkTasksDirectory)ILLink.Tasks.dll</ILLinkTasksAssembly>
</PropertyGroup>

<UsingTask TaskName="ILLink.Tasks.ILLink" AssemblyFile="$(ILLinkTasksAssembly)" />
<UsingTask TaskName="ILLink.Tasks.ComputeManagedAssemblies" AssemblyFile="$(ILLinkTasksAssembly)" />

</Project>
11 changes: 0 additions & 11 deletions src/tools/illink/src/ILLink.Tasks/Utils.cs
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
using System;
using Mono.Cecil;
using Mono.Linker;

public static class Utils
{
Expand All @@ -13,14 +12,4 @@ public static bool IsManagedAssembly (string fileName)
return false;
}
}

public static bool IsCrossgenedAssembly (string fileName)
{
try {
ModuleDefinition module = ModuleDefinition.ReadModule (fileName);
return module.IsCrossgened ();
} catch (BadImageFormatException) {
return false;
}
}
}
3 changes: 3 additions & 0 deletions src/tools/illink/src/linker/runtimeconfig.template.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
{
"rollForwardOnNoCandidateFx": 2
}

0 comments on commit a8d99d4

Please sign in to comment.