Skip to content

Always sign builds#746

Merged
lukebakken merged 1 commit intomasterfrom
rabbitmq-dotnet-client-745
Mar 3, 2020
Merged

Always sign builds#746
lukebakken merged 1 commit intomasterfrom
rabbitmq-dotnet-client-745

Conversation

@lukebakken
Copy link
Collaborator

Fixes #745

@lukebakken lukebakken added this to the 5.2.0 milestone Mar 2, 2020
@lukebakken lukebakken force-pushed the rabbitmq-dotnet-client-745 branch from 452c780 to ed9d7d7 Compare March 2, 2020 16:17
</PropertyGroup>
<PropertyGroup Condition=" '$(Configuration)' == 'SignedRelease' ">
<DelaySign>true</DelaySign>
<PropertyGroup Condition=" '$(Configuration)' == 'Release' ">
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'd remove this PropertyGroup entirely. The only things in it that aren't redudant are AssemblyOriginatorKeyFile and SignAssembly, so you can move those up to the top PropertyGroup and get rid of everything else.

<PackageReference Include="System.Memory" Version="4.5.3" />
</ItemGroup>
<ItemGroup Condition=" '$(Configuration)' == 'SignedRelease' ">
<AssemblyAttribute Include="System.Runtime.CompilerServices.InternalsVisibleToAttribute">
Copy link
Collaborator

@bording bording Mar 2, 2020

Choose a reason for hiding this comment

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

Since it make sense to always strong-name the assembly, I'd move this one out of the project file and into AssemblyInfo.cs as an actual attribute.

@lukebakken
Copy link
Collaborator Author

@bording as always thanks for the guidance.

<IncludeSymbols>true</IncludeSymbols>
<SymbolPackageFormat>snupkg</SymbolPackageFormat>
<AllowedOutputExtensionsInPackageBuildOutputFolder>$(AllowedOutputExtensionsInPackageBuildOutputFolder);.pdb</AllowedOutputExtensionsInPackageBuildOutputFolder>
<OutputType>Library</OutputType>
Copy link
Collaborator

Choose a reason for hiding this comment

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

This can be removed.

<OutputType>Library</OutputType>
<AssemblyOriginatorKeyFile>../rabbit.snk</AssemblyOriginatorKeyFile>
<SignAssembly>true</SignAssembly>
<DelaySign>false</DelaySign>
Copy link
Collaborator

Choose a reason for hiding this comment

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

This can be removed.

<SignAssembly>true</SignAssembly>
</PropertyGroup>
<PropertyGroup Condition=" '$(Configuration)' != 'Release' ">
<Optimize>false</Optimize>
Copy link
Collaborator

Choose a reason for hiding this comment

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

This isn't needed, so you an remove this entire PropertyGroup and the Optimize setting.

<DelaySign>true</DelaySign>
<OutputType>Library</OutputType>
<PropertyGroup Condition=" '$(Configuration)' == 'Release' ">
<Optimize>true</Optimize>
Copy link
Collaborator

Choose a reason for hiding this comment

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

You don't need to set Optmize at all, so you can remove everything related to that.

<GenerateAssemblyCopyrightAttribute>false</GenerateAssemblyCopyrightAttribute>
<AssemblyOriginatorKeyFile>../rabbit.snk</AssemblyOriginatorKeyFile>
<SignAssembly>true</SignAssembly>
<DelaySign>false</DelaySign>
Copy link
Collaborator

Choose a reason for hiding this comment

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

This isn't needed.

using System;
using System.Threading;

#pragma warning disable CS0649
Copy link
Collaborator

Choose a reason for hiding this comment

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

Hmm, this is interesting. What problem are you running into here?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I was getting a build warning but now I'm not. Bizarre.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It was a warning about this line being unassigned and thus keeping its default value.

@lukebakken lukebakken force-pushed the rabbitmq-dotnet-client-745 branch from 0952c7c to 188f23e Compare March 3, 2020 21:37
@lukebakken lukebakken marked this pull request as ready for review March 3, 2020 21:39
@lukebakken lukebakken changed the title WIP: always sign Release builds Always sign builds Mar 3, 2020
https://docs.microsoft.com/en-us/dotnet/core/tools/telemetry

Add strong-named secret file

Removed SignedRelease build, other SNK releated cleanup

Since we're always signing RabbitMQ.Client we need to sign Unit as well
@lukebakken lukebakken force-pushed the rabbitmq-dotnet-client-745 branch from 188f23e to c8a669c Compare March 3, 2020 21:40
@lukebakken lukebakken self-assigned this Mar 3, 2020
@lukebakken lukebakken merged commit 20cc528 into master Mar 3, 2020
@lukebakken lukebakken deleted the rabbitmq-dotnet-client-745 branch March 3, 2020 21:45
lukebakken added a commit that referenced this pull request Mar 3, 2020
Always sign builds

(cherry picked from commit 20cc528)
@lukebakken
Copy link
Collaborator Author

Backported to 5.x

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Always strongly name / sign library

2 participants