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.

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

I'd like to make computeSignToolArgs usable in custom signing. #2397

Closed
wants to merge 2 commits into from

Conversation

spicykoala
Copy link

computeSignToolArgs is available on the configuration object passed into a custom signing method. This was really helpful to me, but in order to make it usable I had to remove the custom sign check from the winPackager.

I'm not sure if it's the best way to solve the issue, but in my view we can just leave out certificateSubjectName and certificateSha1 and certificateFile for the same effect. Either way, I wanted to create this pull request so that it could be discussed.

Thanks for all of your fantastic work - this tool has been awesome.

spicykoala added 2 commits December 18, 2017 11:41
cscInfo should still be calculated for a custom sign if applicable.

-- Making this change because I want to use computeSignToolArgs in my custom signer and it was blowing up due to unexpected null references with this change.
@develar
Copy link
Member

develar commented Dec 18, 2017

CI passed, but I am not sure that solution is correct.

in order to make it usable

Could you please clarify why it is not usable now? If because your custom signer still want to get code singing info, please explain why.

@spicykoala
Copy link
Author

Right now if you call it, for example:

configuration.computeSignToolArgs(true);

It blows up inside windowsCodeSign.ts on

const certificateFile = (options.cscInfo as FileCodeSigningInfo).file

Because it's undefined, undefined.file throws an error.

I thought the args were handy to have, because I'm using Parallels to run the signing, but I'm manually making the calls for other reasons.

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