-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Enable "portable" RID graph when targeting .NET 8 and higher #34279
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
Large diffs are not rendered by default.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,60 @@ | ||
// Licensed to the .NET Foundation under one or more agreements. | ||
// The .NET Foundation licenses this file to you under the MIT license. | ||
|
||
using System; | ||
using System.Collections.Generic; | ||
using System.IO; | ||
using System.Linq; | ||
using System.Security.Cryptography; | ||
using System.Text; | ||
using System.Threading.Tasks; | ||
using Microsoft.Build.Framework; | ||
using Microsoft.Build.Utilities; | ||
using Newtonsoft.Json; | ||
using Newtonsoft.Json.Linq; | ||
|
||
namespace Microsoft.DotNet.Cli.Build | ||
{ | ||
public class UpdatePortableRuntimeIdentifierGraph : Task | ||
{ | ||
[Required] | ||
public string InputFile { get; set; } | ||
|
||
[Required] | ||
public string OutputFile { get; set; } | ||
|
||
|
||
// ItemSpec should be a RID, and "Imports" metadata should be a semicolon-separated list of RIDs that the ItemSpec RID imports | ||
public ITaskItem[] AdditionalRuntimeIdentifiers { get; set; } | ||
|
||
public override bool Execute() | ||
{ | ||
JToken json; | ||
|
||
using (var file = File.OpenText(InputFile)) | ||
using (JsonTextReader reader = new JsonTextReader(file)) | ||
{ | ||
json = JObject.ReadFrom(reader); | ||
} | ||
|
||
JObject runtimes = (JObject) json["runtimes"]; | ||
|
||
if (AdditionalRuntimeIdentifiers != null) | ||
{ | ||
foreach (var rid in AdditionalRuntimeIdentifiers) | ||
{ | ||
var importedRids = rid.GetMetadata("Imports").Split(';'); | ||
runtimes.Add(rid.ItemSpec, new JObject(new JProperty("#import", new JArray(importedRids)))); | ||
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. I think this is correct and updates json["runtimes"] rather than making a copy...but can we have a regression test for that? Or perhaps just add json["runtimes"] = runtimes at the end? |
||
} | ||
} | ||
|
||
using (var file = File.CreateText(OutputFile)) | ||
using (var writer = new JsonTextWriter(file) { Formatting = Formatting.Indented }) | ||
{ | ||
json.WriteTo(writer); | ||
} | ||
|
||
return true; | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -48,6 +48,19 @@ Copyright (c) .NET Foundation. All rights reserved. | |
<_GenerateSingleFileBundlePropertyInputsCache>$([MSBuild]::NormalizePath($(MSBuildProjectDirectory), $(_GenerateSingleFileBundlePropertyInputsCache)))</_GenerateSingleFileBundlePropertyInputsCache> | ||
</PropertyGroup> | ||
|
||
<!-- For .NET 8 and higher, we will by default use a simplified "portable" RID graph --> | ||
<PropertyGroup Condition="'$(UseRidGraph)' == ''"> | ||
<UseRidGraph Condition="'$(TargetFrameworkIdentifier)' == '.NETCoreApp' AND $([MSBuild]::VersionGreaterThanOrEquals($(TargetFrameworkVersion), '8.0'))">false</UseRidGraph> | ||
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. I was tripped up on this once and almost said this is incorrect when it's correct. UseRidGraph to me sounds like it means use the new thing, especially since the new thing is the "portable" rid graph...can we add portable to this name and flip its true/falseness? Or add full and not flip it? |
||
<UseRidGraph Condition="'$(UseRidGraph)' == ''">true</UseRidGraph> | ||
</PropertyGroup> | ||
|
||
<PropertyGroup Condition="'$(RuntimeIdentifierGraphPath)' == ''"> | ||
<RuntimeIdentifierGraphPath Condition="'$(UseRidGraph)' == 'true'">$(BundledRuntimeIdentifierGraphFile)</RuntimeIdentifierGraphPath> | ||
|
||
<!-- The portable RID graph should be in the same directory as the full RID graph --> | ||
<RuntimeIdentifierGraphPath Condition="'$(UseRidGraph)' != 'true'">$([System.IO.Path]::GetDirectoryName($(BundledRuntimeIdentifierGraphFile)))/PortableRuntimeIdentifierGraph.json</RuntimeIdentifierGraphPath> | ||
</PropertyGroup> | ||
|
||
<ItemGroup> | ||
<GenerateRuntimeConfigurationFilesInputs Include="$(ProjectAssetsFile)" /> | ||
<GenerateRuntimeConfigurationFilesInputs Include="$(ProjectAssetsCacheFile)" /> | ||
|
@@ -288,7 +301,7 @@ Copyright (c) .NET Foundation. All rights reserved. | |
ResolvedRuntimeTargetsFiles="@(RuntimeTargetsCopyLocalItems)" | ||
IsSelfContained="$(SelfContained)" | ||
IncludeRuntimeFileVersions="$(IncludeFileVersionsInDependencyFile)" | ||
RuntimeGraphPath="$(BundledRuntimeIdentifierGraphFile)" | ||
RuntimeGraphPath="$(RuntimeIdentifierGraphPath)" | ||
IncludeProjectsNotInAssetsFile="$(IncludeProjectsNotInAssetsFileInDepsFile)" | ||
ValidRuntimeIdentifierPlatformsForAssets="@(_ValidRuntimeIdentifierPlatformsForAssets)"/> | ||
<ItemGroup> | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,66 @@ | ||
// Licensed to the .NET Foundation under one or more agreements. | ||
// The .NET Foundation licenses this file to you under the MIT license. | ||
|
||
using System; | ||
using System.Collections.Generic; | ||
using System.IO; | ||
using System.Linq; | ||
using System.Text; | ||
using System.Threading.Tasks; | ||
using FluentAssertions; | ||
using Microsoft.NET.TestFramework; | ||
using Microsoft.NET.TestFramework.Assertions; | ||
using Microsoft.NET.TestFramework.Commands; | ||
using Microsoft.NET.TestFramework.ProjectConstruction; | ||
using Xunit; | ||
using Xunit.Abstractions; | ||
|
||
namespace Microsoft.NET.Build.Tests | ||
{ | ||
public class RuntimeIdentifierGraphTests : SdkTest | ||
{ | ||
public RuntimeIdentifierGraphTests(ITestOutputHelper log) : base(log) | ||
{ | ||
} | ||
|
||
[Theory] | ||
[InlineData("net7.0", null, true)] | ||
[InlineData("net8.0", null, false)] | ||
[InlineData("net7.0", "false", false)] | ||
[InlineData("net8.0", "true", true)] | ||
public void ItUsesCorrectRuntimeIdentifierGraph(string targetFramework, string useRidGraphValue, bool shouldUseFullRidGraph) | ||
{ | ||
var testProject = new TestProject() | ||
{ | ||
TargetFrameworks = targetFramework, | ||
IsExe = true | ||
}; | ||
|
||
if (useRidGraphValue != null) | ||
{ | ||
testProject.AdditionalProperties["UseRidGraph"] = useRidGraphValue; | ||
} | ||
|
||
testProject.RecordProperties("RuntimeIdentifierGraphPath"); | ||
|
||
var testAsset = _testAssetsManager.CreateTestProject(testProject, identifier: targetFramework + "_" + (useRidGraphValue ?? "null")); | ||
|
||
var buildCommand = new BuildCommand(testAsset); | ||
|
||
buildCommand.Execute() | ||
.Should() | ||
.Pass(); | ||
|
||
var runtimeIdentifierGraphPath = testProject.GetPropertyValues(testAsset.TestRoot)["RuntimeIdentifierGraphPath"]; | ||
|
||
if (shouldUseFullRidGraph) | ||
{ | ||
Path.GetFileName(runtimeIdentifierGraphPath).Should().Be("RuntimeIdentifierGraph.json"); | ||
} | ||
else | ||
{ | ||
Path.GetFileName(runtimeIdentifierGraphPath).Should().Be("PortableRuntimeIdentifierGraph.json"); | ||
} | ||
} | ||
} | ||
} |
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.
Should we have a "fast path" here for if AdditionalRuntimeIdentifiers is null? Then it should just be a copy, right?