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 --config-file param to dotnet nuget push command #4819

Closed
wants to merge 2 commits into from
Closed

Add --config-file param to dotnet nuget push command #4819

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
8f1559b
Implement --config-file param in dotnet.exe
aortiz-msft Sep 26, 2022
ef202fd
remove empty line
aortiz-msft Sep 26, 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
9 changes: 8 additions & 1 deletion src/NuGet.Core/NuGet.CommandLine.XPlat/Commands/PushCommand.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,11 @@ public static void Register(CommandLineApplication app, Func<ILogger> getLogger)
Strings.SymbolSource_Description,
CommandOptionType.SingleValue);

var configurationFile = push.Option(
"--config-file <source>",
Copy link
Member

Choose a reason for hiding this comment

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

It looks like most other dotnet commands use --configfile, not --config-file

NuGet.Client/src/NuGet.Core/NuGet.CommandLine.XPlat/Commands/Commands.xml

Line 14 in 451713c

<SingleValueOption Name="configfile" Help="Option_ConfigFile" />

https://learn.microsoft.com/en-us/dotnet/core/tools/dotnet-restore?tabs=netcore2x#options
https://learn.microsoft.com/en-us/dotnet/core/tools/dotnet-nuget-add-source#options

Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
"--config-file <source>",
"--config-file <file>",

In NuGet vocabulary, a source is a feed of packages. I don't know if this was a copy-paste issue, but source really doesn't feel right to me. I don't know if we should say path instead of file.

dotnet restore -h says --configile <FILE>, while dotnet list package -h says --config <CONFIG_FILE>. Clear as mud. As consistent as flipping a coin.

Copy link
Contributor

Choose a reason for hiding this comment

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

Just fyi --configfile <File> is used for dotnet nuget trust.

Strings.Option_ConfigFile,
CommandOptionType.SingleValue);

var timeout = push.Option(
"-t|--timeout <timeout>",
Strings.Push_Timeout_Description,
Expand Down Expand Up @@ -105,7 +110,9 @@ public static void Register(CommandLineApplication app, Func<ILogger> getLogger)
}

#pragma warning disable CS0618 // Type or member is obsolete
var sourceProvider = new PackageSourceProvider(XPlatUtility.GetSettingsForCurrentWorkingDirectory(), enablePackageSourcesChangedEvent: false);
var sourceProvider = new PackageSourceProvider(
XPlatUtility.ProcessConfigFile(configurationFile.Value()),
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we should add a validation to ensure file path exists before accessing it.

Copy link
Member

Choose a reason for hiding this comment

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

Also, does XPlatUtility.ProcessConfigFile fall back to GetSettingsForCurrentWorkingDirectory when configurationFile.Value() is null or empty (was not provided on the command line)?

This code, the way it's written, makes it look like a breaking change and that passing the --config-file is becoming mandatory. edit: reading the tests, it appears that it does maintain backwards compatibility. It's just not obvious to me.

enablePackageSourcesChangedEvent: false);
#pragma warning restore CS0618 // Type or member is obsolete

try
Expand Down
3 changes: 2 additions & 1 deletion src/NuGet.Core/NuGet.Commands/CommandRunners/PushRunner.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,8 @@ public static Task Run(
bool skipDuplicate,
ILogger logger)
{
return Run(settings: settings,
return Run(
settings: settings,
sourceProvider: sourceProvider,
packagePaths: new[] { packagePath },
source: source,
Expand Down
Loading