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

Allow overriding OS_TYPE and ARCH. This will allow signing locally on e.g. mac #595

Merged
merged 2 commits into from
Jul 27, 2021

Conversation

onetechnical
Copy link
Contributor

@onetechnical onetechnical commented Jul 27, 2021

Summary

To execute make sign you need to be able to issue the signing command. There is a script for this, but it presumes that the architecture you're signing for is the same as what you're running.

This is problematic if you are on mac, as OS_TYPE will register as darwin. To override this, we allow you to override OS_TYPE, e.g. set it to linux. We will also allow overriding ARCH, since this may be set to arm64 on newer macs.

Test Plan

Verify signing works with these new changes, e.g.:

ARCH=amd64 OS_TYPE=linux make sign

Copy link
Contributor

@egieseke egieseke left a comment

Choose a reason for hiding this comment

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

Makes sense.

@codecov-commenter
Copy link

codecov-commenter commented Jul 27, 2021

Codecov Report

Merging #595 (f32ecc7) into develop (c454446) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff            @@
##           develop     #595   +/-   ##
========================================
  Coverage    47.55%   47.55%           
========================================
  Files           25       25           
  Lines         3863     3863           
========================================
  Hits          1837     1837           
  Misses        1745     1745           
  Partials       281      281           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c454446...f32ecc7. Read the comment docs.

@algobarb
Copy link
Contributor

Since this is for signing, would this change only affect our team? Also, would we get a help message or some doc so we know we can pass in these new variables as described in the test plan ARCH=amd64 OS_TYPE=linux make sign ?

@onetechnical
Copy link
Contributor Author

Since this is for signing, would this change only affect our team? Also, would we get a help message or some doc so we know we can pass in these new variables as described in the test plan ARCH=amd64 OS_TYPE=linux make sign ?

Correct, this is internal only. I am updating the release doc right now.

@onetechnical onetechnical merged commit 0d2efe8 into develop Jul 27, 2021
@onetechnical onetechnical deleted the onetechnical/override-make-sign branch July 27, 2021 16:21
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.

4 participants