-
Notifications
You must be signed in to change notification settings - Fork 5k
Add the FX_RESOLUTION Runtime property #644
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -1057,4 +1057,4 @@ int fx_muxer_t::handle_cli( | |
} | ||
|
||
return result; | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -114,15 +114,38 @@ int hostpolicy_context_t::initialize(hostpolicy_init_t &hostpolicy_init, const a | |
fx_definition_vector_t::iterator fx_end; | ||
resolver.get_app_fx_definition_range(&fx_begin, &fx_end); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If I read it correctly these are not used anymore. |
||
|
||
pal::string_t resolved_frameworks; | ||
pal::string_t app_context_deps_str; | ||
fx_definition_vector_t::iterator fx_curr = fx_begin; | ||
while (fx_curr != fx_end) | ||
bool is_app = host_mode != host_mode_t::libhost; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Doesn't the first element in |
||
|
||
for (const auto& fx : fx_definitions) | ||
{ | ||
if (fx_curr != fx_begin) | ||
if (!app_context_deps_str.empty()) | ||
{ | ||
app_context_deps_str += _X(';'); | ||
} | ||
|
||
app_context_deps_str += (*fx_curr)->get_deps_file(); | ||
++fx_curr; | ||
app_context_deps_str += fx->get_deps_file(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Before your change the app.deps.json was not included in this string when running in libhost mode (since only real frameworks should be reported then). Now it will include it. |
||
|
||
if (!resolved_frameworks.empty()) | ||
{ | ||
resolved_frameworks += _X(';'); | ||
} | ||
|
||
if (is_app) | ||
{ | ||
// The first framework entry is skipped, because it corresponds to the app itself. | ||
is_app = false; | ||
} | ||
else | ||
{ | ||
resolved_frameworks += _X("Framework:"); | ||
resolved_frameworks += fx->get_name(); | ||
resolved_frameworks += _X(",Requested:"); | ||
resolved_frameworks += fx->get_requested_version(); | ||
resolved_frameworks += _X(",Resolved:"); | ||
resolved_frameworks += fx->get_found_version(); | ||
} | ||
} | ||
|
||
pal::string_t clr_library_version; | ||
|
@@ -145,6 +168,7 @@ int hostpolicy_context_t::initialize(hostpolicy_init_t &hostpolicy_init, const a | |
coreclr_properties.add(common_property::FxDepsFile, fx_deps_str.c_str()); | ||
coreclr_properties.add(common_property::ProbingDirectories, resolver.get_lookup_probe_directories().c_str()); | ||
coreclr_properties.add(common_property::FxProductVersion, clr_library_version.c_str()); | ||
coreclr_properties.add(common_property::ResolvedFrameworks, resolved_frameworks.c_str()); | ||
|
||
if (!clrjit_path.empty()) | ||
coreclr_properties.add(common_property::JitPath, clrjit_path.c_str()); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,10 @@ | ||
<Project> | ||
|
||
<!-- This project targets a released verison of .net core, don't use workaround. --> | ||
<PropertyGroup> | ||
<UseMicrosoftNETCoreAppInternalWorkaround>false</UseMicrosoftNETCoreAppInternalWorkaround> | ||
</PropertyGroup> | ||
|
||
<Import Project="$([MSBuild]::GetDirectoryNameOfFileAbove($(MSBuildThisFileDirectory).., Directory.Build.props))\Directory.Build.props" /> | ||
|
||
</Project> |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,14 @@ | ||
<Project> | ||
|
||
<Import Project="Sdk.props" Sdk="Microsoft.NET.Sdk" /> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we need to explicitly do the imports for some reason instead of just setting the |
||
|
||
<PropertyGroup> | ||
<AssemblyName>PortableApp</AssemblyName> | ||
<TargetFramework>netcoreapp2.1</TargetFramework> | ||
<OutputType>Exe</OutputType> | ||
<RuntimeFrameworkVersion>$(MNAVersion)</RuntimeFrameworkVersion> | ||
</PropertyGroup> | ||
|
||
<Import Project="Sdk.targets" Sdk="Microsoft.NET.Sdk" /> | ||
|
||
</Project> |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,18 @@ | ||
using System; | ||
|
||
namespace PortableApp | ||
{ | ||
public static class Program | ||
{ | ||
public static void Main(string[] args) | ||
{ | ||
Console.WriteLine("Hello World!"); | ||
Console.WriteLine($"RESOLVED_FRAMEWORKS = {GetRuntimePropertiesFromAppDomain()}"); | ||
} | ||
|
||
private static string GetRuntimePropertiesFromAppDomain() | ||
{ | ||
return System.AppDomain.CurrentDomain.GetData("RESOLVED_FRAMEWORKS") as string; | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,75 @@ | ||
// Licensed to the .NET Foundation under one or more agreements. | ||
// The .NET Foundation licenses this file to you under the MIT license. | ||
// See the LICENSE file in the project root for more information. | ||
|
||
using System; | ||
using Xunit; | ||
|
||
namespace Microsoft.DotNet.CoreSetup.Test.HostActivation | ||
{ | ||
public class ResolvedFrameworksProperty : IClassFixture<ResolvedFrameworksProperty.SharedTestState> | ||
{ | ||
private readonly SharedTestState sharedTestState; | ||
|
||
public ResolvedFrameworksProperty(SharedTestState fixture) | ||
{ | ||
sharedTestState = fixture; | ||
} | ||
|
||
[Fact] | ||
public void ResolvedFrameworksPropertyIsEmptyForStandaloneApps() | ||
{ | ||
var fixture = sharedTestState.StandaloneApp.Copy(); | ||
var dotnet = fixture.BuiltDotnet; | ||
var appDll = fixture.TestProject.AppDll; | ||
|
||
dotnet.Exec(appDll) | ||
.EnableTracingAndCaptureOutputs() | ||
.Execute() | ||
.Should() | ||
.Pass() | ||
.And | ||
.HaveStdErrContaining($"Property RESOLVED_FRAMEWORKS = \r\n"); | ||
} | ||
|
||
[Fact] | ||
public void ResolvedFrameworksPropertyIsComputedForPortableApps() | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can you please add a test for cases where the requested version doesn't match the resolved version? |
||
{ | ||
var fixture = sharedTestState.PortableApp.Copy(); | ||
var dotnet = fixture.BuiltDotnet; | ||
var appDll = fixture.TestProject.AppDll; | ||
var MNAversion = sharedTestState.RepoDirectories.MicrosoftNETCoreAppVersion; | ||
|
||
dotnet.Exec(appDll) | ||
.EnableTracingAndCaptureOutputs() | ||
.Execute() | ||
.Should() | ||
.Pass() | ||
.And | ||
.HaveStdErrContaining($"Property RESOLVED_FRAMEWORKS = Framework:Microsoft.NETCore.App,Requested:{"2.1.0"},Resolved:{MNAversion}\r\n"); | ||
} | ||
|
||
public class SharedTestState : IDisposable | ||
{ | ||
public RepoDirectoriesProvider RepoDirectories { get; } | ||
public TestProjectFixture PortableApp { get; } | ||
public TestProjectFixture StandaloneApp { get; } | ||
public SharedTestState() | ||
{ | ||
RepoDirectories = new RepoDirectoriesProvider(microsoftNETCoreAppVersion: "2.1.0"); | ||
var rid = RepoDirectories.TargetRID; | ||
StandaloneApp = new TestProjectFixture("StandaloneApp", RepoDirectories) | ||
.BuildProject(runtime: rid, restore: true); | ||
|
||
PortableApp = new TestProjectFixture("PortableApp21", RepoDirectories, framework: "netcoreapp2.1", assemblyName: "PortableApp") | ||
.BuildProject(restore: true); | ||
} | ||
|
||
public void Dispose() | ||
{ | ||
PortableApp.Dispose(); | ||
StandaloneApp.Dispose(); | ||
} | ||
} | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This excludes the "App" when in libhost mode - so the loop below would actually skip the first framework.
We may need to tweak the behavior of this helper in correspondence with the loop.