Skip to content

Conversation

@jkoritzinsky
Copy link
Member

Let's rebaseline these now so we can add logic to support Mach-O ReadyToRun files in the right places and then port over to the SDK repo easily.

… recent copy on dotnet/sdk's main

Let's rebaseline these now so we can add logic to support Mach-O ReadyToRun files in the right places and then port over to the SDK repo easily.
@dotnet-policy-service
Copy link
Contributor

Tagging subscribers to this area: @dotnet/runtime-infrastructure
See info in area-owners.md if you want to be subscribed.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR modernizes the Crossgen2Tasks codebase and adds support for composite ReadyToRun compilation with unrooted assemblies. The key changes include:

  • Added support for composite R2R compilation with unrooted input assemblies (assemblies that are only partially compiled based on transitive method calls)
  • Modernized C# code to use target-typed new() expressions, global usings, nullable annotations, and other modern patterns
  • Improved cross-platform compilation support by using target OS metadata instead of runtime OS platform detection
  • Refactored runtime identifier handling and crossgen2 target OS determination
  • Enhanced build configuration with telemetry item renaming and MSBuild dependency tracking

Reviewed Changes

Copilot reviewed 14 out of 14 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
RunReadyToRunCompiler.cs Added support for unrooted composite input, fixed reference handling to use ItemSpec, improved OS detection using metadata, modernized code patterns
ResolveReadyToRunCompilers.cs Refactored RID handling and host detection logic, improved crossgen2 target OS determination with runtime graph support, reorganized platform detection code
PrepareForReadyToRunCompilation.cs Added composite roots support, introduced unrooted input tracking, replaced runtime OS checks with target-aware properties, modernized code patterns
Microsoft.NET.CrossGen.targets Added composite roots configuration, enabled composite mode for single-file by default in .NET 7+, improved telemetry naming, added MSBuild dependency tracking
Crossgen2Tasks.csproj Configured global usings and nullable annotations, suppressed additional analyzer warnings
TaskBase.cs Applied nullable annotations and adjusted null-coalescing pattern
RuntimeGraphCache.cs Applied nullable disable directive, changed class from sealed to non-sealed
NuGetUtils.NuGet.cs Added nullable annotations, introduced RID exclusion support in matching logic, changed string.Contains to IndexOf
MetadataKeys.cs Reorganized metadata constants, added new keys for runtime pack exclusions and debug symbols
MessageLevel.cs Removed unused System import
Message.cs Applied nullable annotations
Logger.cs Applied nullable annotations, removed LogNonSdkError method, relaxed debug assertion for message codes
LogAdapter.cs Converted to primary constructor pattern, fixed comment typo
BuildErrorException.cs Changed from sealed to non-sealed class
Comments suppressed due to low confidence (1)

src/tasks/Crossgen2Tasks/PrepareForReadyToRunCompilation.cs:564

  • The peReader parameter is added to the method signature but is never used in the method body. Either remove the unused parameter or add a comment explaining why it's included for future use.
        private static bool HasILCode(PEReader peReader, MetadataReader mdReader)
        {
            foreach (var methoddefHandle in mdReader.MethodDefinitions)
            {
                MethodDefinition methodDef = mdReader.GetMethodDefinition(methoddefHandle);
                if (methodDef.RelativeVirtualAddress > 0)
                {
                    return true;
                }
            }

            return false;
        }

@jkoritzinsky
Copy link
Member Author

/ba-g arm64 and android queues on the floor

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

2 participants