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

ResolvePackageDependencies doesn't set same metadata as ResolveNuGetPackageAssets #567

Closed
ericstj opened this issue Dec 30, 2016 · 5 comments
Assignees
Labels
Milestone

Comments

@ericstj
Copy link
Member

ericstj commented Dec 30, 2016

ResolveNuGetPackageAssets set NuGetPackageId and NuGetPackageVersion metadata but ResolvePackageDependencies does not. Seems like it should if it is replacing it. I know of a few folks that depend on this metadata being set.

/cc @jasonmalinowski @natidea

See https://github.com/NuGet/NuGet.BuildTasks/blob/4a57a0ef4bee31f122d50a39fe3dddd4ca03fb07/src/Microsoft.NuGet.Build.Tasks/ResolveNuGetPackageAssets.cs#L851-L852

@natidea
Copy link
Contributor

natidea commented Jan 3, 2017

See my comment in #568 (comment) . This metadata can be set downstream as needed. The package name and version are currently set as metadata on the PackageDefinitions item. (We avoided using NuGet-specific metadata names so this could be extended to other packaging systems.)

@srivatsn srivatsn added this to the Unknown milestone Jan 3, 2017
@ericstj
Copy link
Member Author

ericstj commented Jan 4, 2017

I need that metadata for doing conflict resolution. See https://github.com/dotnet/standard/blob/4392c75933f08108615699038b057389a37c7c4c/netstandard/tools/HandlePackageFileConflicts.cs#L140. For now I just parsed out ParentPackage metadata, but I want to make sure I'm depending on something explicit and public that will not break in the future.

I don't think it makes sense for this metadata to be added downstream since ResolvePackageDependencies already has this data (Id & Version) and is in fact the source of it. That's different than #568 where one might argue that its adding data on top of the items through an independent algo.

@ericstj
Copy link
Member Author

ericstj commented Jan 6, 2017

Actually you aren't even preserving the ParentPackage metdata in the Reference items so the code I pointed to is broken. The only way we can get this information back out is to try and parse the path. That's not acceptable. Please preserve package ID and version on Reference and ReferenceCopyLocalPaths. @srivatsn This is not a "feature request" it is a regression from NuGet 3.

@natidea
Copy link
Contributor

natidea commented Jan 6, 2017

I see. Yes when we create Reference items, we copy data from FileDefinitions into a temporary item to make resolved path the itemspec, but we are not copying those fields. I can add those fields.

@srivatsn srivatsn modified the milestones: 1.0 RC3, Unknown Jan 7, 2017
@srivatsn srivatsn modified the milestones: 1.0 RC3, 1.0 RTM Jan 13, 2017
@srivatsn srivatsn modified the milestones: 1.1, 1.0 RTM Jan 27, 2017
natidea added a commit that referenced this issue Mar 1, 2017
…tems (#603)

* Add package id and version to Reference and ReferenceCopyLocalPaths items #567

* Clean up statements in response to PR feedback; use transform to pull in metadata for compile files and framework assemblies

* Use transform for copy local items also
@ericstj
Copy link
Member Author

ericstj commented Mar 1, 2017

Thank you for fixing this @natidea. This will help the perf of our build tasks since they'll no longer need to enumerate the filesystem to find the nuspec.

mmitche pushed a commit to mmitche/sdk that referenced this issue Jun 5, 2020
…0190409.7 (dotnet#567)

- Microsoft.AspNetCore.Mvc.Analyzers - 3.0.0-preview4-19209-07
- Microsoft.AspNetCore.Mvc.Api.Analyzers - 3.0.0-preview4-19209-07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants