dotnet / sdk

Core functionality needed to create .NET Core projects, that is shared between Visual Studio and CLI
https://dot.net/core
MIT License
2.7k stars 1.06k forks source link

ProduceReferenceAssembly should exclude exe by default #16193

Open jaredpar opened 3 years ago

jaredpar commented 3 years ago

Starting with .NET 5 SDK we've enabled the $(ProduceReferenceAssembly) flag by default. I think this is excellent as it will really help our inner loop scenarios by removing unneeded compilations in a number of scenarios.

I think we should disable this by default for exe projects though. The reasons being:

  1. Reference assemblies are only beneficial when there is a good chance the project being built will be referenced by another. Yes this can happen for exes but it's the rare case.
  2. Reference assemblies appear in the bin output directory. That means it's an extra, and often unnecessary, concept that we're exposing to our customers that contributes to the look of us being more complex than our customers.

Believe we should tweak this setting so that it's enabled only when building library projects.

dotnet-issue-labeler[bot] commented 3 years ago

I couldn't figure out the best area label to add to this issue. If you have write-permissions please help me learn by adding exactly one area label.

DamianEdwards commented 3 years ago

+1 on this. We've received feedback numerous times regarding confusion about how complicated the build output of a starter app project is, and the new addition of a ref folder just adds to this. It seems totally sensible that the default for exe output should be off.

marcpopMSFT commented 3 years ago

Put it on the 6.0 backlog but not committed. We'd have to consider the impact of changing this now as it could potentially break someone who depended on this behavior for whatever reason.

dsplaisted commented 3 years ago

Why do the ref assemblies go in the output folder in the first place? I would have thought they would be in the intermediate output path?

jaredpar commented 3 years ago

@dsplaisted

They are initially written out to the intermediate output folder, under a sub-folder named ref, by the compilation task. MSBuild then uses the CopyRefAssembly task to move it to the output folder.

https://github.com/dotnet/msbuild/blob/41441946c883903cef0334ce872e9ce10918e2bf/src/Tasks/Microsoft.Common.CurrentVersion.targets#L4489

Part of this is necessary. The compiler always emits a reference assembly to disk because it's an output of compilation. MSBuild though is timestamp based so we cannot use the reference assembly emitted by the compiler as the output reference assembly consumed by referencing projects. That would cause every referencing project to re-compile even when the reference assembly was identical because the timestamp changed.

This is where the CopyRefAssembly task comes into play. It generally takes the source the reference assembly emitted by the compiler and copies it to the location that other projects will reference it from. That copy operation is "smart" though, it will only do the copy if the MVID of the source and dest are different. So when the build produces the same reference assembly it does not perform a copy and downstream compilations are avoided.

As to why we chose the final location of the reference assembly to be the output folder vs. the intermediate output folder I can't answer. I don't have any memory of that decision, or it even being a decision point. My intuition is that project references always grab from the output folder, not the intermediate output folder, so likely it was put there because it's where we used it previously. @rainersigwald would best be able to answer "why" though.

jaredpar commented 3 years ago

@jcouv as he did this work in the compiler and he may know the answer of why the output folder was chosen here.

jaredpar commented 3 years ago

Ended up finding a number of other issues discussing ref appearing in the bin directory. Think most will end up being dupes but want to let sdk team decide.

marcpopMSFT commented 1 year ago

@jaredpar what's the priority of this since we moved ref assemblies into obj so this is less visible now?

KalleOlaviNiemitalo commented 1 year ago

In my experience, exe projects that use System.CommandLine are referenced by test projects that call CommandLineConfiguration.ThrowIfInvalid (to be renamed per https://github.com/dotnet/command-line-api/issues/1908). OTOH, I could set ProduceReferenceAssembly explicitly in those projects.

jaredpar commented 1 year ago

@marcpopMSFT at this point I think it's mostly about performance as the ref DLL is just unnecessary work. I'm not sure where this would fit on your performance priority list but I imagine it's pretty low.

marcpopMSFT commented 1 year ago

Agreed. Moved to the backlog.