Repository navigation
[native] Require opt-in for assembly loading logs - #13182
Conversation
Routine assembly probes and the Fast Deployment summary currently emit messages without an explicit assembly logging opt-in. Route these messages through the assembly category and use debug logging for missing assembly probes, keeping genuine warnings and errors visible. Document debug.dotnet.log=assembly as the opt-in setting. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
🟢 Approval recommended
The focused logging changes preserve failure diagnostics, with no blocking issues identified.
No open issues 🎉
What changed in this PR
Makes routine CoreCLR assembly-loading logs opt-in, reducing logcat noise while preserving genuine warnings and errors.
Changes:
- Applies category filtering to the Fast Deployment summary.
- Downgrades unsuccessful probes to debug and categorizes external probes as assembly logs.
- Documents how to enable assembly diagnostics.
| File | Description |
|---|---|
| src/native/clr/host/host.cc | Moves external probe traces to the assembly category. |
| src/native/clr/host/fastdev-assemblies.cc | Filters the summary and downgrades missing-assembly probes. |
| src/native/clr/host/assembly-store.cc | Downgrades unsuccessful lookup messages to debug. |
| Documentation/workflow/SystemProperties.md | Explains opt-in diagnostics and restart requirements. |
Review effort: Balanced
|
/review |
simonrozsival
added
the
ready-to-review
|
✅ Android PR Reviewer completed successfully! Warning Firewall blocked 1 domainThe following domain was blocked by the firewall during workflow execution:
To allow these domains, add them to the network:
allowed:
- defaults
- "azcliprod.blob.core.windows.net"See Network Configuration for more information.
|
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
The runtime changes correctly move routine assembly probes and the Fast Deployment summary behind LOG_ASSEMBLY while preserving genuine warnings and errors. The documentation matches the implementation. One existing device test still requires the now-opt-in FastDev summary without enabling assembly logging and should be updated to match the new contract.
CI has no failures reported so far: 40 checks have passed, with the final package lane and aggregate dotnet-android check still pending.
Warning
Firewall blocked 1 domain
The following domain was blocked by the firewall during workflow execution:
azcliprod.blob.core.windows.net
To allow these domains, add them to the network.allowed list in your workflow frontmatter:
network:
allowed:
- defaults
- "azcliprod.blob.core.windows.net"See Network Configuration for more information.
Generated by Android PR Reviewer for #13182 · copilot · gpt56 · 79.2 AIC · ⌖ 7.03 AIC · ⊞ 28.4K
Comment /review to run again
| if (count > 0 && found_corelib && !found_r2r) { | ||
| tpa_in_use = true; | ||
| log_writef ( | ||
| log_infof ( |
There was a problem hiding this comment.
🤖 FastDev: Found ... message is absent unless debug.dotnet.log includes assembly, but InstallAndRunTests.StackTraceContainsLineNumbers still unconditionally asserts that this message and the following explanation are present without enabling that category. Please update that test to opt in (and restore the property afterward), or remove those unrelated log assertions, so the test remains consistent with the new contract.
Rule: Keep behavioral tests aligned with intentional output changes
|
/backport to release/11.0.1xx |
|
Started backporting to |
|
@simonrozsival backporting to git am output$ git cherry-pick 70088d7b1e9ab36616640cdfcfd41c2438f4e5ef
Auto-merging Documentation/workflow/SystemProperties.md
CONFLICT (content): Merge conflict in Documentation/workflow/SystemProperties.md
Auto-merging src/native/clr/host/assembly-store.cc
Auto-merging src/native/clr/host/fastdev-assemblies.cc
Auto-merging src/native/clr/host/host.cc
error: could not apply 70088d7b1... [native] Require opt-in for assembly loading logs (#13182)
hint: After resolving the conflicts, mark them with
hint: "git add/rm <pathspec>", then run
hint: "git cherry-pick --continue".
hint: You can instead skip this commit with "git cherry-pick --skip".
hint: To abort and get back to the state before "git cherry-pick",
hint: run "git cherry-pick --abort".
hint: Disable this message with "git config set advice.mergeConflict false"
$ git am --3way --empty=keep --ignore-whitespace --keep-non-patch changes.patch
Applying: [native] Require opt-in for assembly loading logs
Using index info to reconstruct a base tree...
M Documentation/workflow/SystemProperties.md
M src/native/clr/host/assembly-store.cc
M src/native/clr/host/fastdev-assemblies.cc
M src/native/clr/host/host.cc
Falling back to patching base and 3-way merge...
Auto-merging Documentation/workflow/SystemProperties.md
CONFLICT (content): Merge conflict in Documentation/workflow/SystemProperties.md
Auto-merging src/native/clr/host/assembly-store.cc
Auto-merging src/native/clr/host/fastdev-assemblies.cc
Auto-merging src/native/clr/host/host.cc
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
hint: When you have resolved this problem, run "git am --continue".
hint: If you prefer to skip this patch, run "git am --skip" instead.
hint: To restore the original branch and stop patching, run "git am --abort".
hint: Disable this message with "git config set advice.mergeConflict false"
Patch failed at 0001 [native] Require opt-in for assembly loading logs
Error: The process '/usr/bin/git' failed with exit code 128 |
Routine assembly-loading messages should not clutter logcat unless explicitly requested. The Fast Deployment summary currently bypasses category filtering, unsuccessful probes are emitted as warnings, and the external probe trace uses the general
defaultcategory.Route the Fast Deployment summary through category-filtered logging, downgrade routine unsuccessful probes to debug messages, and put the external probe trace in the
assemblycategory. Genuine warnings and errors remain visible. Enable assembly-loading diagnostics withadb shell setprop debug.dotnet.log assembly(orall) and restart the application. Update the system-property documentation accordingly.No linked issue. The added regression test has been removed as requested; the PR contains only runtime and documentation changes.
Validation:
make all— passed with the final runtime changes.ADB_TARGET='-s emulator-5570' ./dotnet-local.sh test bin/TestDebug/MSBuildDeviceIntegration/net10.0/MSBuildDeviceIntegration.dll --filter 'Name~CoreCLRAssemblyLoadingLogsRequireOptIn' --logger 'trx;LogFileName=assembly-logging-system-property.trx' --results-directory /Users/simon/.copilot/session-state/edb5280d-ccc0-40e9-8303-7b930896814f/files -v minimal— all 8 cases passed on an arm64 Android emulator before the test was removed, covering Debug/Fast Deployment and Release/assembly-store with unset,default,assembly, andalllogging settings../dotnet-local.sh build tests/MSBuildDeviceIntegration/MSBuildDeviceIntegration.csproj --no-restore -v minimal— passed after removing the test (30 warnings, 0 errors).Pull Request
title and
description
should follow the
commit-messages.mdworkflow documentation, and in particular should include: