Fix building coreclr SPC inside VS

[ViktorHofer]

• Use the correct hook points for illink targets (CoreCompileDependsOn)
• Move target imports into a D.B.targets file as inside the project file is too early to override properties.
• Update targetingpacks.targets to not throw error in design time build
• Remove unnecessary target from S.P.CoreLib.csproj

With these changes I can now finally build coreclr S.P.CoreLib inside VS without any errors from a fresh clone and without any upfront steps.

[Copilot]

Pull Request Overview

This PR fixes building System.Private.CoreLib inside Visual Studio by relocating ILLink and optimization imports, updating hook points for ILLink, adjusting design-time build behavior, and removing an obsolete inline target.

• Move ILLink descriptor and codeOptimization imports into Directory.Build.targets
• Update hook points to use CoreCompileDependsOn and add ResolveProjectReferences
• Skip the targeting pack error during design-time builds
• Remove the inlined ILLink target from the project file

Reviewed Changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File	Description
src/coreclr/System.Private.CoreLib/System.Private.CoreLib.csproj	Removed inline imports and the CreateRuntimeRootIlLinkDescFile target
src/coreclr/System.Private.CoreLib/Directory.Build.targets	Added imports for ILLink descriptor and codeOptimization targets
src/coreclr/System.Private.CoreLib/CreateRuntimeRootILLinkDescriptorFile.targets	Switched to CoreCompileDependsOn, added ResolveProjectReferences dependencies
eng/targetingpacks.targets	Modified error condition to skip during design-time builds

Comments suppressed due to low confidence (2)

eng/targetingpacks.targets:127

• Consider adding a test or integration step to verify that design-time builds no longer fail at this error condition.

           Condition="!Exists('$(LocalRefPackDir)\data\FrameworkList.xml') and '$(DesignTimeBuild)' != 'true'" />

src/coreclr/System.Private.CoreLib/Directory.Build.targets:3

• The call to GetPathOfFileAbove is missing quotes around the filename literal; it should be GetPathOfFileAbove('Directory.Build.targets', …) to correctly locate the file.

  <Import Project="$([MSBuild]::GetPathOfFileAbove(Directory.Build.targets, $(MSBuildThisFileDirectory)..))" />

[ViktorHofer]

@hoyosjs @jkotas as Jeremy is out, would you mind reviewing/approving? Thanks

[dotnet-policy-service]

Tagging subscribers to this area: @hoyosjs
See info in area-owners.md if you want to be subscribed.

[jkotas]

Does this fix work for Native AOT and Mono CoreLib's as well?

[jkotas]

LGTM otherwise

[jkoritzinsky]

NativeAOT is difficult because the AsmOffsets.cs file is generated by the native build.

Maybe we can utilize a similar technique to the "runtime root descriptor" task to have it generated in the managed build?

[ViktorHofer]

The other CoreLibs dont have this exact issue but they might run into others as Jeremy hinted.

[jkotas]

Maybe we can utilize a similar technique to the "runtime root descriptor" task to have it generated in the managed build?

You have to build the native runtime to be able to run any tests. What's the point of trying to make corelib build when you cannot do anything with the result?

[jkoritzinsky]

I was just trying to come up with a solution to the mentioned possible issue (building NAOT CoreLib without having built the repo first). If that's not something we want to do, then that's not something we need to do.

[hoyosjs]

Generally, I thought the guidance was dotnet/sourcelink#392 (comment) to do a BeforeCompile for codegen. Is this coming from any behavior worth commenting or any new documentation?

[ViktorHofer]

These targets need to run before CoreCompile runs as Csc reads outputs from the targets. So CoreCompileDependsOn is the right hook point. We don't use Before or After Targets here so there's no sequencing issue.

[ViktorHofer]

I was just trying to come up with a solution to the mentioned possible issue (building NAOT CoreLib without having built the repo first). If that's not something we want to do, then that's not something we need to do.

For libs devs, the coreclr one matters as its the default P2P dependency and is therefore part of the solution files. Being able to dotnet build a managed library without any upfront steps is good for developer efficiency but unless the NativeAOT owners care about that scenario, not a priority.

[jkotas]

Being able to dotnet build a managed library without any upfront steps is good for developer efficiency

Is the idea that once you build it you will throw it against the CI to see whether it actually works? I think it encourages bad engineering habits.

[ViktorHofer]

Is the idea that once you build it you will throw it against the CI to see whether it actually works? I think it encourages bad engineering habits.

Agreed, we still rely on local and CI validation. We don't encourage to skip running tests locally when the build succeeds. For the libraries portion of the managed code we are interested in being able to build managed code with the least amount of upfront steps in at least the following scenarios:

1. Applying new code analysis rules. This is usually done inside VS and gets applied to the entire libs subset or the shared framework part of it. 023b54a as an example. Here it mostly matters whether the code compiles. Any potential issues in special environments won't be caught on i.e. win-x64 but rather on android / ios / wasm which close to no one on my team tests locally.
2. We want to stay on the common path. If a dotnet new console / classlib app is dotnet buildable without any upfront steps, we want to enable that as well. This lowers the barrier for new contributors (including Copilot agents). We still encourage to run tests locally but there's a huge set of potential changes for which we rely on CI validation regardless.
3. Being able to dotnet pack a library for local validation (in another project / repository). This is especially useful in a local VMR clone.