-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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.
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
Microbuild signing #47
Conversation
…to ProducesNoOutput.Settings.targets.
If you'd like to see the output of a signed build, it's on \internal_drop_root\roslyn-project-system\dotnet-core-sdk-testing. If you don't know what the drop root is, you can find it in the MicroBuild variable definitions, or just message me and I'll give you the link. Don't want to post it on a repo that'll eventually go public. |
|
||
<!-- If RealSigning --> | ||
<Message Text="Performing Real Signing" Condition="'$(SignType)'=='real'"/> | ||
<Exec Command="$(SignTool) -nugetPackagesPath "$(NuGet_Packages)" -config "@(SigningConfig)" "$(OutDir.TrimEnd('\'))"" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This change sort of conflicts with my latest PR #46. Basically, the output assembly is getting placed into the PackagesLayout folder during build of the tasks assembly. That's the assembly that needs to be signed.
What do you think about hooking the signing tool up during the Build of the tasks assembly, instead of in an outside project?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The part that tells MicroBuild what to sign is a combination of the path at the end and SignToolConfig.json. I'm not sure how this conflicts with that PR, we'll just have to pass $(OutDir)PackagesLayoutDir
as the path instead of just $(OutDir)
. Is there something I'm missing?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The tasks assembly is located in 2 places in $(OutDir)PackagesLayoutDir
, once in netcoreapp1.0 and once in net46. I'd rather we sign the file once, and then place/copy the signed file into the packages layout directory. So I think I'll move the "layout" code out of the Tasks.csproj and instead into the NuGet.proj file, which solves all these problems.
|
||
<Target Name="SignPackages"> | ||
|
||
<MSBuild BuildInParallel="true" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should ensure that BuildNuGetPackages
is dependent on this target, or else we will build the nuget packages in parallel, and then we won't get our signed assemblies in the nuget packages.
This looks good, @333fred. Thanks for taking care of this. after addressing the "parallel build" comment. |
…es from the signing config
Use jspm_packages instead modules
This adds PublicSigning to local builds, and DelaySigning+RealSigning when running on MicroBuild. I've also updated our MicroBuild definition to include the
-RealSign
parameter, which won't have an effect until this PR is merged.Tagging @eerhardt @piotrpMSFT @livarcocc @brthor @natidea @dotnet/project-system for review.