Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove requirement of setting RuntimeIdentifier in project file for .NET Framework Exe projects #396

Closed
dsplaisted opened this issue Nov 16, 2016 · 3 comments · Fixed by #847

Comments

@dsplaisted
Copy link
Member

Right now we require that .NET Framework Exe projects set a RuntimeIdentifier property. This is because ASP.NET Core projects depend on the native library in the libuv package. In .NET Core we have a way of deploying both x86 and x64 versions and loading the correct one at runtime, but we don't have this for .NET Framework. That means we effectively don't support AnyCPU for projects that depend on Libuv (or other packages authored in the same way).

At a minimum, we should change it so that the RuntimeIdentifier is only required when there is a package in the dependency graph that has architecture-specific assets.

We should also try to get rid of the requirement of settingRuntimeIdentifier in web projects. This could include setting it to a default in the Web SDK targets when targeting .NET Framework, or updating the Libuv and Kestrel packages to deploy both architectures and load the right native DLL at runtime.

@davidfowl
Copy link
Member

or updating the Libuv and Kestrel packages to deploy both architectures and load the right native DLL at runtime.

Kestrel does the right thing already and libuv already has the right bits inside of it. So changing the package would be a mistake. What you really want is to do what we do in .NET Core and have a runtime library that can look for things by RID. That library could be used by libraries that have native dependencies to properly load the right ones. This way we don't need to update each package with native dependencies to have custom msbuild tasks.

@dsplaisted
Copy link
Member Author

@davidfowl

You're right, another option would be to make the same magic that works for .NET Core work for .NET Framework. My understanding was that this would require changes to the .NET Framework itself, so there would be a higher barrier to doing it and it wouldn't apply to versions of the Framework that already shipped.

If you wanted to update the packages to work on .NET Framework as AnyCPU, you don't need any custom MSBuild targets or tasks. You'd just need the packages to deploy the native libraries in different subfolders, which I think NuGet already does for .NET Core, and which in any case you can do by declaring them as content files. Then in the Libuv constructor before you use any of the DllImports from Libuv, you do a LoadLibrary on the right version of the native library, like this:

#if NET451
            string libuvPath;
            if (Marshal.SizeOf<IntPtr>() == 4)
            {
                libuvPath = "x86\\libuv.dll";
            }
            else
            {
                libuvPath = "x64\\libuv.dll";
            }
            NativeMethods.LoadLibrary(libuvPath);
#endif

Then the DllImports for Libuv methods will resolve to the native library you already loaded.

@davidfowl
Copy link
Member

We already have something like this for SQLLite https://github.com/aspnet/Microsoft.Data.Sqlite/blob/f04228cac9c29e2b8e0362749be47f9f6350df86/src/Microsoft.Data.Sqlite/Interop/NativeMethodsImpl.cs#L24. Maybe we should move this into a netstandard library more people should be using.

/cc @bricelam

@srivatsn srivatsn added the Bug label Nov 21, 2016
@srivatsn srivatsn added this to the 1.0 RC2 milestone Nov 21, 2016
@srivatsn srivatsn modified the milestones: 1.0 RC2, 1.0 RC3 Nov 29, 2016
@srivatsn srivatsn modified the milestones: 1.0 RC3, 1.0 RTM Jan 13, 2017
@nguerrera nguerrera added the In PR label Feb 9, 2017
dougbu added a commit to aspnet/IISIntegration that referenced this issue Mar 12, 2017
- not clear either change will clean up CI failures on Win7 and Win2008 but can't hurt
  - don't see other likely suspects
- dotnet/sdk#396 has been fixed; web sites target x86 automatically

nit: sort dependencies
dougbu added a commit to aspnet/IISIntegration that referenced this issue Mar 13, 2017
- remove unnecessary x64 and x86 configurations from the solution
- dotnet/sdk#396 has been fixed; web sites target x86 automatically

nit: sort dependencies
sbomer pushed a commit to sbomer/sdk that referenced this issue Sep 19, 2017
Corrected some grammar the FAQ
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants