Include assets filtered out by nuspec's References in runtime assets

tomkuijsten commented 7 years ago

Details about Problem

NuGet product used:

NuGet version: 3.5.0.1938 (3.5 RTM)
VS version: 2015 update 3

Detailed repro steps so we can see the same problem

Create a package using nuget.exe pack mypackage.nuspec, with content like:

<?xml version="1.0" encoding="utf-8"?>
<package xmlns="http://schemas.microsoft.com/packaging/2010/07/nuspec.xsd">
  <metadata>
    <id>Logging</id>
    <version>1.0.2-dev</version>
    <title>Logging Framework</title>
    <references>
      <reference file="Logging.dll" />
    </references>
  </metadata>
  <files>
    <!-- .NET desktop collection -->
    <file target="lib\net461\" src="..\bin\$configuration$\Logging.dll" />
    <file target="lib\net461\" src="..\bin\$configuration$\Logging.Debug.dll" />
    <file target="lib\net461\" src="..\bin\$configuration$\Logging.EventLog.dll" />
    <!-- Portable collection -->
    <file target="lib\netstandard1.4\" src="..\bin\$configuration$\Logging.dll" />
    <file target="lib\netstandard1.4\" src="..\bin\$configuration$\Logging.Debug.dll" />
  </files>
</package>

Create a .net 4.6.1 console app and add the package, I expected:
- The Logging.dll as a reference in visual studio
- All three dll's in the output folder

The files, however, are not copied to the output folder. Only the Logging.dll is present, the other two (Logging.Debug.dll and Logging.EventLog.dll) are missing. According to the nuspec documentation it should be copied.

What am I missing?

tomkuijsten commented 7 years ago

Anyone?

rohit21agrawal commented 7 years ago

@tomkuijsten does the package have all the DLLs in the correct place? trying to figure out if this is a restore issue or a pack issue.

rohit21agrawal commented 7 years ago

Just noticed that you have this in your nuspec :

<references>
      <reference file="Logging.dll" />
    </references>

If you remove this, you will get references to all the DLLs in your lib folder.

For more details, please refer to : https://docs.microsoft.com/en-us/nuget/schema/nuspec#specifying-explicit-assembly-references

tomkuijsten commented 7 years ago

The whole point was to not have the project reference to .Debug.dll and .EventLog.dll, but only have them copied to the output folder. Actually kind of the same question as #4236. That one solved the problem, thanks for helping out anyway.

maxtoroq commented 7 years ago

Originally, <references> (explicit assembly references) worked just like manually adding an assembly reference in Visual Studio, e.g. if you reference foo.dll which depends on bar.dll then bar.dll also gets copied, but only foo.dll is shown in the references list and in the project file. This works for all project types, even ASP.NET (with .refresh files).

With the new PackageReference element, <references> is treated as the exact list of assemblies to copy, which is a breaking change from its original behavior.

davidfowl commented 7 years ago

@maxtoroq the behavior should be the same.

@emgarten Doesn't the list of compile assemblies get filtered during restore based on the references list? The used to be.

emgarten commented 7 years ago

Doesn't the list of compile assemblies get filtered during restore based on the references list? The used to be.

That's correct. <references> gives you a subset of the selected lib folder.

https://docs.microsoft.com/en-us/nuget/schema/nuspec#specifying-explicit-assembly-references

@maxtoroq you may be thinking of <frameworkAssemblies> which NuGet reads from the nuspec and passes directly to Visual Studio to add. The intent of this was to add refererences to the framework, not *.dlls directly, but this likely still worked for packages.config projects.

maxtoroq commented 7 years ago

@emgarten NuGet does the right thing in copying only what's specified in <references>, but after that there's a step missing. Adding a reference is not just a matter of copying a dll to the bin. Perhaps taking the information from the assembly manifest, dependencies must be copied as well, recursively. If foo.dll depends on bar.dll, which in turn depends on baz.dll, then all three should end up in bin. And if lib contains other assemblies that are not an explicit (using <references>) or implicit dependency, then those should not end up in bin.

maxtoroq commented 7 years ago

Will this issue be re-opened?

emgarten commented 7 years ago

@maxtoroq can you give a detailed example of this? I am not understanding what you expect here. I would expect the package author to determine which assemblies should be added to the project and list them under references appropriately.

maxtoroq commented 7 years ago

@emgarten I've explained as best as I can in my comments, but I'll try one more time.

<PackageReference> is not working the same way as how NuGet have always worked in VS/MsBuild, which is how assembly references have always worked in VS/MsBuild.

Forget about NuGet for a minute. When you manually add an assembly reference in VS by selecting an assembly from your hard-drive, nothing is actually copied but VS remembers the location of that assembly using <HintPath>. When you build the project, VS copies the assembly to the bin, but also copies the assembly's dependencies. I believe dependencies are taken from the assembly manifest. This is done recursively, so if you selected A.dll, and A.dll depends on B.dll, and B.dll depends on C.dll, then all three are copied to bin, but only A.dll is listed as reference VS/MsBuild. If in the same directory there's a D.dll, that assembly is ignored because it's not directly or indirectly related on A.dll.

In NuGet, by default all assemblies in the lib folder are added as reference to the project. Using <references> you can control which assemblies are references and which are not (it's the package author's way of manually selecting an assembly from the hard-drive). The problem with <PackageReference> is that dependencies are not being copied when you build the project.

emgarten commented 7 years ago

@maxtoroq use dotnet publish

Build only requires the APIs you directly depend on, it doesn't bring in indirect assemblies. The publish step is what copies the entire closure of dependencies to an output folder. Is that what you are looking for?

maxtoroq commented 7 years ago

@emgarten No. I'm not even talking about .NET Core. Create a .NET Framework Class Library project and see for yourself.

emgarten commented 7 years ago

@maxtoroq please open a new issue and provide repro steps. What you are describing does not sound related to the issue of the references section in the nuspec, so let's move this discussion.

davidfowl commented 7 years ago

The problem with <PackageReference> is that dependencies are not being copied when you build the project.

The logic that expands the assembly closure is in the ResolveAssemblyReference task which should still be running when you use the <PackageReference> feature. That in itself though has nothing to do with nuget's handling of <reference> in the nuspec. There should be a file called project.assets.json in the obj folder. Look for your package inside of there and paste the section into github. It should have the assembly listed in <reference> in "compile" and "runtime" should have everything else.

maxtoroq commented 7 years ago

Both compile and runtime only have the assembly listed in references:

{
  "version": 2,
  "targets": {
    ".NETFramework,Version=v4.5.2": {
      "Saxon-HE/9.7.0.15": {
        "type": "package",
        "frameworkAssemblies": [
          "System",
          "System.Xml"
        ],
        "compile": {
          "lib/net40/saxon9he-api.dll": {}
        },
        "runtime": {
          "lib/net40/saxon9he-api.dll": {}
        }
      }
    },
    ".NETFramework,Version=v4.5.2/win": {
      "Saxon-HE/9.7.0.15": {
        "type": "package",
        "frameworkAssemblies": [
          "System",
          "System.Xml"
        ],
        "compile": {
          "lib/net40/saxon9he-api.dll": {}
        },
        "runtime": {
          "lib/net40/saxon9he-api.dll": {}
        }
      }
    },
    ".NETFramework,Version=v4.5.2/win-x64": {
      "Saxon-HE/9.7.0.15": {
        "type": "package",
        "frameworkAssemblies": [
          "System",
          "System.Xml"
        ],
        "compile": {
          "lib/net40/saxon9he-api.dll": {}
        },
        "runtime": {
          "lib/net40/saxon9he-api.dll": {}
        }
      }
    },
    ".NETFramework,Version=v4.5.2/win-x86": {
      "Saxon-HE/9.7.0.15": {
        "type": "package",
        "frameworkAssemblies": [
          "System",
          "System.Xml"
        ],
        "compile": {
          "lib/net40/saxon9he-api.dll": {}
        },
        "runtime": {
          "lib/net40/saxon9he-api.dll": {}
        }
      }
    }
  },
  "libraries": {
    "Saxon-HE/9.7.0.15": {
      "sha512": "J75qW11I5trJFbdQvo664RvpVOlbtxoYJDGEs3XgUsORUp6s4ojVeckm78bwZTAfTuaDk9YXfewi4Kc/PBHCYw==",
      "type": "package",
      "path": "saxon-he/9.7.0.15",
      "files": [
        "lib/net40/IKVM.OpenJDK.Charsets.dll",
        "lib/net40/IKVM.OpenJDK.Core.dll",
        "lib/net40/IKVM.OpenJDK.Text.dll",
        "lib/net40/IKVM.OpenJDK.Util.dll",
        "lib/net40/IKVM.OpenJDK.XML.API.dll",
        "lib/net40/IKVM.Runtime.dll",
        "lib/net40/saxon9he-api.dll",
        "lib/net40/saxon9he-api.xml",
        "lib/net40/saxon9he.dll",
        "notices/APACHE-RESOLVER.txt",
        "notices/APACHE-XERCES.txt",
        "notices/CERN.txt",
        "notices/FRIJTERS.txt",
        "notices/GPL+CLASSPATH.txt",
        "notices/JAMESCLARK.txt",
        "notices/LICENSE.txt",
        "notices/THAI.txt",
        "notices/UNICODE.txt",
        "saxon-he.9.7.0.15.nupkg.sha512",
        "saxon-he.nuspec",
        "tools/Query.exe",
        "tools/Query.exe.config",
        "tools/Transform.exe",
        "tools/Transform.exe.config"
      ]
    }
  },
  "projectFileDependencyGroups": {
    ".NETFramework,Version=v4.5.2": [
      "Saxon-HE >= 9.7.0.15"
    ]
  },
  "packageFolders": {
    "C:\\Users\\Max\\.nuget\\packages\\": {}
  },
  "project": {
    "version": "1.0.0",
    "restore": {
      "projectUniqueName": "C:\\temp\\VS\\ConsoleApp5\\ConsoleApp5\\ConsoleApp5.csproj",
      "projectName": "ConsoleApp5",
      "projectPath": "C:\\temp\\VS\\ConsoleApp5\\ConsoleApp5\\ConsoleApp5.csproj",
      "outputPath": "C:\\temp\\VS\\ConsoleApp5\\ConsoleApp5\\obj\\",
      "projectStyle": "PackageReference",
      "originalTargetFrameworks": [
        "net452"
      ],
      "frameworks": {
        "net452": {
          "projectReferences": {}
        }
      }
    },
    "dependencies": {
      "Saxon-HE": {
        "target": "Package",
        "version": "9.7.0.15"
      }
    },
    "frameworks": {
      "net452": {
        "dependencies": {
          "Saxon-HE": {
            "target": "Package",
            "version": "9.7.0.15"
          }
        }
      }
    },
    "runtimes": {
      "win": {
        "#import": []
      },
      "win-x64": {
        "#import": []
      },
      "win-x86": {
        "#import": []
      }
    }
  }
}

maxtoroq commented 7 years ago

@davidfowl did you see the json above?

davidfowl commented 7 years ago

@emgarten references seem to filter both runtime and compile elements. That would break these scenarios. It should only change what is compiled against.

emgarten commented 7 years ago

I've updated the title, this should be investigated to see if changing this behavior will break any other scenarios.

maxtoroq commented 6 years ago

@rohit21agrawal this issue is old, any news?

adamgauthier commented 5 years ago

I believe this issue needs more attention as it is still present and breaking for nuget packages that use <references>. These packages can't be safely used by both packages.config and PackageReference projects.

I am providing all code necessary to reproduce this issue in the hope that maintainers can address it. NuspecReferencesIssue.zip

Repro Steps

Download the attached .zip and extract it. This sample project is based on Nuget's very own documentation.

Change directory to ./MyPackage and observe ./MyPackage/MyPackage.nuspec, it is attempting to be compatible with both packages.config and PackageReference by using <references> and copying to ref\net472:


<?xml version="1.0" encoding="utf-8"?>
<package xmlns="http://schemas.microsoft.com/packaging/2010/07/nuspec.xsd">
<metadata>
    <id>MyPackage</id>
    <version>1.0.0</version>
    <description>MyPackage</description>
    <authors>MyPackage</authors>

    <dependencies>
        <group targetFramework="net472" />
    </dependencies>

    <references>
        <group targetFramework="net472">
            <reference file="MyLib.dll" />
            <reference file="MyHelpers.dll" />
        </group>
    </references>
</metadata>
<files>
    <file src="out\MyLib.dll" target="lib\net472" />
    <file src="out\MyHelpers.dll" target="lib\net472" />
    <file src="out\MyUtilities.dll" target="lib\net472" />
    <file src="out\MyLib.dll" target="ref\net472" />
    <file src="out\MyHelpers.dll" target="ref\net472" />
</files>
</package>

3. Build and pack the package:
```console
dotnet build .\src\MyPackage.sln --configuration Release --output .\out\
nuget pack

Open ../MyConsumers/MyConsumers.sln and add ../MyPackage/ as a nuget package source.
Build and run all 3 consumers (packages.config net472 project, PackageReference SDK project system net472 project and PackageReference SDK project system netcoreapp3.0 project)
Notice that only the PackageReference SDK project system net472 project fails at runtime because MyUtilities.dll is missing from the output directory.

Additional Notes

NuGet Version: 5.3.0.6260
dotnet version: 3.0.100
Related: https://github.com/NuGet/Home/issues/7276, https://github.com/NuGet/docs.microsoft.com-nuget/pull/1443

Conversation

@zivkan Are you aware that "Select Assemblies Referenced By Projects" is misleading as it is not possible to be compatible with both because of this bug.

Can we get any information on the priority of this issue for the team and can you provide at least a workaround? Is it possible to amend the docs in the meantime to mention this bug? Thank you.

zivkan commented 5 years ago

@louistio Thank you for taking the time to create the zip with the sample and writing such a good comment. I really appreciate it.

I confirm that there's a bug and this feature cannot work in it's current implementation for all scenarios. During asset selection, the PackageReference flow has a ApplyReferencesFilter method, but I don't understand why it applies the feature to RuntimeAssemblies. I beleive it should only apply to CompileTimeAssemblies. Note that groups.GetNearest(framework) returns null when the package was restored with AssetTargetFallback, and therefore all the runtime assemblies are selected (I don't understand why the compile time assemblies appear to be filtered correctly). So this functionality appears to accidentally work correctly with ATF, because ATF doesn't behave the same way as compatible packages do (which arguably is itself a bug).

maxtoroq commented 5 years ago

@louistio @zivkan FYI, the OP's example and mine also do not use ref targets.

maxtoroq commented 3 years ago

@zivkan You found the bug, and it doesn't look hard to fix. What is the problem? It's been 4 years now.

zivkan commented 3 years ago

@maxtoroq I've got more work to do than time, hence I need to prioritize what I work on. Few packages on nuget.org use this <references> feature, and we don't have telemetry to estimate how commonly it's used in pack (in case it's more common in private/other feeds), so it doesn't feel like it would benefit a large number of customers.

adamgauthier commented 3 years ago

Without getting into telemetry, I can speak anecdotally about the use case at my company and what led me to this issue initially, which I suspect is similar to other companies.

Our product is made up of a couple dozens legacy NuGet packages that haven't aged super well over the years. We were wanting to move the actual head csproj that aggregates all these dependencies to PackageReference/SDK-style csproj format, to greatly improve the developer experience in our project. In doing so, we were trying to find a way to make our NuGet packages compatible with both formats to migrate in a non-breaking way.

These legacy NuGet packages are hard to work with because they use manually written .nuspec files, have multiple DLLs and don't have a properly defined public facing API, things you don't usually see with more modern packages. After running into multiple issues such as this one, we decided to forget about packages.config compatibility and made a breaking change in packages we updated, moving consumers to PackageReference as soon as we could.

janstaelensskyline commented 2 years ago

Would it be possible to add fixing this bug back to the backlog? I've come across this myself while trying to make a NuGet starting from a NuSpec file and individual DLLs where only some of them should be referenced but the others should be present as dependencies for the referenced DLLs.

zivkan commented 2 years ago

We prioritize work on a number of factors, including upvotes. At the time I'm posting this, this issue has only 1 upvote (despite one of the comments having 6 upvotes 😕 ).

Since this issue is already assigned to me and tagged as quality week, I hope I'll eventually get to it without upvotes. But upvotes will help when we look at quality week issues sorted by upvotes (we do at least one quality week per month currently).

janstaelensskyline commented 2 years ago

It's upvotes on the original post that are used as the metric? edit: I'm asking, because I'm using this to create development packages for my company with DLLs extracted from an installer from another department. We spent some research time to try and workaround this but are currently going to release the development packages as packages.config support only.

But I think it'll cause issues with our developers when we release like this, because some existing solutions already use packagereferences and will have to downgrade to use packages.config.

I can probably get some upvotes for your metrics from my colleagues in my company who will be impacted. But I also don't want to skew your metrics too much...

NuGet / Home