Skip to content
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

Figure out build process for libraries with no reference assembly #19584

Closed
ericstj opened this issue Dec 7, 2016 · 11 comments
Closed

Figure out build process for libraries with no reference assembly #19584

ericstj opened this issue Dec 7, 2016 · 11 comments
Labels
area-Infrastructure-libraries enhancement Product code improvement that does NOT require public API changes/additions
Milestone

Comments

@ericstj
Copy link
Member

ericstj commented Dec 7, 2016

For now I binplace to the TargetingPack directory during source build, but that's not a good long term solution since it will lead folks to add simple name references rather than projectreferences that become race conditions.

Ideas proposed so far:

  1. Add refs for these that we only use for NETCoreApp.
  2. Build them in a separate phase between ref and src builds.
  3. Make them use project references to ref projects and build them during the ref phase.

/cc @mellinoe @weshaggard

@ericstj
Copy link
Member Author

ericstj commented Dec 8, 2016

Thinking about this some more, I think we should auto-generate the reference assembly source. That way it doesn't impose additional complexity on these simple projects while still permitting a reference assembly build. What do you think?

@weshaggard
Copy link
Member

How do you envision auto-generating them? I assume the would still need to be checked in. Do you think having a post step of the src build that automatically updates it?

@mellinoe
Copy link
Contributor

mellinoe commented Dec 8, 2016

Can't we leave everything unchanged and just use P2P references to projects that don't have reference assemblies? I don't see that listed as an option.

@ericstj
Copy link
Member Author

ericstj commented Dec 8, 2016

@mellinoe, do so and leave them out of the TargetingPack folder? If you put them in the TP folder then you won't have any way to force folks to use P2P and they'll introduce race conditions. If you leave them out of the TP folder what do you suggest for the tests?

@mellinoe
Copy link
Contributor

mellinoe commented Dec 8, 2016

Ok, I see what you're getting at. It's problematic if one project uses a ProjectReference and another uses a Reference, because the latter might randomly work depending on the ordering.

I'm not sure that auto-generating will really help, because I think they will still need to be checked in, in order to be built before the source project. In that case, it's not really better than putting them into a ref-stage-2 type thing. How were you envisioning it working?

@ericstj
Copy link
Member Author

ericstj commented Dec 8, 2016

Set a property in the project just autogenerates the ref-cs. That way we can build it with no overhead. We'd only set this property in projects that are intentionally trying to be 1:1. Incidentally the property might also be useful in the case someone wants to code up some API and auto-update the ref-cs (assuming no ifdefs).

@ericstj
Copy link
Member Author

ericstj commented Dec 8, 2016

The reason I prefer this to the staged approach is that the staged approach requires upstack build goo to list out stage 2 projects. It also bloats the package size since we'd have to have the implementation assembly duplicated in the reference folder in our uber packages. So at least to me, that's two strikes against it.

Granted, having a ref project is more complicated, but we mitigate that with autogeneration. At that point its a wash since everything else has a ref and folks can just effectively "forget" about this one since they don't need to touch it. Add to that the benefit of reducing the package size by using a ref in the ref folder and, to me at least, makes this the preferred choice.

@mellinoe mellinoe removed their assignment Mar 8, 2017
@ericstj
Copy link
Member Author

ericstj commented May 23, 2017

@mellinoe why did this get moved to future? These files are bloating our reference package.

@ericstj
Copy link
Member Author

ericstj commented May 23, 2017

Currently this impacts 4 assemblies in netcoreapp:

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

Together these make up 805 KB worth of IL assemblies, about 16% of the total IL in the package. Compressed they make up 340 KB, which is about 14% of the IL in the package.

@mellinoe
Copy link
Contributor

I can quickly create reference projects for these four manually, and we can still think about creating the automated process in the future. I'll try working on it later today.

@weshaggard
Copy link
Member

We have reference assemblies for these and we now have reverse apicompat checks to make sure they stay in-sync.

@msftgits msftgits transferred this issue from dotnet/corefx Jan 31, 2020
@msftgits msftgits added this to the 3.0 milestone Jan 31, 2020
@ghost ghost locked as resolved and limited conversation to collaborators Dec 27, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-Infrastructure-libraries enhancement Product code improvement that does NOT require public API changes/additions
Projects
None yet
Development

No branches or pull requests

4 participants