Fix resolution of the external dependencies. - #11742
Fix resolution of the external dependencies.#11742Nikolay Rovinskiy (nick863) wants to merge 18 commits into
Conversation
microsoft-github-policy-service
Bot
added
the
emitter:client:csharp
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 |
f530a05 to
970c165
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
Suppressed comments (1)
packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/PostProcessing/GeneratedCodeWorkspace.cs:295
- Waiting for the process to exit before draining either redirected stream can deadlock once
dotnet restorefills an OS pipe buffer. This codebase already handles the same failure mode by reading stdout and stderr concurrently inGeneratorHandler.ReadProcessOutput(lines 340-346); start both reads before awaiting process exit here as well.
if (restore.Start())
{
await restore.WaitForExitAsync();
if (restore.ExitCode != 0)
{
string output = await restore.StandardOutput.ReadToEndAsync();
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
Suppressed comments (7)
Previously missed (1) — in code that hasn't changed since the last review.
packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/PostProcessing/GeneratedCodeWorkspace.cs:10
System.Text.RegularExpressionsis not used anywhere in this file; remove the import.
using System.Text.RegularExpressions;
packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/PostProcessing/GeneratedCodeWorkspace.cs:325
new NuGetFramework(x)treats the TFM text as the framework identifier and leavesVersionempty; it does not parsenet8.0/net10.0. Consequently every element compares as version 0 and this method returns the first input (for example,net8.0, net10.0selectsnet8.0), so framework-specific package versions can be resolved from the wrong group. Parse folder TFMs withNuGetFramework.ParseFolderand retain the original short name when selecting the preferred framework.
NuGetFramework? maxFramework = shortNames.Select(x => new NuGetFramework(x)).MaxBy(x => x.Version);
packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/GeneratedCodeWorkspaceTests.cs:389
- Correct the typo in this comment.
// Dislplay is a dll path C:\Users\%username%\AppData\Local\Temp\TestArtifacts\%guid%\NuGetCache\first.package\1.0.0\lib\netstandard2.0\First.Package.dll
packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/GeneratedCodeWorkspaceTests.cs:403
- Use the singular “version” after “more than one.”
Assert.Fail($"Found more than one versions for package {resolvedPackage.Name}: {version} and {resolvedPackage.Version}");
packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/GeneratedCodeWorkspaceTests.cs:673
- Every test using this helper now restores the fake package's
Azure.Coredependency into an otherwise empty temporary cache, making those unit tests depend on network/feed availability. These tests do not exercise transitive dependency resolution, so make the fake nuspec dependency-free (or create the dependency locally).
<dependency id="Azure.Core" version="1.61.0" exclude="Build,Analyzers" />
packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/GeneratedCodeWorkspaceTests.cs:447
- This local is never used; remove it.
var nugetCacheDir = Path.Combine(_tempDirectory!, "NuGetCache");
packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/GeneratedCodeWorkspaceTests.cs:531
- This local is never used; remove it.
var nugetCacheDir = Path.Combine(_tempDirectory!, "NuGetCache");
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.
Suppressed comments (3)
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:237
- This deletion leaves the stated “no MinVersion means latest stable” rule unimplemented. The surviving
FindPackageAssembly(..., null)considers prerelease versions and sorts all parseable cached versions descending, so a cached3.0.0-betais selected over stable2.0.0. Preserve stable-only selection when no minimum is supplied before falling back to arbitrary cached content.
if (assemblyPath == null || !File.Exists(assemblyPath))
packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/GeneratedCodeWorkspaceTests.cs:432
- This test is declared
asyncbut contains noawait, which emits CS1998. Since the generator build treats warnings as errors, the test project will not compile; make the test synchronous.
public async Task TestGetLatestFramework(bool includeGoodVersions, bool includeBadVersions)
packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/PostProcessing/GeneratedCodeWorkspace.cs:342
- The
/ 100conversion misorders valid compact .NET Framework TFMs:net48becomes 2000.48 whilenet472becomes 2004.72, so this method selectsnet472even though .NET Framework 4.8 is newer. Parse compact TFMs as framework versions (for example viaNuGetFramework.ParseFolder) rather than as decimal numbers, and covernet48versusnet472.
if (name.StartsWith("net4", StringComparison.InvariantCultureIgnoreCase))
{
current /= 100;
current += 2000.0;
| { | ||
| Dictionary<string, Dictionary<string, string>> hshFrameworks = []; | ||
| // Read in the resolved direct dependencies | ||
| DirectoryInfo? directory = (new DirectoryInfo(CodeModelGenerator.Instance.Configuration.OutputDirectory)).Parent?.Parent?.Parent; |
There was a problem hiding this comment.
Will this work for other output directory values? I'd prefer we didn't make assumptions about any configuration that is provided by consumers of the generator
There was a problem hiding this comment.
In this logic, we are looking for project.assets.json in the %repo_root%/artifacts/obj/%Project_name% directory as we have it in SDK, or in %Project_root%/src/obj, which is a default location. I think the exact location of this file needs to be read from project configuration, which seems to be out of scope for this PR (unless there is an easy way to get it, it seems to be a bigger task).
There was a problem hiding this comment.
For example this is how the object directory is defined in our projects:
In the Directory.Build.props we define that the Artifacts are located in "artitacts" directory, and ArtifactsObjDir is in artifacts/obj. BaseIntermediateOutputPath is in %repo_root%/artifacts/obj/%Project_name%, and then we apply conditions, so to detect the project.assets.json location we need to parse the structure of a project apply semantics from the configuration files.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
Suppressed comments (5)
packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/GeneratedCodeWorkspaceTests.cs:479
- This local is never used, producing CS0219 under the generator's warnings-as-errors build. Remove it.
var nugetCacheDir = Path.Combine(_tempDirectory!, "NuGetCache");
packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/GeneratedCodeWorkspaceTests.cs:563
- This local is never used, producing CS0219 under the generator's warnings-as-errors build. Remove it.
var nugetCacheDir = Path.Combine(_tempDirectory!, "NuGetCache");
packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/GeneratedCodeWorkspaceTests.cs:688
- NuGet's global-packages layout uses a lowercase manifest filename, as the existing helper does in
test/common/FakeNuGetPackage.cs:111. On case-sensitive agents this casing prevents restore/exact-version lookup from recognizing these fake packages; with multiple cached versions the code then falls back to the highest cache entry instead of the asserted restored version. Lowercase the filename or reuse the shared helper.
File.WriteAllText(Path.Combine(metadataPath, $"{packageName}.nuspec"), $"""
packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/GeneratedCodeWorkspaceTests.cs:180
- The fake assembly does not reference Azure.Core, but this manifest makes
dotnet restorefetch Azure.Core into the fresh per-test cache. That makes the unit test network-dependent and slower; the shared fake-package helper is explicitly designed to avoid network access (test/common/FakeNuGetPackage.cs:15-18). Keep an empty dependency group or use that helper.
<dependencies>
<group targetFramework="net10.0">
<dependency id="Azure.Core" version="1.61.0" exclude="Build,Analyzers" />
</group>
packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/GeneratedCodeWorkspaceTests.cs:706
- These generated assemblies do not reference Azure.Core, so declaring it forces every
dotnet restoretest case to download an unrelated package into its fresh cache. This defeats the hermetic fake-package pattern documented intest/common/FakeNuGetPackage.cs:15-18and makes CI/offline runs unnecessarily dependent on an external feed. Emit an empty dependency group or reuse the shared helper.
<dependencies>
<group targetFramework="net10.0">
<dependency id="Azure.Core" version="1.61.0" exclude="Build,Analyzers" />
</group>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.
Suppressed comments (8)
packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/GeneratedCodeWorkspaceTests.cs:432
- This test contains no
await, soasyncproduces CS1998. The generator tree treats warnings as errors (generator/Directory.Build.props:29), which prevents the test project from compiling. Make the test synchronous.
public async Task TestGetLatestFramework(bool includeGoodVersions, bool includeBadVersions)
packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/GeneratedCodeWorkspaceTests.cs:479
- This local is never used, producing CS0219 under the generator's warnings-as-errors build. Remove it.
var nugetCacheDir = Path.Combine(_tempDirectory!, "NuGetCache");
packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/GeneratedCodeWorkspaceTests.cs:563
- This local is never used, producing CS0219 under the generator's warnings-as-errors build. Remove it.
var nugetCacheDir = Path.Combine(_tempDirectory!, "NuGetCache");
packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/PostProcessing/GeneratedCodeWorkspace.cs:342
- Dividing compact .NET Framework TFMs by 100 is not order-preserving:
net48becomes0.48, whilenet472becomes4.72, so this selects .NET Framework 4.7.2 as newer than 4.8 and can read dependency versions from the wrong target. Parse folder TFMs withNuGetFramework.ParseFolderand compare their framework identity/version with the intended family precedence instead of treating the suffix as a decimal.
if (name.StartsWith("net4", StringComparison.InvariantCultureIgnoreCase))
{
current /= 100;
current += 2000.0;
packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/GeneratedCodeWorkspaceTests.cs:162
- NuGet's global-packages layout uses a lower-cased nuspec filename. Writing
My.External.Library.nuspecin the lower-cased package directory makesNuGetv3LocalRepository.FindPackagemiss this fake package on case-sensitive systems, so restore/exact-version lookup can fail in Linux CI.
File.WriteAllText(Path.Combine(metadataPath, $"{externalPkgName}.nuspec"), $"""
packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/GeneratedCodeWorkspaceTests.cs:688
- NuGet's global-packages layout uses a lower-cased nuspec filename. Keeping the original package casing here makes
NuGetv3LocalRepository.FindPackagemiss these fake packages on case-sensitive systems, defeating the exact-version assertions in Linux CI.
File.WriteAllText(Path.Combine(metadataPath, $"{packageName}.nuspec"), $"""
packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/GeneratedCodeWorkspaceTests.cs:180
- This fake package now declares Azure.Core even though the test does not use it. Because setup points
NUGET_PACKAGESat a fresh temporary cache and the method runsdotnet restore, the test must fetch Azure.Core from a remote feed, making it non-hermetic and prone to offline/feed failures. Remove this dependency block (the sharedtest/common/FakeNuGetPackage.cshelper also creates dependency-free packages by default).
<dependencies>
<group targetFramework="net10.0">
<dependency id="Azure.Core" version="1.61.0" exclude="Build,Analyzers" />
</group>
packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/GeneratedCodeWorkspaceTests.cs:706
- Every package created by this helper unnecessarily depends on Azure.Core. Since these tests restore against a fresh temporary global-packages folder, this forces network access and makes the tests dependent on feed availability although the emitted assemblies do not reference Azure.Core. Omit the dependency block or delegate to
FakeNuGetPackage.Create, which defaults to no dependencies.
<dependencies>
<group targetFramework="net10.0">
<dependency id="Azure.Core" version="1.61.0" exclude="Build,Analyzers" />
</group>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
Suppressed comments (5)
Previously missed (1) — in code that hasn't changed since the last review.
packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/GeneratedCodeWorkspaceTests.cs:345
- These cases intentionally omit
First.Package, but the method now runsdotnet restoreagainst the machine's configured feeds. The test can therefore block/fail with feed availability or unexpectedly download a same-named package, changing the asserted result. Configure the temporary project with an isolated local-only NuGet source (and provide all required fake packages) so this remains deterministic.
[TestCase(true, true)]
[TestCase(false, true)]
[TestCase(true, false)]
[TestCase(false, false)]
packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/GeneratedCodeWorkspaceTests.cs:479
- This local is never used, producing CS0219. Because the generator test project inherits
TreatWarningsAsErrors=true, this prevents the tests from compiling; remove it.
var nugetCacheDir = Path.Combine(_tempDirectory!, "NuGetCache");
packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/GeneratedCodeWorkspaceTests.cs:688
- NuGet's global-packages layout requires the nuspec filename to be lowercase; the shared
FakeNuGetPackagehelper does this attest/common/FakeNuGetPackage.cs:111. With package IDs such asFirst.Package, this mixed-case filename makes exact-version lookup/restore fail on case-sensitive agents.
File.WriteAllText(Path.Combine(metadataPath, $"{packageName}.nuspec"), $"""
packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/GeneratedCodeWorkspaceTests.cs:180
- The fake assembly does not use Azure.Core, but declaring this dependency forces the new restore step to contact external feeds (or fail offline). Keep the fake package dependency-free so this unit test is hermetic.
<dependencies>
<group targetFramework="net10.0">
<dependency id="Azure.Core" version="1.61.0" exclude="Build,Analyzers" />
</group>
packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/GeneratedCodeWorkspaceTests.cs:706
- Every fake package created by this helper unnecessarily depends on Azure.Core, so the new restore step requires an external feed even though the emitted assembly has no such reference. Remove the dependency to keep these tests offline and deterministic.
<dependencies>
<group targetFramework="net10.0">
<dependency id="Azure.Core" version="1.61.0" exclude="Build,Analyzers" />
</group>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.
Suppressed comments (6)
Previously missed (2) — in code that hasn't changed since the last review.
packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/PostProcessing/GeneratedCodeWorkspace.cs:461
- This reports the maximum version directory, not the version whose assembly
FindPackageAssemblyactually selected. If the highest cached version lacks a usable DLL, resolution falls back to an older version while the debug message claims the higher one;maxVersion.Versionalso drops prerelease labels. Derive the version fromresolvedAssemblyPathinstead.
NuGetVersion? maxVersion = allDirs.Select(dir => NuGetVersion.TryParse(Path.GetFileName(dir), out var v) ? v : null)
.Where(t => t != null)
.Max();
packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/PostProcessing/GeneratedCodeWorkspace.cs:449
- The PR's central fallback is still untested: every case creates the exact requested version (
Second.Package3.5.0), so these assertions never exercise restore selecting a newer version when the minimum is unavailable or the prerelease-inclusive rule. Add hermetic fake-package cases where only a newer stable/prerelease version exists and assert that exact restored version is referenced.
string? resolvedAssemblyPath = version is null
? NugetPackageResolver.FindPackageAssembly(globalPackagesFolder, refPackageName)
: NugetPackageResolver.FindPackageAssemblyInVersion(globalPackagesFolder, refPackageName, version);
packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/GeneratedCodeWorkspaceTests.cs:180
- This fake package now declares an Azure.Core dependency, but the test replaces
NUGET_PACKAGESwith an empty temporary directory before invoking the newly addeddotnet restore. Restore must therefore contact an external feed (and fails offline) even though this test does not exercise Azure.Core. Keep the fake package dependency-free, astest/common/FakeNuGetPackage.cs:14-18does for hermetic package layouts.
<dependencies>
<group targetFramework="net10.0">
<dependency id="Azure.Core" version="1.61.0" exclude="Build,Analyzers" />
</group>
packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/GeneratedCodeWorkspaceTests.cs:479
- This local is unused and produces CS0219; generator projects treat warnings as errors (
generator/Directory.Build.props:29), so the test project will not build. Remove it.
var nugetCacheDir = Path.Combine(_tempDirectory!, "NuGetCache");
packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/GeneratedCodeWorkspaceTests.cs:705
- The shared fake-package helper is explicitly hermetic, but this dependency makes every call to
CreateFakeNuGetPackagerequire downloading Azure.Core into the test's empty temporary cache. These tests do not use Azure.Core, so remove the dependency block to keep them reliable offline (test/common/FakeNuGetPackage.cs:14-18).
<dependencies>
<group targetFramework="net10.0">
<dependency id="Azure.Core" version="1.61.0" exclude="Build,Analyzers" />
</group>
packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/GeneratedCodeWorkspaceTests.cs:687
- NuGet's global-packages layout requires the manifest filename to use the lowercased package ID. This mixed-case filename makes
NuGetv3LocalRepositorymiss these fake packages on case-sensitive agents, so restore can fail or query remote feeds. Match the shared helper attest/common/FakeNuGetPackage.cs:111.
File.WriteAllText(Path.Combine(metadataPath, $"{packageName}.nuspec"), $"""
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.
Suppressed comments (4)
packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/GeneratedCodeWorkspaceTests.cs:479
- This local is unused, so it produces CS0219; because the generator test tree treats warnings as errors, the test project will not build. Remove the declaration.
var nugetCacheDir = Path.Combine(_tempDirectory!, "NuGetCache");
packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/GeneratedCodeWorkspaceTests.cs:687
- NuGet's global-packages layout expects the manifest filename to use the lowercased package ID. With mixed-case IDs, this writes (for example)
First.Package.nuspec, soNuGetv3LocalRepository.FindPackageused by the exact-version lookup cannot recognize the fake package on case-sensitive systems. Lowercase the filename, asFakeNuGetPackage.WriteNuspecdoes.
File.WriteAllText(Path.Combine(metadataPath, $"{packageName}.nuspec"), $"""
packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/GeneratedCodeWorkspaceTests.cs:180
- This fake package declares an
Azure.Coredependency that is not created in the isolatedNUGET_PACKAGESdirectory. The newly addeddotnet restoretherefore reaches an external feed even for the successful-cache test, making it fail or emit a restore error offline. The package does not use Azure.Core, so omit the dependency block to keep the test hermetic.
<dependencies>
<group targetFramework="net10.0">
<dependency id="Azure.Core" version="1.61.0" exclude="Build,Analyzers" />
</group>
packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/GeneratedCodeWorkspaceTests.cs:705
- Every package produced by this helper declares
Azure.Core, but the helper does not install it in the isolated cache. As a result, all otherwise-local package-reference cases require a network restore and can fail or report restore errors for reasons unrelated to the behavior under test. Remove this unused dependency block (or create the dependency locally).
<dependencies>
<group targetFramework="net10.0">
<dependency id="Azure.Core" version="1.61.0" exclude="Build,Analyzers" />
</group>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.
Suppressed comments (5)
packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/GeneratedCodeWorkspaceTests.cs:479
- This local is never used, so it produces CS0219; warnings are errors for the generator test projects (
generator/Directory.Build.props:29). Remove it so the test assembly builds.
var nugetCacheDir = Path.Combine(_tempDirectory!, "NuGetCache");
packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/GeneratedCodeWorkspaceTests.cs:687
- NuGet's global-packages layout expects the manifest filename to use the normalized lowercase package ID, as
test/common/FakeNuGetPackage.cs:111does. On a case-sensitive agent this manifest is not found byFindPackageAssemblyInVersion, so these new exact-version assertions fail to resolve the fake packages.
File.WriteAllText(Path.Combine(metadataPath, $"{packageName}.nuspec"), $"""
packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/GeneratedCodeWorkspaceTests.cs:704
- Every package created by this helper declares Azure.Core even though its generated assembly does not use it. Since each test uses a fresh empty NuGet cache, the production
dotnet restorenow contacts an external feed for this unrelated dependency, making all four test cases network-dependent. Emit an empty dependency list instead, matchingFakeNuGetPackage.Create's default behavior.
<dependency id="Azure.Core" version="1.61.0" exclude="Build,Analyzers" />
packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/GeneratedCodeWorkspaceTests.cs:179
- The fake assembly does not reference Azure.Core, but this manifest makes the newly added
dotnet restoreresolve Azure.Core from an external feed because each test starts with an empty temporary package cache. That makes this unit test unnecessarily network-dependent; omit the unused dependency declaration.
<dependency id="Azure.Core" version="1.61.0" exclude="Build,Analyzers" />
packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Utilities/ExternalTypeReferenceResolver.cs:239
- After removing the feed lookup, this failure path still reports that configured feeds were searched. The resolver now checks only the global cache, so the message should not tell users that remote sources were consulted.
if (assemblyPath == null || !File.Exists(assemblyPath))
{
var versionQualifier = string.IsNullOrEmpty(external.MinVersion)
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: