Skip to content

Build our own packages using FrameworkReference #4257

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

Open
natemcmaster opened this issue Nov 27, 2018 · 5 comments
Open

Build our own packages using FrameworkReference #4257

natemcmaster opened this issue Nov 27, 2018 · 5 comments
Labels
affected-very-few This issue impacts very few customers area-infrastructure Includes: MSBuild projects/targets, build scripts, CI, Installers and shared framework severity-nice-to-have This label is used by an internal tool
Milestone

Comments

@natemcmaster
Copy link
Contributor

Depends on NuGet/Home#7342.
Follow up to #4178

The packages we ship which reference the shared framework should be built using FrameworkReference. Once NuGet/Home#7342 is resolved, the metadata for this item will be preserved in the nuspec, which should ensure consumers get all the necessary runtime settings and compiler references.

Workaround
Consumers using our AspNetCore packages, such as Microsoft.AspNetCore.NodeServices, need to make sure they also have <FrameworkReference Include="Microsoft.AspNetCore.App" />. This will be implicitly added with Microsoft.NET.Sdk.Web, but not other MSBuild SDKs.

Changes required

  1. We need to change our own packages to build with FrameworkReference. (Related work Convert projects in this repo to use ProjectReference #4246)
  2. Update our SDK in BuildTools to support using FrameworkReference
@natemcmaster natemcmaster added the feature-platform Deprecated: Cross-cutting issues related to ASP.NET Core as a platform label Nov 27, 2018
@natemcmaster natemcmaster added this to the 3.0.0-preview2 milestone Nov 27, 2018
@natemcmaster natemcmaster added the area-infrastructure Includes: MSBuild projects/targets, build scripts, CI, Installers and shared framework label Dec 18, 2018
@natemcmaster natemcmaster changed the title 3.0 Preview 1 packages require adding a FrameworkReference to the project Build our own packages using FrameworkReference Jan 7, 2019
@natemcmaster natemcmaster self-assigned this Jan 18, 2019
@natemcmaster
Copy link
Contributor Author

Prototype in progress: https://github.com/aspnet/AspNetCore/compare/natemcmaster:local-fx

Current challenges:

  • I get compiler errors due to duplicate APIs in the same namespace. I think this is the result of having InternalsVisibleTo and duplicate copies of API due to shared source.
  • Visual Studio go-to-definition is broken when navigating from something outside the shared framework into a shared framework assembly.
  • Working around https://github.com/dotnet/core-setup/issues/4809 - the only way to make tests run against the shared framework is to install the shared framework into DOTNET_ROOT. My workaround is to make DOTNET_ROOT = "$repoRoot/.dotnet/$(arch)/".
  • Requires VS 2019 and a Preview 2 SDK (blocked on Make Visual Studio 2019 a prerequisite to building this repo #7005).

cc @pakrym @ryanbrandenburg

@natemcmaster
Copy link
Contributor Author

natemcmaster commented Jul 24, 2019

@Pilchie I think this is still worth addressing, but this is a challenging issue to address. The challenges I described in the previous comment are still unresolved. We would probably need help from the teams which own VS, the runtime host, and the SDK to completely resolve this. It's extremely unlikely this will land in 3.0. 3.1 or 5.0 is a more appropriate timeframe for this.

What we have now in the code currently is a workaround for this. There is a post-build step which manually alters the metadata in .nuspec file in NuGet packages to remove incorrect <dependency> items and add a <frameworkReference>. IMO it would be ideal if dotnet pack produced the right nuspec in the first place so this post-build hack wasn't necessary.

The other major workaround we have is that test projects don't actually run against the shared framework bits that ship to customers. The host doesn't provide a way to easily swap in a new shared framework, so test projects run on ASP.NET Core binaries as if they are simple class libraries projects. This means test code isn't actually executing the same crossgened binaries that ship to customers.

@Pilchie Pilchie modified the milestones: 3.0.0-preview8, Backlog Aug 5, 2019
@JunTaoLuo JunTaoLuo added severity-major This label is used by an internal tool and removed feature-platform Deprecated: Cross-cutting issues related to ASP.NET Core as a platform labels Dec 10, 2020
@JunTaoLuo
Copy link
Contributor

@dotnet/aspnet-build Let's chat about this once everyone is back in the office and see if there is any work we need to do here. I suspect some of the details mentioned here is out of date.

@dougbu dougbu added affected-very-few This issue impacts very few customers severity-nice-to-have This label is used by an internal tool and removed severity-major This label is used by an internal tool labels Jan 4, 2021
@dougbu
Copy link
Member

dougbu commented Jan 4, 2021

Seems minor and won't affect anyone using our packages. The cleanup task run after every build runs reliably and does the needful.

About the only external improvement coming from building against framework references would be cleanup of the very few .deps.json files we ship. But, this comes with the major downside of basically requiring two builds to get the tests using the just-built ASP.NET Core assemblies.

@Pilchie
Copy link
Member

Pilchie commented Jan 15, 2021

I think I agree that it's probably ok to let this go.

cleanup of the very few .deps.json files we ship

@dougbu could you give me an example of the before and after here?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
affected-very-few This issue impacts very few customers area-infrastructure Includes: MSBuild projects/targets, build scripts, CI, Installers and shared framework severity-nice-to-have This label is used by an internal tool
Projects
None yet
Development

No branches or pull requests

7 participants
@pranavkm @Eilon @Pilchie @JunTaoLuo @natemcmaster @dougbu and others