remove dead code from nativemethods

[kasperk81]

followup on #9223

[JanKrivanek]

Hello @kasperk81 - thank you for your contribution! MSBuild team for sure appraciates contributions. Upfront heads up via bug or other form of discussion is preferred (plus an explicit confirmation of wanting to fix).

More details: https://github.com/dotnet/msbuild/blob/main/documentation/wiki/Contributing-Code.md

Each PR should ideally have motivation and goal in the description (links are big advantage, but important context should be part of the PR).

By no means would I want to kill the interest and flow :-) But it should be just little extra work that will save us considerable time.

---

In this specific PR - it mentiones it's a followup of the other PR that uses IO.Redist on NetFx, while here only single implementation is used - it's bit confusing to me. It looks great as code cleanup. Though as mentioned followup I'd expect the usage of IO.Redist or justification why not using it

[kasperk81]

while here only single implementation is used

the change in WindowsFileSystem.cs is following the other pr, MSBuildTaskHostFileSystem.cs is only built on netfx so it didn't required netcore tfm branch and that's the only difference there.

[JanKrivanek]

while here only single implementation is used

the change in WindowsFileSystem.cs is following the other pr, MSBuildTaskHostFileSystem.cs is only built on netfx so it didn't required netcore tfm branch and that's the only difference there.

Is it just code removal though? The previous implementation of MSBuildTaskHostFileSystem.FileOrDirectoryExists ended up in FileOrDirectoryExistsWindows

msbuild/src/Framework/NativeMethods.cs

Lines 1865 to 1869 in d8d42ef

	internal static bool FileOrDirectoryExistsWindows(string path)
	{
	WIN32_FILE_ATTRIBUTE_DATA data = new WIN32_FILE_ATTRIBUTE_DATA();
	return GetFileAttributesEx(path, 0, ref data);
	}

Now it calls System.IO.[File|Directory]

It looks like a change in a good direction. It however doesn't look like dead code removal. Let's please add a motivation + reaso ning into PR description so that we can proceed with reviewing.

[maridematte]

I agree with Jan here, please add more about the context as it is a bit hard to understand where these changes are coming from.

[kasperk81]

@JanKrivanek seems like we are reading too much into semantics in a simple code cleanup moving stuff from native to managed. not the first pr of its kind in this repo. p/invoking win32 api for file.exists check looks to be some legacy from 90s which is no longer needed as green ci status is showing up here