LT-22728: Scope local libraries to one build - #1105
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1105 +/- ##
=======================================
Coverage 38.35% 38.35%
=======================================
Files 1507 1507
Lines 350617 350617
Branches 40298 40298
=======================================
Hits 134471 134471
Misses 186916 186916
Partials 29230 29230 🚀 New features to boost your workflow:
|
jasonleenaylor
left a comment
jasonleenaylor
left a comment
There was a problem hiding this comment.
The diagnosis in LT-22728 is correct: an extracted package in packages/ survives the
deletion of its source .nupkg, because nuget.config points globalPackagesFolder at the
repository's packages directory and NuGet does not invalidate an extracted ID/version when
the feed copy disappears. That is a real trap and worth closing.
My concern is scope. Before going through the individual findings, I want to put the
proportionality question first, because it may make most of them moot.
The reported bug is really two, with very different severity
Mode 1 — the version pin was never reverted. This is what the ticket's repro actually
does: pack Machine, build, delete the .nupkg, build again. Manage-LocalLibraries.ps1 wrote
the local version into Build/SilVersions.props, so the second build resolves the local
package because the pin still names it. This affects all five libraries — and it is
visible: SilVersions.props is tracked, git status shows it dirty, and the script prints
"To revert: git checkout Build/SilVersions.props" immediately after packing. The only genuinely
surprising part is that deleting the .nupkg produces a silent success rather than a restore
failure, because the stale extraction covers for it.
Mode 2 — the local pack produced the same version string as the published pin. Then even
after a correct revert, the extracted local build masquerades as the published package. No
dirty file, no signal, nothing to notice. This is the dangerous one, and in practice it is
specific to SIL.Machine: Build/SilVersions.props:20 pins SilMachineVersion to 3.9.2,
and Machine's local pack produces 3.9.2. For palaso, lcm, chorus and l10nsharp a local
checkout produces a different version, so once the pin is reverted the cached local package is
inert.
A smaller fix for the dangerous half
Make a local pack produce a version that cannot collide with a published pin — a -local
suffix, a build stamp, anything distinguishable. Then:
- A local package can never masquerade as the published one, for any library, including Machine.
- Mode 2 disappears entirely.
- Mode 1 remains, which is acceptable: it is already visible in
git statusand already has a
printed revert instruction. If you want to harden it further, the cheap addition is a warning
at pack time when the packed version equals the pin — the single case where a stale cache
entry can quietly change a later build.
That closes the silent failure without changing how anyone works.
What the current approach costs
As written, this PR changes the local-library workflow for all five libraries in service of a
problem whose silent form belongs to one:
Manage-LocalLibraries.ps1 -Palaso(and the rest) now throws, redirecting to
build.ps1 -LocalLibraries palaso.- The persistent user-level NuGet source is removed, and
nuget.config:31adds<clear />,
which discards every inherited user- and machine-level package source for every restore in
this repository — including for developers who never run a local-library build. - Every build, whether or not
-LocalLibrarieswas passed, sweeps both the package cache and
$env:LOCAL_NUGET_REPO, a user-owned folder outside the repository that may be shared with
other SIL projects and is shared across worktrees. - Selection moves from a visible edit to a tracked file to an invisible per-invocation MSBuild
property.
The persistent local source in particular is the intended path for painless local library
development, not a defect to be eliminated. It is also inert by construction: with exact
version pins in Directory.Packages.props, a local feed containing SIL.LibPalaso 15.0.1-local
is never consulted while the pin says 15.0.0. It becomes live only when a version override
selects it — which is exactly the per-invocation mechanism you built. Please drop <clear />
and keep the persistent source.
Worth knowing, since it is directly upstream of all of this: Build/NuGet.targets used to
carry a CleanNuGet target that did ForceDelete Files="$(fwrt)/packages/" — a standalone,
manually invoked wipe of the package cache. It was removed in 5711bf6be ("Modernize .NET
tooling and enable AI workflows", #678) when that file was deleted, and nothing replaced it.
The gap this PR is filling with automatic cleanup was previously filled by an explicit one.
Cleanup should be deliberate and visible, not automatic
Local library development is uncommon, deliberate, developer-driven work. The person doing it
knows when they have finished. That argues for a well-known post-activity cleanup rather than a
sweep on every build — and it dissolves most of the machinery in this PR:
- Cleanup can afford to be blunt, because the developer asked for it.
packages/is repo-local
and disposable; restore repopulates it. Prefix matching over the library families is
sufficient — which is whatUpdate-VersionAndClearCachealready does today via
CachePrefixes, and what this PR replaces with.nupkg.metadatainspection. - The metadata inspection is what creates the failure modes below. Dropping it removes them
rather than requiring them to be fixed:LocalLibraries.psm1:145reads$metadata.sourceoutside thetry/catchat:138-144,
underSet-StrictMode -Version Latest. A version-1.nupkg.metadata(older NuGet) has only
versionandcontentHash, so a long-livedpackages/folder containing one aborts the
build with a property-not-found error. The fixture only ever writesversion = 2.- A version directory with no
.nupkg.metadataat all is skipped (:140-142), so a partially
extracted package survives the cleanup that exists to remove it. - The cleanup at
build.ps1:591-592sits outside every guard.-LocalLibrariesis correctly
refused with-SkipRestore(:597-598), but the reverse order is not: run
build.ps1 -LocalLibraries machine, thenbuild.ps1 -SkipRestore, and the second run
deletes the package and then skips the restore that would replace it.
- Raise the visibility where the developer already is: print the cleanup command at the end
of a-LocalLibrariesbuild. The person who just did the uncommon thing is told, in that
moment, how to undo it. That is the part worth engineering.
If the current approach is kept, these still need fixing
The version override does not reach the nested restore.
Build/PackageRestore.targets:100-119 Execs a fresh dotnet restore carrying only
/p:Configuration and /p:Platform. The native build reaches it every run
(mkall.targets:73 DebugProcs -> CopyDlls -> downloadDlls -> RestorePackages), and
native runs first. MSBuild global properties do not cross an Exec boundary, so that restore
resolves versions from SilVersions.props — the published values — with no local feed, and
rewrites project.assets.json before the managed traversal compiles.
I have not run this, so I am not asserting it as fact. What I am confident about is that the
existing evidence cannot settle it: the verification is a local machine 3.9.2 build, and
SilVersions.props:20 pins SilMachineVersion to 3.9.2, so the override was a no-op and the
nested restore was harmless by construction. Machine is simultaneously the only library that
was tested and the only one that could not have exposed this.
Please run it end to end with palaso or lcm, where the local version differs from the pin.
Either the binary is built against the local package and I am wrong, or it is not and the gap
is real. Both outcomes are worth having before merge. If it needs closing, a generated,
gitignored Build/LocalLibraries.props imported by Directory.Packages.props would reach every
MSBuild process including nested Execs — and matches the LibraryDevelopment.properties
precedent.
The test harness never fails properly. Build/LocalLibraries.Tests.ps1:2 sets
$ErrorActionPreference = 'Stop', so the first Write-Error at :129 is terminating: only one
failure ever prints and exit 1 at :131 is unreachable. Build/Agent/CommentHygiene.Tests.ps1
— the file this was modelled on — uses Write-Host -ForegroundColor Red in the loop and then
exit 1. The structure was copied; the detail that makes it work was not.
The new PowerShell escapes the repository's compatibility check. LocalLibraries.psm1 and
LocalLibraries.Tests.ps1 sit in Build/, not Build/Agent/, so powershell-compat.ps1:52-55
does not scan them and .github/workflows/CI.yml:34-50 does not run them under both PowerShell
5.1 and 7 — even though build.ps1 and test.ps1 load the module under 5.1 in CI.
Six of the 25 assertions are invalidated by the changes above, so the test rework is larger
than it looks. Two are incorrect: :102 ($managerText -notmatch 'dotnet nuget add source')
pins the absence of the persistent source, and :119 pins the presence of <clear />. Four are
obsolete: :63 and :65 test the filesystem-versus-HTTP source discrimination, and :110
and :114 test build.ps1 wiring that goes away.
The eleven behavioural assertions survive a design change; the twelve source-text regexes do
not — several would fail on a rename that preserved behaviour exactly, and two now make
correcting a design decision look like breaking tests. Given how much of it the adjustments
invalidate, what does the remainder need a 133-line file and a bespoke harness for? That is a
genuine question, not a rhetorical one — if the surviving behaviour justifies it, keep it.
Smaller things:
build.ps1:614-617invokesManage-LocalLibraries.ps1with&and then tests
$LASTEXITCODE -ne 0. That script signals failure bythrow, never by an exit code, so the
check reads whatever native process ran last — or, if$LASTEXITCODEis unset,
$null -ne 0is true and the build throws "Local library packing failed." spuriously. The
real failure path is the exception, which is already handled.- Pack order is now load-bearing and documented nowhere. It was
# Pack order: libpalaso first (other libraries may depend on it)with an explicit list; it
is now$PackOrder = @($LibraryConfig.Keys)(Manage-LocalLibraries.ps1:112), taking its
order from the declaration order of an[ordered]hashtable in a different file. Alphabetising
the catalogue — the obvious future tidy-up — silently breaks dependent packs, and no test
covers ordering. Restore the comment at both sites. - The PR body says selected libraries "evict same-version published cache entries". The
function's own synopsis (LocalLibraries.psm1:85-88) correctly says "every cached version",
and the test at:83-86evicts3.9.3while packing3.9.2. The code is right; the body
promises less than it does. build.ps1:683-686(FwBuildTasksrestore/build) and:564-566(native freshness refresh)
run without the/p:Sil*Versionproperties or the local feed — same class of gap as the
nested restore, lower risk.- The five library names now live in four places:
LocalLibraries.psm1:4-42, the-Library
ValidateSet, the-LocalLibrariesValidateSet atbuild.ps1:202-203, and the docs table.
The test asserts the catalogue has five entries but not that the ValidateSets match it.
Comments
The three .SYNOPSIS blocks in LocalLibraries.psm1 (:85-88, :106-109, :114-117) are
one accurate sentence each, stating only their own contract — those are right, and so are the
rewritten .EXAMPLE blocks in Manage-LocalLibraries.ps1:20-27.
Four things to fix:
Manage-LocalLibraries.ps1:69-70—.PARAMETER VersionOutputPath, "JSON output consumed by
build.ps1 for invocation-scoped version overrides." Describe the file the parameter names, not
who reads it; callers change silently.nuget.config:33-36— the rewritten comment keeps "See
Docs/architecture/local-library-debugging.md for the full workflow." Editing the block was the
moment to drop the.mdpointer.- Two genuine WHY comments were lost in the move.
# Pack only the projects FieldWorks uses (avoids native CMake deps)explained something the code cannot show and arrived bare at
LocalLibraries.psm1:38-41; the pack-order rationale is gone from both files. - The one genuinely non-obvious thing is uncommented: the whole design turns on
.nupkg.metadata'ssourcefield being a safe local-versus-published discriminator
(:56-66,:145), and nothing says why. If the metadata approach survives, that comment is
the one worth writing.
This review was assisted by Claude Fable 5.
b267b2f to
beb75c3
Compare
A locally packed library used to reuse the published version string, and NuGet resolves an already-extracted (id, version) before it consults a folder feed. A local pack could therefore be shadowed by the published package, or shadow it, with nothing to tell them apart. SIL.Machine was the clearest case: it has no GitVersion, so it packed as a flat 3.9.2, identical to the package on nuget.org. Derive each pack's version from the checkout instead, as <core>-<branch>.<commit>, taking the core from the library's own GitVersion where it has one. GitVersion.MsBuild assigns Version inside a target, which outranks a command-line property, so it is switched off for the pack. Because a clean commit identifies its contents, a second build from the same commit reuses the package already in the feed. An uncommitted checkout has no stable identity, so it is marked dirty, repacked every time, and the build names the paths responsible. Write the selected versions and the feed to a generated Build/LocalLibraries.props that Build/SilVersions.props imports, rather than passing them on the command line. The restore in Build/PackageRestore.targets runs through Exec, which starts an MSBuild process that does not inherit global properties, so a version passed that way never reached it: the build reported using a local library while every project resolved the published one. Build each library before packing it. A package may include output from a target framework its own project does not build, and pack alone does not produce those, which left L10NSharp unpackable. Keep the feed inside the working tree as .localfeed. A machine-wide feed let one working tree's build delete packages another had just produced, which is also why the existing cleanup could not be trusted; scoped to one working tree, it can be. Skip that cleanup when the build will not restore, so it cannot remove packages nothing will put back. Add Setup-LocalLibraries.ps1 to make a library branch available. It finds the checkout beside FieldWorks or through the library's path variable, and uses an existing worktree for the branch where there is one, since git refuses to check a branch out twice and that worktree may hold work in progress. It fetches but never merges, never switches a branch in a checkout that already has one, and never prompts. Leave inherited package sources in place: a local build adds its own feed for that build only, and the versions it packs cannot collide. Read the cache metadata defensively. Version 1 records no source, which under Set-StrictMode ended the build, and a version directory with no metadata is a partial extraction rather than something to keep. Report every failing assertion instead of stopping at the first, and cover Build with the PowerShell compatibility check, which scanned only Build/Agent. Verified through build.ps1 against liblcm: 114 projects resolved the local package where none did before, and an ordinary build then restored the published one without redownloading it.
beb75c3 to
049b692
Compare
|
Thank you — this review found a bug that would have shipped, and the analysis was right on every point I could check. Head is now You were right about the nested restore, and it was worse than "not settled"I ran it end to end with …and all 120 Your reasoning was exactly the mechanism: Fixed the way you suggested: a generated, gitignored
And the reverse: an ordinary build removes the overrides, empties the feed, and returns all 114 to The naming fixAdopted, and made content-addressed rather than a fixed suffix: One thing worth flagging, since it bit me: Everything else you raised
On the test fileFair question, and the honest answer is that the source-text regexes are the weaker half — several would fail on a rename that preserved behaviour. I kept them where the thing being asserted is a property of the source (that a ScopeI have not shrunk the branch, and I want to be straight about why rather than quietly disagreeing. The nested-restore fix required the generated props file; the props file is what the deferred per-worktree paths needed anyway; and the worktree-local feed is what makes the cleanup you objected to defensible instead of merely convenient. What is left beyond your minimal fix is the setup command, and I would drop that if you want it separate — it is self-contained. The one thing I did not do is make cleanup fully manual. With the feed inside the working tree and the |
3 participants
.\build.ps1 -LocalLibraries lcmnow builds FieldWorks against your localliblcm checkout with no machine-wide setup, and the next ordinary build puts
the published package back without redownloading it.
The unknown a reviewer starts with is why the previous version deleted cache
entries so aggressively. Because a local pack reused the published version
string, and NuGet resolves an already-extracted
(id, version)before itconsults a folder feed — so a local pack could shadow the published package, or
be shadowed by it, with nothing to tell them apart. SIL.Machine showed it
plainly: no GitVersion, so it packed as a flat
3.9.2, identical in name tonuget.org's. The eviction was a workaround for a naming problem. This branch
fixes the name, which turns the cleanup into ordinary housekeeping.
Where to look
LocalLibraries.psm1) —<core>-<branch>.<commit>. Apublished version can no longer be reused; a dirty tree gets
.dirtyandrepacks every time, having no stable identity.
-p:DisableGitVersionTask=true— GitVersion assignsVersioninside anMSBuild target, which outranks a command-line property. Without this the stamp
is silently overridden; proven on libpalaso in the evidence below.
Build/LocalLibraries.props, generated and imported bySilVersions.props— the restore inPackageRestore.targetsruns throughExec, a new MSBuild process inheriting no global properties, so a versionpassed on a command line never reached it.
.localfeed) — a machine-wide feed letone working tree's build delete packages another had just produced. This is
what makes the existing cleanup safe rather than destructive.
Setup-LocalLibraries.ps1— reuses an existing worktree for a branchbefore creating one, fetches but never merges, and never switches a branch.
Deliberately not here
pass CI.
use different branches of one library.
Verification —
.�uild.ps1 -LocalLibraries lcmend to end: 114 projectsresolved the local package where none did before the props file existed, and
an ordinary build then put
11.0.0-beta0178back without redownloading it. Allfive libraries pack from a clean worktree and reuse on a second run.
gitlint,comment hygiene, the PowerShell 5.1/7 compatibility check and the 70-assertion
test script are clean.
LT-22728
Reading this a year from now — start here
The reasoning behind this branch lives here rather than in the tree, on purpose.
The investigation that produced it was a one-time diagnosis, and
Docs/architecture/local-library-debugging.mddeliberately carries only what adeveloper needs in order to use the workflow.
The one fact worth keeping: NuGet's resolution order is global-packages folder
→ non-HTTP sources (folder feeds) → http-cache → HTTP. If
(id, version)isalready extracted in
packages/, the folder feed is never consulted at all.Every design choice below follows from that sentence.
Decisions, and why
Content-addressed, not branch-addressed. An earlier draft named packages by
branch alone. That is a stable string, so a second pack from a dirty tree
would hit the extracted cache and silently serve the previous build. The commit
hash makes a clean version identify its contents, which is also what makes pack
reuse correct rather than a gamble. A dirty tree cannot be identified this way
at all, so it is marked
.dirtyand always repacked.Untracked files count as dirty. The rule is "whatever git would report",
which honours
.gitignoreand so excludesbin/obj. A new source file addedwhile prototyping changes the build without changing the commit, so treating it
as clean would serve a stale package. The cost is that a stray note beside the
source keeps the slow path, so the build now names the offending paths.
The core version comes from the library's own GitVersion, probed with
dotnet msbuild -restore -t:GetVersion, so a version bump in the library showsup in the local package.
-restoreis required: the target ships inside theGitVersion package, which a never-built checkout has not restored yet.
SIL.Machine has no GitVersion and falls back to the consumed version.
Discovery before creation.
git worktree addrefuses a branch alreadychecked out elsewhere. Rather than fight that, the setup flow reads
git worktree list --porcelainand uses whatever already holds the branch —both the fast path and the only path git allows.
Build before pack. A package may include output from a target framework its
own project does not build, and packing first fails on the missing file.
Paths not taken
A branch name in the config filename (
localLibs.<branch>.props). In a gitworktree
.gitis a file, not a directory, so discovering the branch atMSBuild evaluation time — the only point at which
PackageReferenceitems canbe declared — breaks in exactly the multi-worktree case that motivates this
work. Branch names also contain
/, a detached HEAD has no branch name, andrenaming a branch would silently drop the config. Go, Cargo and Gradle all use a
fixed filename in a per-checkout location for the same reason.
Publishing per-branch prereleases from the library repositories so a
FieldWorks PR could point at an unreleased library. liblcm already does this to
GitHub Packages for same-repo PRs. Dropped deliberately: it needs a
read:packagestoken for every developer and every CI job, and the intendedpolicy is that only a real release passes CI.
Nesting a dependency's worktree pinned to a live branch. No established tool
does this. Chromium's
DEPS, Zephyr'swestand Android'srepoallmaterialise to a detached HEAD or a pinned revision, and
westdoes soprecisely to avoid the same-branch-checked-out-twice refusal.
Having the build create the worktrees. Every comparable tool keeps sync as an
explicit command. A build that mutates the source tree outside its output
directory is the failure mode, which is why
Setup-LocalLibraries.ps1isseparate and the build only verifies.
Surprising findings
GitVersion silently wins.
-p:Version=is a global property, butGitVersion.MsBuild assigns
<Version>inside a target, and a target canoverride a global property. This was found only because the first end-to-end
test ran against SIL.Machine — the one library with no GitVersion, and therefore
the one case that could not exhibit the bug.
A build can report using a local library while compiling against the published
one. MSBuild global properties do not cross an
Execboundary, andPackageRestore.targetsrestores through one, reached by the native build beforethe managed traversal. The first end-to-end run printed "Using local libraries:
lcm", exited 0, and left every one of 120
project.assets.jsonfiles naming thepublished version. Reviewer analysis predicted this before it was measured; the
earlier evidence could not have caught it, because the only library tested end to
end was SIL.Machine, whose local version equalled the pin and made the override a
no-op.
git rev-parse --git-common-dirreturns a path relative to the caller, notto the repository. A first cut used it to locate
.git/info/excludeandsilently resolved against the wrong directory;
--path-format=absoluteisrequired. The same trap applies to locating a sibling checkout from inside a
worktree, where the parent directory is
.tmp/worktreesrather than therepositories root.
L10NSharp was not broken by this change. It could not be packed at all —
NU5026, missingoutput/Debug/net461. The discriminating test was to packwith the original flags, which failed identically, and the main checkout had no
such output either. Its packages include output from target frameworks its own
projects do not build, so it needs a full build first. It packs cleanly now.
Two libraries' symbol directories named paths that are never written, so the
PDB copy did nothing and said nothing. A miss is now reported together with the
directories that were searched.
What this does NOT authorize
This branch does not establish a way to reference an unreleased library from a
merged commit. Derived versions are passed to restore and MSBuild as properties
and never enter
SilVersions.props;.localfeedis gitignored; the feed isnever added to
nuget.config. A pushed branch therefore carries no reference toa local package, and CI restores from nuget.org only. If a FieldWorks change
needs a new library API, it cannot go green until that library is released —
the intended constraint, not an oversight.
Deferred, and what would unblock it
LIBLCM_PATHand its siblings are machineglobal, so two working trees cannot use two different branches of the same
library. The fix is the pattern this repository already uses twice
(
GlobalInclude.properties,LibraryDevelopment.properties): a fixed-name,gitignored
Build/LocalLibraries.propsimported fromDirectory.Build.props.Setup-LocalLibraries.ps1would then write the resolved path there instead ofprinting it.
Build/Localize.targetsstill records the original move awayfrom that pattern.
instead of as a compile error.
now configured.
Evidence
GitVersion override, libpalaso, same commit:
SIL.Core.18.0.0-lt22728-vp.1e46149.nupkgSIL.Core.18.0.0-lt22728-vp0033.nupkgAll five libraries, packed from a clean worktree:
Full chain, liblcm. A probe class committed to a scratch branch produced
11.0.0-lt22728-e2e.08cb359. The marker was present in the packednet462,net8.0andnetstandard2.0assemblies;dotnet restoreresolved it in 8s;the published
11.0.0-beta0178and the local version coexisted inpackages/,with
.nupkg.metadatarecording the filesystem feed as the source; an ordinarybuild then cleared 9 local cache entries and 18 feed packages and left
beta0178intact.Symbol fix, SIL.Machine.
Output/Debugwent from 0 to 2SIL.MachinePDBs,where the previously configured path reported the directory missing.
Test assertions were mutation-tested rather than assumed: flipping
.localfeed, the reuse message, and the L10NSharp symbol directory each madethe suite fail, and pass again once restored.
End to end through
build.ps1. With-LocalLibraries lcm, 114project.assets.jsonfiles named11.0.0-feature-grammar-json-exp.dirty; the sixstill naming the published version were stale from four days earlier and untouched
by the run. An ordinary build then removed the overrides, emptied the feed, and
returned all 114 to
11.0.0-beta0178.Not run: the full managed test suite. The diff is PowerShell, markdown,
.gitignoreandnuget.config, with no compiled code.Preflight review details
Code Review Summary
Branch: LT-22728-local-library-selection
Base: origin/main
Date: 2026-08-26
Review model: Claude Opus 5
Files changed: 10
Overview
Local library packs reused the published version string. NuGet resolves an
already-extracted
(id, version)from the packages folder before it consults afolder feed, so a local pack could shadow the published package or be shadowed
by it with nothing to distinguish them. SIL.Machine showed it plainly: with no
GitVersion it packed as a flat
3.9.2, byte-different from but identicallynamed to the package on nuget.org.
The branch derives each pack's version from the checkout as
<core>-<branch>.<commit>, keeps the local feed inside the working tree, buildseach library before packing it, and adds a setup command that makes a library
branch available as a worktree without disturbing uncommitted work.
Contract/API Changes
No public API change. Build-surface changes only:
build.ps1 -LocalLibrariesno longer requiresLOCAL_NUGET_REPO; the feeddefaults to
.localfeedin the working tree and the variable still overrides.Build/Setup-LocalLibraries.ps1with-Library <name>:<branch>.Build/LocalLibraries.psm1exports eight new functions.Manage-LocalLibraries.ps1now stamps-p:Versionand sets-p:DisableGitVersionTask=truewhile packing.Findings
Critical - Must address before merge
None.
Important - Should address before merge
Docs/architecture/dependencies.mdquick start still setLOCAL_NUGET_REPO(fixed during review: removed the line and reworded thesurrounding text; it contradicted the updated local-library-debugging.md)
NU5026, missing net461/net48output) (fixed during review: each library is now built before it is
packed, because a package may include output from a target framework its own
project does not build)
Minor - Consider
PdbRelativeDirnamed directories two libraries do not write, so thesymbol copy did nothing and said nothing (fixed during review: l10nsharp now
uses
output/Debug/net48, machine takes one directory per pack project, anda miss now reports the directories searched. Proven on machine: symbols in
Output/Debug went from 0 to 2 where the old path reported nothing)
test.ps1ranLocalLibraries.Tests.ps1unconditionally, ignoring-TestProjectand-TestFilter(fixed during review: it now runs only whenneither is given)
nuget.config<clear />dropped inherited user-level sources with nohint (fixed during review: the comment now says a private feed belongs
here rather than in user-level configuration)
Required Validation / Evidence
Run and passing:
Build/LocalLibraries.Tests.ps1- passes; 55 assertions. Mutation-testedtwice (
.localfeedto.wrongfeed, and the reuse message) to confirm the newassertions actually fire rather than passing vacuously.
Build/Agent/comment-hygiene.ps1 -BaseRef origin/main- clean.gitlint --ignore body-is-missing --commits origin/main..HEAD- clean.carries the derived version, and a second run reuses it.
SIL.Machine 7s to 0s.
SIL.Core.18.0.0-lt22728-vp.1e46149.nupkg; without themSIL.Core.18.0.0-lt22728-vp0033.nupkg.net462, net8.0 and netstandard2.0; the published
11.0.0-beta0178and thelocal version coexisted in the cache; an ordinary build then cleared 9 local
cache entries and 18 feed packages and left
beta0178intact.Not run:
./build.ps1full compile of FieldWorks against a local library. Pack andrestore are verified; the subsequent compile is not.
./test.ps1full managed suite. The diff is PowerShell, markdown, gitignoreand nuget.config only, with no compiled code.
./Build/Agent/Setup-InstallerBuild.ps1 -ValidateOnly- no installer or WiXfiles changed.
Positive Observations
sanctioned mitigation: NuGet documents no restore flag that bypasses the
extraction cache for a version already present.
post-pack clear be deleted;
Manage-LocalLibraries.ps1is net shorter.worktree add, plus afetch. A test asserts noswitch,checkout,resetorcleanappears.Interview Notes
satisfied - derived versions travel as MSBuild properties and never enter
SilVersions.props,.localfeedis gitignored, and the feed is never addedto
nuget.config, so a pushed branch carries no reference to a local package.-p:DisableGitVersionTask=truealso disables GitVersion'sDefineConstantsand assembly-info stamping, so a local package's assembly metadata differs
from a CI-built one. Author accepted: local versions cannot be mistaken for
real ones and dirty detection says when to rebuild. The narrower
-p:UpdateVersionProperties=falsewas offered and not taken.documented, on the grounds that it was probably already solvable. That was
correct: it needed a build before the pack, not a toolchain change.
to what git itself would report rather than classifying file types.
rather than deferred, so all three were addressed here.
In-Review Quality Check
Docs/architecture/dependencies.mdcorrected; markdown fences verifiedbalanced after a bad
seddeleted the wrong line mid-edit.work: branch, HEAD, worktree list, branch list, status and
.git/info/excludeall match, including three repositories' pre-existing uncommitted files.
Suggested Review Focus
where a package spans target frameworks.
means a stray note beside the source keeps the slow path.
This change is