Skip to content
This repository has been archived by the owner on Jan 23, 2023. It is now read-only.
/ corefx Public archive

Add a "GenerateReferenceSource" target and add four ref assembly projects. #20211

Merged
merged 5 commits into from
May 24, 2017

Conversation

mellinoe
Copy link
Contributor

@mellinoe mellinoe commented May 24, 2017

The GenerateReferenceAssembly target can be run as a manual step on a project. When run, it produces a source code file .cs, in the reference assembly folder for the library. This source file can be used to compile an equivalent reference assembly.

Use the target to generate reference assemblies for the following libraries:

  • System.Collections.Immutable
  • System.Reflection.Metadata
  • System.Threading.Tasks.Dataflow
  • System.Threading.Tasks.Extensions

This will cut a chunk out of the total size of the reference assemblies we are shipping. This is part of https://github.com/dotnet/corefx/issues/14291.

@ericstj @weshaggard

@karelz
Copy link
Member

karelz commented May 24, 2017

I assume the PR should be ported to rel/2.0.0 after it is done ... (based on the original 2.0.0 milestone)

dir.targets Outdated
@@ -110,4 +110,15 @@
<Message Importance="High" Condition="'$(ConfigurationErrorMsg)' != ''" Text="$(MSBuildProjectFullPath) ConfigurationErrorMessage: $(ConfigurationErrorMsg)" />
</Target>

<Target Name="GenerateReferenceAssembly">
Copy link
Member

Choose a reason for hiding this comment

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

This name isn't accurate, can it be "GenerateReferenceSource" instead?

<?xml version="1.0" encoding="utf-8"?>
<Project ToolsVersion="14.0" DefaultTargets="Build" xmlns="http://schemas.microsoft.com/developer/msbuild/2003">
<Import Project="$([MSBuild]::GetDirectoryNameOfFileAbove($(MSBuildThisFileDirectory), dir.props))\dir.props" />
<PropertyGroup Condition="'$(Configuration)|$(Platform)' == 'netcoreapp-Debug|AnyCPU'" />
Copy link
Member

@ericstj ericstj May 24, 2017

Choose a reason for hiding this comment

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

I would have expected a PropertyGroup here with at least a ProjectGuid. If you run UpdateVSConfigurations from root it should fix that. You may need to privately pick up the new buildtools that @weshaggard is merging, #20195. It has a fix to UpdateVSConfigurations to handle some projects recently checked in.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll rebase and then run the target.

Copy link
Member

@ericstj ericstj left a comment

Choose a reason for hiding this comment

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

LGTM, modulo a couple small suggestions

dir.targets Outdated
@@ -110,4 +110,15 @@
<Message Importance="High" Condition="'$(ConfigurationErrorMsg)' != ''" Text="$(MSBuildProjectFullPath) ConfigurationErrorMessage: $(ConfigurationErrorMsg)" />
</Target>

<Target Name="GenerateReferenceAssembly">
<PropertyGroup>
Copy link
Member

Choose a reason for hiding this comment

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

For reference I do this in the standard repo at https://github.com/dotnet/standard/blob/master/netstandard/src/netstandard.csproj#L45 we should share the common pattern.

dir.targets Outdated
@@ -110,4 +110,15 @@
<Message Importance="High" Condition="'$(ConfigurationErrorMsg)' != ''" Text="$(MSBuildProjectFullPath) ConfigurationErrorMessage: $(ConfigurationErrorMsg)" />
</Target>

<Target Name="GenerateReferenceAssembly">
Copy link
Member

Choose a reason for hiding this comment

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

Should we hook this target up to the build so it will start to pend changes if any APIs change and hopefully we don't accidentally forget to update a ref or someone update it manually?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Right now this will just be manual. @ericstj and I were discussing a few different things we would need to work through if we want to make it automatic in some way. And like you mentioned, the ref's aren't fully "standardized" right now, so it would cause a lot of churn at first.

@@ -135,5 +135,10 @@
<None Include="Resources\Misc\Signed.cs" />
<EmbeddedResource Include="Resources\Misc\Signed.exe" />
</ItemGroup>
<ItemGroup>
<!-- Some internal types are needed, so we reference the implementation assembly, rather than the reference assembly. -->
Copy link
Member

Choose a reason for hiding this comment

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

boo...

@@ -0,0 +1,6 @@
// These attributes should be excluded from reference assemblies.

Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll sync my version up with that.

@weshaggard
Copy link
Member

We should consider moving this target into BuildTools as some future point. We should also consider running this for all the ref's across the repo to get them consistent. I know that @stephentoub did some work in the past to try and normalize the refs.

mellinoe added 4 commits May 24, 2017 11:13
* The GenerateReferenceAssembly target can be run as a manual step on a
  project. When run, it produces a source code file <Assembly>.cs, in the
  reference assembly folder for the library. This source file can be used
  to compile an equivalent reference assembly.
* Use the target to generate reference assemblies for the following
  libraries:
  * System.Collections.Immutable
  * System.Reflection.Metadata
  * System.Threading.Tasks.Dataflow
  * System.Threading.Tasks.Extensions
@mellinoe mellinoe changed the title Add a "GenerateReferenceAssembly" target and generate four assemblies. Add a "GenerateReferenceSource" target and add four ref assembly projects. May 24, 2017
@mellinoe
Copy link
Contributor Author

@weshaggard I've filed https://github.com/dotnet/corefx/issues/20258 to track moving the target into buildtools.

@karelz
Copy link
Member

karelz commented May 24, 2017

@mellinoe is it must-have for 2.0? If yes, can you please file tracking bug to port it to 2.0?

@mellinoe
Copy link
Contributor Author

@dotnet-bot Test Innerloop netfx Windows_NT Release x64 Build and Test

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants