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.

Sign up for GitHub

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

Jump to bottom

Add EV certificate signing for Windows builds #1186

Merged
merged 3 commits into from
Sep 14, 2023
Merged

Add EV certificate signing for Windows builds #1186

merged 3 commits into from
Sep 14, 2023

Conversation

philippitts
Copy link
Member

  • Replaces code signing for Windows builds with Extended Validation code signing which provides additional guarantees to end-user systems and reduces warning about untrusted software
  • Extracts code signing along with other OS specific logic from the make_release.py script
  • Decomposes the make_release.py script into a build.py and package.py script

@philippitts philippitts linked an issue Sep 12, 2023 that may be closed by this pull request
Add EV certificate signing for Windows builds #1185
Closed
@philippitts philippitts self-assigned this Sep 12, 2023
@philippitts philippitts changed the base branch from master to development September 12, 2023 19:35
@philippitts philippitts force-pushed the 1185-add-ev-certificate-signing-for-windows-builds branch 4 times, most recently from df339e6 to 481f3c2 Compare September 12, 2023 20:55
@retiutut
Copy link
Member

Looking great so far. Did an early review.

@philippitts philippitts force-pushed the 1185-add-ev-certificate-signing-for-windows-builds branch from c03f6ea to fc422f5 Compare September 12, 2023 21:27
@philippitts philippitts marked this pull request as ready for review September 12, 2023 22:55
@philippitts
Copy link
Member Author

@retiutut can you double check that the BrainFlow library setup for MacOS is still correct? I ran builds on Windows and MacOS and they seem correct, but it would be great to have you confirm it.

@philippitts
Copy link
Member Author

The v5 GUI used to store build artifacts with timestamps in the names, but we've taken those out for v6. If you prefer to have timestamps in the name we can just drop the related commit and it should work. However, I think we can use versioning for the artifacts in AWS and it will work better.

retiutut
retiutut requested changes Sep 13, 2023
View reviewed changes
release/package.py Outdated

if LOCAL_OS == MAC:
shutil.move(flavors[LOCAL_OS] + ".dmg", new_name + "macosx.dmg")
else:
Copy link
Member

Choose a reason for hiding this comment

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

It's not explicit here that this is the case for Windows. Can we make it more obvious?

retiutut
retiutut requested changes Sep 13, 2023
View reviewed changes
.github/workflows/mac_build_deploy.yml Outdated
GUI_VERSION_STRING=`cat temp/versionstring.txt`
python $GITHUB_WORKSPACE/release/build.py
cp $GITHUB_WORKSPACE/OpenBCI_GUI/sketch.icns $GITHUB_WORKSPACE/application.macosx/OpenBCI_GUI.app/Contents/Resources/sketch.icns
dmgbuild -s release/mac/dmgbuild_settings.py -D app=$GITHUB_WORKSPACE/application.macosx/OpenBCI_GUI.app \
Copy link
Member

@retiutut retiutut Sep 13, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

We need to sign both the Mac app (application) and the dmg (package).

@philippitts
Copy link
Member Author

Updated the PR based on your feedback and tested on Windows, MacOS, and Linux. Things are working as expected and it's ready for another review pass.

retiutut
retiutut approved these changes Sep 14, 2023
View reviewed changes
Copy link
Member

@retiutut retiutut left a comment

Choose a reason for hiding this comment

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

Awesome! Thanks Phil!

@retiutut retiutut merged commit afe4595 into development Sep 14, 2023
4 checks passed
@retiutut retiutut deleted the 1185-add-ev-certificate-signing-for-windows-builds branch September 14, 2023 16:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@retiutut retiutut retiutut approved these changes

Assignees

@philippitts philippitts

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add EV certificate signing for Windows builds
2 participants
@philippitts @retiutut