Add default value when FeatureDynamicCodeCompiled is undefined

[BrzVlad]

This property is normally passed to msbuild when building runtime.proj, with a true or false value. An exception happens when building from vs, with ./build.cmd -vs coreclr.slnx. Feature defines need to have a default value for this scenario. Rather than adding a default in clrfeatures.cmake, which would require duplicating target os logic from the build scripts, this commit handles the default at a higher level in runtime.proj.

[BrzVlad]

Issue hit by @jakobbotsch in #124168 (comment)

[dotnet-policy-service]

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

[Copilot]

Pull request overview

This PR fixes a build issue when opening CoreCLR in Visual Studio using ./build.cmd -vs coreclr.slnx. When building through this path, the FeatureDynamicCodeCompiled MSBuild property is undefined, causing CMake configuration to fail. The fix adds a default value by treating an empty string as true, which aligns with the default behavior in build scripts for Windows platforms.

Changes:

• Modified the conditional check for FeatureDynamicCodeCompiled in runtime.proj to default to enabled when the property is undefined

[jakobbotsch]

Thanks!

[akoeplinger]

This seems like the wrong fix to me. The flag should probably be initialized in msbuild rather than the build.sh/cmd scripts so the logic is handled there. E.g. ILCompiler.ReadyToRun.csproj and clr.featuredefines.props are also using it.

[BrzVlad]

@akoeplinger Hmm, if I'm understanding correctly, then it is not necessarily this fix that is wrong, but the original implementation. So you would recommend the arch handling to be removed from here: https://github.com/dotnet/runtime/blob/main/eng/build.ps1#L357 and here: https://github.com/dotnet/runtime/blob/main/eng/build.sh#L597, pass the FeatureDynamicCodeCompiled property to msbuild only if it is explicitly set via --dynamiccodecompiled and in clr.featuredefines.props have the default value of the property, if it is not set, depending on the target ?

edit: I'm not sure if runtime.proj would see properties from clr.featuredefines.props

[akoeplinger]

Yeah though it looks like clr.featuredefines.props is not imported in runtime.proj. I think this might be the first "enabled-by-default" feature.

e.g. if you look at FeatureXplatEventSource we set it to true based on some conditions in

runtime/src/coreclr/clr.featuredefines.props

Lines 30 to 31 in 9268ae6

	<PropertyGroup Condition="'$(TargetsUnix)' == 'true'">
	<FeatureXplatEventSource Condition="'$(FeatureXplatEventSource)' == '' AND '$(TargetOS)' == 'linux'">true</FeatureXplatEventSource>

but only pass it to the runtime build when it's false (which happens to work):

runtime/src/coreclr/runtime.proj

Line 55 in 9268ae6

<_CoreClrBuildArg Condition="'$(FeatureXplatEventSource)' == 'false'" Include="-cmakeargs &quot;-DFEATURE_EVENTSOURCE_XPLAT=0&quot;" />

[BrzVlad]

A problem with dynamic code compiled option is that, even if you don't pass it explicitly to the build it needs to have a platform dependent value, both for runtime/crossgen2 and SPC. So it seems clr.featuredefines.props is not the unified solution to this problem and we would still need to have defaults in other places, for runtime.proj and crossgen2.

I believe we have 2 options:

• Adding the pattern from this PR to clr.featuredefines.props and ILCompiler.ReadyToRun.csproj, by adding the default as true if it was not set.
• what I believe you might be suggesting:
  • we remove the target checks from build.sh and build.ps1
  • in runtime.proj, clr.featuredefines.props and ILCompiler.ReadyToRun.csproj, if FeatureDynamicCodeCompiled is undefined (meaning the build scripts didn't set an explicit value), we will do the target platform checks to produce the value of this property.

[jkotas]

I see the top-level build option as a non-production quality testing hook. I think the current implementation is good enough for that.

A production quality option would be much more complicated. For example, we would not want to have the ifdef in ILCompiler.ReadyToRun.csproj at all. Instead, we would want it to be a crossgen2 switch, or have crossgen2 auto-detect the setting from the CoreLib.