-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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.
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
Conversation
Co-authored-by: dsplaisted <[email protected]>
@copilot What about the tests for the consumption of the tools manifest file? Do they use the .config subfolder or not? Probably most of them should be changed to not use the .config folder, but we should still have some coverage of the .config folder to ensure backwards compatibility. |
…d compatibility Co-authored-by: dsplaisted <[email protected]>
I've analyzed the test coverage and made the necessary updates. Most consumption tests already use the direct path format (not .config), which is good. The main issues were:
Current test coverage:
The distribution is now appropriate - most tests use the new direct format while maintaining coverage for .config backward compatibility. Commit: 5c42f9d |
@@ -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() |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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:
- ✅ The manifest under the .config folder has been updated (checks file exists, contains the tool, and content differs from original)
- ✅ No manifest exists in the root folder after the install command runs
All ToolInstallLocalCommandTests continue to pass, ensuring backward compatibility is maintained.
Commit: 9099124
…ecks Co-authored-by: dsplaisted <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR changes the default behavior for creating new tool manifests to avoid automatically creating .config
subdirectories. Previously, dotnet tool install --create-manifest-if-needed
would create manifests at .config/dotnet-tools.json
, but now they are created directly as dotnet-tools.json
in the target directory.
- Simplified manifest creation by removing automatic
.config
directory creation - Updated tests to reflect new creation behavior while maintaining backward compatibility
- Preserved discovery logic to support existing
.config/dotnet-tools.json
files
Reviewed Changes
Copilot reviewed 4 out of 6 changed files in this pull request and generated no comments.
File | Description |
---|---|
src/Cli/dotnet/ToolManifest/ToolManifestFinder.cs |
Updated WriteManifestFile() to create manifests directly in target folder |
test/dotnet.Tests/ToolManifestTests/ToolManifestFinderTests.cs |
Added test to verify new manifests are created without .config directory |
test/dotnet.Tests/CommandTests/Tool/Install/ToolInstallLocalCommandTests.cs |
Updated test expectations and added backward compatibility test |
test/dotnet-new.IntegrationTests/TemplateDiscoveryTool.cs |
Updated test to expect manifest in root directory |
You can also share your feedback on Copilot code review for a chance to win a $100 gift card. Take the survey.
test/dotnet.Tests/CommandTests/Tool/Install/ToolInstallLocalCommandTests.cs
Outdated
Show resolved
Hide resolved
Co-authored-by: Jacques Eloff <[email protected]>
This change removes the automatic creation of
.config
subdirectories when creating new tool manifests. Previously, when usingdotnet tool install --create-manifest-if-needed
or similar commands, the CLI would create manifests at.config/dotnet-tools.json
. Now, manifests are created directly asdotnet-tools.json
in the target directory.Changes Made
WriteManifestFile()
method to create manifests directly in the target folder instead of in a.config
subdirectory.config/dotnet-tools.json
todotnet-tools.json
Backward Compatibility
The change maintains full backward compatibility for reading existing manifests:
.config/dotnet-tools.json
files continue to be discovered and used.config/dotnet-tools.json
anddotnet-tools.json
locationsThis simplifies the local tools experience by reducing directory clutter while preserving support for existing projects.
Fixes #49296.
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.