NuGet / Home

Repo for NuGet Client issues
Other
1.49k stars 252 forks source link

Custom metadata from nuspec file is lost #4751

Open heaths opened 7 years ago

heaths commented 7 years ago

Details about Problem

NuGet product used (NuGet.exe | VS UI | Package Manager Console | dotnet.exe): nuget.exe

NuGet version (x.x.x.xxx): any (verified in 2x and 3x)

dotnet.exe --version (if appropriate): N/A

VS version (if appropriate): N/A

OS version (i.e. win10 v1607 (14393.321)): N/A

Worked before? If so, with which NuGet version: No

Detailed repro steps so we can see the same problem

  1. Add custom metadata, such as docsUrl (Chocolatey recommendation).
  2. Read in nuspec using Manifest.ReadFrom (the NuProj project does this). At this point the custom metadata is already lost.
  3. Continuing with what NuProj does, update metadata.
  4. Write metadata back to file.

Custom metadata is not present.

Sample Project

Will be uploaded soon to https://github.com/Microsoft/vswhere/tree/develop/pkg/vswhere.

heaths commented 7 years ago

Sample project uploaded to https://github.com/Microsoft/vswhere/tree/choco/pkg/vswhere. You don't need to install NuProj - just restore packages.

I could fix this by adding a property bag to IPackageMetadata and anything not explicitly referenced would be added to that.

If I were to fix this, would you accept changes to the latest 3x release, or are you only working on 4x at this point? Not sure if NuProj would want to take a beta release just yet (their still using 2x so will have to upgrade either way).

bbowman commented 7 years ago

@kzu

kzu commented 7 years ago

Care to elaborate @bbowman? ;)

bbowman commented 7 years ago

@kzu Sure :-)

The issue here is that Nugetizer can not be used to produce complete chocolatey packages ( https://chocolatey.org/ ). Chocolatey has extended the .nuspec to include several new items that they really want their packages to include (See the below image). These items are specified in the nuspec with extra metadata elements.

The fix to this issue is two part:

  1. Enchance IPackageMetadata as suggested by @heaths to allow the core Nuget packager to accept extra metadata on the nuspec.
  2. Ehance Nugetizer to allow extra metadata to flow into the manifest used to make the nuget.

I'm not sure who to engage for 1? @emgarten @rrelyea may know? For 2 seems like something that we might want to discuss how Nugetizer should handle custom metadata.

@gep13 and @ferventcoder might have input on this from the chocolatey end.

Thoughts?

ferventcoder commented 7 years ago

To give you a general idea, this is the output of choco new test

<?xml version="1.0" encoding="utf-8"?>
<!-- Read this before creating packages: https://chocolatey.org/docs/create-packages -->
<!-- It is especially important to read the above link to understand additional requirements when publishing packages to the community feed aka dot org (https://chocolatey.org/packages). -->

<!-- Test your packages in a test environment: https://github.com/chocolatey/chocolatey-test-environment -->

<!--
This is a nuspec. It mostly adheres to https://docs.nuget.org/create/Nuspec-Reference. Chocolatey uses a special version of NuGet.Core that allows us to do more than was initially possible. As such there are certain things to be aware of:

* the package xmlns schema url may cause issues with nuget.exe
* Any of the following elements can ONLY be used by choco tools - projectSourceUrl, docsUrl, mailingListUrl, bugTrackerUrl, packageSourceUrl, provides, conflicts, replaces 
* nuget.exe can still install packages with those elements but they are ignored. Any authoring tools or commands will error on those elements 
-->

<!-- You can embed software files directly into packages, as long as you are not bound by distribution rights. -->
<!-- * If you are an organization making private packages, you probably have no issues here -->
<!-- * If you are releasing to the community feed, you need to consider distribution rights. -->
<!-- Do not remove this test for UTF-8: if “Ω” doesn’t appear as greek uppercase omega letter enclosed in quotation marks, you should use an editor that supports UTF-8, not this one. -->
<package xmlns="http://schemas.microsoft.com/packaging/2015/06/nuspec.xsd">
  <metadata>
    <!-- == PACKAGE SPECIFIC SECTION == -->
    <!-- This section is about this package, although id and version have ties back to the software -->
    <!-- id is lowercase and if you want a good separator for words, use '-', not '.'. Dots are only acceptable as suffixes for certain types of packages, e.g. .install, .portable, .extension, .template -->
    <!-- If the software is cross-platform, attempt to use the same id as the debian/rpm package(s) if possible. -->
    <id>test</id>
    <!-- version should MATCH as closely as possible with the underlying software -->
    <!-- Is the version a prerelease of a version? https://docs.nuget.org/create/versioning#creating-prerelease-packages -->
    <!-- Note that unstable versions like 0.0.1 can be considered a released version, but it's possible that one can release a 0.0.1-beta before you release a 0.0.1 version. If the version number is final, that is considered a released version and not a prerelease. -->
    <version>__REPLACE__</version>
    <!-- <packageSourceUrl>Where is this Chocolatey package located (think GitHub)? packageSourceUrl is highly recommended for the community feed</packageSourceUrl>-->
    <!-- owners is a poor name for maintainers of the package. It sticks around by this name for compatibility reasons. It basically means you. -->
    <!--<owners>__REPLACE_YOUR_NAME__</owners>-->
    <!-- ============================== -->

    <!-- == SOFTWARE SPECIFIC SECTION == -->
    <!-- This section is about the software itself -->
    <title>test (Install)</title>
    <authors>__REPLACE_AUTHORS_OF_SOFTWARE_COMMA_SEPARATED__</authors>
    <!-- projectUrl is required for the community feed -->
    <projectUrl>https://_Software_Location_REMOVE_OR_FILL_OUT_</projectUrl>
    <!--<iconUrl>http://cdn.rawgit.com/__REPLACE_YOUR_REPO__/master/icons/test.png</iconUrl>-->
    <!-- <copyright>Year Software Vendor</copyright> -->
    <!-- If there is a license Url available, it is is required for the community feed -->
    <!-- <licenseUrl>Software License Location __REMOVE_OR_FILL_OUT__</licenseUrl>
    <requireLicenseAcceptance>true</requireLicenseAcceptance>-->
    <!--<projectSourceUrl>Software Source Location - is the software FOSS somewhere? Link to it with this</projectSourceUrl>-->
    <!--<docsUrl>At what url are the software docs located?</docsUrl>-->
    <!--<mailingListUrl></mailingListUrl>-->
    <!--<bugTrackerUrl></bugTrackerUrl>-->
    <tags>test admin SPACE_SEPARATED</tags>
    <summary>__REPLACE__</summary>
    <description>__REPLACE__MarkDown_Okay </description>
    <!-- <releaseNotes>__REPLACE_OR_REMOVE__MarkDown_Okay</releaseNotes> -->
    <!-- =============================== -->      

    <!-- Specifying dependencies and version ranges? https://docs.nuget.org/create/versioning#specifying-version-ranges-in-.nuspec-files -->
    <!--<dependencies>
      <dependency id="" version="__MINIMUM_VERSION__" />
      <dependency id="" version="[__EXACT_VERSION__]" />
      <dependency id="" version="[_MIN_VERSION_INCLUSIVE, MAX_VERSION_INCLUSIVE]" />
      <dependency id="" version="[_MIN_VERSION_INCLUSIVE, MAX_VERSION_EXCLUSIVE)" />
      <dependency id="" />
      <dependency id="chocolatey-core.extension" version="1.1.0" />
    </dependencies>-->
    <!-- chocolatey-core.extension - https://chocolatey.org/packages/chocolatey-core.extension
         - You want to use Get-UninstallRegistryKey on less than 0.9.10 (in chocolateyUninstall.ps1)
         - You want to use Get-PackageParameters and on less than 0.11.0
         - You want to take advantage of other functions in the core community maintainer's team extension package
    -->

    <!--<provides>NOT YET IMPLEMENTED</provides>-->
    <!--<conflicts>NOT YET IMPLEMENTED</conflicts>-->
    <!--<replaces>NOT YET IMPLEMENTED</replaces>-->
  </metadata>
  <files>
    <!-- this section controls what actually gets packaged into the Chocolatey package -->
    <file src="tools\**" target="tools" />
    <!--Building from Linux? You may need this instead: <file src="tools/**" target="tools" />-->
  </files>
</package>
ferventcoder commented 7 years ago

Pulling out the specific elements:

<packageSourceUrl>Where is this Chocolatey package located (think GitHub)? packageSourceUrl is highly recommended for the community feed</packageSourceUrl>
<projectSourceUrl>Software Source Location - is the software FOSS somewhere? Link to it with this</projectSourceUrl>
 <docsUrl>At what url are the software docs located?</docsUrl>
<mailingListUrl></mailingListUrl>
<bugTrackerUrl></bugTrackerUrl>

<!-- eventually -->
<provides>NOT YET IMPLEMENTED</provides>
<conflicts>NOT YET IMPLEMENTED</conflicts>
<replaces>NOT YET IMPLEMENTED</replaces>

We have plans to implement more items in the nuspec (those may change) as well. Chocolatey has different needs than NuGet, so while compatible in many respects, we understand the authoring side may never be compatible.

Yes, I know about arbitrary properties. Unfortunately they do not provide a good user experience - they are a workaround, and don't make introduced elements first class citizens.

Yes, I realize it is very difficult to do what is being asked here, and would require some possibly very obtuse code to support these kinds of situations.

bbowman commented 7 years ago

Another possible solution is have @ferventcoder and friends fork nugetizer to use the custom nuget.core. Chocotizer?

heaths commented 7 years ago

@bbowman why fork? I don't want to have to install choco on build machines that already have nuget. As I offered above, I'm happy to contribute a change that, basically, is little more than capturing non-standard metadata into a dictionary and spit it back out. Nuget doesn't have to understand it. I've seen lots of property systems work this way, including Windows own IPropertyStore, various JSON serializers (i.e. to capture non-declared properties into a bag) including Json.NET and the one in the .NET BCL, some implementation of dynamic objects (including one I wrote for a static analysis project), and more.

If nuget is open to accepting it, I can work on it relatively soon.

ferventcoder commented 7 years ago

@heaths that would be great if it could be added.

Just one statement here I would like to address:

I don't want to have to install choco on build machines that already have nuget.

IMHO this is the equivalent of saying "I don't want to install NPM on systems that have Bower." or "I don't want to install Ruby on systems that have Python."

NuGet != Chocolatey

Just because the technologies use a similar framework does not make them the same thing.

Like I said though, if you and NuGet is/are open to getting this in, then from Chocolatey side, we're 👍 .

bbowman commented 7 years ago

@heaths I would definitely prefer not forking as well. I was just throwing it out there as an option since @ferventcoder was wanting first class citizen support. i think maybe the optimal route here is that Nuget.Core is enhanced for property bag support as you state and then @ferventcoder could move to using that version in Chocolatey / make first class citizen support on top of it. In terms of msbuild integrated authoring, @kzu / someone would need to enhance nugetizer to accept the property bag at pack time and then later if we want, maybe add a "IsChocolateyPackage" flag or something that knows about the new stuff or maybe fork to make those new things more first class like on the authoring side.

Let me ping some nuget folks to get their input.

ferventcoder commented 7 years ago

@ferventcoder could move to using that version in Chocolatey / make first class citizen support on top of it

We have other customizations to NuGet as well. It would take quite a bit more to merge back to using the original NuGet. We do have work coming up to move to NuGet 3/4, but it is on the schedule for later this year. One thing to consider - we've added first class support for package release version notation, but will keep it dark until more of the ecosystem can support it. See the huge warnings and read this for more info - https://github.com/chocolatey/nuget-chocolatey/issues/11

We fully understand Chocolatey's needs are different than NuGet's needs - and have had conversations about such things with @jeffhandley and others on the NuGet team in the past. While Chocolatey will always be compatible with NuGet packages, it may not always be that Chocolatey packages will be compatible with NuGet.

kzu commented 7 years ago

Well, given that the nugetizer authoring is CPS-based, extending it with additional property pages and capabilities shouldn't be hard.

As far as passing done additional properties during pack, it could be as easy as (say) a new <PackAdditionalProperties>Foo=Bar;...</PackAdditionalProperties> that we pass down to NuGet... (provided they support it).

ferventcoder commented 7 years ago

As far as passing done additional properties during pack, it could be as easy as (say) a new Foo=Bar;... that we pass down to NuGet... (provided they support it).

The end using system here is Chocolatey, so it needs to pack it in a way that will be recognized by Chocolatey. As long as it packs those properties appropriately, 👍 . My guess is the devil is in the details.