diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md index 4759ce7542b..1f41806ee2c 100644 --- a/.github/copilot-instructions.md +++ b/.github/copilot-instructions.md @@ -17,6 +17,16 @@ - All files in the repository are nullable by default (project-level nullable enable). No need to add `#nullable enable` directives to individual files. +## Bug Fixing Guidance + +- Prefer a test-first workflow for bug fixes. Write a focused repro test that fails for the correct reason before changing the implementation. +- Make the failure details explicit in the assertion or expected message so the test proves the actual bug, not just that something failed. +- Treat TDD as the default for bug fixing: reproduce the issue, fix it, and keep the regression test in place. +- When a bug may flow through shared infrastructure or multiple entry points, add parity coverage across the affected surfaces instead of validating only one caller. +- For restore bugs involving settings, source resolution, or command-line properties, preserve parity across `nuget.exe`, `dotnet restore`, and `msbuild /t:restore`. +- When testing explicit restore source inputs (`-Source`, `--source`, `/p:RestoreSources`), cover both direct paths/URLs and named package sources defined in `NuGet.Config`. A source value may be a configured source name, not just a filesystem path. +- Prefer a layered test strategy for restore and pack changes: add fast unit coverage around the shared helper or factory logic first, then add at least one integration/functional regression for the relevant client entry points. + ## Nullable Migration Rules - **Shipped.txt format must be precise** — e.g. `string![]!` not `string![]`, `byte[]?` not `byte?[]`. Always match the format of existing base class entries in the same file. diff --git a/src/NuGet.Core/NuGet.Build.Tasks/BuildTasksUtility.cs b/src/NuGet.Core/NuGet.Build.Tasks/BuildTasksUtility.cs index 11b15e0967f..809a9424c85 100644 --- a/src/NuGet.Core/NuGet.Build.Tasks/BuildTasksUtility.cs +++ b/src/NuGet.Core/NuGet.Build.Tasks/BuildTasksUtility.cs @@ -652,12 +652,14 @@ private static IEnumerable ProcessFailedEventsIntoRestoreLogs public static string[] GetSources(string startupDirectory, string projectDirectory, string[] sources, string[] sourcesOverride, IEnumerable additionalProjectSources, ISettings settings) { + var configuredSources = SettingsUtility.GetEnabledSources(settings).ToList(); + // Sources var currentSources = RestoreSettingsUtils.GetValue( - () => sourcesOverride?.Select(MSBuildRestoreUtility.FixSourcePath).Select(e => UriUtility.GetAbsolutePath(startupDirectory, e)).ToArray(), + () => sourcesOverride?.Select(MSBuildRestoreUtility.FixSourcePath).Select(e => ResolveSourceValue(startupDirectory, e, configuredSources)).ToArray(), () => MSBuildRestoreUtility.ContainsClearKeyword(sources) ? Array.Empty() : null, - () => sources?.Select(MSBuildRestoreUtility.FixSourcePath).Select(e => UriUtility.GetAbsolutePath(projectDirectory, e)).ToArray(), - () => (PackageSourceProvider.LoadPackageSources(settings)).Where(e => e.IsEnabled).Select(e => e.Source).ToArray()); + () => sources?.Select(MSBuildRestoreUtility.FixSourcePath).Select(e => ResolveSourceValue(projectDirectory, e, configuredSources)).ToArray(), + () => configuredSources.Select(e => e.Source).ToArray()); // Append additional sources // Escape strings to avoid xplat path issues with msbuild. @@ -669,6 +671,20 @@ public static string[] GetSources(string startupDirectory, string projectDirecto return AppendItems(projectDirectory, currentSources, filteredAdditionalProjectSources); } + private static string ResolveSourceValue(string rootDirectory, string source, IReadOnlyList configuredSources) + { + PackageSource configuredSource = configuredSources.FirstOrDefault( + e => string.Equals(e.Name, source, StringComparison.OrdinalIgnoreCase) + || string.Equals(e.Source, source, StringComparison.OrdinalIgnoreCase)); + + if (configuredSource != null) + { + return configuredSource.Source; + } + + return UriUtility.GetAbsolutePath(rootDirectory, source); + } + /// /// Gets the package fallback folders for a project. /// diff --git a/src/NuGet.Core/NuGet.Commands/RestoreCommand/Utility/PackageSpecFactory.cs b/src/NuGet.Core/NuGet.Commands/RestoreCommand/Utility/PackageSpecFactory.cs index 1d690ed9de2..63aa5ac79b4 100644 --- a/src/NuGet.Core/NuGet.Commands/RestoreCommand/Utility/PackageSpecFactory.cs +++ b/src/NuGet.Core/NuGet.Commands/RestoreCommand/Utility/PackageSpecFactory.cs @@ -831,12 +831,14 @@ internal static List GetSources(IProject project, ISettings setti private static string[] GetSources(string startupDirectory, string projectDirectory, string[]? sources, string[]? sourcesOverride, IEnumerable additionalProjectSources, ISettings settings) { + List configuredSources = SettingsUtility.GetEnabledSources(settings).ToList(); + // Sources var currentSources = GetValue( - () => sourcesOverride?.Select(MSBuildRestoreUtility.FixSourcePath).Select(e => UriUtility.GetAbsolutePath(startupDirectory, e)).ToArray(), + () => sourcesOverride?.Select(MSBuildRestoreUtility.FixSourcePath).Select(e => ResolveSourceValue(startupDirectory, e, configuredSources)).ToArray(), () => MSBuildRestoreUtility.ContainsClearKeyword(sources) ? Array.Empty() : null, - () => sources?.Select(MSBuildRestoreUtility.FixSourcePath).Select(e => UriUtility.GetAbsolutePath(projectDirectory, e)).ToArray(), - () => (PackageSourceProvider.LoadPackageSources(settings)).Where(e => e.IsEnabled).Select(e => e.Source).ToArray()); + () => sources?.Select(MSBuildRestoreUtility.FixSourcePath).Select(e => ResolveSourceValue(projectDirectory, e, configuredSources)).ToArray(), + () => configuredSources.Select(e => e.Source).ToArray()); // Append additional sources // Escape strings to avoid xplat path issues with msbuild. @@ -849,6 +851,20 @@ private static string[] GetSources(string startupDirectory, string projectDirect return AppendItems(projectDirectory, currentSources!, filteredAdditionalProjectSources); } + private static string ResolveSourceValue(string rootDirectory, string source, IReadOnlyList configuredSources) + { + PackageSource? configuredSource = configuredSources.FirstOrDefault( + e => string.Equals(e.Name, source, StringComparison.OrdinalIgnoreCase) + || string.Equals(e.Source, source, StringComparison.OrdinalIgnoreCase)); + + if (configuredSource != null) + { + return configuredSource.Source; + } + + return UriUtility.GetAbsolutePath(rootDirectory, source)!; + } + private static string[] AppendItems(string projectDirectory, string[] current, IEnumerable? additional) { if (additional == null || !additional.Any()) diff --git a/test/NuGet.Clients.Tests/NuGet.CommandLine.Test/RestoreNETCoreTest.cs b/test/NuGet.Clients.Tests/NuGet.CommandLine.Test/RestoreNETCoreTest.cs index 95342d932c5..6d055c153ad 100644 --- a/test/NuGet.Clients.Tests/NuGet.CommandLine.Test/RestoreNETCoreTest.cs +++ b/test/NuGet.Clients.Tests/NuGet.CommandLine.Test/RestoreNETCoreTest.cs @@ -440,6 +440,47 @@ public async Task RestoreNetCore_VerifyProjectConfigCanOverrideSolutionConfigAsy } } + [Fact] + public async Task RestoreNetCore_WithNuGetExe_WhenSourceArgUsesConfiguredSourceName_RestoresFromConfigSourceAsync() + { + using (var pathContext = new SimpleTestPathContext()) + { + var solution = new SimpleTestSolutionContext(pathContext.SolutionRoot); + var project = SimpleTestProjectContext.CreateNETCore( + "projectA", + pathContext.SolutionRoot, + NuGetFramework.Parse("net45")); + + var package = new SimpleTestPackageContext() + { + Id = "packageA", + Version = "1.0.0" + }; + + project.AddPackageToAllFrameworks(package); + project.Properties.Clear(); + solution.Projects.Add(project); + solution.Create(); + + await SimpleTestPackageUtility.CreateFolderFeedV3Async( + pathContext.PackageSource, + PackageSaveMode.Defaultv3, + package); + + pathContext.Settings.RemoveSource("source"); + pathContext.Settings.AddSource("source_name", pathContext.PackageSource); + + var result = Util.Restore( + pathContext, + project.ProjectPath, + expectedExitCode: 0, + additionalArgs: $"-Source source_name -ConfigFile \"{pathContext.Settings.ConfigPath}\""); + + result.Success.Should().BeTrue(because: result.AllOutput); + project.AssetsFile.Libraries.Select(e => e.Name).Should().Contain(package.Id); + } + } + [Fact] public async Task RestoreNetCore_VerifyProjectConfigChangeTriggersARestoreAsync() { diff --git a/test/NuGet.Core.FuncTests/Dotnet.Integration.Test/DotnetRestoreTests.cs b/test/NuGet.Core.FuncTests/Dotnet.Integration.Test/DotnetRestoreTests.cs index cff890ce240..100a78aff51 100644 --- a/test/NuGet.Core.FuncTests/Dotnet.Integration.Test/DotnetRestoreTests.cs +++ b/test/NuGet.Core.FuncTests/Dotnet.Integration.Test/DotnetRestoreTests.cs @@ -100,6 +100,58 @@ public void DotnetRestore_SolutionRestoreVerifySolutionDirPassedToProjects() } } + [Fact] + public async Task DotnetRestore_WhenSourceUsesConfiguredSourceName_RestoresFromConfigSource() + { + using (SimpleTestPathContext pathContext = _dotnetFixture.CreateSimpleTestPathContext()) + { + var package = new SimpleTestPackageContext() + { + Id = "TestPackage", + Version = "1.0.0" + }; + + await SimpleTestPackageUtility.CreateFolderFeedV3Async( + pathContext.PackageSource, + PackageSaveMode.Defaultv3, + package); + + pathContext.Settings.RemoveSource("source"); + pathContext.Settings.AddSource("source_name", pathContext.PackageSource); + + var projectName = "ClassLibrary1"; + var workingDirectory = Path.Combine(pathContext.SolutionRoot, projectName); + var projectFile = Path.Combine(workingDirectory, $"{projectName}.csproj"); + + _dotnetFixture.CreateDotnetNewProject(pathContext.SolutionRoot, projectName, "classlib", testOutputHelper: _testOutputHelper); + + using (var stream = File.Open(projectFile, FileMode.Open, FileAccess.ReadWrite)) + { + var xml = XDocument.Load(stream); + + var attributes = new Dictionary() { { "Version", package.Version } }; + + ProjectFileUtils.AddItem( + xml, + "PackageReference", + package.Id, + string.Empty, + new Dictionary(), + attributes); + + ProjectFileUtils.WriteXmlToFile(xml, stream); + } + + var result = _dotnetFixture.RunDotnetExpectSuccess( + workingDirectory, + $"restore {projectName}.csproj --source source_name --configfile \"{pathContext.Settings.ConfigPath}\"", + testOutputHelper: _testOutputHelper); + + result.ExitCode.Should().Be(0, because: result.AllOutput); + File.Exists(Path.Combine(workingDirectory, "obj", "project.assets.json")).Should().BeTrue(because: result.AllOutput); + } + } + [Fact] public void DotnetRestore_WithAuthorSignedPackage_Succeeds() { diff --git a/test/NuGet.Core.FuncTests/Msbuild.Integration.Test/MsbuildRestoreTaskTests.cs b/test/NuGet.Core.FuncTests/Msbuild.Integration.Test/MsbuildRestoreTaskTests.cs index 58cf9721beb..e9cb2604485 100644 --- a/test/NuGet.Core.FuncTests/Msbuild.Integration.Test/MsbuildRestoreTaskTests.cs +++ b/test/NuGet.Core.FuncTests/Msbuild.Integration.Test/MsbuildRestoreTaskTests.cs @@ -740,6 +740,74 @@ await SimpleTestPackageUtility.CreateFolderFeedV3Async( } } + [PlatformTheory(Platform.Windows)] + [InlineData(true, true)] + [InlineData(true, false)] + [InlineData(false, false)] + public async Task MsbuildRestore_WithConfiguredSourceName_ResolvesFromConfig(bool isStaticGraphRestore, bool usePackageSpecFactory) + { + // Arrange + using (var pathContext = new SimpleTestPathContext()) + { + // Set up solution, project, and packages + var solution = new SimpleTestSolutionContext(pathContext.SolutionRoot); + + var net461 = NuGetFramework.Parse("net472"); + + var project = SimpleTestProjectContext.CreateLegacyPackageReference( + "a", + pathContext.SolutionRoot, + net461); + + var packageX = new SimpleTestPackageContext() + { + Id = "x", + Version = "1.0.0" + }; + + packageX.Files.Clear(); + project.AddPackageToAllFrameworks(packageX); + packageX.AddFile("lib/net472/a.dll"); + + solution.Projects.Add(project); + solution.Create(); + + await SimpleTestPackageUtility.CreateFolderFeedV3Async( + pathContext.PackageSource, + packageX); + + pathContext.Settings.RemoveSource("source"); + pathContext.Settings.AddSource("source_name", pathContext.PackageSource); + + var projectOutputPaths = new[] + { + project.AssetsFileOutputPath, + project.PropsOutput, + project.TargetsOutput, + project.CacheFileOutputPath, + }; + + var environmentVariables = new Dictionary(); + environmentVariables.AddRange(_msbuildFixture.DefaultProcessEnvironmentVariables); + environmentVariables["NUGET_USE_NEW_PACKAGESPEC_FACTORY"] = usePackageSpecFactory.ToString(); + + var result = _msbuildFixture.RunMsBuild( + pathContext.WorkingDirectory, + $"/t:restore {project.ProjectPath} /p:RestoreSources=\"source_name\" /p:RestoreConfigFile=\"{pathContext.Settings.ConfigPath}\"" + + (isStaticGraphRestore ? " /p:RestoreUseStaticGraphEvaluation=true" : string.Empty), + ignoreExitCode: true, + testOutputHelper: _testOutputHelper, + environmentVariables); + + result.Success.Should().BeTrue(because: result.AllOutput); + + foreach (var asset in projectOutputPaths) + { + new FileInfo(asset).Exists.Should().BeTrue(because: result.AllOutput); + } + } + } + [PlatformFact(Platform.Windows)] public Task MsbuildRestore_WithStaticGraphRestore_MessageLoggedAtDefaultVerbosityWhenThereAreNoProjectsToRestore() { diff --git a/test/NuGet.Core.Tests/NuGet.Build.Tasks.Console.Test/BuildTasksUtilityTests.cs b/test/NuGet.Core.Tests/NuGet.Build.Tasks.Console.Test/BuildTasksUtilityTests.cs index 17d2735ecf2..e507c42b3aa 100644 --- a/test/NuGet.Core.Tests/NuGet.Build.Tasks.Console.Test/BuildTasksUtilityTests.cs +++ b/test/NuGet.Core.Tests/NuGet.Build.Tasks.Console.Test/BuildTasksUtilityTests.cs @@ -135,6 +135,46 @@ public void GetSources_WithRestoreSourcesGlobal_Property_ResolvesAgainstWorkingD } } + [Fact] + public void GetSources_WithConfiguredSourceName_UsesConfiguredSourceValue() + { + using (var testDir = TestDirectory.CreateInTemp()) + { + // Arrange + var startupDirectory = Path.Combine(testDir, "startup"); + var projectDirectory = Path.Combine(testDir, "project"); + var configuredSource = "https://configured-source/v3/index.json"; + Directory.CreateDirectory(projectDirectory); + File.WriteAllText( + Path.Combine(projectDirectory, Settings.DefaultSettingsFileName), + $@" + + + + + +"); + + var settings = Settings.LoadDefaultSettings( + root: projectDirectory, + configFileName: null, + machineWideSettings: null); + + // Act + var effectiveSources = BuildTasksUtility.GetSources( + startupDirectory: startupDirectory, + projectDirectory: projectDirectory, + sources: new[] { "source_name" }, + sourcesOverride: new[] { "source_name" }, + additionalProjectSources: Array.Empty(), + settings: settings + ); + + // Assert + effectiveSources.Should().BeEquivalentTo(new[] { configuredSource }); + } + } + [Theory] [InlineData("0", "false", 0)] [InlineData("0", "true", 0)] diff --git a/test/NuGet.Core.Tests/NuGet.Build.Tasks.Console.Test/MSBuildStaticGraphRestoreTests.cs b/test/NuGet.Core.Tests/NuGet.Build.Tasks.Console.Test/MSBuildStaticGraphRestoreTests.cs index 2a981d5f570..1850c55a31f 100644 --- a/test/NuGet.Core.Tests/NuGet.Build.Tasks.Console.Test/MSBuildStaticGraphRestoreTests.cs +++ b/test/NuGet.Core.Tests/NuGet.Build.Tasks.Console.Test/MSBuildStaticGraphRestoreTests.cs @@ -696,6 +696,46 @@ public void GetSources_WhenRestoreSourcesAndRestoreSourcesOverrideSpecified_Corr }); } + [Fact] + public void GetSources_WhenRestoreSourcesMatchesConfiguredSourceName_UsesConfiguredSourceValue() + { + using (var testDir = TestDirectory.CreateInTemp()) + { + var configuredSource = "https://configured-source/v3/index.json"; + var project = new MockMSBuildProject( + properties: new Dictionary + { + ["RestoreSources"] = "source_name", + }, + globalProperties: new Dictionary + { + ["RestoreSources"] = "source_name" + }); + + File.WriteAllText( + Path.Combine(testDir, Settings.DefaultSettingsFileName), + $@" + + + + + +"); + + var settings = Settings.LoadDefaultSettings( + root: testDir, + configFileName: null, + machineWideSettings: null); + + var actual = MSBuildStaticGraphRestore.GetSources(project, new[] { project }, settings); + + actual.Should().BeEquivalentTo(new[] + { + new PackageSource(configuredSource), + }); + } + } + [Fact] public void GetSources_WhenRestoreSourcesSpecified_CorrectSourcesDetected() { diff --git a/test/NuGet.Core.Tests/NuGet.Build.Tasks.Test/GetRestoreSettingTaskTests.cs b/test/NuGet.Core.Tests/NuGet.Build.Tasks.Test/GetRestoreSettingTaskTests.cs index a2e23081089..45abe59785a 100644 --- a/test/NuGet.Core.Tests/NuGet.Build.Tasks.Test/GetRestoreSettingTaskTests.cs +++ b/test/NuGet.Core.Tests/NuGet.Build.Tasks.Test/GetRestoreSettingTaskTests.cs @@ -677,6 +677,44 @@ public void GetRestoreSettingsTask_WithRestoreSourcesOverride_ResolvesAgainstWor } } + [Fact] + public void GetRestoreSettingsTask_WithRestoreSourcesOverrideAndConfiguredSourceName_UsesConfiguredSourceValue() + { + using (var testDir = TestDirectory.CreateInTemp()) + { + // Arrange + var buildEngine = new TestBuildEngine(); + var startupDirectory = Path.Combine(testDir, "innerPath"); + var configuredSource = "https://configured-source/v3/index.json"; + + File.WriteAllText( + Path.Combine(testDir, Settings.DefaultSettingsFileName), + $@" + + + + + +"); + + var task = new GetRestoreSettingsTask() + { + BuildEngine = buildEngine, + MSBuildStartupDirectory = startupDirectory, + ProjectUniqueName = Path.Combine(testDir, "a.csproj"), + RestoreSourcesOverride = new[] { "source_name" }, + RestoreSettingsPerFramework = Array.Empty() + }; + + // Act + var result = task.Execute(); + + // Assert + result.Should().BeTrue(); + task.OutputSources.Should().BeEquivalentTo(new[] { configuredSource }); + } + } + [Fact] public void GetRestoreSettingsTask_WithFallbackFoldersOverride_ResolvesAgainstWorkingDirectory() {