dotnet build continues when restore fails

[livarcocc]

From @forki on October 19, 2017 15:43

Steps to reproduce

1. git clone https://github.com/forki/restorebug.git
2. cd restorebug/console
3. dotnet build

Expected behavior

process stops after restore error (which we set from MyTargets,targets)

Actual behavior

the build continues after restore. And if you still have the assets file for weird reasons then build even succeeds.

Environment data

dotnet --info output: 2.0.2

Copied from original issue: dotnet/cli#7870

[livarcocc]

From @forki on October 19, 2017 15:52

This is really important for Paket since we try to let the build fail from targets file when certain conditions are not met. But instead it just continues. ¯_(ツ)_/¯

[livarcocc]

@forki Is this just a regular restore or are you doing some form of customization? I just tried it out on a project by updating a package reference to a non-existing version and I see that the restore failure is actually stopping the build.

Also, we have this piece of code in place: https://github.com/dotnet/cli/blob/master/src/dotnet/commands/RestoringCommand.cs#L52

So, if restore fails, it should stop the next command from running.

[livarcocc]

From @forki on October 19, 2017 16:37

@livarcocc did you follow the repro steps? This is calling dotnet build on a project that has targets. And the targets have an error task to explicitly stop the build

[livarcocc]

From @forki on October 19, 2017 16:41

But it does not. The error is only shown on command line and then build continuous

[livarcocc]

From @forki on October 19, 2017 16:46

So in other words: it's not the nuget task that fails, but targets along the way in "dotnet restore".

[livarcocc]

This may be a regression in 2.0.2:

git/restorebug/console
$ dotnet build
git/restorebug/MyTargets.targets(4,5): error : Explcit restore error. [git/restorebug/lib/lib.csproj]
git/restorebug/MyTargets.targets(4,5): error : Explcit restore error. [git/restorebug/console/console.csproj]

/git/restorebug/console
$ dotnet --version
2.0.0

Trying that locally with 2.0.2 now.

[livarcocc]

Yes, this is a regression in NuGet. So, in 2.0.2 we took in a new version of NuGet. It seems like their targets have changed and now your targets are not causing the restore operation to fail anymore.

I am going to move this issue over to NuGet/Home to have the NuGet team take a look at it.

cc @emgarten @rrelyea

[forki]

According to @livarcocc on twitter: this stops the build as expected if we use dotnet 2.0.0.

[jainaashish]

@emgarten can you take a look at this on priority if this is indeed a regression, then we might want to consider this for 15.5.

[emgarten]

Removing CollectPackageReferences from MyTargets.targets brings back the 2.0.0 behavior.

In 2.0.2 NuGet started calling CollectPackageReferences from command line restores, and that moves up the time this target is called to an earlier stage where the restore is walking the project to project graph. During this stage ContinueOnError is used to ignore projects which are valid, but do not import restore targets, this ends up as an error when the target is called in the project.

The solution to this is for NuGet to use the skip non-existent targets feature which recently went into MSBuild, and then we'll be able to remove ContinueOnError, and fail appropriately when a project has an actual error. This wasn't available for 2.0.2.

MSBuild feature issue: dotnet/msbuild#2471

[mishra14]

@emgarten Since the msbuild issue is closed, can we get this in?

[emgarten]

@mishra14 we need to use the msbuild feature in our targets. Let's track that work there.

[emgarten]

Add support for skipping non-existent targets this sprint.