search

microsoft / ProjFS-Managed-API

A managed-code API for the Windows Projected File System
Other
142 stars 34 forks source link

Add .NET Core 3.1 targeting and package generation #40

Closed erikmav closed 4 years ago

erikmav commented 4 years ago

NetCore3.1 in VS 16.4 adds C++/CLI support for .NET Core. Add a second 3.1 targeting to the Framework version:

Issue #38

abhijeeg commented 4 years ago

Should we target both .Net Core and .Net Framework as seems to be the case in this PR ? @wilbaker, @cgallred - What's your take on this ?

Upgrade the NetFramework version to 4.7.2 minimum. Add a NetCore 3.1 target for ProjFS.Managed by factoring out common vcxproj portions and creating two distinct vcxproj files with different targets.

wilbaker commented 4 years ago

Should we target both .Net Core and .Net Framework as seems to be the case in this PR ? @wilbaker, @cgallred - What's your take on this ?

If we target .NET Core and .NET Framework then customers will not be forced to move to .NET Core (though they may need to move to a later version of .NET Framework).

Unless there is a large cost to supporting both I don't see a reason to force people onto .NET Core. @mjcheetham, what are your thoughts?

mjcheetham commented 4 years ago

If we target .NET Core and .NET Framework then customers will not be forced to move to .NET Core (though they may need to move to a later version of .NET Framework).

Unless there is a large cost to supporting both I don't see a reason to force people onto .NET Core. @mjcheetham, what are your thoughts?

Agreed. If it's not too much effort, keeping a .NET Framework target would be helpful for consumers still targeting Framework.

erikmav commented 4 years ago

RE: NetFramework vs. NetCore: My strong opinion is we should target both. As it stands most projects I'm working on will take years to move to NetCore simply because of the effort needed to move dependencies (like this one!) to Core or Standard one by one then update package references and retest end-to-end. Hundreds of packages at differing Framework vs. Core vs. Standard versions mean a lot of pain. Framework support should never go away in this package until Framework itself is removed from Windows (meaning: effectively never).

cgallred commented 4 years ago

@erikma I assume you changed the build script to require >= VS 16.4 because that has .NET Core 3.1, correct? That is what is breaking the validation checks. The Azure Pipelines pool ("Hosted Windows 2019 with VS2019") has "VisualStudio/16.3.6+29418.71". The build script is failing because it can't find a VS 16.4 or greater.

erikmav commented 4 years ago

@cgallred Correct on the 16 4 requirement, it is a strong requirement. I bet the 2019 image gets the update very soon, as I was able to successfully update to netcore31 in another project in mseng and everything was fine. IIRC they usually test there (ring 0) then roll outward.

cgallred commented 4 years ago

I bet the 2019 image gets the update very soon

@erikma okay, we'll re-submit the build/test periodically. Once it passes we'll complete the PR.

cgallred commented 4 years ago

@wilbaker the 2019 Azure image has been updated to VS 16.4 and the tests are now passing. I've approved the PR but I'd appreciate somebody with more .NET experience putting their stamp of approval on it before I merge. :-)

erikmav commented 4 years ago

@wilbaker RE Any CPU: See comment in resolved conversation, we have to leave it since VS locks up on trying to remove. The packaging target needs AnyCPU, and the test and main assembly targets are all explicitly set to x64 in each proj so the sln targeting doesn't matter anyway

wilbaker commented 4 years ago

@wilbaker RE Any CPU: See comment in resolved conversation, we have to leave it since VS locks up on trying to remove. The packaging target needs AnyCPU, and the test and main assembly targets are all explicitly set to x64 in each proj so the sln targeting doesn't matter anyway

@erikma thanks! I am having trouble finding the conversation you mentioned, but the details you included in this comment sound good to me.

erikmav commented 4 years ago

@wilbaker Latest commit downgrades to net461 to match previous

erikmav commented 4 years ago

I hadn’t known you still had an RS1 deployment targeting, net472 is what I’ve moved all our other projects to until we can get to netcore. I pulled it back to net461 as it was before.

From: Christian Allred notifications@github.com Sent: Tuesday, December 17, 2019 12:00 PM To: microsoft/ProjFS-Managed-API ProjFS-Managed-API@noreply.github.com Cc: Erik Mavrinac erikmav@microsoft.com; Mention mention@noreply.github.com Subject: Re: [microsoft/ProjFS-Managed-API] Add .NET Core 3.1 targeting and package generation (#40)

@cgallred commented on this pull request.

In NugetPackage/ProjectedFSLib.Package.csprojhttps://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fmicrosoft%2FProjFS-Managed-API%2Fpull%2F40%23discussion_r358999289&data=02%7C01%7Cerikmav%40microsoft.com%7Cb220aa3424a24bcc4b0e08d7832ba666%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637122095807758631&sdata=rnbbrNJc8Ectq%2B0QoE%2FuTd9hGVDHN54IvmLECdYfM9g%3D&reserved=0:

+

I don't know if there was a reason to upgrade the Framework version. @erikmahttps://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Ferikma&data=02%7C01%7Cerikmav%40microsoft.com%7Cb220aa3424a24bcc4b0e08d7832ba666%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637122095807768625&sdata=9Tb1LCqhifku6JiYomMw36O3tlAsXUicrSETHAo%2FhRc%3D&reserved=0?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHubhttps://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fmicrosoft%2FProjFS-Managed-API%2Fpull%2F40%3Femail_source%3Dnotifications%26email_token%3DAAHYCQQRXQPHQRVKJHXQUEDQZEVSVA5CNFSM4JR7D36KYY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOCPQXM6I%23discussion_r358999289&data=02%7C01%7Cerikmav%40microsoft.com%7Cb220aa3424a24bcc4b0e08d7832ba666%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637122095807778618&sdata=0Y9GnqlCfICYgBkW3Gbsc0BYyszVejv0kmcoWs03b7M%3D&reserved=0, or unsubscribehttps://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fnotifications%2Funsubscribe-auth%2FAAHYCQTPD6TUZUOS6YDXJOLQZEVSVANCNFSM4JR7D36A&data=02%7C01%7Cerikmav%40microsoft.com%7Cb220aa3424a24bcc4b0e08d7832ba666%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637122095807778618&sdata=%2Fhc982mwkDrZDRSArPG53%2FtFTqPUPZjCwy4BkkPXtJM%3D&reserved=0.

cgallred commented 4 years ago

@wilbaker @jrbriggs @mjcheetham Erik reverted to Framework 4.6.1. Do these changes look good to you now?

wilbaker commented 4 years ago

@wilbaker @jrbriggs @mjcheetham Erik reverted to Framework 4.6.1. Do these changes look good to you now?

These look good to me

cgallred commented 4 years ago

@wilbaker @jrbriggs @mjcheetham

FYI this has been uploaded to NuGet.org. Thanks again to @erikma for doing this work.

Cc: @abhijeeg @PingXie