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

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

Open
jbaehr opened this issue Nov 14, 2019 · 10 comments
Open

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

jbaehr opened this issue Nov 14, 2019 · 10 comments

Comments

@jbaehr
Copy link

jbaehr commented Nov 14, 2019

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 #3623 (comment))

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 jbaehr changed the title paket pack and developmentDependency: Inconsistent behaviour on Winfdows/Linux paket pack and developmentDependency: Inconsistent behaviour on Windows/Linux Nov 14, 2019
@jbaehr jbaehr changed the title paket pack and developmentDependency: Inconsistent behaviour on Windows/Linux paket pack and developmentDependency: Fail on Linux (works on Windows) Nov 15, 2019
@forki
Copy link
Member

forki commented Nov 15, 2019

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

@jbaehr
Copy link
Author

jbaehr commented Nov 18, 2019

@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.

@forki
Copy link
Member

forki commented Nov 21, 2019

there is not lock file in the zip

@jbaehr
Copy link
Author

jbaehr commented Nov 22, 2019

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
Copy link
Author

jbaehr commented May 7, 2020

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.

@ruhullahshah
Copy link
Contributor

Root cause of the defect (explanation considers the project provided by @jbaehr and the code references are w.r.t commit a891adc 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.

@jbaehr
Copy link
Author

jbaehr commented Jul 10, 2020

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.

@ruhullahshah
Copy link
Contributor

ruhullahshah commented Jul 24, 2020

@jbaehr You are welcome. I have improved the fix for Windows further in #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
Copy link
Contributor

@jbaehr I have fixed and tested this for Linux as well. Please see #3888 for the root cause and the fix.

@jbaehr
Copy link
Author

jbaehr commented Aug 10, 2021

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants