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

Signing CD Action #397

Merged
merged 9 commits into from
Jun 24, 2024
Merged

Signing CD Action #397

merged 9 commits into from
Jun 24, 2024

Conversation

jasonleenaylor
Copy link
Contributor

@jasonleenaylor jasonleenaylor commented Apr 17, 2024

This change is Reviewable

@jasonleenaylor jasonleenaylor force-pushed the feature/gha-signing branch 18 times, most recently from f1e5cd0 to 2c77ba7 Compare April 23, 2024 21:28
@jasonleenaylor jasonleenaylor force-pushed the feature/gha-signing branch 10 times, most recently from f4fd936 to af97707 Compare May 6, 2024 19:02
@jasonleenaylor jasonleenaylor force-pushed the feature/gha-signing branch 4 times, most recently from 00bba6a to 6411066 Compare June 7, 2024 04:50
* Add workflow dispatch input
* Update to latest overcrowdin version
* Update generic installer to handle delayed signing
* Build/upload/and sign the msi
* actions/runner-images#9667
* Should be removed as soon as the runner version works
* Download the signed msi artifact and then and build bundles
* Add a description parameter for signing
* Also update framework dependency to net48
* Sign engines, reattach engines, upload and sign bundles
@jasonleenaylor jasonleenaylor changed the title WIP - Signing CD Action Signing CD Action Jun 7, 2024
@megahirt
Copy link
Contributor

This is a monumental effort, @jasonleenaylor nice work! I assume you are happy for this to get merged in with a review and not wait for a signing service to be developed.

@jasonleenaylor
Copy link
Contributor Author

Yes @megahirt I would like to get this merged to use until we have a signing service.

@jasonleenaylor
Copy link
Contributor Author

jasonleenaylor commented Jun 14, 2024

From Ken Zook:
"It installed. I was able to get project from colleague from LD, and did a S/R project to LD and it worked. I think we have a winner. It's version FLEx Bridge 4.2.0-alpha.20 13/Jun/2024. It has mercurial 6.5.1 installed. Looks like it's ready for the test team to give it a good workout. "

I would like to get some PRs in the codesign repo merged and update this PR with a real tagged version but otherwise it is ready.
sillsdev/codesign#18
sillsdev/codesign#19

Copy link
Contributor

@megahirt megahirt left a comment

Choose a reason for hiding this comment

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

LGTM

.github/workflows/ci-cd.yml Show resolved Hide resolved
if: github.event_name != 'pull_request'


- name: Downgrade Wix Toolset - remove when runner has 3.14.2
Copy link
Contributor

@megahirt megahirt Jun 24, 2024

Choose a reason for hiding this comment

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

Suggested change
- name: Downgrade Wix Toolset - remove when runner has 3.14.2
- name: Downgrade Wix Toolset - remove when runner has 3.14.2
# See: https://github.com/actions/runner-images/issues/9667

Copy link
Contributor

Choose a reason for hiding this comment

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

I'd rather put that in a comment rather than make an already-long step name even longer.

Copy link
Contributor

Choose a reason for hiding this comment

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

good call. I moved it to the next line as a comment

Copy link
Contributor

@rmunn rmunn left a comment

Choose a reason for hiding this comment

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

I have no experience with WiX so if there are any mistakes in that part of the PR then I won't have caught them. But the GHA workflow changes look good to me.

@jasonleenaylor jasonleenaylor requested review from megahirt and rmunn June 24, 2024 13:24
Copy link
Contributor Author

@jasonleenaylor jasonleenaylor left a comment

Choose a reason for hiding this comment

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

Dismissed @rmunn from a discussion.
Reviewable status: 0 of 8 files reviewed, 2 unresolved discussions (waiting on @megahirt and @rmunn)


src/LibFLExBridge-ChorusPluginTests/LibFLExBridge-ChorusPluginTests.csproj line 15 at r1 (raw file):

Previously, rmunn (Robin Munn) wrote…

Why this downgrade? SIL.Chorus.Mercurial 6.5.1.25 fixed running Mercurial on Linux under Python 3, and AFAIK made no other changes. Is there something that I broke in creating 6.5.1.25 that was working correctly with 6.5.1.22 which made the downgrade necessary?

Done.

Copy link
Contributor Author

@jasonleenaylor jasonleenaylor left a comment

Choose a reason for hiding this comment

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

Reviewable status: 0 of 8 files reviewed, 2 unresolved discussions (waiting on @megahirt and @rmunn)


.github/workflows/ci-cd.yml line 56 at r4 (raw file):

Previously, megahirt (Christopher Hirt) wrote…

good call. I moved it to the next line as a comment

Done.

Copy link
Contributor Author

@jasonleenaylor jasonleenaylor left a comment

Choose a reason for hiding this comment

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

Reviewable status: 0 of 8 files reviewed, 2 unresolved discussions (waiting on @megahirt and @rmunn)


.github/workflows/ci-cd.yml line 23 at r4 (raw file):

Previously, rmunn (Robin Munn) wrote…

It's used by FLExBridge.proj to get the root directory of the project. We should keep it until the TeamCity build is finally gone, then we can rename it both here and in FLExBridge.proj.

Done.

* Also only downgrade WIX when building installers (not PRs)
@jasonleenaylor jasonleenaylor merged commit e27a57a into develop Jun 24, 2024
9 of 10 checks passed
@jasonleenaylor jasonleenaylor deleted the feature/gha-signing branch June 24, 2024 14:08
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.

3 participants