[Proposal] Pack ProjectReferences
#11708
Conversation
|
Thank you so much for your contribution, @maxkoshevoi. I have edited the description to help out and will schedule an initial review with the NuGet team for next week. Please feel free to update the proposal as questions & feedback come in, and I'll do my best to support. Thanks again! |
|
|
||
| ## Unresolved Questions | ||
|
|
||
| None |
There was a problem hiding this comment.
One of the primary use cases of bundling third party package content within your own package is custom build logic - analyzers, custom msbuild tasks, bundling satellite applications. There will be certain pressure to not only resolve a single level of package but a xcopy ready representation of that dependency's own dependency tree. To give some examples:
- a custom build task i am working on at the moment uses
Microsoft.EntityFrameworkCore.Sqliteto write out gathered information as a database file- that package incurs a slew of packages dependent on TFM & platform - Do we stop after bundling the upper node of the dependency tree specified?
- an analyzer package needs a parser library at runtime - where does that
ProprietaryFileFormatParserXLibland without further instructions ? A naive implementation would drop this under lib which would mean the analyzer wouldn't work and the analyzer consumer would reference that parser library to add insult to injury -PackagePathanyone?
A possible basic implementation of the feature would look like a 1:1 copy of PackageDependencyA/lib/[TFM or any]/ to PackageToBePacked/lib/[TFM or any]/ - this would mean that any library package added into a bundled package would end up being referenced by consumers of that bundled package - is that a desired outcome?
I would recommend adding the following questions:
| None | |
| - **How do we flow/assign TFM based `lib`/`content` items?** (Is `PackagePath` a desired parameter?) | |
| - **How is conflict resolution going to work with consumers taking in a dependency already present in the bundle?** | |
| - Should license flow/validation be considered right from the start? | |
| - **Should a `PackageReference` or `ProjectReference` tagged with `Pack="true"` be resolved transitively?** |
There was a problem hiding this comment.
- I think usually one can check that as an implementation detail using:
$(TargetFramework)
However there comes another task it would have to do as well:
- use the value of
$(RuntimeIdentifier)if set in project (otherwise ignore it when it ==any).
-
I think if analyzers/generators are referenced in project and are tagged with
Pack="true"that it should package everything under the analyzer's folder in it's package AS IS which means conflicts between internal deps of the analyzers could possibly happen. -
As for the license flow, what it could do is add these to the package metadata to support it:
licenses/<package name>(bundle them in that package path) where it would bundle the licenses from the included projects/packages. -
That just depends entirely on a lot of factors, if the package is meant only for an generator / analyzer (nothing more) than I say that resolving them would not be needed, otherwise yes they would be needed (we do not want to resolve dependencies of analyzers that would never be used on the consuming project and so wastes disc space).
Co-authored-by: Meik Tranel <tranelmeik@gmail.com>
Co-authored-by: Meik Tranel <tranelmeik@gmail.com>
|
Hi all, Just commenting that we did review this last Friday and I'm trying to find time to write-up some notes. I apologize as this week we had a few large releases as per our blogs. |
|
Do you also get to choose the location in the package? Here's a similar mess I had to figure out recently: |
Good catch. Updated the proposal to allow to choose the location |
jeffkl
added
the
Community
|
Regarding the empty "Drawbacks" section: I detailed a real-life scenario in my blog: |
| 2) If `ProjectReference` has `Pack` property and it's set to `True` - include it's artifacts into the NuGet (to the default location or one specified in `PackagePath`). | ||
| 3) Do the same steps for `PackageReference`s | ||
|
|
||
| ## Drawbacks |
There was a problem hiding this comment.
It may be worth to add that one of the best practices in NuGet today is to have one assembly per NuGet package.
There was a problem hiding this comment.
I think some of the feedback around compatibiltiy and redistribution of packages should be listed in the drawbacks here.
|
|
||
| Allow referenced projects' artifacts to be packed inside the NuGet. | ||
|
|
||
| ## Motivation |
There was a problem hiding this comment.
Great motivation. Thanks for writing it!
| - **How do we flow/assign TFM based `lib`/`content` items?** (Is `PackagePath` a desired parameter?) | ||
| - **How is conflict resolution going to work with consumers taking in a dependency already present in the bundle?** | ||
| - Should license flow/validation be considered right from the start? | ||
| - **Should a `PackageReference` or `ProjectReference` tagged with `Pack="true"` be resolved transitively?** |
There was a problem hiding this comment.
We'll have to consider the ProjectReference side of this problem with a broader team. I can include them.
There was a problem hiding this comment.
This is a very good question, which is why I think the motivation isn't the correct one here. Understanding the scenarios would guide this answer (the answer would usually be yes.)
|
|
||
| This one is less flexible since it cannot switch on per-`ProjectReference` basis. And it isn't applicable to `PackageReferences`. | ||
|
|
||
| - Another solution is to introduce a `IncludeReferencedProjects` switch for `nuget` CLI. |
There was a problem hiding this comment.
Would this be for dotnet CLI instead?
There was a problem hiding this comment.
If a Project is being included into a joined package and never gets packed itself, it's dependencies would need to be something that has to be represented in the joined package.
Example:
Project 1 -> Project 2 -> Package A.
Say, Project 1 includes the reference project, and it is intended to be redistributed as a library, it's extremely likely that Project 1 needs Package A as a dependency.
This is a reverse restore. Instead of given the dependency, give me a resolved graph, it starts with a resolved graph and it tries to generate the correct top level dependencies. Note that we'd need to account for package include/excludes as well.
There was a problem hiding this comment.
Would this be for
dotnetCLI instead?
Correct. It'd be for dotnet CLI and the msbuild pack target (which the dotnet pack is just a wrapper for)
|
I apologize in advance for forgetting about this proposal until now. I wanted to let people know that the earliest we can think about this work will start in September. Thanks again for writing it and everyone's contributions and upvotes thus far. It significantly helps us in our upcoming planning cycles. |
| ## Motivation | ||
|
|
||
| When SDK-style project is being packed as a NuGet, all projects that it references are considered as NuGet themselves that this NuGet depends on. | ||
| This is not always a desired behavior, sometimes user needs artifacts from referenced project(s) to be packed into the NuGet. |
There was a problem hiding this comment.
I think the motivation needs to capture the particular scenarios where this is helpful. We'd appreciate a bigger focus on the scenarios.
|
|
||
| When SDK-style project is being packed as a NuGet, all projects that it references are considered as NuGet themselves that this NuGet depends on. | ||
| This is not always a desired behavior, sometimes user needs artifacts from referenced project(s) to be packed into the NuGet. | ||
| Currently there's no easy way to override this behavior except for ditching `csproj`, and going back to `nuspec`. In many cases that involves piping lots of information from the `MSBuild` context into the `nuspec` evaluation via `key=value` parameters. |
There was a problem hiding this comment.
I think it's an unfair characterization to say the only way to pack more than assembly is by going with a nuspec, as None items can glob in an equivalent way the nuspec files elements can.
Furthermore the issue body does link to a comment that contains a workaround:
#3891 (comment), that can be implemented with the documented pack extensibility. https://docs.microsoft.com/en-us/nuget/reference/msbuild-targets#advanced-extension-points-to-create-customized-package
| When SDK-style project is being packed as a NuGet, all projects that it references are considered as NuGet themselves that this NuGet depends on. | ||
| This is not always a desired behavior, sometimes user needs artifacts from referenced project(s) to be packed into the NuGet. | ||
| Currently there's no easy way to override this behavior except for ditching `csproj`, and going back to `nuspec`. In many cases that involves piping lots of information from the `MSBuild` context into the `nuspec` evaluation via `key=value` parameters. | ||
| Chaining together dozens of arguments to achieve parity with metadata usually automagically introduced by the SDK becomes tedious, which eventually weakens the otherwise rich set of information attached to a modern NuGet package. |
There was a problem hiding this comment.
Someone brought up:
One of the primary use cases of bundling third party package content within your own package is custom build logic - analyzers, custom msbuild tasks, bundling satellite applications
I think this what the motivation should cover.
| If `Pack` is `False` or not present - consider the project (or package) as a NuGet's dependency, if it's `True` - pack its artifacts in. | ||
| If `PackagePath` is not present - pack artifacts to the default location within the package. If it's present - pack them to the specified path. | ||
|
|
||
| For `PackageReference`s it's useful in case referenced package is from private feed, but NuGet that's being packed will be public. |
There was a problem hiding this comment.
nit: This needs to go into the motivation.
There was a problem hiding this comment.
Additionally an aside, packing assemblies from packages published on private feeds only can lead to a lot of issues.
If a package is in a private feed, it must have been done so for a reason. Redistributing assemblies for packages you don't own might lead into some licensing violations.
There's no guarantee that this package currently on a private feed (maybe because it's in previews) does not make it to NuGet.org. Redistributing the same assembly in multiple packages is something that's known to cause a lot of problems for the consumers.
The build and runtime work best when only 1 reference of the same assembly is used in an application. NuGet ensures that you cannot have multiple versions of the package in the project, but that guarantee cannot be transferred to the assemblies themselves. If an assembly is in multiple packages, it's very possible that someone ends up referencing both. The more dependency issues can be resolved at restore time the better.
The above concerns can be generalized to Never include the same assembly in multiple library packages.
Note the focus on library packages. In some scenarios, packages contain tools, and those often need the entry assembly + some of it's dependencies.
|
|
||
| ## Explanation | ||
|
|
||
| ### Functional explanation |
There was a problem hiding this comment.
I think multiple asks are being conflated into 1 here.
- Packaging multiple projects into 1 package.
- Packaging assemblies from packages into a different package.
These are obviously related, but the scenarios vary and the potential for misuses of a feature that does things automatically varies.
It's worth nothing that IncludeReferenceProjects handled projects only, so I don't think it'd be unfair to suggest that these features are not all combined into one.
|
|
||
| ## Unresolved Questions | ||
|
|
||
| - **How do we flow/assign TFM based `lib`/`content` items?** (Is `PackagePath` a desired parameter?) |
There was a problem hiding this comment.
I'm not sure I fully understand this example. Can you clarify better what the ask is?
There was a problem hiding this comment.
Referring to the conversation earlier - here: #11708 (comment)
There was a problem hiding this comment.
Thanks for that link. In that case, I think the compat side would probably need to not be configurable.
In other words, given a project reference today, the particular assembly chosen by NuGet/SDK is the best compatible framework one, so I think the switch would only really need to be copy for project, or don't.
The conflict resolution between package items and references is more complex as that happens in a part that NuGet doesn't understand. So we'd need NuGet and the SDK to work through that in some way.
There was a problem hiding this comment.
I understand there is potentially a better framework for some of the dependent assemblies, but accounting for that would make things too complicated imo. NuGet today chooses a full lib folder, so we can't really pick and choose from different folders.
| ## Unresolved Questions | ||
|
|
||
| - **How do we flow/assign TFM based `lib`/`content` items?** (Is `PackagePath` a desired parameter?) | ||
| - **How is conflict resolution going to work with consumers taking in a dependency already present in the bundle?** |
There was a problem hiding this comment.
It unfortunately doesn't. The controls for assembly includes are on a package level. Furthermore, it's not something that NuGet tries to address today. The build itself does go through conflict resolution itself.
| - **How do we flow/assign TFM based `lib`/`content` items?** (Is `PackagePath` a desired parameter?) | ||
| - **How is conflict resolution going to work with consumers taking in a dependency already present in the bundle?** | ||
| - Should license flow/validation be considered right from the start? | ||
| - **Should a `PackageReference` or `ProjectReference` tagged with `Pack="true"` be resolved transitively?** |
There was a problem hiding this comment.
This is a very good question, which is why I think the motivation isn't the correct one here. Understanding the scenarios would guide this answer (the answer would usually be yes.)
| - **How is conflict resolution going to work with consumers taking in a dependency already present in the bundle?** | ||
| - Should license flow/validation be considered right from the start? | ||
| - **Should a `PackageReference` or `ProjectReference` tagged with `Pack="true"` be resolved transitively?** | ||
| - What happens to NuGets that are referenced in `PackageReference` tagged with `Pack="true"`? Do they become dependencies of the resulting NuGet or being packed into it. |
There was a problem hiding this comment.
This a reverse restore cases similar to the above one.
|
|
||
| ## Future Possibilities | ||
|
|
||
| This proposal would allow users more flexibility in choosing what is being packed as part of a NuGet, and how it's dependencies look like. |
There was a problem hiding this comment.
nit: More of a motivation thing.
| When SDK-style project is being packed as a NuGet, all projects that it references are considered as NuGet themselves that this NuGet depends on. | ||
| This is not always a desired behavior, sometimes user needs artifacts from referenced project(s) to be packed into the NuGet. | ||
| Currently there's no easy way to override this behavior except for ditching `csproj`, and going back to `nuspec`. In many cases that involves piping lots of information from the `MSBuild` context into the `nuspec` evaluation via `key=value` parameters. | ||
| Chaining together dozens of arguments to achieve parity with metadata usually automagically introduced by the SDK becomes tedious, which eventually weakens the otherwise rich set of information attached to a modern NuGet package. |
There was a problem hiding this comment.
One specific motivating example for this feature is MSBuild Tasks. Happy to see this brought up!
ghost
added
the
|
This PR has been automatically marked as stale because it has no activity for 30 days. It will be closed if no further activity occurs within another 330 days of this comment. If it is closed, you may reopen it anytime when you're ready again, as long as you don't delete the branch. |
|
Ping |
|
This PR has been automatically marked as stale because it has no activity for 30 days. It will be closed if no further activity occurs within another 330 days of this comment. If it is closed, you may reopen it anytime when you're ready again, as long as you don't delete the branch. |
|
Ping |
|
ping, anyone going to review this again? This would be useful in the future. |
|
Hi, we have removed our "proposed" folder, so please move this proposal to the "accepted" folder. |
|
This PR has been automatically marked as stale because it has no activity for 30 days. It will be closed if no further activity occurs within another 330 days of this comment. If it is closed, you may reopen it anytime when you're ready again, as long as you don't delete the branch. |
|
We still waiting. |
|
Also shouldn't this be moved to 2024 folder? |
|
This PR has been automatically marked as stale because it has no activity for 30 days. It will be closed if no further activity occurs within another 330 days of this comment. If it is closed, you may reopen it anytime when you're ready again, as long as you don't delete the branch. |
|
What is the proposal for nested references? For example, when we were manually creating nuspec files, the referenced projects that we weren't creating packages for (IsPackable=false) were included in the nuspec as files instead of dependencies, but the packages that those referenced projects themselves referenced were added as dependencies - because they actually were packages. If we go down the route of Pack=false on a project reference, then I'd hope this wouldn't prevent it recursively checking the references within that project to determine what else should be added as a dependency and what as a file. But this may then require setting Pack=false against a lot more project references across a solution and becomes a bit messy and confusing. That's why I have to say I prefer it if we just used the IsPackable setting on the project. It just feels more intuitive and clearer and also means we don't need to set Pack=false everywhere that the project is referenced. It'd be a property of the project rather than of references to it. |
dotnet-policy-service
bot
removed
the
Status:No recent activity
|
This PR has been automatically marked as stale because it has no activity for 30 days. It will be closed if no further activity occurs within another 330 days of this comment. If it is closed, you may reopen it anytime when you're ready again, as long as you don't delete the branch. |
Related discussion: #3891
Rendered Proposal
Please 👍 or 👎 this comment to help us with the direction of this feature & leave as much feedback/questions/concerns as you'd like on this issue itself and we will get back to you shortly.
Thank You 🎉