Fix resolution of the external dependencies. - #11742
Fix resolution of the external dependencies.#11742Nikolay Rovinskiy (nick863) wants to merge 2 commits into
Conversation
commit: |
There was a problem hiding this comment.
Pull request overview
Updates the C# generator’s external NuGet dependency resolution so it can (when needed) fall back to the latest available version (optionally including prereleases) instead of only using a requested minimum version or whatever is already cached.
Changes:
- Added a helper to enumerate available package versions across enabled NuGet sources.
- Updated external type resolution to select a version based on
MinVersionpresence and prerelease status before downloading.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Utilities/NugetPackageResolver.cs | Adds GetAllVersions helper for collecting versions from enabled NuGet sources. |
| packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Utilities/ExternalTypeReferenceResolver.cs | Uses version enumeration to choose a download version when the requested MinVersion isn’t available and to include prereleases when appropriate. |
Suppressed comments (1)
packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Utilities/ExternalTypeReferenceResolver.cs:260
new NuGetVersion(external.MinVersion)will throw for an invalid/unsupported version string, which changes behavior compared to the previous string-based flow and ends up being swallowed by the broad catch (reported as "package not found"). Also,versions.Max()throws on an empty sequence, so missing packages/feeds can trigger an exception and skip the intended fallback selection.
NuGetVersion minVersion = new(external.MinVersion);
IList<NuGetVersion> versions = await NugetPackageResolver.GetAllVersions(external.Package!, nugetSettings, allowPrerelease: minVersion.IsPrerelease);
if (versions.Any(x => x == minVersion))
{
resolvedVersion = external.MinVersion;
}
else
{
resolvedVersion = versions.Max()?.ToString();
}
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
|
No changes needing a change description found. |
|
The resolved version should be based on the csproj/central package management. The MinVersion from the decorator is only meant to be used as a safety check - it doesn't influence the version that is used, but it can trigger an error if the min version doesn't align with the resolved version. Can you clarify what issue this is solving? |
I have updated the description. |
The minVersion should not be used to influence the version that is downloaded by the generator. It is only meant to be used as a compatibility floor. It is optional - it doesn't have to be specified at all. I'm not sure what problem this is solving. |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
Suppressed comments (2)
Previously missed (1) — in code that hasn't changed since the last review.
packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Utilities/ExternalTypeReferenceResolver.cs:260
- If the configured feeds return no versions (e.g., package doesn't exist, or only prerelease versions exist but
allowPrereleaseis false),versions.Max()will throw on an empty sequence and the resolver will fall into the catch path. Handle the empty list explicitly so resolution can fail cleanly without relying on exceptions.
NuGetVersion minVersion = new(external.MinVersion);
IList<NuGetVersion> versions = await NugetPackageResolver.GetAllVersions(external.Package!, nugetSettings, allowPrerelease: minVersion.IsPrerelease);
if (versions.Any(x => x == minVersion))
{
resolvedVersion = external.MinVersion;
}
else
{
resolvedVersion = versions.Max()?.ToString();
}
packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Utilities/ExternalTypeReferenceResolver.cs:266
- New behavior adds multiple version-selection branches (minVersion present + exact version missing -> pick latest; prerelease minVersion -> include prerelease; minVersion absent -> latest stable). There are existing unit tests for ExternalTypeReferenceResolver, but none cover these new branches. Add tests that exercise: (1) minVersion not in feed selects latest available version; (2) prerelease minVersion allows selecting a prerelease latest; (3) stable minVersion does not select prerelease when only prerelease versions exist.
if (!string.IsNullOrEmpty(external.MinVersion))
{
// If min version was provided, we
// 1. Search if it is in our repositories;
// 2. Get the latest one if it is not.
// 3. If our version is a pre release, include pre released versions in our search.
NuGetVersion minVersion = new(external.MinVersion);
IList<NuGetVersion> versions = await NugetPackageResolver.GetAllVersions(external.Package!, nugetSettings, allowPrerelease: minVersion.IsPrerelease);
if (versions.Any(x => x == minVersion))
{
resolvedVersion = external.MinVersion;
}
else
{
resolvedVersion = versions.Max()?.ToString();
}
}
else
{
// If min version was not provided, get the latest stable version.
resolvedVersion = await NugetPackageResolver.ResolveLatestPackageVersion(external.Package!, nugetSettings);
}
The problem is that we did not released the new stable version yet, while the downloaded version is 2.0.0. The logic in |
|
Thanks, that clarifies the reproduction. I think the root fix should be in project-reference resolution rather than selecting a package version from
With that flow, the reported case resolves the project's --generated by Copilot |
Problem: Assume, we have the external assembly defined in a typespec as follows:
If the version 3.0.0-alpha.20260820.5 is not present in the repository, the
ExternalTypeReferenceResolverwill not download the needed assembly and the one already present will be used. This will result in some classes not being found as by default the latest stable version is being downloaded.Solution: Currently, the external package is resolved as follows:
In this PR we are adding more logic: