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

[Packaging] Define the way/ways that we want to create the nuget packages #383

Closed
maririos opened this issue Jul 26, 2018 · 29 comments
Closed
Assignees
Labels
area-Infrastructure-libraries Area maintained by .NET libraries team: APICompat, AsmDiff, GenAPI, GenFacades, PkgProj, etc

Comments

@maririos
Copy link
Member

Should we have by default dotnet pack and if the needs of a repo ask for more, then let them implement their own packaging way?

@maririos maririos changed the title Define the way/ways that we want to create the nuget packages [Packaging] Define the way/ways that we want to create the nuget packages Jul 26, 2018
@tmat
Copy link
Member

tmat commented Jul 27, 2018

It's gotta be dotnet pack. That's what our customers use, we should use it as well.

@maririos
Copy link
Member Author

As a default, I agree, but yesterday @weshaggard was talking about how corefx needs custom nuget packages because of the nature of the content.

@tmat
Copy link
Member

tmat commented Jul 27, 2018

I believe any package can be built from a project using Pack target. We have projects in Roslyn, Roslyn Analyzers, and other repos that pack meta-packages, tool packages, analyzer packages, etc.

We either use pack with handwritten nuspec, or we generate nuspec in a target.

If package just aggregates outputs of other projects (like meta-package or analyzer package) we have a dummy .csproj with no-op Build and Test targets, so that the only target that's executed is Pack target.

It works well. In future, I expect these kind of packages to be supported directly by NuGet Pack task. Once they will we can just remove our custom target without the need of changing anything else.

@weshaggard
Copy link
Member

While I agree with the general push to use the shipping tools there are a lot of things not currently supported so we will likely be on our own custom packaging targets/tasks for the core repo's for a while to come. We can look at converting that system but that is way down on our backlog.

To give some idea we have defined our on project type call pkgproj and it handles a lot of stuff for us, such as supporting refs and multi-targeting for many TFMs that are not necessary supported by the SDK. We also have a mechanism to harvest binaries from our older packages so we can maintain compatibility with our old packages. Our package tools also have a lot of static validation built in to ensure we ship consistent and compatible packages.

@ericstj designed most of it so if you want more details I'm sure he would be happy to chat about it.

@tmat
Copy link
Member

tmat commented Jul 27, 2018

Custom targets are fine as long as they are chained to Pack target.
Custom project type is not ideal since it can't be part of a solution opened in VS, but still ok since your build has multiple entry points (solutions/projects) anyways.

@JohnTortugo JohnTortugo self-assigned this Aug 24, 2018
@JohnTortugo
Copy link
Contributor

I was assigned to work on what we decide here. :-)

@ericstj Do you think that we can manage to chain the custom packaging that you implemented in the Pack target?

Cc'ing @natemcmaster as this discussion might interest him as well.

@natemcmaster
Copy link
Contributor

Let me lob a hand grenade.

IMO we should take a close look at using https://github.com/nuproj/nuproj. It has most of the things I want for NuGet package authoring -- dedicated project files ✅ , VS integration ✅ , simple project schema ✅, written by Microsoft (or at least, written by msft devs) ✅ .

@markwilkie
Copy link
Member

markwilkie commented Aug 24, 2018

I like using things already done. :)

What are the downsides? I presume that some of the things that Wes noted are not supported?

Also, there's much to be said for incremental steps. Implementing something we KNOW works is sometimes advantageous.

@natemcmaster - do you know yet if the current implementation in CoreFx would work for ASP?

@natemcmaster
Copy link
Contributor

What are the downsides?

I'm not sure if dotnet pack works on nuproj. We might need to make a contribution to add support for running on MSBuild for .NET Core - cc @AArnott @terrajobst

I presume that some of the things that Wes noted are not supported?

Correct. AFAIK the harvesting and validation is very specific to dotnet/corefx. I have not seen stuff like this in other repos.

From my (admittedly limited) understanding of nuproj, I don't think it would be too hard to port the custom harvesting and validation from pkgproj to run in nuproj.

do you know yet if the current implementation in CoreFx would work for ASP?

I don't think it would work without a few tweaks. ASP.NET has two pieces of custom stuff.

  1. One of those was this PR Add PackNuspec MSBuild task #57 which wasn't merged due to reasons I think we can resolve.
  2. The other is our "NuGetPackageVerifier" console tool which runs some simple acceptance tests on .nupkg's. I think @Eilon would be happy to move this into dotnet/arcade.

@AArnott
Copy link

AArnott commented Aug 24, 2018

There's no one working on nuproj any more. It probably works (or can be made to work) with dotnet pack merely by supporting the Pack target, if it doesn't already. But it's woefully outdated in many ways. I never use nuproj any more.

My vote in virtually any use case is to use csproj's self-packing capabilities now. Even in cases where a package has to ship multiple assemblies.

@weshaggard
Copy link
Member

weshaggard commented Aug 24, 2018

It should be pointed out that our existing pkgproj stuff started with nuproj and then morphed to handle the complexities of corefx. So I think we could attempt to use the pkgproj stuff and make it work for asp.net.

The other is our "NuGetPackageVerifier" console tool which runs some simple acceptance tests on .nupkg's

It would be interesting to see what verification you guys do as we include a lot of validation in our pkgproj support.

Custom project type is not ideal since it can't be part of a solution opened in VS, but still ok since your build has multiple entry points (solutions/projects) anyways.

One of the biggest reasons we needed a custom project type was because we need to build multiple other projects in order to get all the assets necessary to create our new packages.

@AArnott
Copy link

AArnott commented Aug 24, 2018

Custom project types can load in VS so long as you follow the rules necessary for CPS to load your project. No VS extension required, potentially.

@tmat
Copy link
Member

tmat commented Aug 24, 2018

One of the biggest reasons we needed a custom project type was because we need to build multiple other projects in order to get all the assets necessary to create our new packages.

That's doable without custom projects. We use csproj with overridden Build target for meta-packages and tools packages.

See dotnet/roslyn#29408.

@natemcmaster
Copy link
Contributor

I read dotnet/roslyn#29408 as more evidence of how hard it is for devs to use the built-in pack targets for anything other than vanilla packages. It might be doable for an MSBuild expert like @tmat, but my experience has been that you basically have to read and understand NuGet's pack targets before you can customize package layout.

@tmat
Copy link
Member

tmat commented Aug 24, 2018

@natemcmaster Yes, it took some figuring out and there are definitely rough edges. But once it's figured out and documented it is not that hard to follow patterns that are already established in the code base. We can add more helper targets that make it even easier and ultimately work towards including these improvements in the shipping NuGet packaging targets.

@weshaggard
Copy link
Member

That's doable without custom projects. We use csproj with overridden Build target for meta-packages and tools packages.

What advantage do you get but forcing this into a csproj over a custom project? In either case you need to override a number of targets and by having a custom project type it makes it clear that is is building a package and not a library.

I have no problems getting the packaging tasks/targets we use to hang off the Pack target but I do think it will still take a fair amount of work to get everything plumbed into the existing Packaging targets as I suspect we will completely override all of them initially and slowly transition to use what is in the product as we can.

@tmat
Copy link
Member

tmat commented Aug 25, 2018

What advantage do you get but forcing this into a csproj over a custom project?

You can have the project in solution without installing any extensions. It's actually very simple - you don't need to override a number of targets, just one. It's a few lines of trivial code: https://github.com/dotnet/roslyn/pull/29408/files#diff-20beb43ddafee4bc74e74b1c060d828a

CoreFX packages might be very special since CoreFX is special. I'm not suggesting you have to do it the same way. As long as your projects implement all targets that the Arcade build pipeline expect them to have they can be outside of any solution and should work fine.

@markwilkie
Copy link
Member

I thought custom projects do load in VS so long as the rules are followed?

@tmat
Copy link
Member

tmat commented Aug 27, 2018

No, CPS project system does not currently support custom projects. It is possible to write a VSIX extension that adds the support for particular project kind, but it's not built-in.

@AArnott
Copy link

AArnott commented Aug 27, 2018

@tmat What do you mean? CPS certainly does (or used to!) load any project that conforms to its requirements. What are you suggesting is lacking in support without a VSIX?

This is my understanding:

Without a VSIX:

  1. A project must have the right items/properties so that CPS knows how to handle the project. This usually comes mostly for free by importing the Microsoft.Common.targets.
  2. The project must use a project file extension that VS already recognizes as associated with CPS (e.g. .msbuildproj).

With a VSIX:

  1. A custom file extension can be used for the project.
  2. Project templates can be included.
  3. Custom project extensions that run within the IDE process can be added to further tailor the project experience.

@tmat
Copy link
Member

tmat commented Aug 27, 2018

I've got the info from @jmarolf.

I guess the misunderstanding might be that .msbuildproj isn't considered a custom project since it's a known CPS extension.

@tmat
Copy link
Member

tmat commented Aug 27, 2018

Just tried with .msbuildproj. Importing Microsoft.Common.targets doesn't seem sufficient:

C:\Temp\a\a.msbuildproj : error  : Project file is incomplete. Expected imports are missing.

@AArnott
Copy link

AArnott commented Aug 27, 2018

This usually comes mostly for free by importing the Microsoft.Common.targets.

@tmat, I did say "mostly for free" -- importing that isn't the only requirement. The CPS SDK available on the VS Gallery includes a project template that does this and the rest of the requirements for you (with perhaps some fluff that isn't actually required).

BTW, that error you shared may be an issue with your project itself. Have you tried building it from the command line? If it fails there, of course you need MSBuild to be able to build it before even thinking about getting VS to load it. :)

@JohnTortugo
Copy link
Contributor

JohnTortugo commented Aug 28, 2018

Updating with an outline of key points so far (IMO):

  • Topic question is: Should we work on a custom packaging target/tasks?

  • Can we plug it (the CoreFX custom packaging) on the default Pack logic? Yes, with some work. @weshaggard

  • What do we get from using it?

    • More flexibility as shown by current CoreFX usage.
    • ASPNet also has some custom packaging logic and might benefit from this as well.
    • Seems CoreFX and ASPNet could share some of the static validation/verification tasks.
  • Will the project type show up in VS? Yes, but might needs to adjust the custom project type extension.

Follow-up question: Disadvantages of plugging the custom logic + new project type?

Perhaps @mmitche @rainersigwald has some opinion on this.

@markwilkie
Copy link
Member

Here's what I'd like to do:

  • Strongly recommend 'dotnet pack' for every repo who can possibly make it work.
  • For those repos that have complex requirements (like corefx, and asp for starters), we'll want to converge on the right custom packaging solution.

Tactically, this means:

  • In preparation for teams onboarding, (9/21) let's make sure our guidance is solid regarding 'dotnet pack'
  • Over the next month or two, create the solution for use only when the added complexity is needed. Complex Packaging ready for use #578

@JohnTortugo - this means that this epic will be fulfilled once the details of dotnet pack are sorted. The other epic I created (#578) can be picked up again once @maririos is back.

@rainersigwald
Copy link
Member

@rrelyea should be involved in the discussions around where the existing pack targets aren't sufficient or are hard to customize.

@natemcmaster
Copy link
Contributor

For context, we have had a similar discussion about that subject in #57. This was in March, but I don't think a whole lot has changed.

@weshaggard
Copy link
Member

Is the plan to track this work as part of #578?

@weshaggard
Copy link
Member

weshaggard commented Sep 21, 2018

Also did we document your packing plan somewhere @markwilkie?

edit: I found it from this PR #586

@ericstj ericstj added the area-Infrastructure-libraries Area maintained by .NET libraries team: APICompat, AsmDiff, GenAPI, GenFacades, PkgProj, etc label Mar 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-Infrastructure-libraries Area maintained by .NET libraries team: APICompat, AsmDiff, GenAPI, GenFacades, PkgProj, etc
Projects
None yet
Development

No branches or pull requests

9 participants