Home: Regression in preview nuget.exe v4.4.0 install command

[devlead]

Details about Problem

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

NuGet version (x.x.x.xxx): v.4.4.0 (preview)

OS version (i.e. win10 v1607 (14393.321)): 17025, but also confirmed on AppVeyor CI Windows 2016 Server

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

Detailed repro steps so we can see the same problem

Have a folder with packages.config in it, in that folder execute following command

1. nuget install
2. nuget install

Basically

1. no packages installed and get installed
2. packages already installed and as expected doesn't install anything

The issue comes with that when already installed at step to nuget.exe now returns exit code 1 and not 0 as before. And non 0 exit code indicates error for basically all existing CI systems which means this is a serious regression.

This is a exit code table comparing 4.4.0 vs. previous versions

|Version|Step|ExitCode|
|-------|----| --------|
|v4.4.0 |First|0|
|v4.4.0 |Second|1|
|v4.3.0 |First|0|
|v4.3.0 |Second|0|
|v4.1.0| First|0|
|v4.1.0 |Second|0|
|v3.5.0 |First|0|
|v3.5.0 |Second|0|

Verbose Logs

Attched detailed logs to issue
first_v3.5.0.log
second_v4.1.0.log
first_v4.1.0.log
second_v4.3.0.log
first_v4.3.0.log
second_v4.4.0.log
first_v4.4.0.log
second_v3.5.0.log

Sample Project

Used this PowerShell script to repro

$nugetExes =    @(
                    [PSCustomObject][Ordered]@{
                            Version = 'v4.4.0'
                            Url = 'https://dist.nuget.org/win-x86-commandline/v4.4.0/nuget.exe'
                    },
                    [PSCustomObject][Ordered]@{
                            Version = 'v4.3.0'
                            Url = 'https://dist.nuget.org/win-x86-commandline/v4.3.0/nuget.exe'
                    },
                    [PSCustomObject][Ordered]@{
                            Version = 'v4.1.0'
                            Url = 'https://dist.nuget.org/win-x86-commandline/v4.1.0/nuget.exe'
                    },
                    [PSCustomObject][Ordered]@{
                            Version = 'v3.5.0'
                            Url = 'https://dist.nuget.org/win-x86-commandline/v3.5.0/nuget.exe'
                    }
                )

$nugetExes | % {
    $packagesFolder = Join-Path (Resolve-Path "./") $_.Version
    if (Test-Path $packagesFolder)
    {
        Remove-Item -Recurse -Force $packagesFolder
    }
    New-Item -Type Directory $packagesFolder | Out-Null

    Copy-Item .\packages.config $packagesFolder

    Push-Location

    Set-Location $packagesFolder

    Invoke-WebRequest -Uri $_.Url -OutFile "$packagesFolder/nuget.exe"

    ./nuget.exe install -Source https://api.nuget.org/v3/index.json -Verbosity detailed | Out-File "$packagesFolder/first_$($_.Version).log"

    [PSCustomObject][Ordered]@{
        Version = $_.Version
        Step = 'First'
        ExitCode = $LASTEXITCODE
    }

    ./nuget.exe install -Source https://api.nuget.org/v3/index.json -Verbosity detailed  | Out-File "$packagesFolder/second_$($_.Version).log"

    [PSCustomObject][Ordered]@{
        Version = $_.Version
        Step = 'Second'
        ExitCode = $LASTEXITCODE
    }

    Pop-Location
}

Used this packages.config

<?xml version="1.0" encoding="utf-8"?>
<packages>
    <package id="Cake" version="0.23.0" />
</packages>

[nkolev92]

My guess is, it was probably introduced in https://github.com/NuGet/NuGet.Client/pull/1634

It seems like we don't differentiate between no packages being isntalled and a package restore failed.
https://github.com/NuGet/NuGet.Client/blob/dev/src/NuGet.Clients/NuGet.CommandLine/Commands/InstallCommand.cs#L211

//cc
@zhili1208

[zhili1208]

I can repro this, it's a regression

[devlead]

Ok great that you can repro. As most CI use exit codes to determine if a tool fails the build is not - this could potentially break quite a few builds.

[anangaur]

@devlead Agree. We plan to fix this before making 4.4.0 the default

[devlead]

Excellent 👍

[anangaur]

/cc: @rrelyea

[gep13]

@zhili1208 @anangaur can you help me understand why the milestone on this issue has been changed? Surely this is a serious regression that should be fixed in the next release?!?

[zhili1208]

@gep13 we just put all items in backlog for now, this will be fixed ASAP

[gep13]

@zhili1208 thanks for getting back to me. Will keep an eye out for an update, and help test/verify once it is available.

[emgarten]

Before 4.4.0 nuget.exe install was always returning an exit code of 0, even if restore failed completely. https://github.com/NuGet/Home/issues/5017

[emgarten]

As a workaround I suggest using nuget.exe restore instead of nuget.exe install.

Install calls the same restore behind the scenes, however it is less supported/tested. Really it should be deprecated and removed to avoid confusion.

[devlead]

There's thousands and thousands of scripts using the install command, removing it would IMHO be a far worse regression.

Also install & restore are slightly semantically different(especially when package id is used), so it’s just not find and replace, so while it might save maintainers some time to have one less command, it would cause several users a serious bag of hurt.

[emgarten]

nuget.exe install supports package id and also packages.config
nuget.exe restore supports solutions and also packages.config

Install makes sense when using a package id, but for packages.config is confusing that install and restore do almost the same thing.

[gep13]

@emgarten said...
Really it should be deprecated and removed to avoid confusion.

In the time frame of 5.x.x of NuGet this would make sense, and I would suggest another issue be raised to track that effort.

However, as we stand right now, given that this is accepted as a regression in the public facing API of NuGet.exe, I feel that the only correct course of action is to correct this regression, without the need for a workaround across an indeterminate (but likely huge) number of builds and automation tasks.

[emgarten]

@gep13 yes this will be fixed. My point was, for packages.config restore you should use nuget.exe restore instead of nuget.exe install.

[gep13]

@emgarten that is great news! Yip, understood, going forward, we will make a point of recommending that people do this.

Since the PR is now merged, is there a place that we can take the latest code for a spin? Or when will this be released? Thanks.

[emgarten]

4.4.0 nuget.exe will be released soon, it was held up only by this issue.

Thanks again for reporting the issue! I'm glad it was caught before 4.4.0 went out to everyone :smile:

[gep13]

@emgarten Perfect. Just to clarify, does that mean that 4.4.0 will switch to being available on the latest download URL?

[emgarten]

This fix will go into 4.4.1 and will move to the latest.

[gep13]

@emgarten did this fix go into 4.4.1? We have just had a report that this is still happening, as a result of using the latest download URL.

[gep13]

@emgarten I have just verified that the reported regression has indeed been fixed. Working with the person who reported still having an issue to figure out what is going on.

[emgarten]

@gep13 keep me posted if you find any other problems, thanks!