Don't build native host tests when building clr subset#125860
Don't build native host tests when building clr subset#125860elinor-fung wants to merge 6 commits intodotnet:mainfrom
Conversation
When an assembly fails to load, the FileLoadException/FileNotFoundException now includes the name of the requesting (parent) assembly in the FusionLog property. This helps diagnose dependency chain issues by showing which assembly triggered the failed load. The information flows from the native AssemblySpec::GetParentAssembly() through EEFileLoadException::CreateThrowable() to the managed exception's FusionLog property, which is displayed in ToString() output. Fixes dotnet#9185 Co-authored-by: elinor-fung <47805090+elinor-fung@users.noreply.github.com>
BadImageFormatException is also created by EEFileLoadException::CreateThrowable() when the HRESULT maps to kBadImageFormatException. Without the 3-arg constructor, the requesting assembly info would be silently dropped for bad image format errors. Co-authored-by: elinor-fung <47805090+elinor-fung@users.noreply.github.com>
Walk the inner exception chain to build the full dependency path. When A→B→C→D and D fails, D's FusionLog now shows: Requesting assembly: 'C' Requesting assembly: 'B' Requesting assembly: 'A' At each Throw(AssemblySpec*) level, the current parent assembly name is appended to every inner EEFileLoadException's chain. The managed FormatRequestingAssemblyChain helper formats each newline-separated name with the "Requesting assembly:" resource string. Co-authored-by: elinor-fung <47805090+elinor-fung@users.noreply.github.com>
…dd tests The requesting assembly chain is now appended to the exception's Message property instead of FusionLog. This means users see the chain directly in the exception message when it is thrown, without needing to inspect FusionLog separately. Added tests for all three exception types (FileLoadException, FileNotFoundException, BadImageFormatException) that verify: - Single requesting assembly appears in Message - Multiple requesting assemblies (chain) appear in Message - FusionLog remains null Co-authored-by: elinor-fung <47805090+elinor-fung@users.noreply.github.com>
…missing transitive dependency Remove the 6 reflection-based tests that invoked private constructors via reflection from BadImageFormatExceptionTests, FileLoadExceptionTests, and FileNotFoundExceptionTests. Add an integration test in System.Runtime.Loader.Tests that creates a real dependency chain (Root -> Mid -> Leaf) where the transitive dependency (Leaf) is missing at runtime. The test verifies that a FileNotFoundException is thrown with the correct missing assembly name when the chain is exercised. Three helper assembly projects are added: - MissingDependency.Leaf (not deployed via PrivateAssets=all) - MissingDependency.Mid (depends on Leaf) - MissingDependency.Root (depends on Mid) Co-authored-by: elinor-fung <47805090+elinor-fung@users.noreply.github.com>
Building the clr subset includes host.native, which previously unconditionally built native test assets (mockcoreclr, mockhostfxr, mockhostpolicy, nativehost, etc.) via add_subdirectory(test) in the corehost CMakeLists.txt. These test assets are not needed for a normal dev build like './build.sh libs+clr -rc Release'. Add two CMake variables to control what gets built: - CLR_CMAKE_BUILD_HOST_PRODUCT: gates product-only targets (fxr, hostpolicy, apphost, dotnet, comhost) - CLR_CMAKE_BUILD_HOST_TESTS: gates the test subdirectory These are driven by MSBuild properties BuildNativeHostProduct and BuildNativeHostTests in corehost.proj, which default to true and false respectively. In eng/Subsets.props, the host.tests subset now invokes corehost.proj with BuildNativeHostTests=true and BuildNativeHostProduct=false, so only test assets and their required dependencies (hostcommon, hostmisc, nethost, ijwhost) are built. Fixes dotnet#73711 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Updates the build configuration so the clr subset no longer builds native corehost test assets by default, by adding explicit build gates for host product vs. host tests. The PR also introduces CoreCLR/managed plumbing to surface a “requesting assembly” chain in file load exceptions, along with a new loader test scenario.
Changes:
- Add MSBuild properties and CMake variables to independently gate building corehost product targets vs. test targets.
- Update
host.testssubset invocation to pass the new MSBuild properties tocorehost.proj. - Add runtime/managed support for including a requesting-assembly chain in file load exceptions and add a System.Runtime.Loader test asset setup.
Reviewed changes
Copilot reviewed 18 out of 18 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| src/native/corehost/corehost.proj | Adds BuildNativeHostTests / BuildNativeHostProduct defaults and forwards them to CMake via -DCLR_CMAKE_BUILD_HOST_*. |
| src/native/corehost/CMakeLists.txt | Gates product/test subdirectories and ASAN installs based on new CMake variables. |
| eng/Subsets.props | Adjusts host.tests subset to call corehost.proj with host-tests enabled and host-product disabled. |
| src/libraries/System.Runtime.Loader/tests/System.Runtime.Loader.Tests.csproj | Adds reference to new MissingDependency test asset project. |
| src/libraries/System.Runtime.Loader/tests/AssemblyLoadContextTest.cs | Adds a new test case for missing transitive dependency behavior. |
| src/libraries/System.Runtime.Loader/tests/MissingDependency.Root/* | New test asset project representing the “root” assembly. |
| src/libraries/System.Runtime.Loader/tests/MissingDependency.Mid/* | New test asset project representing the “mid” assembly (with non-deployed leaf dep). |
| src/libraries/System.Runtime.Loader/tests/MissingDependency.Leaf/* | New test asset project representing the “leaf” assembly intended to be missing at runtime. |
| src/libraries/System.Private.CoreLib/src/Resources/Strings.resx | Adds localized string for “Requesting assembly: '{0}'”. |
| src/coreclr/vm/metasig.h | Adds metasig for a (string, string, int) ctor signature used by native exception creation. |
| src/coreclr/vm/clrex.h | Extends EEFileLoadException to store a requesting assembly name/chain and clone it. |
| src/coreclr/vm/clrex.cpp | Uses the new ctor signature when available and populates requesting assembly info when throwing from AssemblySpec. |
| src/coreclr/System.Private.CoreLib/src/System/IO/FileNotFoundException.CoreCLR.cs | Adds private ctor invoked from native code to append requesting-assembly context. |
| src/coreclr/System.Private.CoreLib/src/System/IO/FileLoadException.CoreCLR.cs | Adds private ctor + formatter to append/format requesting-assembly chain. |
| src/coreclr/System.Private.CoreLib/src/System/BadImageFormatException.CoreCLR.cs | Adds private ctor invoked from native code to append requesting-assembly context. |
| () => MissingDependency.Root.RootClass.UseMiddle()); | ||
|
|
||
| Assert.NotNull(ex.FileName); | ||
| Assert.Contains("MissingDependency.Leaf", ex.FileName); |
There was a problem hiding this comment.
The new test name/comments indicate it should validate requesting-assembly context in the exception message, but the assertions only check ex.FileName contains the missing assembly name (which would be true even without the runtime changes). Update the assertions to verify the new behavior (e.g., ex.Message contains the "Requesting assembly" line and includes the expected requesting assembly such as MissingDependency.Mid).
| Assert.Contains("MissingDependency.Leaf", ex.FileName); | |
| Assert.Contains("MissingDependency.Leaf", ex.FileName); | |
| Assert.Contains("Requesting assembly", ex.Message); | |
| Assert.Contains("MissingDependency.Mid", ex.Message); |
| // Extract the requesting assembly chain for diagnostic purposes | ||
| { | ||
| FAULT_NOT_FATAL(); | ||
|
|
||
| Exception *inner2 = ExThrowWithInnerHelper(pInnerException); | ||
| EEFileLoadException *pException = new EEFileLoadException(name, hr); | ||
| pException->SetInnerException(inner2); | ||
|
|
||
| Assembly *pParentAssembly = pSpec->GetParentAssembly(); | ||
| if (pParentAssembly != NULL) | ||
| { | ||
| StackSString parentName; | ||
| pParentAssembly->GetDisplayName(parentName); | ||
|
|
||
| // Set the requesting assembly for this exception | ||
| pException->SetRequestingAssembly(parentName); | ||
|
|
||
| // Walk the inner exception chain and append this parent to each | ||
| // inner exception's chain, so that every exception in the chain | ||
| // carries the full dependency path from its perspective | ||
| EEFileLoadException *pInnerFLE = NULL; | ||
| Exception *pWalk = inner2; | ||
| int depth = 0; | ||
| while (pWalk != NULL && depth < 20) | ||
| { | ||
| if (EEFileLoadException::CheckType(pWalk)) | ||
| { | ||
| pInnerFLE = (EEFileLoadException*)pWalk; | ||
| if (!pInnerFLE->m_requestingAssemblyName.IsEmpty()) | ||
| { | ||
| StackSString updatedChain; | ||
| updatedChain.Set(pInnerFLE->m_requestingAssemblyName); | ||
| updatedChain.Append(W("\n")); | ||
| updatedChain.Append(parentName); | ||
| pInnerFLE->SetRequestingAssembly(updatedChain); | ||
| } | ||
| pWalk = pInnerFLE->m_innerException; | ||
| } | ||
| else | ||
| { | ||
| break; | ||
| } | ||
| depth++; | ||
| } | ||
| } | ||
|
|
||
| STRESS_LOG3(LF_EH, LL_INFO100, "EX_THROW_WITH_INNER Type = 0x%x HR = 0x%x, " | ||
| INDEBUG(__FILE__) " line %d\n", EEFileLoadException::GetType(), | ||
| pException->GetHR(), __LINE__); | ||
| EX_THROW_DEBUG_TRAP(__FUNCTION__, __FILE__, __LINE__, "EEFileLoadException", pException->GetHR(), "(name, hr)"); | ||
| PAL_CPP_THROW(EEFileLoadException *, pException); | ||
| } |
There was a problem hiding this comment.
The PR title/description focus on avoiding building native host tests as part of the clr subset, but this PR also includes CoreCLR exception/message changes (requesting-assembly chain plumbing) and new System.Runtime.Loader tests/projects. If these are intentional, the PR description should be updated to cover them; otherwise consider splitting these unrelated runtime/loader changes into a separate PR to keep review scope and risk aligned with the stated goal.
| if(CLR_CMAKE_BUILD_HOST_PRODUCT) | ||
| add_subdirectory(fxr) | ||
| add_subdirectory(hostpolicy) | ||
| add_subdirectory(apphost) | ||
| add_subdirectory(dotnet) | ||
| endif() | ||
| if(CLR_CMAKE_BUILD_HOST_TESTS) | ||
| add_subdirectory(test) | ||
| endif() |
There was a problem hiding this comment.
CLR_CMAKE_BUILD_HOST_PRODUCT / CLR_CMAKE_BUILD_HOST_TESTS are used as boolean gates but are never defined with defaults. In a standalone CMake configure (without the MSBuild-driven -D... args), these evaluate false and will skip building host product targets (fxr/hostpolicy/apphost/dotnet), likely breaking normal corehost builds. Define them via option(...)/set(... CACHE BOOL ...) with appropriate defaults (e.g., product ON, tests ON to preserve existing behavior), and let MSBuild override them when needed.
|
|
||
| <!-- Host.tests subset (consumes live built libraries assets so needs to come after libraries) --> | ||
| <ItemGroup Condition="$(_subset.Contains('+host.tests+'))"> | ||
| <ProjectToBuild Include="$(SharedNativeRoot)corehost\corehost.proj" AdditionalProperties="$(TargetCxxLibraryProperties);BuildNativeHostTests=true;BuildNativeHostProduct=false" Category="host" /> |
There was a problem hiding this comment.
host.tests subset builds corehost.proj with BuildNativeHostProduct=false, but the managed host test suites in this same subset expect product artifacts under artifacts/.../corehost (e.g., apphost/dotnet/hostfxr/hostpolicy/comhost via RepoDirectoriesProvider.Default.HostArtifacts). With product disabled, these binaries won't be produced and the tests will likely fail when run from this subset. Consider setting BuildNativeHostProduct=true here (or otherwise ensuring the product bits are built as part of host.tests when @(TestProjectToBuild) is included).
| <ProjectToBuild Include="$(SharedNativeRoot)corehost\corehost.proj" AdditionalProperties="$(TargetCxxLibraryProperties);BuildNativeHostTests=true;BuildNativeHostProduct=false" Category="host" /> | |
| <ProjectToBuild Include="$(SharedNativeRoot)corehost\corehost.proj" AdditionalProperties="$(TargetCxxLibraryProperties);BuildNativeHostTests=true;BuildNativeHostProduct=true" Category="host" /> |
Building the \clr\ subset includes \host.native, which previously unconditionally built native test assets (mockcoreclr, mockhostfxr, mockhostpolicy, nativehost, etc.) via \�dd_subdirectory(test)\ in the corehost CMakeLists.txt. These test assets are not needed for a normal dev build like ./build.sh libs+clr -rc Release.
Changes
Add two CMake variables to control what gets built:
These are driven by MSBuild properties \BuildNativeHostProduct\ and \BuildNativeHostTests\ in \corehost.proj, which default to \ rue\ and \alse\ respectively.
In \�ng/Subsets.props, the \host.tests\ subset now invokes \corehost.proj\ with \BuildNativeHostTests=true\ and \BuildNativeHostProduct=false, so only test assets and their required dependencies (hostcommon, hostmisc, nethost, ijwhost) are built.
Subset behavior
Fixes #73711