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

Produce a nuget package with just reference assemblies for AspNetCore.App #7355

Merged
merged 11 commits into from
Feb 8, 2019

Conversation

natemcmaster
Copy link
Contributor

This implements the first part of #6501.

To be done in a separate PR, if necessary. Still unclear on the details of native installers...

  • Debian/Windows/Rpm installers
  • Add zip's maybe?

cref dotnet/designs#50

@natemcmaster natemcmaster added the feature-platform Deprecated: Cross-cutting issues related to ASP.NET Core as a platform label Feb 7, 2019

<!-- Subject to change: see https://github.com/dotnet/designs/pull/50 -->
<PackageType>TargetingPack</PackageType>
<RefAssemblyPackagePath>ref/$(TargetFramework)/</RefAssemblyPackagePath>
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@nguerrera what was the result of your battle with NuGet on this one? I had to step out early from that meeting on Monday.

Copy link
Contributor

Choose a reason for hiding this comment

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

Not resolved. I will put down all of the options and book a meeting. We can change this when we have agreement. Thanks for putting it in one place.

Microsoft.NETCore.App.Ref is going with PackageType=Data packagePath=data/ right now. This was our proposal going into last meeting, but we have not settled it.

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 don't really care either way, just wanted to know which to use to make this actually work with the SDK. Is the SDK being updated soon to look for data/? Last I checked, the FrameworkReference resolution still looks for ref/<TFM>/

Copy link
Contributor

Choose a reason for hiding this comment

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

I believe this is because we're using the existing packages still. @dsplaisted

Copy link
Contributor

Choose a reason for hiding this comment

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

Anyway, I think we can follow up with a PR to these two lines when we have the decision.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok, let me and @pakrym know when a decision is reached. I'll leave as ref/<tfm> for now cuz I want to test this with the SDK to find the holes in this implementation.

@natemcmaster
Copy link
Contributor Author

Btw, is this package supposed to include docxml, too?

@dsplaisted
Copy link
Member

Btw, is this package supposed to include docxml, too?

@natemcmaster Yes, it should (if you want intellisense to show ASP.NET API documentation :-) )

Copy link
Member

@dagood dagood left a comment

Choose a reason for hiding this comment

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

In Core-Setup I continued with custom packaging infra (which is shared with CoreFX), so I can't really comment on the infra. Good to see where this is and what it takes with what I assume are the SDK packaging targets, though.

src/Framework/ref/Microsoft.AspNetCore.App.Ref.csproj Outdated Show resolved Hide resolved
@natemcmaster
Copy link
Contributor Author

Updated for include xmldocs for all assemblies except the two from dotnet/core-setup (see dotnet/core-setup#5109)

@natemcmaster
Copy link
Contributor Author

Alright, I think this is ready to merge. Had to to react to changes merged earlier today which breaks XML docs generation.

@natemcmaster natemcmaster merged commit cc065f0 into dotnet:master Feb 8, 2019
@natemcmaster natemcmaster deleted the targeting-pack branch February 8, 2019 03:47
@dsplaisted
Copy link
Member

@natemcmaster @pakrym Can you let me know the version of the Ref package that's published to the internal feed, so I can start using it in the SDK?

@natemcmaster
Copy link
Contributor Author

The first build with this package was 3.0.0-preview-19107-09.

@dsplaisted
Copy link
Member

@natemcmaster When testing the new package, I'm getting a compile error with the following code:

        public static IHostBuilder CreateHostBuilder(string[] args) =>
            Host.CreateDefaultBuilder(args)
                .ConfigureWebHostDefaults(webBuilder =>
                {
                    webBuilder.UseStartup<Startup>();
                });

The error is: error CS0103: The name 'Host' does not exist in the current context

Is this a mismatch between the targeting pack I'm using and the template version for dotnet new mvc?

@natemcmaster
Copy link
Contributor Author

This is very strange. Microsoft.AspNetCore.App.Ref is missing the assembly with this API. Let me debug further to see why ResolveReferences isn't putting this into the item group I'm using to create the package.

@natemcmaster
Copy link
Contributor Author

Found the problem. I'll send a PR today.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature-platform Deprecated: Cross-cutting issues related to ASP.NET Core as a platform
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants