Skip to content
This repository has been archived by the owner on Jul 30, 2024. It is now read-only.
/ NuGet.Jobs Public archive

Configure MicroBuild to sign NuGet.Jobs #590

Merged
merged 23 commits into from
Oct 25, 2018
Merged

Configure MicroBuild to sign NuGet.Jobs #590

merged 23 commits into from
Oct 25, 2018

Conversation

scottbommarito
Copy link

https://github.com/NuGet/Engineering/issues/1728

  • Added sign.thirdparty.targets, a target shared by all projects in the solution, that configures all the third-party assemblies in the project to be signed.
  • Added MicroBuild dependencies to projects that didn't have them.
  • Added sign.scripts.targets to projects that have PowerShell scripts so that it signs them.
  • Removed Validation.Callback.Vcs, which is no longer needed and would have needed to be signed.

Note: This doesn't sign NuGetCDNRedirect, but because it's an App Service we are not required to sign it yet.

NuGet.Jobs.sln Outdated
@@ -1,7 +1,7 @@

Microsoft Visual Studio Solution File, Format Version 12.00
# Visual Studio 15
VisualStudioVersion = 15.0.27121.1
# Visual Studio Version 16
Copy link
Member

Choose a reason for hiding this comment

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

What if I am using dev15 😓 ?

Copy link
Author

Choose a reason for hiding this comment

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

Wasn't aware of exactly what this did...will revert.

<Import Project="..\..\build\sign.targets" Condition="Exists('..\..\build\sign.targets')" />
<Import Project="$(SignPath)\sign.microbuild.targets" Condition="Exists('$(SignPath)\sign.microbuild.targets')" />
<ItemGroup>
<PowerShellScriptsToSign Include="InstallAzCopy.ps1"/>
Copy link
Member

Choose a reason for hiding this comment

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

👍

@@ -0,0 +1,4 @@
# This file is a placeholder--it does nothing.
# "sign.scripts.targets", which is imported by the project from ServerCommon, requires that "nssm.exe", "Functions.ps1", "PreDeploy.ps1", and "PostDeploy.ps1" all exist.
Copy link
Member

Choose a reason for hiding this comment

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

Let's find a way to make this place-holder file unnecessary. Perhaps a no-op powershell or an ItemGroup Remove="..."

@@ -26,6 +26,7 @@
<dependency id="NuGetGallery.Core" version="4.4.4-master-41290" />
<dependency id="Serilog" version="2.5.0" />
<dependency id="System.Net.Http" version="4.3.3" />
<dependency id="MicroBuild.Core" version="0.3.0" />
Copy link
Member

Choose a reason for hiding this comment

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

We should check IncludeAssets/PrivateAssets to exclude dev dependencies in the unit test requiring this change.

Copy link
Member

@joelverhagen joelverhagen left a comment

Choose a reason for hiding this comment

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

:shipit: after addressing comments.

@joelverhagen
Copy link
Member

This should go into dev not master.

@scottbommarito
Copy link
Author

@joelverhagen - I thought the priority of this was enough to make it a hotfix-level change. I can change to dev if you think that's best however.

@joelverhagen
Copy link
Member

@scottbommarito, that's a good point. We don't want to block this work and deployments on some issue in dev. Carry on 👍

Scott Bommarito added 2 commits October 25, 2018 14:42
@scottbommarito scottbommarito merged commit 44fdc21 into master Oct 25, 2018
@scottbommarito scottbommarito deleted the sb-micro branch October 25, 2018 22:43
joelverhagen pushed a commit that referenced this pull request Sep 27, 2019
joelverhagen pushed a commit that referenced this pull request Oct 26, 2020
joelverhagen pushed a commit that referenced this pull request Oct 26, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants