Skip to content

Conversation

@lerouxb
Copy link
Contributor

@lerouxb lerouxb commented Feb 11, 2025

  • assert on required env vars in signtool.go
  • log errors in signtool.go (the stdout and stderr redirect wasn't working)
  • have @mongodb-js/signing-utils where signtool.go expects it
  • actually assert that the setup .exe is signed when verifying artifacts

@lerouxb lerouxb added no release notes Fix or feature not for release notes no-title-validation Skips validation of PR titles (conventional commit adherence + JIRA ticket inclusion) and removed no release notes Fix or feature not for release notes labels Feb 11, 2025
@lerouxb lerouxb changed the title chore: log checksums before and after signing chore: log checksums before and after signing, assert env vars, error out if there is a failure Feb 11, 2025
@lerouxb lerouxb changed the title chore: log checksums before and after signing, assert env vars, error out if there is a failure chore: log checksums before and after signing, assert env vars, log errors Feb 11, 2025
@lerouxb lerouxb force-pushed the log-checksums-before-after-signing branch from 40e6372 to 7e16bc0 Compare February 11, 2025 20:35
@lerouxb lerouxb changed the title chore: log checksums before and after signing, assert env vars, log errors chore: assert env vars, log errors, have @mongodb-js/signing-utils where signtool.go expects it Feb 12, 2025
@lerouxb lerouxb changed the title chore: assert env vars, log errors, have @mongodb-js/signing-utils where signtool.go expects it chore: assert env vars, log errors, have @mongodb-js/signing-utils where signtool.go expects it COMPASS-8945 Feb 12, 2025
@lerouxb lerouxb force-pushed the log-checksums-before-after-signing branch from 0817504 to 3890616 Compare February 12, 2025 10:31
"@mongodb-js/my-queries-storage": "^0.22.0",
"@mongodb-js/prettier-config-compass": "^1.2.0",
"@mongodb-js/sbom-tools": "^0.7.0",
"@mongodb-js/signing-utils": "^0.3.7",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

So the bug was that signtool.go is doing node -e 'require("@mongodb-js/signing-utils")' from the packages/compass folder and that node module doesn't exist there.

In the past it probably worked because the dep was hoisted and then it broke because now it is not.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think doing something like npx run sign -w @mongodb-js/hadron-build -- path-to-file from signtool.go would be a cleaner fix. I'll try that in a follow-up.

@lerouxb lerouxb changed the title chore: assert env vars, log errors, have @mongodb-js/signing-utils where signtool.go expects it COMPASS-8945 chore: assert env vars, log errors, have @mongodb-js/signing-utils where signtool.go expects it COMPASS-8945 COMPASS-8950 Feb 12, 2025
@lerouxb lerouxb changed the title chore: assert env vars, log errors, have @mongodb-js/signing-utils where signtool.go expects it COMPASS-8945 COMPASS-8950 fix: sign the windows setup .exe COMPASS-8945 COMPASS-8950 Feb 12, 2025
@github-actions github-actions bot added the fix label Feb 12, 2025
@lerouxb lerouxb added release notes and removed no-title-validation Skips validation of PR titles (conventional commit adherence + JIRA ticket inclusion) labels Feb 12, 2025
if err != nil {
fmt.Println("Error signing the file")
fmt.Printf("%s\n", stdoutStderr)
log.Println(err)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ideally this would be log.fatal(err) but then we don't see any of the logging and electron winstaller throws a huge error, none of which is helpful. Maybe it doesn't flush its subprocess's output? I'm not sure.

@lerouxb lerouxb merged commit 89eb2f2 into main Feb 12, 2025
36 of 39 checks passed
@lerouxb lerouxb deleted the log-checksums-before-after-signing branch February 12, 2025 11:45
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