Skip to content

Conversation

@cinderblocks
Copy link

This adds .nupkg signing via nuget sign to Authenticode signing. nuget.exe needs to be installed on not-windows and path set to it. (Nuget still requires mono on other platforms.)

I also made the timestamp service a configurable.

Copy link
Owner

@Daniel15 Daniel15 left a comment

Choose a reason for hiding this comment

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

Looks good, thanks! Just some minor comments inline.

if (!RuntimeInformation.IsOSPlatform(OSPlatform.Windows))
{
await RunProcessAsync(
"mono",
Copy link
Owner

Choose a reason for hiding this comment

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

Is Mono really needed here? Could it use .NET Core instead?

Copy link
Author

Choose a reason for hiding this comment

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

Unfortunately, it doesn't work in .NET Core yet. NuGet/NuGet.Client#2545

$"-CertificatePath \"{CommandLineEncoder.Utils.EncodeArgText(certFile)}\"",
$"-CertificatePassword \"{CommandLineEncoder.Utils.EncodeArgText(certPassword)}\"",
$"-Timestamper {_pathConfig.Timestamper}",
$"\"{CommandLineEncoder.Utils.EncodeArgText(inputFile)}\"",
Copy link
Owner

Choose a reason for hiding this comment

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

These arguments are all identical across both - Can you please remove the duplication (eg. pull them out into a separate array)?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants