Thank you for the PR.

Since this is disabling builds on VS2015 (which are very important for CI) and is also touching the installer, I will not be able to merge your PR for the upcoming v1.12 (WinFsp 2022.2) release timeframe. However I should be able to merge your PR for the release after v1.12.

The major change in the release after v1.12 is that it will finally allow uninstall or upgrade of WinFsp without reboot. For this reason that release will be version 2.0 of WinFsp. (There are no backwards incompatible API changes in that release, but nevertheless enough things change that warrant a version change.)

The work for the release that will become version 2.0 of WinFsp is currently in the private branch pvt-sxs. I would advise against rebasing your work against that branch just yet: although the work on it is mostly complete I may still move things around.

My current plan is to release v1.12B2 (WinFsp 2022.2 Beta2) by mid-October, v1.12 (WinFsp 2022.2) by end of October and v2.0B1 (WinFsp 2023 Beta1) by end of October or early November. I will target to include your PR in v2.0B1.

Caveat: it is very important that we find a solution for building on VS2015, because it is used for CI. I faced the same problem of VS2015 not supporting new features when I ported WinFsp to ARM64. I ended up writing the custom script remove-build-arm64 that removes ARM64 builds from vcxproj files so that they can be built with VS2015. We may have to come up with a similar solution in this case as well.

Thank you for the feedback, CI should work now.

For the VS 2015 build I wrote a script to exclude anything that matches *dotnet* from winfsp.sln, so the library and memfs-dotnet are excluded.
memfs-dotnet had to be ported to .NET SDK and excluded because VS was trying to build the library as a dependency. Both projects are now built using dotnet build
Also the VS2015 image is provisioned with .NET Core 2.2 which doesn't work, so I use the official dotnet-install.ps1 script to get the latest version.

WinFsp 2022.2 (v1.12) has now been released. As previously discussed I am looking to merge your PR into the current master branch which will be released as WinFsp 2023 Beta1 (v2.0B1) in the coming days.

Disclaimer: I am not very familiar with the recent changes in the .NET Core tooling as my detailed knowledge about the .NET ecosystem dates from sometime before 2010. Therefore some of my comments may be misguided or plain wrong.

My comments regarding this PR:

• The WinFsp project has no external dependencies. This rule is slightly relaxed for the .NET components, but the intent is to have no dependencies other than what ships with the OS (Windows 7 and above) by default. I do not believe that your PR changes anything in this regard, but wanted to mention this as a general guideline.

• Ideally we would have a single .NET DLL (winfsp-msil.dll) that could be used in all variants of .NET, whether it is .NET Framework, .NET Core, .NET Standard, etc. Is this possible?

• If the above is not possible, then we would have to ship the current DLL (winfsp-msil.dll) but also other DLL's that target different variants of .NET. Which variants should we ship? Currently this PR continues to ship the Framework 3.5 DLL.

• The memfs-dotnet executable is used for testing and no external code should depend on it directly. Furthermore memfs-dotnet annoyingly depends on Framework 4.5. Would it be possible to "modernize" memfs-dotnet (e.g. ship the .NET Core version) without breaking the no "external dependencies" rule on old OS'es like Windows 7?

• You introduce files Directory.build.props into the build in order to set the properties OutputPath, BaseIntermediateOutputPath, and IntermediateOutputPath. The WinFsp project uses the single file build.common.props for project wide settings. But the above properties could also be maintained in the individual project files winfsp.net.csproj and memfs-dotnet.csproj. This would be my preference in order to minimize the changes introduced by this PR.

  • While it would make sense to move all such properties into the file build.common.props I recall it would cause problems with some versions of Visual Studio. For this reason I did not do this previously. (Note also that the C++ versions of these properties have different names OutDir, IntDir.)

• There is a lot of churn in files winfsp.net.csproj and memfs-dotnet.csproj (especially in the latter). My preference is for smaller changes that can be easily understood, ideally the minimal necessary to support the .NET core tooling. I understand however that these many changes may be necessary due to tooling differences.

Additionally note that some of properties for the NuGet package seem wrong or empty:

Hi, thanks for the CR

Ideally we would have a single .NET DLL (winfsp-msil.dll) that could be used in all variants of .NET, whether it is .NET Framework, .NET Core, .NET Standard, etc. Is this possible?

.NET Standard is basically only for libraries, which they must target so that they're compatible with a wide range of .NET versions
Unfortunately those versions don't include Framework 3.5 (only 4.6.1+ and Core) which is why we have to target both Standard and Framework 3.5 and generate two DLLs

If the above is not possible, then we would have to ship the current DLL (winfsp-msil.dll) but also other DLL's that target different variants of .NET. Which variants should we ship? Currently this PR continues to ship the Framework 3.5 DLL.

I was thinking of keeping Framework 3.5 only to not break existing projects, and encourage new ones to use the NuGet package

Even if we ship the Standard DLL as well, .NET Core projects won't be able to use it unless developers manually install the required dependencies in their project (System.IO.FileSystem.AccessControl and Microsoft.Win32.Registry -- the NuGet package does this automatically)

The memfs-dotnet executable is used for testing and no external code should depend on it directly. Furthermore memfs-dotnet annoyingly depends on Framework 4.5. Would it be possible to "modernize" memfs-dotnet (e.g. ship the .NET Core version) without breaking the no "external dependencies" rule on old OS'es like Windows 7?

Normally The .NET Core version will still need the .NET Core runtime (which no Windows version ships with, so it'd break the rule on all versions)
However we can publish a self-contained, single file executable which will bundle the runtime inside the executable, but it would increase the size substantially (a Hello World app that does this weighs 65MB)
We can explore trimming to reduce the size. The self-contained Hello World goes from 65MB to 11MB with this

This will need additional CI changes (for all VMs) because .NET Core .exe files aren't generated with dotnet build or normal Visual Studio build. We'd have to use dotnet publish or a Visual Studio publish profile
(In this PR, memfs-dotnet is an SDK project but still targets Framework only for this reason)

Perhaps this can be a separate issue/PR?

You introduce files Directory.build.props into the build in order to set the properties OutputPath, BaseIntermediateOutputPath, and IntermediateOutputPath. The WinFsp project uses the single file build.common.props for project wide settings. But the above properties could also be maintained in the individual project files winfsp.net.csproj and memfs-dotnet.csproj. This would be my preference in order to minimize the changes introduced by this PR.

While it would make sense to move all such properties into the file build.common.props I recall it would cause problems with some versions of Visual Studio. For this reason I did not do this previously. (Note also that the C++ versions of these properties have different names OutDir, IntDir.)

Setting those properties inside the .csproj files causes the build to fail with error CS0579 (duplicate attributes)
This is because in SDK projects, MSBuild already sets these properties by default earlier in the build process, so it's mandatory to override them in a Directory.Build.props file specifically

There is a lot of churn in files winfsp.net.csproj and memfs-dotnet.csproj (especially in the latter). My preference is for smaller changes that can be easily understood, ideally the minimal necessary to support the .NET core tooling. I understand however that these many changes may be necessary due to tooling differences.

The changes are not necessary per se, but all the properties removed are completely useless (either already defaults or don't do anything anymore).
SDK projects are immensely more compact and straightforward compared to Framework ones. So much so that in these migration examples, Microsoft replaces those massive .csproj files with a few lines
I can add them back to reduce the number of changes, but I think they would be a big burden down the line

I'll work on rebase & CR changes in the next few days

Ideally we would have a single .NET DLL (winfsp-msil.dll) that could be used in all variants of .NET...

.NET Standard is basically only for libraries... Unfortunately those versions don't include Framework 3.5 (only 4.6.1+ and Core) which is why we have to target both Standard and Framework 3.5 and generate two DLLs

Understood.

If the above is not possible, then we would have to ship the current DLL (winfsp-msil.dll) but also other DLL's that target different variants of .NET...

I was thinking of keeping Framework 3.5 only to not break existing projects, and encourage new ones to use the NuGet package.

This means that the new DLL would have to be distributed via the alternative NuGet mechanism. How does one publish a NuGet package? Would the WinFsp project need a nuget.org account and push the new DLL on every release?

(Note that the WinFsp release process is already quite complex and error prone (despite attempts to automate some of it). Adding steps increases the complexity and possibility for errors.)

Even if we ship the Standard DLL as well, .NET Core projects won't be able to use it unless developers manually install the required dependencies in their project (System.IO.FileSystem.AccessControl and Microsoft.Win32.Registry -- the NuGet package does this automatically).

I agree then that we should not ship the Standard DLL, at least for now. But let's assume for a second that these required dependencies did not exist. Is shipping a DLL via the WinFsp installer something that modern .NET developers expect or can work with?

FWIW I looked into what it would take to remove the cited dependencies:

• The System.IO.FileSystem.AccessControl dependency is baked into the public winfsp.net API and removing it would be a breaking change. This makes it a non-starter.
• The Microsoft.Win32.Registry dependency could easily be replaced with native Win32 calls.
• To clarify I do not expect you to do any of these for this PR, which we should keep minimal.

Would it be possible to "modernize" memfs-dotnet (e.g. ship the .NET Core version) without breaking the no "external dependencies" rule on old OS'es like Windows 7?

Normally The .NET Core version will still need the .NET Core runtime (which no Windows version ships with, so it'd break the rule on all versions)

Understood. We will not do this then.

However we can publish a self-contained, single file executable which will bundle the runtime inside the executable, but it would increase the size substantially (a Hello World app that does this weighs 65MB)
We can explore trimming to reduce the size. The self-contained Hello World goes from 65MB to 11MB with this

This will need additional CI changes (for all VMs) because .NET Core .exe files aren't generated with dotnet build or normal Visual Studio build. We'd have to use dotnet publish or a Visual Studio publish profile
(In this PR, memfs-dotnet is an SDK project but still targets Framework only for this reason)

Perhaps this can be a separate issue/PR?

I think you have completely convinced me that shipping a .NET core version of memfs-dotnet is not something we want! 😄

You introduce files Directory.build.props into the build in order to set the properties OutputPath, BaseIntermediateOutputPath, and IntermediateOutputPath. The WinFsp project uses the single file build.common.props for project wide settings. But the above properties could also be maintained in the individual project files winfsp.net.csproj and memfs-dotnet.csproj. This would be my preference in order to minimize the changes introduced by this PR.

Setting those properties inside the .csproj files causes the build to fail with error CS0579 (duplicate attributes)
This is because in SDK projects, MSBuild already sets these properties by default earlier in the build process, so it's mandatory to override them in a Directory.Build.props file specifically

I was just able to change the OutputPath property by modifying the project properties from Visual Studio. Visual Studio produced this diff:

diff --git a/build/VStudio/dotnet/winfsp.net.csproj b/build/VStudio/dotnet/winfsp.net.csproj
index 34dbc2f2..8a287ae3 100644
--- a/build/VStudio/dotnet/winfsp.net.csproj
+++ b/build/VStudio/dotnet/winfsp.net.csproj
@@ -79,4 +79,8 @@
[snipped]
+^M
+  <PropertyGroup Condition="'$(Configuration)|$(TargetFramework)|$(Platform)'=='Debug|netstandard2.0|AnyCPU'">^M
+    <OutputPath>C:\Users\billziss\Projects\winfsp\build\VStudio\build\Debug\winfsp.net2</OutputPath>^M
+  </PropertyGroup>^M
[snipped]

There is a lot of churn in files winfsp.net.csproj and memfs-dotnet.csproj (especially in the latter). My preference is for smaller changes that can be easily understood, ideally the minimal necessary to support the .NET core tooling. I understand however that these many changes may be necessary due to tooling differences.

The changes are not necessary per se, but all the properties removed are completely useless (either already defaults or don't do anything anymore)... I can add them back to reduce the number of changes, but I think they would be a big burden down the line.

Perhaps we can add those changes in future commits. My preference for minimal changes is because I want to be able to understand what is being changed, especially in areas where my own knowledge is lacking (like the modern .NET tooling).

This means that the new DLL would have to be distributed via the alternative NuGet mechanism. How does one publish a NuGet package? Would the WinFsp project need a nuget.org account and push the new DLL on every release?

I've set <GeneratePackageOnBuild> to true so a .nupkg file is generated when building the library (or we can call dotnet pack).
It does need to be pushed to nuget.org on every release, but it can be automated with dotnet nuget push (link)

Additionally, the NuGet package contains both DLLs, so even old Framework 3.5 projects that can't use the Standard DLL will be able to migrate (NuGet picks the appropriate DLL based on the project)

Also as you said, NuGet properties are missing (I only set PackageId and Version)
The rest are either optional or use AssemblyName by default (Authors/Company). Which should we set? I'll set Company and Description to those from build.version.props, what about Authors (same as Company maybe, or new prop)?

Is shipping a DLL via the WinFsp installer something that modern .NET developers expect or can work with?

Not really. There's no advantage to that and every other library in the project is going to be NuGet anyway
It's also kind of a hassle because we either have to use <Reference> and assume the DLL's location at compile-time, or read the InstallDir key at runtime and load the DLL using Reflection (which causes issues with trimming).

I was just able to change the OutputPath property by modifying the project properties from Visual Studio. Visual Studio produced this diff:

I was wrong about that, it's only BaseIntermediateOutputPath that causes problems:

When setting it in the .csproj, MSBuild generates AssemblyInfo in two folders which causes the errors

Perhaps we can add those changes in future commits. My preference for minimal changes is because I want to be able to understand what is being changed, especially in areas where my own knowledge is lacking (like the modern .NET tooling).

Got it, I'll only keep the .csproj changes that are necessary

It does need to be pushed to nuget.org on every release, but it can be automated with dotnet nuget push (link)

Understood.

The rest are either optional or use AssemblyName by default (Authors/Company). Which should we set? I'll set Company and Description to those from build.version.props, what about Authors (same as Company maybe, or new prop)?

Please set them from build.version.props. For Authors you can leave it blank or if it cannot be blank then use the Company name.

Not really. There's no advantage to that and every other library in the project is going to be NuGet anyway
It's also kind of a hassle because we either have to use <Reference> and assume the DLL's location at compile-time, or read the InstallDir key at runtime and load the DLL using Reflection (which causes issues with trimming).

I think I will eventually add the new DLL to the installer as an alternative to nuget.org. For now let us proceed with the plan as is however.

I was wrong about that, it's only BaseIntermediateOutputPath that causes problems.

When I was reorganizing the project build structure a couple of years ago, I tried to use file Directory.build.props but I seem to recall some problem with it. It was either not supported by VS2015 (which was even more important at the time than it is now) or it was not supported by C++ / kernel-mode projects or both. This is why I ended up creating the alternative build.common.props file instead.

If the Directory.build.props file really cannot be avoided, then please only introduce a single Directory.build.props file in the build root build\VStudio.

Thanks for the updated PR. I have some minor comments that do not require any changes on your part. I can make any necessary changes when the PR is ready for merging.

Unfortunately there is a problem on CI in test sample-passthrough-dotnet which is a show stopper:

warning MSB3270:
There was a mismatch between the processor architecture of the project being
built "MSIL" and the processor architecture of the reference "winfsp-msil,
Version=1.0.0.0, Culture=neutral, PublicKeyToken=b099876d8fa9b1f3,
processorArchitecture=MSIL", "AMD64". This mismatch may cause runtime failures.
Please consider changing the targeted processor architecture of your project
through the Configuration Manager so as to align the processor architectures
between your project and references, or take a dependency on references with a
processor architecture that matches the targeted processor architecture of your
project.

This means that the new winfsp-msil.dll is not a perfect replacement for the old one. In this case it looks like the problem is with the new winfsp-msil.dll, but even if the problem is actually caused by a setting in sample-passthrough-dotnet I would still want it to build and work without any changes with the new DLL. (In other words: I would like all existing projects that use winfsp-msil.dll to continue to work without any changes and without having to be rebuilt.)

BTW, I will be making a release of WinFsp 2023 Beta1 soon and we can hopefully include this PR in the next release (WinFsp 2023 Beta2).

UPDATE: WinFsp 2023 Beta1 has been released. (You do not need to rebase your PR against the latest changes.)

@Noire001 thank you very much for your contribution. It has been merged in.

I have added a few commits after your PR, most importantly:

• Removed the Directory.Build.props file by using this incantation (commit db72b57).

• Changed the output location for the assemblies/executables targeting the old .NET frameworks to be the same as prior to your PR (e.g. output the NET35 winfsp-msil.dll to build\VStudio\build\$(Configuration) instead of build\VStudio\build\$(Configuration)\winfsp.net\net35) (commit 0488451).

• Changed the Nuget package name to winfsp.net.