hvanbakel / CsprojToVs2017

Tooling for converting pre 2017 project to the new Visual Studio 2017 format.
MIT License
1.08k stars 120 forks source link

Migration removes all .cs files #252

Closed josh-degraw closed 5 years ago

josh-degraw commented 5 years ago

I've used this tool before without issue, but I just ran it on a solution with several projects and encountered a weird issue where it added a line in the modified .csproj files that for each MyFile.cs it found, it added the line:

<Compile Remove="MyFile.cs"/>

which, as the syntax implies, completely removed the files from the project-- they were all empty. It was simple enough to fix, but it seems like something that just shouldn't happen in the first place. This seems like a similar issue to #210, but in my case these were all just class library projects utilized in an ASP.NET MVC project, not WinForms, and none of the files were SubType'd.

mungojam commented 5 years ago

Were the files included in the original project? It's meant to do this just for files that weren't included in the original so that they don't automatically get included in the converted project

superstrom commented 5 years ago

I've seen this happen if you run migrate on an already VS2017-style project.

josh-degraw commented 5 years ago

@mungojam No they weren't explicitly referenced in the project, and @superstrom I think that might have been the case. Is there any sort of fix that can be done for that? I had manually updated a few of the projects, and I was hoping to clean up some elements that were still left over from that in the process.

mungojam commented 5 years ago

What command did you run to do the conversion and did you use the wizard? I thought it blocked conversions for VS2017 projects and made you run modernise instead

If you're able to share an example that would be really helpful

@andrew-boyarshin

superstrom commented 5 years ago

I can reproduce with

dotnet new console
dotnet migrate-2017 wizard

With Project2015To2017.Migrate2017.Tool-4.0.0

Project D:\dev\migtest\migtest.csproj is already CPS-based It appears you already have everything converted to CPS. Would you like to process CPS projects to clean up and reformat them? (Y/n) Yes Processing migtest... Project migtest has been processed Modernization can be progressed a little further but it might lead to unexpected behavioral changes. Would you like to modernize projects? (Y/n) Yes Would you like to create backups (Y/n) Yes Modernizing migtest... Backing up to D:\dev\migtest\Backup Project migtest has been modernized

<Project Sdk="Microsoft.NET.Sdk">
  <PropertyGroup>
    <OutputType>Exe</OutputType>
    <TargetFramework>netcoreapp2.1</TargetFramework>
  </PropertyGroup>
  <ItemGroup>
     <Compile Remove="Program.cs" />
  </ItemGroup>
</Project>
josh-degraw commented 5 years ago

@superstrom Yep, that's basically how it happened for me, and the output looks just like that.

hvanbakel commented 5 years ago

Yeah I think I can see how this happens: migrate looks for all the files in the csproj there are none, so all the files on disk must be excluded from the project thus insert all those removes 😄

I guess we can fix that, yes.

hvanbakel commented 5 years ago

Would you mind trying with: https://www.nuget.org/packages/Project2015To2017.Migrate2017.Tool/4.1.0-beta.3

It does seem to work for me given the repro case above.

superstrom commented 5 years ago

Yeah, the update is working for me.