Skip to content

Don't create tool manifests in a .config subfolder by default #50242

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Aug 14, 2025
Merged

Don't create tool manifests in a .config subfolder by default #50242

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
eab62eb
Initial plan
Copilot Aug 13, 2025
610c1b8
Remove .config subfolder creation for tool manifests by default
Copilot Aug 13, 2025
5c42f9d
Update test coverage for tool manifest consumption - maintain backwar…
Copilot Aug 13, 2025
9099124
Enhanced test to verify .config manifest update and root directory ch…
Copilot Aug 13, 2025
8ad435c
Code review feedback
dsplaisted Aug 14, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions src/Cli/dotnet/ToolManifest/ToolManifestFinder.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -250,8 +250,7 @@ private string WriteManifestFile(DirectoryPath folderPath)
"tools": {}
}
""";
_fileSystem.Directory.CreateDirectory(Path.Combine(folderPath.Value, Constants.DotConfigDirectoryName));
string manifestFileLocation = Path.Combine(folderPath.Value, Constants.DotConfigDirectoryName, Constants.ToolManifestFileName);
string manifestFileLocation = Path.Combine(folderPath.Value, Constants.ToolManifestFileName);
_fileSystem.File.WriteAllText(manifestFileLocation, manifestFileContent);

return manifestFileLocation;
Expand Down
0 ...nt/ToolManifest/.config/dotnet-tools.json → ...es/content/ToolManifest/dotnet-tools.json
View file
Open in desktop
File renamed without changes.
0 ...l-manifest-file/.config/dotnet-tools.json → ...ocal-tool-manifest-file/dotnet-tools.json
View file
Open in desktop
File renamed without changes.
2 changes: 1 addition & 1 deletion test/dotnet-new.IntegrationTests/TemplateDiscoveryTool.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ public TemplateDiscoveryTool(IMessageSink messageSink)
testOutputHelper = new SharedTestOutputHelper(messageSink);
string home = Utilities.CreateTemporaryFolder("home");
dotnetNewTestExecutionDir = Utilities.GetTestExecutionTempFolder();
string toolManifestPath = Path.Combine(dotnetNewTestExecutionDir, @".config\dotnet-tools.json");
string toolManifestPath = Path.Combine(dotnetNewTestExecutionDir, "dotnet-tools.json");
if (!File.Exists(toolManifestPath))
{
new DotnetNewCommand(
Expand Down
28 changes: 24 additions & 4 deletions test/dotnet.Tests/CommandTests/Tool/Install/ToolInstallLocalCommandTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -315,6 +315,26 @@ out ToolCommand restoredCommand
"But restore do not need to 'revert' since it just set in nuget global directory");
}

[Fact]
public void WhenRunWithExistingManifestInConfigDirectoryItShouldAddToExistingManifest()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot This test looks good, but can you add additional checks to it to verify that the manifest under the .config folder has been updated, and that no manifest exists in the root folder after the install command is run?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added the requested checks to the test. The enhanced test now verifies:

  1. ✅ The manifest under the .config folder has been updated (checks file exists, contains the tool, and content differs from original)
  2. ✅ No manifest exists in the root folder after the install command runs

All ToolInstallLocalCommandTests continue to pass, ensuring backward compatibility is maintained.

Commit: 9099124

{
// Test backward compatibility: ensure tools can be added to existing manifests in .config directories
_fileSystem.File.Delete(_manifestFilePath);
var configDirectory = Path.Combine(_temporaryDirectory, ".config");
_fileSystem.Directory.CreateDirectory(configDirectory);
var configManifestPath = Path.Combine(configDirectory, "dotnet-tools.json");
_fileSystem.File.WriteAllText(configManifestPath, _jsonContent);

var toolInstallLocalCommand = GetDefaultTestToolInstallLocalCommand();

toolInstallLocalCommand.Execute().Should().Be(0);

// Verify the tool was added to the existing .config manifest
var manifestPackages = _toolManifestFinder.Find();
manifestPackages.Should().HaveCount(1);
manifestPackages.First().PackageId.Should().Be(_packageIdA);
}

private ToolInstallLocalCommand GetDefaultTestToolInstallLocalCommand()
{
return new ToolInstallLocalCommand(
Expand Down Expand Up @@ -418,7 +438,7 @@ public void GivenNoManifestFileAndCreateManifestIfNeededFlagItShouldCreateManife
_reporter);

installLocalCommand.Execute().Should().Be(0);
_fileSystem.File.Exists(Path.Combine(_temporaryDirectory, ".config", "dotnet-tools.json")).Should().BeTrue();
_fileSystem.File.Exists(Path.Combine(_temporaryDirectory, "dotnet-tools.json")).Should().BeTrue();
}

[Fact]
Expand All @@ -440,7 +460,7 @@ public void GivenNoManifestFileItUsesCreateManifestIfNeededByDefault()
_reporter);

installLocalCommand.Execute().Should().Be(0);
_fileSystem.File.Exists(Path.Combine(_temporaryDirectory, ".config", "dotnet-tools.json")).Should().BeTrue();
_fileSystem.File.Exists(Path.Combine(_temporaryDirectory, "dotnet-tools.json")).Should().BeTrue();
}

[Fact]
Expand All @@ -465,7 +485,7 @@ public void GivenNoManifestFileAndCreateManifestIfNeededFlagItShouldCreateManife
_reporter);

installLocalCommand.Execute().Should().Be(0);
_fileSystem.File.Exists(Path.Combine(_temporaryDirectory, ".config", "dotnet-tools.json")).Should().BeTrue();
_fileSystem.File.Exists(Path.Combine(_temporaryDirectory, "dotnet-tools.json")).Should().BeTrue();
}

[Fact]
Expand All @@ -487,7 +507,7 @@ public void GivenNoManifestFileAndCreateManifestIfNeededFlagItShouldCreateManife
_reporter);

installLocalCommand.Execute().Should().Be(0);
_fileSystem.File.Exists(Path.Combine(_temporaryDirectory, ".config", "dotnet-tools.json")).Should().BeTrue();
_fileSystem.File.Exists(Path.Combine(_temporaryDirectory, "dotnet-tools.json")).Should().BeTrue();
}

private IToolPackageDownloader GetToolToolPackageInstallerWithPreviewInFeed()
Expand Down
16 changes: 16 additions & 0 deletions test/dotnet.Tests/ToolManifestTests/ToolManifestFinderTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -764,6 +764,22 @@ public void GivenNoManifestInspectShouldNotThrow()
a.Should().NotThrow();
}

[Fact]
public void GivenNoManifestFileWhenCreatingNewManifestItShouldCreateInDirectFolder()
{
var toolManifest =
new ToolManifestFinder(
new DirectoryPath(_testDirectoryRoot),
_fileSystem,
new FakeDangerousFileDetector());

FilePath createdManifest = toolManifest.FindFirst(createIfNotFound: true);

createdManifest.Value.Should().Be(Path.Combine(_testDirectoryRoot, "dotnet-tools.json"));
_fileSystem.File.Exists(Path.Combine(_testDirectoryRoot, "dotnet-tools.json")).Should().BeTrue();
_fileSystem.Directory.Exists(Path.Combine(_testDirectoryRoot, ".config")).Should().BeFalse("New manifests should not create .config directories");
}

private string _jsonContent =
@"{
""version"":1,
Expand Down