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

Enable RID-specific Apps on Shared Framework #1053

Merged
merged 5 commits into from
Apr 3, 2017

Conversation

eerhardt
Copy link
Member

Allow .NET Core applications to be RID-specific, but still run on the Shared Framework. This enables an application to carry only the native assets it needs for the specific runtime it intends to run on.

For example, if an application uses a graphics library and wants to run on the shared framework, it would carry that graphics library for all the runtimes the graphics library supported (win, osx, linux, x86, x64, arm, etc). But if the application is only intended to run on one runtime, these extra assemblies bloat the app.

@ericstj @nguerrera @dsplaisted @livarcocc

/cc @Petermarcu

Notes:

There are 2 other changes that need to go into other repos:

  1. dotnet/cli will add a new option to publish --self-contained[:false|true], which flows into the MSBuild $(SelfContained) property added here.
  2. We need to change a single line in the DependencyModel library, which assumes "IsPortable" means "FrameworkDependent". https://github.com/dotnet/core-setup/blob/master/src/Microsoft.Extensions.DependencyModel/DependencyContextLoader.cs#L74-L76

A future, potential scenario is to publish self-contained, but don't specify any RID. This won't work today because dotnet restore hasn't restored for that RID, thus the necessary assets are not available. This can be revisited when restore occurs during publish. For now, that scenario raises an error with a message saying it is unsupported.

ericstj and others added 2 commits March 28, 2017 12:52
This option enables a RID-specific application to still use the shared
framework.
Allow .NET Core applications to be RID-specific, but still run on the Shared Framework.  This enables an application to carry only the native assets it needs for the specific runtime it intends to run on.

For example, if an application uses a graphics library and wants to run on the shared framework, it would carry that graphics library for all the runtimes the graphics library supported (win, osx, linux, x86, x64, arm, etc).  But if the application is only intended to run on one runtime, these extra assemblies bloat the app.
@@ -125,7 +124,7 @@ public DependencyContext Build()
_projectContext.LockFileTarget.TargetFramework.DotNetFrameworkName,
_projectContext.LockFileTarget.RuntimeIdentifier,
runtimeSignature,
_projectContext.IsPortable);
isPortable: string.IsNullOrEmpty(_projectContext.LockFileTarget.RuntimeIdentifier));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we use the RID check here but plumb IsSelfContained elsewhere?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I need to add a comment here.

Basically, IsPortable != IsFrameworkDependent and IsPortable != !IsSelfContained.

But IsFrameworkDependent == !IsSelfContained. (for now until we add some other app model where it doesn't run on a shared framework and still isn't self-contained.)

IsPortable means your app can run on any machine. It is portable across architectures, OSes, etc.
IsFrameworkDependent means your app runs on the shared framework, but it can still be runtime-specific. note: IsPortable implies IsFrameworkDependent
IsSelfContained means everything required to run your app is located in your publish output.

In this case, DependencyModel is strictly asking for IsPortable, not IsFrameworkDependent or !IsSelfContained. And looking under the hood, DependencyModel treats IsPortable and an empty RuntimeIdentifier the same. In a perfect world, this boolean wouldn't be needed to be passed here.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the thorough explanation.

Add a comment for DependencyContext IsPortable usage.
eerhardt added a commit to eerhardt/core-setup that referenced this pull request Mar 31, 2017
After dotnet/sdk#1053 is merged, it is possible for an app to run on the shared framework, but not be "portable".  DependencyContextLoader should always load the "runtime" context, if it is present.

Also, with #1597, there may be other deps files for the app. It should load those as well.

Fix #1886
@eerhardt
Copy link
Member Author

eerhardt commented Apr 3, 2017

@nguerrera @dsplaisted - can you take a look?

I'd like to get this merged today.

@@ -24,7 +24,8 @@ public void ItResolvesAssembliesFromProjectLockFiles(string projectName, string
ProjectContext projectContext = lockFile.CreateProjectContext(
FrameworkConstants.CommonFrameworks.NetCoreApp10,
runtime,
Constants.DefaultPlatformLibrary);
Constants.DefaultPlatformLibrary,
false);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please use named arguments for bools.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done on both of these.

@@ -42,7 +43,8 @@ public void ItResolvesAssembliesFromProjectLockFilesWithCacheLayout(string proje
ProjectContext projectContext = lockFile.CreateProjectContext(
FrameworkConstants.CommonFrameworks.NetCoreApp10,
runtime,
Constants.DefaultPlatformLibrary);
Constants.DefaultPlatformLibrary,
false);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ditto, named argument.

if (isFrameworkDependent)
{
Debug.Assert(platformLibrary != null);
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Debug.Assert(!isFrameworkDependent || platformLibrary != null) ?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I kind of like the readability of the way it is. It is very straight-forward to understand (at least to me). The compiler will remove the if statement in release mode, since it is empty, so it isn't like it will affect the outputted assembly code.

Do you have a strong opinion on this?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't have a strong opinion.

@eerhardt eerhardt merged commit 09c2a6b into dotnet:master Apr 3, 2017
@eerhardt eerhardt deleted the LowFatApps branch April 3, 2017 20:12
eerhardt added a commit to eerhardt/cli that referenced this pull request Apr 3, 2017
This flows to the $(SelfContained) property added in dotnet/sdk#1053
eerhardt added a commit to eerhardt/cli that referenced this pull request Apr 3, 2017
This flows to the $(SelfContained) property added in dotnet/sdk#1053
mmitche pushed a commit to mmitche/sdk that referenced this pull request Jun 5, 2020
…0191030.11 (dotnet#1053)

- Microsoft.AspNetCore.Analyzers - 3.1.0-preview3.19530.11
- Microsoft.AspNetCore.Mvc.Api.Analyzers - 3.1.0-preview3.19530.11
- Microsoft.AspNetCore.Mvc.Analyzers - 3.1.0-preview3.19530.11
- Microsoft.AspNetCore.Components.Analyzers - 3.1.0-preview3.19530.11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants