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

Don't use absolute path for signtool in sign_binaries.py #160

Merged
merged 1 commit into from Jun 12, 2018

Conversation

@RyanJarv
Copy link
Contributor

RyanJarv commented Jun 12, 2018

The env var checked before doesn't seem to get passed to this script and the fallback signtool path doesn't exist in a standard visual studios setup.

There's likely a better way to do this but this with setting the path before hand is the simplest fix for now. During the build it will look something like this:

$env:PATH = "C:\Program Files (x86)\Windows Kits\10\bin\10.0.15063.0\x64\;$env:PATH"
yarn run create_dist Release --debug_build=false --official_build=false --channel=beta

Submitter Checklist:

  • Submitted a ticket for my issue if one did not already exist.
  • Used Github auto-closing keywords in the commit message.
  • Added/updated tests for this change (for new code or code which already has tests).
  • Ran git rebase -i to squash commits (if needed).
  • Tagged reviewers and labelled the pull request as needed.
  • Request a security/privacy review as needed.

Test Plan:

Reviewer Checklist:

  • New files have MPL-2.0 license header.
  • Request a security/privacy review as needed.
  • Adequate test coverage exists to prevent regressions
'C:\\Program Files (x86)\\Windows Kits\\10'))
cmd = "{}\\bin\\x64\\signtool.exe {}".format(sdk_dir, signtool_args)
# signtool should be in the path if it was set up correctly by gn through src/build/vs_toolchain.py
cmd = 'signtool ' + format(signtool_args)

This comment has been minimized.

Copy link
@bridiver

bridiver Jun 12, 2018

Collaborator

actually shouldn't this be 'signtool {}'.format(signtool_args)? I guess both ways work?

This comment has been minimized.

Copy link
@RyanJarv

RyanJarv Jun 12, 2018

Author Contributor

yeah that's not intentional, will fix this.

This comment has been minimized.

Copy link
@RyanJarv

RyanJarv Jun 12, 2018

Author Contributor

@bridiver mind taking a look at this again?

The env var checked before doesn't seem to get passed to this
script and the fallback signtool path doesn't exist in a
standard visual studios setup.
@RyanJarv RyanJarv force-pushed the fix/win-signing branch from 21a0bf8 to 26c134d Jun 12, 2018
@RyanJarv
Copy link
Contributor Author

RyanJarv commented Jun 12, 2018

This allows us to work around the this issue brave/brave-browser#323.

@RyanJarv RyanJarv merged commit 0a6c2c7 into master Jun 12, 2018
@RyanJarv RyanJarv deleted the fix/win-signing branch Jun 12, 2018
NejcZdovc added a commit that referenced this pull request Dec 10, 2018
Grant checks are now performed every 24 hours.
NejcZdovc added a commit that referenced this pull request Sep 18, 2019
Adds single panel option to the slider
NejcZdovc added a commit that referenced this pull request Sep 19, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants
You can’t perform that action at this time.