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.

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

Jump to bottom

Add spec of dotnet nuget config command #12172

Merged
merged 30 commits into from
Jan 4, 2023
Merged

Add spec of dotnet nuget config command #12172

Changes from 8 commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
e8fdc34
work
heng-liu Oct 19, 2022
006cb0f
fix
heng-liu Oct 19, 2022
335e6ae
address feedback
heng-liu Oct 19, 2022
204ba1a
fix
heng-liu Oct 19, 2022
5dff9f6
address feedback
heng-liu Oct 21, 2022
abc436d
address feedback
heng-liu Oct 24, 2022
ffd5e71
address comments
heng-liu Oct 26, 2022
a25ea5d
address feedback
heng-liu Oct 27, 2022
9da2a9f
address comments & add more examples
heng-liu Oct 28, 2022
a1f68cf
add verbosity detailed
heng-liu Nov 14, 2022
3fe2fe3
address feedback, list merged settings by default
heng-liu Nov 15, 2022
1bcd070
add set and unset
heng-liu Nov 15, 2022
c261b0b
fix and add a question
heng-liu Nov 15, 2022
54133da
add consideration
heng-liu Nov 15, 2022
91e47c0
fix format
heng-liu Nov 15, 2022
a64746c
work
heng-liu Nov 16, 2022
dfae3bd
address comments
heng-liu Nov 18, 2022
209f3f0
edit the open questions
heng-liu Nov 19, 2022
efb1a2e
fix
heng-liu Nov 19, 2022
b56944d
fix format
heng-liu Nov 19, 2022
4150a2b
fix
heng-liu Nov 21, 2022
b74b829
fix option working-dir
heng-liu Nov 21, 2022
b679074
address feedback
heng-liu Nov 22, 2022
3b4ef11
Reformat the output of dotnet nuget config all
heng-liu Nov 22, 2022
46aec6c
address feedback
heng-liu Nov 22, 2022
0836860
address feedback
heng-liu Nov 22, 2022
e4b6ae5
change WORKING_DIRECTORY from option to argument
heng-liu Dec 6, 2022
160cda0
address feedbacks
heng-liu Dec 12, 2022
779cfc8
fix a typo
heng-liu Dec 12, 2022
45f081f
address feedback
heng-liu Dec 12, 2022
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
84 changes: 84 additions & 0 deletions proposed/2022/dotnet-nuget-config-spec.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,84 @@
# 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 •
edited
Loading

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
1. The `dotnet nuget config list` is a community ask. We will consider adding more commands, like add/update/delete, in the future.
2. We will discuss if adding this command into NuGet.exe CLI, in the future.
3. 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?

## Open Questions
1. When the specified `CURRENT_DIRECTORY` doesn't exist, shall we list user-wide and machine-wide config files? Or show a warning saying the working directory doesn't exist? Or do both?
heng-liu marked this conversation as resolved.
Show resolved Hide resolved

We decided to show a warning/error.
zivkan marked this conversation as resolved.
Show resolved Hide resolved
When user has a spelling mistake when passing `CURRENT_DIRECTORY` without knowing, if we don't show a warning/error, that would mislead the user.
Showing them user-wide and machine-wide config files doesn't help.

## Considerations