kswoll / ReactiveUI.Fody

C# Fody extension to generate RaisePropertyChange notifications for properties and ObservableAsPropertyHelper properties.
MIT License
154 stars 31 forks source link

2.1.0-beta67 Odd behavior (VS2017) #37

Closed SProst closed 7 years ago

SProst commented 7 years ago

I was able to use 2.0.66 without any issues, but as soon as I upgraded to the prelease 2.1.0-beta67 version (and changed to KirkFody) Reactive tagged properties no longer functioned correctly. I did not have any build issues however. Is there a list of known issues with the prerelease?

Visual Studio 2017, .NET 4.6.2 C#7

kswoll commented 7 years ago

So the goal of that pre-release is to try to fix fody/cecil's obliteration of round trip pdb information. It works, either the fody beta or the cecil beta is unfortunately leaving a file lock on some assemblies forcing you to restart VS or manually kill msbuild.exe processes. I'm intending to get back to it at some point. But if you're interested in trying to get it to work, do make sure you do not have the regular Fody package installed. And if that's going to cause problems because, for example, you have other fody packages that require the standard fody, you're basically out of luck for now. Hopefully Fody and Cecil ship their releases, fix their bugs, and we can just go back to using standard stuff.

SProst commented 7 years ago

Would detailed or diagnostic output during the msbuild process provide more insight into what is happening (or not happening) during build that could be causing reactiveui properties to not wire up correctly? Just want to provide helpful information for this issue rather than ambiguity.

kswoll commented 7 years ago

Thanks. I think you'll have to manually update your .csproj file to point to the correct Fody.targets file as well. i.e. it should point to the one in KirkFody rather than the one in Fody. Can you check if that is the case? (search for "Fody.targets")

SProst commented 7 years ago

Okay, so in a normal packages.config csproj project this is added only with version 2.0.66

<Import Project="..\packages\Fody.1.30.0-beta01\build\dotnet\Fody.targets" Condition="Exists('..\packages\Fody.1.30.0-beta01\build\dotnet\Fody.targets')" />

Otherwise, with version 2.1.0-beta67 or if the csproj file is PackageReference, that does NOT get added to the csproj file, which is obviously the source of the issues.

Regarding PackageReference csproj files -- what exactly is preventing the install script from adding that to the csproj file during NuGet installation? That seems like aberrant behavior, correct?

kswoll commented 7 years ago

Yeah, not sure. This pre-release stuff is mostly experimental -- was trying to fix debugging for our projects at work. And Fody seems like it's unmaintained (it's repo has been in a broken state for about half a year), so was trying to figure out how to get it all working.

Probably KirkFody didn't run the script properly because I just forked the Fody repo and probably someing in the .ps1 file was not expecting it to be in the new location. I'll spend some time ironing out the kinds at some point, but because of the assembly locking issue, we've reverted using this repo at work so it's not as high a priority as it was (for me).

SProst commented 7 years ago

It looks like this is a documented issue with PackageReference. https://github.com/NuGet/Home/issues/4942

At this point, it may make more sense for me to hold off using ReactiveUI.Fody until either Fody is patched or more information on the script installation is provided so this can be fixed. Based on your comment, it appears that they are not actively managing pull requests?

kswoll commented 7 years ago

@SProst all is well in Fody-land again. Fody 2.0 has been released, and this package has been updated to point to it.