Paket icon indicating copy to clipboard operation
Paket copied to clipboard

paket pack and developmentDependency: Fail on Linux (works on Windows)

Open jbaehr opened this issue 4 years ago • 10 comments

Description

tl;dr; paket pack handles development depencencies correctly (aka not placing it in the resulting dependency list) on Windows, but fails so on Linux (i.e. does add dependencies on development dependencies).

I have a library targeting netstandard20, developed using .NET Core SDK 2.2. I'm using a paket.template, type project, to create the nupkg via paket pack. Using paket v5.219.0, installed as a local dotnet tool.

The library depends on some other nugets, two of them being "development dependencies" (marked with <developmentDependency>true</developmentDependency> in their nuspec).

Running paket pack on a Windows 10 workstation results in a nupkg that does not have those development dependency nugets listed in its nuspec, as expected. Running the very same command, using the very same version of paket, on a Linux CI server, running Microsoft's official SDK-2.2 docker image, paket generates a nuspec that does have those dependencies listed.

Repro steps

(paket bootstrapping as in https://github.com/fsprojects/Paket/issues/3623#issuecomment-541098823)

paket-issue-3723.zip

On Windows:

git clone ....
dotnet restore .paket
dotnet build
.paket\paket pack

results in a correct nuget.

On Linux

git clone ....
dotnet restore .paket
dotnet build
.paket/paket pack

results in a nuget falsely depending on the development dependencies, too. (And having only \n as line separator, opposed to the \r\n in the windows-created nuget, but that's the only difference when diffing the two packages and doesn't matter at all)

Expected behavior

Running paket pack on Linux should honor the development dependencies correctly, just like it's doing already on Windows.

Actual behavior

On Linux, paket pack adds wrongly dependencies to development-only dependencies.

Known workarounds

Use excludeddependencies in the templates and list all development dependencies explicitly. -- This defeats the whole purpose of having developmentDependency, though.

I haven't checked dotnet pack on Linux, as on Windows it does not handle development dependencies (see #2370). In addition, dotnet pack reads the package version from the PackageVersion msbuild property while paket pack (with type project) infers it from the AssemblyInformationalVersion attribute, which fits better for us.

jbaehr avatar Nov 14 '19 16:11 jbaehr

can you please upload a small zip which I can use as test case?

forki avatar Nov 15 '19 15:11 forki

@forki please find the attached zip. You may need to git init after unpacking; I think a .git is required by GitVersionTask, the developer-dependency that should not end up in the nuspec of ClassLibrary1.
I also replaced the URLs of our internal nuget feed; hope that didn't broke anything.

The actual paket pack happens from within ClassLibrary1.csproj.

jbaehr avatar Nov 18 '19 14:11 jbaehr

there is not lock file in the zip

forki avatar Nov 21 '19 13:11 forki

there is not lock file in the zip

True. I ripped it off because it contained URL of internal feeds. I thought it would be easier/safer to recreate it with a paket install instead of me fiddling around in an otherwise generated file... If it helps you, I'll do it though.

jbaehr avatar Nov 22 '19 09:11 jbaehr

We now see the issue also on some windows machines building .NET Framework projects. Using the very same paket.exe (an older v5.181.1) and the very same source code, some windows-10 machines show this issue while others give reproducible good results. Maybe Paket relies on some undefined behaviour of some system/framework libs...

I'll investigate the differences of the win10 installations.

jbaehr avatar May 07 '20 09:05 jbaehr

Root cause of the defect (explanation considers the project provided by @jbaehr and the code references are w.r.t commit a891adcd0d1c97f785b99c614b5350e98a74f837 on Paket master):

Upon firing paket pack, Paket does the following:

  1. Reads the reference file of ClassLibrary1, that declares two dependencies: Newtonsoft.Json & GitVersionTask (findDependencies in PackageMetaData.fs)

  2. Reads the nuspec of each referenced packaged and decides whether to add it to the dependencies section of the resultant nupkg using: let nuspec = Nuspec.LoadFromCache(np.Name,rp.Version) not nuspec.IsDevelopmentDependency

  3. The question naturally arises that if Paket has a check to not include development dependencies, then why does it add GitVersionTask on some systems.

  4. The answer to 3) lies within the LoadFromCache method of the Nuspec type (Nuspec.fs), it tries to load the GitVersionTask nuspec from the local nuget cache (C:\Users\some_username\.nuget\packages) 4.1. If the nuspec is not found in the nuget cache, it returns a default nuspec wherein the isDevelopmentDependency is set to false! Nuspec.All {{References = All; Dependencies = Value is not created.; OfficialName = ""; Version = ""; LicenseUrl = ""; IsDevelopmentDependency = false; FrameworkAssemblyReferences = [];}} Dependencies: ThreadSafetyMode=ExecutionAndPublication, IsValueCreated=false, IsValueFaulted=false, Value=null FrameworkAssemblyReferences: Length = 0 IsDevelopmentDependency: false LicenseUrl: "" OfficialName: "" References: All Version: ""

  5. This causes GitVersionTask (a development dependency) to pass the test described in 2) and show up with a smiling face in the resultant nupkg, in this case ClassLibrary1....nupkg. Please note that if GitVersionTask was present in the local nuget cache, it would have failed the test in 2) and all would have been fine.

Potential Fix:

  1. Either the isDevelopmentDependency could be set to true in the default Nuspec.All as there seems to be no plausible explanation, atleast in my mind, to set it to false by default. This is a fragile solution though.
  2. Ignore the nupkg whose nuspec was not found in the local nuget cache. Again a workaround, no solid rationale behind the fix.
  3. Try to load the nuspec from the project packages directory, if it is not found in the local nuget cache and if that fails as well then return a Nuspec.All. In my opinion, this is a reliable solution and has a rationale behind it.

I would be happy to take up suggestions on other potential solutions that are not listed here, but for now I am happy to contribute a PR with solution 3.

ruhullahshah avatar Jun 26 '20 15:06 ruhullahshah

Thanks to the work of @ruhullahshah this issue was not observed on Windows any more. On Linux however, when building in a fresh docker container (debian with SDK 3.1.301 from here https://hub.docker.com/_/microsoft-dotnet-core-sdk) it is still reproducible with Paket-5.247.4.

jbaehr avatar Jul 10 '20 14:07 jbaehr

@jbaehr You are welcome. I have improved the fix for Windows further in https://github.com/fsprojects/Paket/pull/3886. I confirm that the issue is still reproducible on Linux, please stay tuned for a fix that I aim to contribute after analysing the problem on Linux.

ruhullahshah avatar Jul 24 '20 13:07 ruhullahshah

@jbaehr I have fixed and tested this for Linux as well. Please see https://github.com/fsprojects/Paket/pull/3888 for the root cause and the fix.

ruhullahshah avatar Jul 24 '20 21:07 ruhullahshah

Unfortunately, the issue still exists on Linux, lastly tried with Paket-5.257.0

jbaehr avatar Aug 10 '21 13:08 jbaehr