search

DotNetAnalyzers / PropertyChangedAnalyzers

Roslyn analyzers for INotifyPropertyChanged
MIT License
44 stars 4 forks source link

Could not load file or assembly Gu.Roslyn.Extensions #111

Open bklebe opened 5 years ago

bklebe commented 5 years ago 
Analyzer 'PropertyChangedAnalyzers.AssignmentAnalyzer' threw an exception of type 'System.IO.FileNotFoundException' with message 'Could not load file or assembly 'Gu.Roslyn.Extensions, Version=0.4.4.1, Culture=neutral, PublicKeyToken=4b04740f2fd5868f' or one of its dependencies. The system cannot find the file specified.'.

I attempted installing the package in the error. It did not help. Any ideas?

JohanLarsson commented 5 years ago

Did this happen after updating? If so it is a bug in nuget.exe paket, handles it well.

For current version of this analyzer the project file should have:

<Analyzer Include="..\packages\PropertyChangedAnalyzers.2.7.1.0\analyzers\dotnet\cs\Gu.Roslyn.Extensions.dll" />
<Analyzer Include="..\packages\PropertyChangedAnalyzers.2.7.1.0\analyzers\dotnet\cs\PropertyChangedAnalyzers.dll" />

Assuming old .csproj. As you can see Gu.Roslyn.Extensions.dll is distributed with the analyzer.

I'm not sure there is a good way to pack analyzers with helper libraries.

JohanLarsson commented 5 years ago

I've been thinking about doing whet fody costura does so we can ship the helper lib as a resource in the analyzer. That would avoid bugs like this, chances are it will not work when running in VS. Another alternative could be to IL-merge it.

bklebe commented 5 years ago

No, it didn't happen after an update. This was a clean install on VS2019, originally with the brand spanking new .NET Framework 4.8, but I retried it with 4.7.2 after I got these errors, to no avail.

On Sun, Apr 21, 2019 at 9:15 AM Johan Larsson notifications@github.com wrote:

I've been thinking about doing whet fody costura does so we can ship the helper lib as a resource in the analyzer. That would avoid bugs like this, chances are it will not work when running in VS. Another alternative could be to IL-merge it.

JohanLarsson commented 5 years ago

I tried installing it in a new project with VS2019 and 4.7.2, no repro, everyhing worked as far as I can tell. Is your project on GitHub? Are you using other analyzers?

JohanLarsson commented 5 years ago

Is it still an issue? We need to find a way to reproduce and fix it if so.

bklebe commented 5 years ago

Yeah, I still need to nail down a repro. Sorry about that. I will note I also attempted to manually install Gu.Roslyn.Extensions, which ended up polluting my packages directory so badly I decided to create a new project (I have a test solution for experimenting with different analyzers)

JohanLarsson commented 5 years ago

Can it be that you have disabled running powershell scripts in nuget packages? Gu.Roslyn.Extensions is added via install.ps1 when using old csproj.

bddckr commented 5 years ago

I have the same issue, weirdly enough only in my Azure DevOps pipeline CI builds...

I believe the issue is solved with this commit https://github.com/GuOrg/Gu.Roslyn.Extensions/commit/78c17dff9d8056ab2873bf4426b3840bbecbab59#diff-62a535edf871d30ecbb5443ca2691860 and all that is missing right now is to create a new NuGet package release for PropertyChangedAnalyzers that uses the latest Gu.Roslyn.Extensions.

JohanLarsson commented 5 years ago

I have the same issue, weirdly enough only in my Azure DevOps pipeline CI builds...

@bddckr are you using other analyzers that depend on Gu.Roslyn.Extensions? Is the project file available on GitHub, if so I can have a look?

I'll try to find time to make a release tomorrow to see if we can get you unstuck.

For the future we are planning: https://github.com/GuOrg/Gu.Roslyn.Extensions/issues/59 Hopefully it will end this pain. We need to bump version to netstandard 2.0 for doing it as AppDomain is not available before that. Bumping means VS2019+ moving forward so we should try to close some issues while we are still supporting VS2017.

bddckr commented 5 years ago

@bddckr are you using other analyzers that depend on Gu.Roslyn.Extensions?

I just checked and the only easy way for me to tell whether I do is by looking at the analyzer dependencies shown in Visual Studio, as that currently still works until the referenced work in GuOrg/Gu.Roslyn.Extensions#59 is done I suppose.

Reading through that issue it sounds like that is actually my issue here, too? Gu.Roslyn.Extensions is listed twice in my project:

assemblyref://Gu.Roslyn.Extensions (C:\Users\bddckr\.nuget\packages\idisposableanalyzers\2.1.2\analyzers\dotnet\cs\Gu.Roslyn.Extensions.dll)
assemblyref://Gu.Roslyn.Extensions (C:\Users\bddckr\.nuget\packages\propertychangedanalyzers\2.7.2\analyzers\dotnet\cs\Gu.Roslyn.Extensions.dll)

Is the project file available on GitHub, if so I can have a look?

Sadly it's a private project.

I'll try to find time to make a release tomorrow to see if we can get you unstuck.

Does knowing I have two analyzers reference it (see above) change anything? I double-checked and I can at least say for certain the only logs I get during my builds are because of PropertyChangedAnalyzers. Not a single one for IDisposableAnalyzers.

For the future we are planning: GuOrg/Gu.Roslyn.Extensions#59 Hopefully it will end this pain. We need to bump version to netstandard 2.0 for doing it as AppDomain is not available before that. Bumping means VS2019+ moving forward so we should try to close some issues while we are still supporting VS2017.

Thankfully this wouldn't be an issue for me! 😄

JohanLarsson commented 5 years ago

Does knowing I have two analyzers reference it (see above) change anything?

Yes, it makes it very likely that a bug in nuget broke things for you when you updated.

You want the project file to look like:

<Analyzer Include="..\packages\PropertyChangedAnalyzers.2.7.2.0\analyzers\dotnet\cs\Gu.Roslyn.Extensions.dll" />
<Analyzer Include="..\packages\PropertyChangedAnalyzers.2.7.2.0\analyzers\dotnet\cs\PropertyChangedAnalyzers.dll" />
<Analyzer Include="..\packages\IDisposableAnalyzers.2.1.2.0\analyzers\dotnet\cs\Gu.Roslyn.Extensions.dll" />
<Analyzer Include="..\packages\IDisposableAnalyzers.2.1.2.0\analyzers\dotnet\cs\IDisposableAnalyzers.dll" />

When updating nuget removes the first instance it finds of Gu.Roslyn.Extensions and that breaks things if more than one analyzer relies on different versions of the helper library.

bddckr commented 5 years ago

Is there a way to check or configure this with the new project file format? I'm using PackageReference:

    <PackageReference Include="PropertyChangedAnalyzers" Version="2.7.2">
      <PrivateAssets>all</PrivateAssets>
      <IncludeAssets>runtime; build; native; contentfiles; analyzers</IncludeAssets>
    </PackageReference>
    <PackageReference Include="IDisposableAnalyzers" Version="2.1.2">
      <PrivateAssets>all</PrivateAssets>
      <IncludeAssets>runtime; build; native; contentfiles; analyzers</IncludeAssets>
    </PackageReference>
JohanLarsson commented 5 years ago

I don't know nuget well, have only used paket the last years.

jnm2 commented 5 years ago

Is there a way to check or configure this with the new project file format? I'm using PackageReference:

I don't think so. By design, \ and \ elements are managed for you by the .NET SDK.

JohanLarsson commented 5 years ago

You can run dotnet restore and check the contents of obj/project.assets.json. I can try to repro tomorrow, a bit busy with another library right now.

jnm2 commented 5 years ago

If you were publishing a nupkg artifact at https://ci.appveyor.com/project/JohanLarsson/gu-roslyn-extensions/builds/26405646/artifacts, @bddckr could test his theory that the master branch is already fixed.

bddckr commented 5 years ago

You can run dotnet restore and check the contents of obj/project.assets.json. I can try to repro tomorrow, a bit busy with another library right now.

I can see each package lists analyzers/dotnet/cs/Gu.Roslyn.Extensions.dll properly:

...
    "IDisposableAnalyzers/2.1.2": {
      "sha512": "hwlTC7QQq3+YY5wr94DETqDgU0sUBDbd0zoxI5hMnePZqooZHAwy3HvRwOmE1jZ4tX+iENBLKyWycKBaIFB2Yw==",
      "type": "package",
      "path": "idisposableanalyzers/2.1.2",
      "hasTools": true,
      "files": [
        ".nupkg.metadata",
        ".signature.p7s",
        "analyzers/dotnet/cs/Gu.Roslyn.Extensions.dll",
        "analyzers/dotnet/cs/IDisposableAnalyzers.dll",
        "idisposableanalyzers.2.1.2.nupkg.sha512",
        "idisposableanalyzers.nuspec",
        "tools/install.ps1",
        "tools/uninstall.ps1"
      ]
    },
    "PropertyChangedAnalyzers/2.7.2": {
      "sha512": "14loYCit6A7t5ogmEaVwr6ht2LlZ5oYuaAtW7uNtSc4krAS4Gmn8Vg4+8aMCRG4YT2nSeD6op6HfpGUiEMHM2w==",
      "type": "package",
      "path": "propertychangedanalyzers/2.7.2",
      "hasTools": true,
      "files": [
        ".nupkg.metadata",
        ".signature.p7s",
        "analyzers/dotnet/cs/Gu.Roslyn.Extensions.dll",
        "analyzers/dotnet/cs/PropertyChangedAnalyzers.dll",
        "propertychangedanalyzers.2.7.2.nupkg.sha512",
        "propertychangedanalyzers.nuspec",
        "tools/install.ps1",
        "tools/uninstall.ps1"
      ]
    },
...

The versions of Gu.Roslyn.Extensions in each:

I think my theory is moot: If I downgrade PropertyChangedAnalyzers to 2.7.1 I can see it's using 0.4.4.1-dev, too. I can confirm that actually ends up without any of the FileLoadExceptions and my build output is completely clean that way. You mentioned this above already, I just didn't check it so far 😅

Here's a minimal project to reproduce the issue: https://github.com/bddckr/AD0001-FileLoadException Open PowerShell in the root of that project, then run dotnet build --no-incremental to see the warnings pop up. If you repeatedly build with just dotnet build you'll notice only the first build (i.e. the fresh one) is giving the warnings. So that's why I'm only seeing this in my CI. I'm not sure why these warnings never show up in Visual Studio...

I noticed if I install the packages via Visual Studio it actually adds buildtransitive to the IncludeAssets for both packages. That doesn't change the outcome, though, and I'm still getting the warning.

Is this a NuGet bug? An MSBuild one? I mean I'd expect each package to be fine coming with its own version of the same Gu.Roslyn.Extensions but maybe that's exactly what you mentioned earlier being the NuGet bug. Is there an issue for this open already? This hopefully isn't by design...

JohanLarsson commented 5 years ago

Awesome research!

If I downgrade PropertyChangedAnalyzers to 2.7.1

We must fix this, quickest way forward is to release new versions of everything that depends on Gu.Roslyn.Extensions

Is this a NuGet bug? An MSBuild one? I mean I'd expect each package to be fine coming with its own version of the same Gu.Roslyn.Extensions but maybe that's exactly what you mentioned earlier being the NuGet bug. Is there an issue for this open already?

I created https://github.com/NuGet/Home/issues/8225 and I think another for old csproj. Maybe you can open a new issue with your awesome repro? My guess is that this is a nuget bug. The bug I mentioned was when updating with old csproj, may be due to the same root cause.

Shot in the dark for fixing CI: Maybe an explicit dotnet restore in the build script can help?

bddckr commented 5 years ago

Shot in the dark for fixing CI: Maybe an explicit dotnet restore in the build script can help?

I'm doing that already before the dotnet build. Running twice and ignoring the first run's output works, but adds to the CI duration which I don't like...

I'll look at opening an issue on the NuGet repo soon. I'll test with the old csproj first.

jnm2 commented 5 years ago

Unless you're doing 'dotnet build --no-restore' in CI, explicit 'dotnet restore' is unnecessary.

bddckr commented 5 years ago

It is necessary as I have a caching setup and I am saving the cached state before it potentially being "polluted" by the build output.

With or without --no-restore doesn't change anything and I can't make the issue crop up at all after the initial build logs those warnings.

I'll check with the old project format, then raise an issue, but in general I think this can be treated as a wont-fix since all analyzers getting updated to use the latest is probably easiest (and hoping this bug in NuGet/MSBuild gets fixed).

JohanLarsson commented 5 years ago

I did not suggest using old csproj, just to be clear. <PackageReference> and new csproj is nicer. Just that my old repro was for old csproj.

bddckr commented 5 years ago

Totally clear here, just thinking it's valuable information to pass to the NuGet team when raising a new issue. I'm still trying to figure out why Visual Studio seems to be fine, while dotnet build (which runs through the same MSBuild as VS AFAIK!) is giving the warnings.