CsprojToVs2017 icon indicating copy to clipboard operation
CsprojToVs2017 copied to clipboard

Exclude migration of SDK projects

Open moh-hassan opened this issue 5 years ago • 12 comments

I run dotnet-migrate-2017 from the solution root of SDK projects:

	dotnet-migrate-2017 migrate

The tool discovered the projects and start to backup, process the project file and showed many info W011 messages:

W011: path\to\project.csproj Unsupported 'NotEqual' expression in conditional

(no migration done)

I expect the tool exclude any csproj of type SDK , which can be detected by the first line

             <Project Sdk="Microsoft.NET.Sdk">

with an information message like:

Finding project myproj.csproj of type SDK , excluding migration

Also, i run the command passing a name of SDK project:

    dotnet-migrate-2017 migrate  Demo.csproj

An error message is displayed:

[.. ERR] TargetFramework cannot be determined from project file. The scenario is not supported and highly bug-prone.

It's nice if the tool can detect the SDK project with a message express excluding the project.

moh-hassan avatar Jan 08 '19 14:01 moh-hassan

We're intentionally not excluding them as we might be able to apply additional fixes with newer versions.

As to your warnings/exceptions. Do you have a project file for those?

hvanbakel avatar Jan 09 '19 04:01 hvanbakel

A demo solution containing 3 projects as a test case to repro the warning

moh-hassan avatar Jan 09 '19 14:01 moh-hassan

@moh-hassan TargetFramework-related non-fatal error is absolutely valid, since it is missing from the original project file. Not specifying TargetFrameworkVersion is bad, and the non-documented legacy project system behavior (net40 by default) is stupid.

andrew-boyarshin avatar Jan 09 '19 14:01 andrew-boyarshin

@moh-hassan NotEqual-related warning is issued correctly, and no errors are introduced by tool. The tool only warns that it might lead to missing possible project file simplifications, and again, it correctly detects dangerous subexpression (!= in Condition) and plays it safe not to introduce errors after conversion.

andrew-boyarshin avatar Jan 09 '19 14:01 andrew-boyarshin

We might later implement advanced support for such scenarios, but for now we simply make less assumptions and pay attention not to introduce errors after conversion.

andrew-boyarshin avatar Jan 09 '19 14:01 andrew-boyarshin

Any more questions regarding the output of the tool? Something unexpected?

andrew-boyarshin avatar Jan 09 '19 14:01 andrew-boyarshin

You are right. This project was initially migrated from the source and there was no warning from the dotnet-migrate-2017. In the second time when trying to migrate it again as SDK project it raises the error. It's nice if that warning is displayed when migrating vs2015 and no target framework is defined. BTW vs 2017 raise a warning and add ToolsVersion="15.0"

            <Project Sdk="Microsoft.NET.Sdk" ToolsVersion="15.0">

The warning of vs 2017 after migrating the project to SDK type is:

One-way upgrade Visual Studio will automatically make functional changes to the following projects in order to open them. You will not be able to open these projects in the version of Visual Studio in which they were originally created. - demo, "F:\CsprojToVs2017_224\sln1\Demo\demo.csproj"

moh-hassan avatar Jan 09 '19 14:01 moh-hassan

ToolsVersion="15.0"

Weird indeed.

andrew-boyarshin avatar Jan 09 '19 14:01 andrew-boyarshin

Can I know what is the valid expression of the NotEqual-related warning ? Is this because BuildTarget variable element isn't defined.

    <BuildTarget> fsharp </BuildTarget>

moh-hassan avatar Jan 09 '19 15:01 moh-hassan

@moh-hassan There is no issue with your project file. Warning is more like a notice for us, the developers of this tool, reminding us to implement more advanced expression evaluator. The tool still works correctly, no changes from user side needed.

andrew-boyarshin avatar Jan 09 '19 15:01 andrew-boyarshin

@andrew-boyarshin .Thanks for reply and explanation.

Sure, as said by @hvanbakel for SDK projects -We're intentionally not excluding them as we might be able to apply additional fixes with newer versions - is a good feature.

Just a suggestion, Let the re-process of SDK projects is controlled by a user option, so developers can concentrate on the messages that need a correction and avoid re-run the tool (by mistake) on the migrated projected.

moh-hassan avatar Jan 09 '19 16:01 moh-hassan

@moh-hassan ref #189.

andrew-boyarshin avatar Jan 09 '19 16:01 andrew-boyarshin