Skip to content
This repository was archived by the owner on Jan 23, 2023. It is now read-only.

Updating S.R.CS.Unsafe to be built using the ilproj sdk. #31512

Merged
merged 1 commit into from
Aug 24, 2018
Merged

Updating S.R.CS.Unsafe to be built using the ilproj sdk. #31512

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions Directory.Build.props
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,8 @@
<!-- Input Directories -->
<PackagesDir>$(DotNetRestorePackagesPath)</PackagesDir>
<PackagesDir Condition="'$(PackagesDir)'==''">$(ProjectDir)packages/</PackagesDir>
<RestorePackagesPath Condition="'$(RestorePackagesPath)'==''">$(PackagesDir)</RestorePackagesPath>
<NuGetPackageRoot Condition="'$(NuGetPackageRoot)'==''">$(PackagesDir)</NuGetPackageRoot>
Copy link
Member Author

@tannergooding tannergooding Aug 9, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Due to the way restore happens for CoreFX, we need to ensure that this is set so that it can be used by the ilproj-sdk.

<ToolsDir Condition="'$(ToolsDir)'==''">$(ProjectDir)Tools/</ToolsDir>
<IlasmToolPath>$(ToolsDir)ilasm/ilasm</IlasmToolPath>
<CodeAnalysisRuleset>$(MSBuildThisFileDirectory)CodeAnalysis.ruleset</CodeAnalysisRuleset>
Expand Down Expand Up @@ -149,6 +151,7 @@
<_runtimeOS Condition="'$(_runtimeOS)' == 'tizen.4.0.0'">ubuntu.14.04</_runtimeOS>
<_runtimeOS Condition="'$(PortableBuild)' == 'true'">$(_portableOS)</_runtimeOS>
<ToolRuntimeRID>$(_runtimeOS)-x64</ToolRuntimeRID>
<MicrosoftNetCoreIlasmPackageRuntimeId Condition="'$(MicrosoftNetCoreIlasmPackageRuntimeId)' == ''">$(ToolRuntimeRID)</MicrosoftNetCoreIlasmPackageRuntimeId>
Copy link
Member Author

@tannergooding tannergooding Aug 22, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated to use ToolRuntimeRID... Noting that I don't think this works today for building on a 32-bit OS, is that a "known" limitation?

Copy link
Member

@eerhardt eerhardt Aug 22, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we have any builds that work on a 32-bit OS.... Do we? The only 32-bit builds that I know of are win-x86 and win-arm (which we build on an x64 Windows machine) and linux-arm (which we build on an x64 Linux machine).

Copy link
Member Author

@tannergooding tannergooding Aug 22, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we have any CI legs, but this was more a question of "Do we support building 32-bit CoreFX from 32-bit Windows?". If we don't, this is probably fine; otherwise, this should probably be updated to be "correct".

Copy link
Member

@eerhardt eerhardt Aug 22, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure if that is supported. @weshaggard ?

Copy link
Member

@weshaggard weshaggard Aug 22, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not aware of us supporting 32-bit OS's. I think using the ToolRuntimeRID here should be just fine.

Copy link
Member

@eerhardt eerhardt Aug 24, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This property is only used in 1 spot. Does it need to be in the root .props file that everyone imports?

Copy link
Member Author

@tannergooding tannergooding Aug 24, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we can move it into the ilproj, but I will handle in a follow-up PR to avoid more maestro merge-conflicts.


<!-- support cross-targeting by choosing a RID to restore when running on a different machine that what we're build for -->
<_portableOS Condition="'$(OSGroup)' == 'Unix' AND '$(_runtimeOSFamily)' != 'osx' AND '$(_runtimeOSFamily)' != 'FreeBSD'">linux</_portableOS>
Expand Down
17 changes: 12 additions & 5 deletions dependencies.props
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
<CoreFxExpectedPrerelease>preview1-26824-01</CoreFxExpectedPrerelease>
<MicrosoftNETCorePlatformsPackageVersion>3.0.0-preview1-26824-01</MicrosoftNETCorePlatformsPackageVersion>
<MicrosoftNETCoreRuntimeCoreCLRPackageVersion>3.0.0-preview1-26824-01</MicrosoftNETCoreRuntimeCoreCLRPackageVersion>
<MicrosoftNetCoreIlasmPackageVersion>3.0.0-preview1-26824-01</MicrosoftNetCoreIlasmPackageVersion>
<ProjectNTfsExpectedPrerelease>beta-26823-00</ProjectNTfsExpectedPrerelease>
<ProjectNTfsTestILCExpectedPrerelease>beta-26823-00</ProjectNTfsTestILCExpectedPrerelease>
<ProjectNTfsTestILCPackageVersion>1.0.0-beta-26823-00</ProjectNTfsTestILCPackageVersion>
Expand Down Expand Up @@ -136,6 +137,17 @@
<ElementName>MicrosoftNETCoreRuntimeCoreCLRPackageVersion</ElementName>
<PackageId>Microsoft.NETCore.Runtime.CoreCLR</PackageId>
</XmlUpdateStep>
<XmlUpdateStep Include="CoreClr">
<Path>$(MSBuildThisFileFullPath)</Path>
<ElementName>MicrosoftNetCoreIlasmPackageVersion</ElementName>
<PackageId>Microsoft.NETCore.ILAsm</PackageId>
</XmlUpdateStep>
<UpdateStep Include="CoreCLR">
<UpdaterType>MSBuildSdk</UpdaterType>
Copy link
Member

@weshaggard weshaggard Aug 21, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice I didn't realize we already had this updater.

Copy link
Member

@eerhardt eerhardt Aug 21, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We didn't. 😄 @tannergooding added it. dotnet/buildtools#2117

Welcome back, BTW.

Copy link
Member

@weshaggard weshaggard Aug 21, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great thanks @tannergooding for adding this support.

<Path>$(MSBuildThisFileDirectory)global.json</Path>
<PackageId>Microsoft.NET.Sdk.IL</PackageId>
<MSBuildSdkName>Microsoft.NET.Sdk.IL</MSBuildSdkName>
</UpdateStep>
<XmlUpdateStep Include="Standard">
<Path>$(MSBuildThisFileFullPath)</Path>
<ElementName>NETStandardLibraryPackageVersion</ElementName>
Expand Down Expand Up @@ -180,11 +192,6 @@
<UpdaterType>File</UpdaterType>
<Path>$(MSBuildThisFileDirectory)BuildToolsVersion.txt</Path>
<PackageId>Microsoft.DotNet.BuildTools</PackageId>
</UpdateStep>
<UpdateStep Include="BuildTools">
<UpdaterType>File</UpdaterType>
<Path>$(MSBuildThisFileDirectory)tools-local/ILAsmVersion.txt</Path>
<PackageId>Microsoft.NETCore.ILAsm</PackageId>
</UpdateStep>
<XmlUpdateStep Include="BuildTools">
<Path>$(MSBuildThisFileFullPath)</Path>
Expand Down
1 change: 1 addition & 0 deletions external/dir.proj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
<Project Include="netstandard/netstandard.depproj" />
<Project Include="netfx/netfx.depproj" />
<Project Include="runtime/runtime.depproj" />
<Project Include="tools/tools.depproj" />
<Project Include="test-runtime/XUnit.Runtime.depproj" Condition="'$(DotNetBuildFromSource)' != 'true'" />
<Project Include="harvestPackages/harvestPackages.depproj" />
<Project Include="binplacePackages/binplacePackages.depproj" />
Expand Down
15 changes: 15 additions & 0 deletions external/tools/Configurations.props
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
<?xml version="1.0" encoding="utf-8"?>
<Project ToolsVersion="14.0" DefaultTargets="Build" xmlns="http://schemas.microsoft.com/developer/msbuild/2003">
<PropertyGroup>
<BuildConfigurations>
netcoreapp-Windows_NT;
netcoreappaot-Windows_NT;
netcoreapp-Unix;
uap10.0.16299;
uap10.0.16299aot;
uap;
uapaot;
mono;
</BuildConfigurations>
</PropertyGroup>
</Project>
22 changes: 22 additions & 0 deletions external/tools/tools.depproj
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
<?xml version="1.0" encoding="utf-8"?>
<Project ToolsVersion="14.0" xmlns="http://schemas.microsoft.com/developer/msbuild/2003" DefaultTargets="Build">
<Import Project="$([MSBuild]::GetDirectoryNameOfFileAbove($(MSBuildThisFileDirectory), Directory.Build.props))\Directory.Build.props" />

<PropertyGroup>
<EnableBinPlacing>false</EnableBinPlacing>
Copy link
Member Author

@tannergooding tannergooding Aug 23, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Needed to set EnableBinPlacing=false to fix the x86 failures (the x64 coreclr.dll was being binplaced in the test folder otherwise)

<NugetRuntimeIdentifier>$(ToolRuntimeRID)</NugetRuntimeIdentifier>
<RidSpecificAssets>true</RidSpecificAssets>
<NoWarn>$(NoWarn);NU1603;NU1605</NoWarn>
</PropertyGroup>

<ItemGroup>
<PackageReference Include="Microsoft.NETCore.ILAsm">
<Version>$(MicrosoftNetCoreIlasmPackageVersion)</Version>
</PackageReference>
<PackageReference Include="Microsoft.NETCore.ILDAsm">
<Version>$(MicrosoftNetCoreIlasmPackageVersion)</Version>
Copy link
Member

@eerhardt eerhardt Aug 24, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there ever a case where these version numbers will be different? I see we are using the ILAsm version number to get the ILDAsm package.

Copy link
Member Author

@tannergooding tannergooding Aug 24, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They should always be the same.

</PackageReference>
</ItemGroup>

<Import Project="$([MSBuild]::GetDirectoryNameOfFileAbove($(MSBuildThisFileDirectory), Directory.Build.targets))\Directory.Build.targets" />
</Project>
5 changes: 5 additions & 0 deletions global.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
{
"msbuild-sdks": {
"Microsoft.NET.Sdk.IL": "3.0.0-preview1-26824-01"
}
}
3 changes: 0 additions & 3 deletions init-tools.cmd
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,9 +72,6 @@ if NOT exist "%BUILD_TOOLS_PATH%\init-tools.cmd" (

:afterbuildtoolsrestore

:: Ask init-tools to also restore ILAsm
set /p ILASMCOMPILER_VERSION=< "%~dp0tools-local\ILAsmVersion.txt"

echo Initializing BuildTools...
echo Running: "%BUILD_TOOLS_PATH%\init-tools.cmd" "%~dp0" "%DOTNET_CMD%" "%TOOLRUNTIME_DIR%" >> "%INIT_TOOLS_LOG%"
call "%BUILD_TOOLS_PATH%\init-tools.cmd" "%~dp0" "%DOTNET_CMD%" "%TOOLRUNTIME_DIR%" "%PACKAGES_DIR%" >> "%INIT_TOOLS_LOG%"
Expand Down
15 changes: 0 additions & 15 deletions init-tools.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ if [ -z "${__BUILDTOOLS_SOURCE:-}" ]; then __BUILDTOOLS_SOURCE=https://dotnet.my
export __BUILDTOOLS_USE_CSPROJ=true
__BUILD_TOOLS_PACKAGE_VERSION=$(cat "$__scriptpath/BuildToolsVersion.txt" | sed 's/\r$//') # remove CR if mounted repo on Windows drive
__DOTNET_TOOLS_VERSION=$(cat "$__scriptpath/DotnetCLIVersion.txt" | sed 's/\r$//') # remove CR if mounted repo on Windows drive
__ILASM_VERSION=$(cat "$__scriptpath/tools-local/ILAsmVersion.txt" | sed 's/\r$//') # remove CR if mounted repo on Windows drive
__BUILD_TOOLS_PATH="$__PACKAGES_DIR/microsoft.dotnet.buildtools/$__BUILD_TOOLS_PACKAGE_VERSION/lib"
__INIT_TOOLS_RESTORE_PROJECT="$__scriptpath/init-tools.msbuild"
__BUILD_TOOLS_SEMAPHORE="$__TOOLRUNTIME_DIR/$__BUILD_TOOLS_PACKAGE_VERSION/init-tools.complete"
Expand Down Expand Up @@ -157,20 +156,6 @@ if [ ! -e "$__BUILD_TOOLS_PATH" ]; then
fi
fi

if [ -z "${__ILASM_RID-}" ]; then
__ILASM_RID=$__PKG_RID
fi

echo "Using RID $__ILASM_RID for BuildTools native tools"

export ILASMCOMPILER_VERSION=$__ILASM_VERSION
export NATIVE_TOOLS_RID=$__ILASM_RID

if [ -n "${DotNetBootstrapCliTarPath-}" ]; then
# Assume ilasm is not in nuget yet when bootstrapping...
unset ILASMCOMPILER_VERSION
fi

echo "Initializing BuildTools..."
echo "Running: $__BUILD_TOOLS_PATH/init-tools.sh $__scriptpath $__DOTNET_CMD $__TOOLRUNTIME_DIR $__PACKAGES_DIR" >> "$__init_tools_log"

Expand Down
3 changes: 1 addition & 2 deletions src/System.Runtime.CompilerServices.Unsafe/Directory.Build.props
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,5 @@
<AssemblyVersion>4.0.5.0</AssemblyVersion>
<AssemblyKey>MSFT</AssemblyKey>
<IsUAP>true</IsUAP>
<LanguageTargets Condition="'$(MSBuildProjectExtension)' == '.ilproj'">$(ToolsDir)\IL.targets</LanguageTargets>
Copy link
Member

@eerhardt eerhardt Jul 31, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are we going to delete the IL.targets from buildtools? It would be good to rip it out so we know that it is no longer being used.

Copy link
Member Author

@tannergooding tannergooding Jul 31, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

First, CoreCLR needs to move all of its ilproj to the SDK as well.

Copy link
Member Author

@tannergooding tannergooding Jul 31, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CC. @AaronRobinsonMSFT

</PropertyGroup>
</Project>
</Project>
19 changes: 17 additions & 2 deletions src/System.Runtime.CompilerServices.Unsafe/src/System.Runtime.CompilerServices.Unsafe.ilproj
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
<Project Sdk="Microsoft.NET.Sdk">
<Project Sdk="Microsoft.NET.Sdk.IL">
<PropertyGroup>
<DocumentationFile>$(MSBuildThisFileDirectory)System.Runtime.CompilerServices.Unsafe.xml</DocumentationFile>
<ProjectGuid>{04BA3E3C-6979-4792-B19E-C797AD607F42}</ProjectGuid>
Expand All @@ -8,18 +8,29 @@
<!-- cannot build on unix, but package as OS-agnostic -->
<PackageTargetRuntime />
<Configurations>netcoreapp-Debug;netcoreapp-Release;netcoreapp2.0-Debug;netcoreapp2.0-Release;netstandard-Debug;netstandard-Release;netstandard1.0-Debug;netstandard1.0-Release</Configurations>
<MicrosoftNetCoreIldasmPackageName>runtime.$(MicrosoftNetCoreIlasmPackageRuntimeId).microsoft.netcore.ildasm</MicrosoftNetCoreIldasmPackageName>
</PropertyGroup>

<ItemGroup>
<Compile Include="System.Runtime.CompilerServices.Unsafe.il" />
<Reference Include="System.Runtime" />
<PackageReference Include="$(MicrosoftNetCoreIldasmPackageName)" Version="$(MicrosoftNetCoreIlasmPackageVersion)" PrivateAssets="all" />
</ItemGroup>

<Target Name="AddResourcesFileToIlasm"
Copy link
Member Author

@tannergooding tannergooding Aug 22, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This target is very CoreFX centric and doesn't readily apply to the more general ilproj, so I didn't port it over.

BeforeTargets="CoreCompile"
Condition="'$(OS)'=='Windows_NT'"
Inputs="$(ContractOutputPath)/$(MSBuildProjectName).dll"
Outputs="$(IntermediateOutputPath)$(MSBuildProjectName).ref.res.obj">
<ItemGroup>
<_IlasmSourceFiles Include="$(NuGetPackageRoot)\$(MicrosoftNetCoreIldasmPackageName)\$(MicrosoftNetCoreIlasmPackageVersion)\runtimes\$(MicrosoftNetCoreIlasmPackageRuntimeId)\native\**\*" />
<_IlasmSourceFiles Include="$(NuGetPackageRoot)\$(MicrosoftNetCoreRuntimeCoreClrPackageName)\$(MicrosoftNetCoreIlasmPackageVersion)\runtimes\$(MicrosoftNetCoreIlasmPackageRuntimeId)\native\**\*" />
<_IlasmSourceFiles Include="$(NuGetPackageRoot)\$(MicrosoftNetCoreJitPackageName)\$(MicrosoftNetCoreIlasmPackageVersion)\runtimes\$(MicrosoftNetCoreIlasmPackageRuntimeId)\native\**\*" />
</ItemGroup>

<PropertyGroup>
<_IlasmDir>$(ToolsDir)\ilasm</_IlasmDir>

<_IldasmCommand>$(ToolsDir)ilasm/ildasm.exe</_IldasmCommand>
<_IldasmCommand>$(_IldasmCommand) $(ContractOutputPath)/$(MSBuildProjectName).dll</_IldasmCommand>
<_IldasmCommand>$(_IldasmCommand) /OUT=$(IntermediateOutputPath)/$(MSBuildProjectName).ref.il</_IldasmCommand>
Expand All @@ -36,6 +47,10 @@
<FileWrites Include="$(IntermediateOutputPath)$(MSBuildProjectName).ref.*" />
</ItemGroup>

<!-- Having to copy these binaries is really inefficient. https://github.com/dotnet/coreclr/issues/18892 tracks making the ilasm tool self-contained -->
<MakeDir Directories="$(_IlasmDir)" />
<Copy DestinationFolder="$(_IlasmDir)" SourceFiles="@(_IlasmSourceFiles)" />

<!-- Getting the res file by disassemblying the contract assembly -->
<Exec Command="$(_IldasmCommand)" ConsoleToMSBuild="true" StandardOutputImportance="Low">
<Output TaskParameter="ExitCode" PropertyName="_IldasmCommandExitCode" />
Expand All @@ -52,4 +67,4 @@
<IlasmResourceFile>$(IntermediateOutputPath)$(MSBuildProjectName).ref.res.obj</IlasmResourceFile>
</PropertyGroup>
</Target>
</Project>
</Project>
1 change: 0 additions & 1 deletion tools-local/ILAsmVersion.txt
View file
Open in desktop

This file was deleted.