Skip to content
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

Add spec of dotnet nuget config command #12172

Merged
merged 30 commits into from
Jan 4, 2023
Merged
Changes from 6 commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
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
79 changes: 79 additions & 0 deletions proposed/2022/dotnet-nuget-config-spec.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,79 @@
# NuGet configuration CLI for dotnet.exe

- Author: [Heng Liu](https://github.com/heng-liu)
- GitHub Issue [8420](https://github.com/NuGet/Home/issues/8420)

## Problem Background

Currently, there is no NuGet configuration CLI for dotnet.exe. It's inconvenient for NuGet users to know about the NuGet configuration file locations and figure out where is the merged configuration coming from.

## Who are the customers

This feature is for dotnet.exe users.

## Goals
Design and implement `dotnet nuget config` command.

## Non-Goals
Design and implement `dotnet nuget config` command with commands other than `list`, E.g. add/update/delete

## Solution
The following command will be implemented in the `dotnet.exe` CLI.
Comment on lines +20 to +21
Copy link
Member

Choose a reason for hiding this comment

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

I really wish GitHub had threaded/resolvable conversations for comments without a specific file/line number 😔

I'd like to know what the design is for error scenarios, specifically when one or more of the XML files contains syntax errors (from an implementation point of view, this means that Settings.Load* apis will fail, so we can't use loadedConfig.GetConfigFilePaths()).

Is dotnet nuget config list going to report an error and fail to list all the files? Will it at least tell us the filename of the XML that couldn't be parsed? Or will all the XML files be listed despite the parsing failure? Will it tell us which file(s) contain parsing errors?

Copy link
Contributor Author

@heng-liu heng-liu Oct 26, 2022

Choose a reason for hiding this comment

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

That's a good question!
I tested with NuGet.exe and dotnet.exe, and both of them are able to tell us the filename of the XML that couldn't be parsed, as below:

nuget list source
NuGet.Config is not valid XML. Path: 'C:\Users\username\Source\Repos\NuGet.Config'.
  The 'disabledPackageSources' start tag on line 19 position 4 does not match the end tag of 'configuration'. Line 20, position 3.
dotnet nuget list source
error: NuGet.Config is not valid XML. Path: 'C:\Users\username\Source\Repos\NuGet.Config'.
error:   The 'disabledPackageSources' start tag on line 19 position 4 does not match the end tag of 'configuration'. Line 20, position 3.

So I think the dotnet nuget config list is able to do this as well.

If we'd like to list all config files despite the parsing failure, we'd consider if it worth doing.
Since running all nuget commands will encounter this invalid XML error, users need to fix this invalid XML before running any NuGet commands. So this seems to be a prerequisite and there is no harm if user needs to do this first before knowing the list of config files.
In addition, this error message clearly points out the file path and location, so it should be easy for users to correct the invalid XML.

The only scenario needs to be considered is, with the invalid XML error, user should able to specifying a specific correct config file to run nuget commands. And if we list all config file locations despite the parsing failure, it might help people to find the path easier. But it seems "listing all config file locations despite the parsing failure" adds very limited value.

Those are my preliminary thoughts. Please let me know if you have any other thoughts :)

The other error scenario I'd like to talk about is, the specified working directory doesn't exist. Shall we go ahead listing user-wide and machine-wide config files? Or show a warning saying the working directory doesn't exist? Or both?


### `dotnet nuget config`

#### Commands

- List

Lists all the NuGet configuration file locations. This command will include all the NuGet configuration file that will be applied, when invoking NuGet command from the current working directory path. The listed NuGet configuration files are in priority order. So the order of loading those configurations is reversed, that is, loading order is from the bottom to the top. So the configuration on the top will apply.
You may refer to [How settings are applied](https://learn.microsoft.com/en-us/nuget/consume-packages/configuring-nuget-behavior#how-settings-are-applied) for more details.

#### Arguments

- CURRENT_DIRECTORY
Copy link
Contributor

Choose a reason for hiding this comment

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

Incase if we are referring to CURRENT_DIRECTORY as working directory, dotnet store command has -w|--working-dir <WORKING_DIRECTORY> option https://learn.microsoft.com/dotnet/core/tools/dotnet-store#optional-options. I think it is better to use the same option name for this command also.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for reviewing! Renamed it as WORKING_DIRECTORY, but still an argument based on previous discussions. Pls let me know if you have any different thoughts :)


Run this command as if current directory is set to the specified directory.

#### Options

- -?|-h|--help

Prints out a description of how to use the command.

#### Examples

- List all the NuGet configuration file that will be applied, when invoking NuGet command in the current directory.

```
dotnet nuget config list

C:\Test\Repos\Solution\NuGet.Config
C:\Test\Repos\NuGet.Config
C:\Test\NuGet.Config
C:\Users\username\AppData\Roaming\NuGet\NuGet.Config
C:\Program Files (x86)\NuGet\Config\Microsoft.VisualStudio.FallbackLocation.config
C:\Program Files (x86)\NuGet\Config\Microsoft.VisualStudio.Offline.config
```

- List all the NuGet configuration file that will be applied, when invoking NuGet command in the specific directory.

```
dotnet nuget config list C:\Test\Repos

C:\Test\Repos\NuGet.Config
C:\Test\NuGet.Config
C:\Users\username\AppData\Roaming\NuGet\NuGet.Config
C:\Program Files (x86)\NuGet\Config\Microsoft.VisualStudio.FallbackLocation.config
C:\Program Files (x86)\NuGet\Config\Microsoft.VisualStudio.Offline.config
```

## Future Work
The `dotnet nuget config list` is a community ask. We will consider adding more commands, like add/update/delete, in the future.

## Open Questions
1. Do we need to add this into NuGet.exe CLI?
heng-liu marked this conversation as resolved.
Show resolved Hide resolved

## Considerations
1. NuGet.exe [config command](https://learn.microsoft.com/en-us/nuget/reference/cli-reference/cli-ref-config) is implemented. But there is no `list` command. And the behavior is confusing (the `set` command will set the property which appears last when loading, so sometimes it's not updating the closest NuGet configuration file). Do we want to implement those subcommand(e.g.`set`) in the future in dotnet.exe differently?
heng-liu marked this conversation as resolved.
Show resolved Hide resolved