RicoSuter/DNT

dnt switch-to-packages doesn't restore recursive

cluen opened this issue · 7 comments

cluen commented

Following setup:

  • CommonA
    -- CommonA.sln
    -- CommonA
    --- CommonA.csproj
  • CommonB
    -- CommonB.sln
    -- CommonB
    --- CommonB.csproj
  • ConsumcerC
    -- ConsumerC.sln (includes currently only ConsumerC.csproj)
    -- ConsumerC
    --- ConsumerC.csproj

CommonB references CommonA via nuget
ConsumerC references CommonA and CommonB via nuget

Created switcher.json for ConsumerC to replace CommonA and CommonB

{
    "solution": ConsumerC.sln,
    "mappings": {
    "CommonA": "../CommonA/CommonA.csproj",
    "CommonB": "../CommonB/CommonB.csproj"
    }
}

dnt switch-to-project switcher.json

nuget References get switched recursively to project references.
CommonB references CommonA as a project reference!
ConsumerC.csproj references CommonA and CommonB as a project reference!
ConsumerC.sln contains CommonA and CommonB + ConsumerC

dnt switch-to-packages switcher.json

Package references get switched back for ConsumerC.csproj, ConsumerC.sln contains only ConsumerC.csproj. Correct so far. But CommonB.csproj still contains the project reference to CommonA.csproj.

DNT (DotNetTools, https://github.com/RSuter/DNT, v1.2.2.0)

Hmm maybe we need to restore in the reverse order to fix this scenario?

cluen commented

@RicoSuter

Hmm maybe we need to restore in the reverse order to fix this scenario?

Didn't looked at your code, but it was the first thought crossed my mind.

We are having the same issue. Ideally dnt would not switch the recursive 'child' projects at all. We would rather run 3 separate switcher commands to keep everything in sync.

Same issue with me. Dnt is great tool, but this issue is stopping me using it at full potential.

Can confirm this behaviour. I'm presently still using the tool and just manually keeping track of cleaning up the recursive fixes for going back from projects to packages, but it'd be nice to have the tool handle it. I'll try and take a peek at the code and see how involved it'd be to fix.

So, it seems this is the culprit:

SwitchProjectsToPackagesCommand line 123

if (!mappedProjectFilePaths.Contains(projectFileName) || !configuration.RemoveProjects)

mappedProjectFilePaths contains a list of all projects in the mapping file. I'd argue that if switch-to-projects was used, and replaced something, switch-to-packages should undo that.. switch-to-projects after all keeps track of what was replaced in every fle it touched, but the condition above prevents a full rollback unless you set RemoveProjects to false.

Alternatively, switch-to-projects should be made to only swap out packages that have been specified - so if a file looked like this:

{ "solution": ConsumerC.sln, "mappings": { "CommonC": [ "../CommonA/CommonA.csproj" "../CommonB/CommonB.csproj" ] } }

With this file, I'd expect the project CommonB to remain unchanged, and CommonC would have CommonA and CommonB replaced by .csproj files. But in my tests with a more complex file (
{ "solution": ConsumerC.sln, "mappings": { "CommonA": "../CommonA/CommonA.csproj", "CommonB": [ "../CommonA/CommonA.csproj" ], "CommonC": [ "../CommonA/CommonA.csproj" "../CommonB/CommonB.csproj" ] } }

) I ended up having duplicate .csproj references.

@RicoSuter been debugging both switch-to-projects and switch-to-packages. And one thing I don't get:

switch-to-projects parses the solution, adds any .csproj from the switcher config that's not already present in the .sln, then goes through all projects in the .sln (AFTER adding the additional .csproj files), and replaces any package reference with the project reference. So, that it recursive- every project added to the .sln gets its packages replaced - including projects that were not part of the original .sln.
switch-to-packages on the other hand does not touch any .csproj that was only added due to execution of switch-to-projects unless removeProjects is false.

So, the operation is asymmetrical and leaves some the final result different from the starting point. Now, switcher.json being updated and containing the entirety of package replacements for all csproj, shouldn't dnt simply skip line 123 in SwitchProjectsToPackagesCommand ? We have configuration.Restore that tells us how packages were switched to projects - I'd argue that these changes should be fully restored on switch-to-packages regardless of whether projects added to the solution are removed again. I guess I'm saying I can't figure out the argument for doing it differently. Before I make a PR that gets rid of that condition I'd rather check with you. My alternative approach would just be adding a new switcher file flag to override the skip condition.