fix(api-docs/dotnet): read package versions from MSBuild, not a csproj regex - #133
Merged
Merged
Conversation
…j regex
The .NET template scraped <Version>/<VersionPrefix> out of the .csproj
files to label each package in the generated API index. That only works
for repos that commit a literal version string. A repo that derives its
version from git tags carries no such literal on purpose -- a literal in
the tree is something an inbound sync can revert -- so the scrape fell
through to its "unknown" default and the index rendered `vunknown` for
every package.
Ask MSBuild instead, which is authoritative for both shapes: it reports
the literal where one exists and the tag-derived value where it does not.
The .csproj scan is kept behind it as a fallback for when the query is
unavailable (no -getProperty before SDK 8.0.200) or the project cannot be
evaluated, so nothing regresses for repos that still use a literal.
Two details that the obvious version of this fix gets wrong:
- `-getProperty:Version` on its own only *evaluates* the project, and a
tag-derived version is computed inside a target. A bare query returns
the SDK's implicit 1.0.0. The query runs `-t:GetAssemblyVersion`, a
stock SDK target whose job is to settle the version. It compiles
nothing -- the solution is already built by an earlier step -- and
costs ~0.5s per project, ~3s for the six.
- An MSBuild answer of exactly 1.0.0 is indistinguishable from the SDK
default, so it is treated as inconclusive and the literal scan still
runs. That preserves the existing multi-candidate behaviour, where a
version lives only in the packaging variant of a duplicated .csproj.
Also set fetch-depth: 0 on the source checkout. Tag-derived versioning
reads the tag graph at build time, and under the default shallow, tagless
checkout it does not fail -- it silently settles on 0.0.0-alpha.<height>.
Without this the index would render `v0.0.0-alpha.0` instead of
`vunknown`, which is worse: a plausible-looking wrong version. The
existing persist-credentials: false is unchanged.
Verified against a full-history clone of the .NET SDK repo: the old logic
yields "unknown" for all six packages, the new logic yields the real
MinVer version (0.6.1-alpha.0.42 at HEAD, 0.7.0 with a release tag
applied), and the same tree at depth 1 yields 0.0.0-alpha.0. A
literal-version repo returns its literal unchanged under both the old and
new logic.
|
Warning Review limit reachedNext included review available in 56 minutes. View limit detailsLimit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (1)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
This was referenced Sep 22, 2026
Merged
WomB0ComB0
added a commit
to resq-software/dotnet-sdk
that referenced
this pull request
Sep 22, 2026
#106) Every package in the generated reference is currently labelled `vunknown`. The version lookup scrapes `<Version>`/`<VersionPrefix>` out of the .csproj and returns "unknown" when neither is present -- and since this repo moved to MinVer, no project carries a literal version by design. The scrape therefore misses on all of them. This replaces that step with the version already fixed upstream in resq-software/docs#133: ask MSBuild via `-t:GetAssemblyVersion -getProperty:Version`. GetAssemblyVersion is a stock SDK target, so MinVer runs and reports the tag-derived value; the .csproj scrape is kept as a fallback for SDKs older than 8.0.200, which have no -getProperty. The file is taken verbatim from the upstream template, so the next template sync is a no-op here instead of a revert. Verified locally against this tree at e727403 (full history, tags present, after restore): ResQ.Storage unknown -> 0.6.1-alpha.0.42 ResQ.Core unknown -> 0.6.1-alpha.0.42 Note the repo contains two ResQ.Core.csproj files -- the real `ResQ.Core/` in the solution, and an orphaned `resq-core/` copy that is not. The orphan is never restored, so MSBuild reports the SDK default 1.0.0 for it. The upstream logic already handles this: it skips any candidate whose value equals the SDK default and only falls back to it last, so the real 0.6.1-alpha.0.42 wins. Confirmed by replaying the selection over both candidates.
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
1 participant
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 broke
The .NET API-docs template labels each package in the generated index by
regex-scraping
<Version>/<VersionPrefix>out of the.csprojfiles,falling back to the string
unknown.The .NET SDK repo has just moved to tag-derived versioning (MinVer), so it
deliberately carries no literal version anywhere — a literal in the tree is
exactly the thing an inbound sync can revert, and one did, twice. The scrape
therefore finds nothing and the generated reference now renders:
Reproduced by running the template's own
read_version()against afull-history clone of the post-migration tree, not inferred.
Two things were wrong, and fixing either alone leaves it broken
1. The version came from the wrong source. Replaced with an MSBuild
query, which is authoritative for both repo shapes — it reports the literal
where one exists and the tag-derived value where it does not. The
.csprojscan stays behind it as a fallback for when the query is unavailable (no
-getPropertybefore SDK 8.0.200) or the project cannot be evaluated, thenunknownas before.Two details the obvious version of this fix gets wrong:
-getProperty:Versionon its own only evaluates the project, and atag-derived version is computed inside a target. A bare query returns the
SDK's implicit
1.0.0. Measured:dotnet msbuild <proj> -getProperty:Version→
1.0.0;dotnet build <proj> -getProperty:Version→1.0.0too, because-getPropertywithout an explicit target skips target execution. The queryhere runs
-t:GetAssemblyVersion, a stock SDK target whose job is to settlethe version →
0.6.1-alpha.0.42.1.0.0is indistinguishable from the SDKdefault, so it is treated as inconclusive and the literal scan still runs.
That preserves the existing multi-candidate behaviour the old comment
describes, where the version lives only in the packaging variant of a
duplicated
.csproj.2. The checkout was shallow.
fetch-depth: 0added. Tag-derivedversioning reads the tag graph at build time and, given a shallow tagless
checkout, does not fail — it silently settles on
0.0.0-alpha.<height>. Soafter fixing the lookup alone the index would render
v0.0.0-alpha.0, whichis worse than
vunknown: a plausible-looking wrong version.persist-credentials: falseis unchanged, and the separate docs-repo checkout is untouched.
Verification
Against a full-history clone of the .NET SDK repo at
e727403, with thesolution restored and built exactly as the workflow does.
Before (current
read_version, post-migration tree):After (the
run:block extracted from this PR's YAML and executedverbatim, full history + tags):
git describeat that commit isv0.6.0-46-ge727403; the.42height iswhat the tool actually computes, checked rather than assumed. With a release
tag applied to the same commit the same code returns
v0.7.0, so the releasepath — the one the
on: push: tags: v*trigger exercises — is covered too.Why
fetch-depth: 0is load-bearing. Same new code, same tree, depth-1clone (1 commit, 0 tags):
No regression for literal-version repos.
ResQ.BuildingBlocks.Templatesin the building-blocks repo carries a literal
<Version>0.1.0</Version>:old logic →
v0.1.0, new logic →v0.1.0. The fallback path itself wasexercised separately by making the MSBuild query fail (shimmed
dotnetthatexits non-zero): still
v0.1.0, via the regex scan. A project with no matchat all still yields
unknown.A synthetic fixture covers the two remaining branches: a project whose version
lives only in
packages/<Proj>/<Proj>.csprojstill resolves to2.3.4, and aproject that genuinely declares
1.0.0still reports1.0.0rather than beingswallowed by the SDK-default check.
Cost. ~0.5s per project, 2.9s for all six — the solution is already built
by the earlier step, so
GetAssemblyVersioncompiles nothing.Lint.
actionlint(with shellcheck available) is clean on the changedtemplate, exit 0, same as the pre-change baseline. The embedded Python
compiles and passes
ruff check -select E,F,W.Notes
.github/workflows/api-docs.ymlcopy wastouched; the sync script propagates this.
package.json,Cargo.toml,vcpkg.jsonandpyproject.toml, so none of them has thisproblem today. Worth revisiting if any of those repos moves to tag-derived
versioning — they would need the same
fetch-depth: 0.has; CI pins 9.0.x via
global.json.-getPropertyandGetAssemblyVersionboth predate 9.0, so the mechanism is the same, but a CI run on a real tag
is the confirming test.
Test plan
v*tag on the .NET SDK reposdks/dotnet/api/README.mdx