Drop the LibrariesAndTests.slnf split for a one-pass managed build#613
Merged
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR removes the LibrariesAndTests.slnf split introduced in #609 and restores a single, parallel, whole-solution CI build by preventing RID/self-contained properties from flowing from executable projects into referenced libraries (eliminating the AnyCPU vs win-x64 dual-build race) and by embedding PDBs to avoid parallel copy races.
Changes:
- Add
Directory.Build.targetsto stripRuntimeIdentifier/SelfContainedfromProjectReferencebuilds originating from executable projects. - Set
DebugType=embeddedglobally inDirectory.Build.propsto avoid.pdbcopy contention under parallel builds. - Simplify PR CI to a single
dotnet build EventLogExpert.slnx ... -p:WindowsPackageType=Nonestep and deleteLibrariesAndTests.slnf.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
LibrariesAndTests.slnf |
Removes the solution filter now that CI no longer depends on a split build. |
Directory.Build.targets |
Prevents RID/self-contained properties from propagating to referenced projects when building executables, addressing the root parallel-build race. |
Directory.Build.props |
Embeds debug symbols across the repo to avoid parallel .pdb copy races. |
.github/workflows/PullRequest.yml |
Switches PR validation to one-pass whole-solution build and removes the separate app build step. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Replaces the
LibrariesAndTests.slnfbuild split (and the-m:1workaround) with a single one-passdotnet build EventLogExpert.slnx. Stacked on #609.Why
#609 introduced the slnf split so CI could build the libraries+tests separately from the MAUI app, working around a parallel-build race: the RID-qualified app compiles the shared libraries as
win-x64while the tests compile themAnyCPU, so each library is built twice concurrently and races on itsobj/bin(CS2012 / MSB3030). The slnf is a maintenance burden (it must list every project). This PR removes the race at the source so the whole solution builds in one parallel pass - no slnf to maintain, matching common .NET practice (PowerToys / eShop / CommunityToolkit build the whole solution/filter in parallel; none use-m:1).This supersedes #609's "build the app separately" approach. Net effect once both land: one-pass build, no slnf, no
-m:1.How
Directory.Build.targets(new) - stripsRuntimeIdentifier;SelfContainedfrom every executable'sProjectReferenceitems (GlobalPropertiesToRemove), so the shared libraries buildAnyCPUexactly once regardless of the app's RID. The libraries get no RID (the SDK-recommended pattern - pinning a RID on libraries is an anti-pattern); the app keeps its own RID for its per-archpublish -r win-x64/win-arm64output.Directory.Build.props- globalDebugType=embedded, so there is no separate.pdbfor dependent projects to race on copying (the second parallel race).dotnet/eShopandCommunityToolkit/Windowsuse the same setting for the same reason; consistent with this repo's existing helper executable.PullRequest.yml- onedotnet build EventLogExpert.slnx -p:WindowsPackageType=None(managed-only, matching the prior PR-CI coverage; the native shell extension + MSIX remain a release-pipeline concern). Drops the slnf step, the separate "Build app" step, and the now-contradictory "Don't recombine" comment.LibrariesAndTests.slnf(its only consumer was the rewritten workflow step).Validation
-p:WindowsPackageType=None): 0 warnings / 0 errors.EventLogExpert.UI) buildsAnyCPU(nowin-x64lib output); the app buildswin-x64. 0 separate.pdbfiles.--no-build): Logging, DatabaseTools, Runtime (1629), UI (958).e_sqlite3.dll+ the AnyCPU libraries.Out of scope
The separate
EventLogExpert-ADOrelease-pipeline repo still passes-m:1(itsbuild.yml+ twopackage.ymlpublish tasks) - a follow-up in that repo. The MSIX-only ElevationHelper publish inside the app csproj retains its own-m:1(single self-contained publish, not the build-graph one).