fsprojects / Paket

A dependency manager for .NET with support for NuGet packages and Git repositories.
https://fsprojects.github.io/Paket/
MIT License
2k stars 520 forks source link

Change to Paket.Restore.targets breaks restore for Paket 5.174.0 #3284

Open garfoot opened 6 years ago

garfoot commented 6 years ago

Description

Performing a build with Paket 5.174.0 of .NET Standard 2.0 projects resulted in an updated Paket.Restore.targets file. This update includes the section below from PR https://github.com/fsprojects/Paket/pull/3273.

true

This stops the package restore on build when using Paket 5.174.0 and breaks all future builds for .NET Standard / Core projects which require a package restore to get their references.

Repro steps

Please provide the steps required to reproduce the problem

  1. Create a new .NET Standard project with Paket 5.174.0

  2. Add packages using Paket 5.174.0

  3. Build

  4. Check NuGet references

If possible then please create a git repository with a repro sample or attach a zip to the issue.

Expected behavior

The NuGet references should be visible in VS and usable in code.

Actual behavior

Build breaks with missing references and NuGet packages are not visible.

Known workarounds

Downgrade to Paket 5.173.4.

forki commented 6 years ago

I think something is wrong with the formatting above. I don't understand what error you have

forki commented 6 years ago

are you referring to

<PropertyGroup Condition="'$(PaketPropsVersion)' != '5.174.0' ">
  <PaketRestoreRequired>true</PaketRestoreRequired>
</PropertyGroup>

?

this should always call restore when the props files are not generated yet or are in wrong version.

garfoot commented 6 years ago

Sorry forgot to quote out the code block so it got messed up.

I just performed a build on a project that I went home building ok last night and when I built it this morning it replaced the Paket.Restore.targets file with a new one including the statement below.

<PropertyGroup Condition="'$(PaketPropsVersion)' != '5.174.0' ">
      <PaketRestoreRequired>true</PaketRestoreRequired>
</PropertyGroup>

This seems to force the build to not restore packages if using Paket 5.174.0 on build. The behaviour I'm seeing is that subsequent build now fail as all of the NuGet packages are missing from all .NET Standard projects as though the restore step had not been performed. I'm guessing it's something to do with the change in the referenced PR.

Reverting the changes to the Paket.Restore.targets or downgrading to 5.173.4 fixes the build, but trying to build with 5.174.0 updates the Paket.Restore.targets with that change and breaks all subsequent builds.

forki commented 6 years ago

@garfoot can you please upload a repro? because this all works for me.

garfoot commented 6 years ago

I'll try, unfortunately my repro has started working and I can't upload the project that's having a problem. I'm seeing the paket.props file in the obj folder but it's looking like this on the failing project

<?xml version="1.0" encoding="utf-8" standalone="no"?>
<Project ToolsVersion="14.0" xmlns="http://schemas.microsoft.com/developer/msbuild/2003">
    <PropertyGroup>
        <MSBuildAllProjects>$(MSBuildAllProjects);$(MSBuildThisFileFullPath)</MSBuildAllProjects>
        <!-- <RestoreSuccess>False</RestoreSuccess> -->
    </PropertyGroup>
</Project>

I'm guessing that it's finding the props file and not restoring again even though it failed?

forki commented 6 years ago

can you please nuke all /obj in the failing project and retry?

forki commented 6 years ago

also: are we talking for build inside VS or build from cmd line?

garfoot commented 6 years ago

Building in VS. I tried nuking the obj folder and it regenerated the props file ok but still wouldn't build.

I've uploaded a repro to https://github.com/garfoot/PaketTest which seems to repro on my machine. If I set Paket version to 5.174.0 it fails to build with missing references. If I set it to 5.173.4 and rebuild a couple of times it works fine.

forki commented 6 years ago

can you please take a look at yoir csproj. The TargetFramework should be netstandard2.0 right? did you miss the .?

/cc @matthid

garfoot commented 6 years ago

You're right, I had missed the . from it and it fixes it. Odd that VS detects it correctly without though and compiles. I also had to go and nuke the obj folders in all of the other projects too but it's all compiling now.

Thanks for the help getting to the bottom of it, PEBKAC strikes again.

forki commented 6 years ago

actually I do wonder if "netstandard20" is valid, then we need to fix our props file generator

garfoot commented 6 years ago

Well VS did seem to have no problem with it either in compiling it or showing the correct version in project properties so it could be a valid value. But new projects that I've not manually tweaked do use netstandard2.0.

I also tried netstandard16 without the . and VS detects that correctly as .NET Standard 1.6 so it's not just defaulting to 2.0.

forki commented 6 years ago

ok. let me fix that

garfoot commented 6 years ago

The official list of target framework monikers here https://docs.microsoft.com/en-us/dotnet/standard/frameworks does include the . though, so whilst it works in VS I don't think it's an offically supported moniker.

forki commented 6 years ago

ok latest version should also accept without .

garfoot commented 6 years ago

Great, thanks.

matthid commented 6 years ago

Yes I don’t think it actually is a officially supposed moniker. And afaik I left it out on purpose because it didn’t work properly. we might emit a warning...