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

Generate and upload checksums for released ZIPs to GitHub #6588

Merged
merged 2 commits into from Aug 1, 2016

Conversation

Projects
None yet
3 participants
@malept
Copy link
Member

malept commented Jul 25, 2016

When generating an Electron release, create a sha256sum-compatible file for each ZIP file, and upload them to the corresponding GitHub release. This is primarily to confirm that the download of a given ZIP completed successfully, as opposed to verifying that an Electron team member uploaded the given ZIP files (which would require using a trusted GPG key).

The reason why I would like to add this feature to the release process is that I'm a maintainer of the Electron Packager project, and we sometimes get GitHub issues filed where ultimately, the problem is that the download of Electron from GitHub releases didn't complete successfully. If this PR is merged, my goal is to add functionality to electron-download that emulates sha256sum -c on the downloaded file.

@zcbenz

This comment has been minimized.

Copy link
Member

zcbenz commented Jul 25, 2016

I'm 👍 on adding checksum files, but I think we should put all checksum files in one file, like what Node did. It would be easier for us to sign it in future.

The only problem is the implementation would be much more complex, since each build is uploaded in a different machine.

@malept

This comment has been minimized.

Copy link
Member Author

malept commented Jul 25, 2016

The only problem is the implementation would be much more complex, since each build is uploaded in a different machine.

Right, I did briefly look at doing an SHA256SUMS.txt file (it would actually be preferable for the electron-download implementation) until I realized that about the build system.

Given that, the only way I can see how a one-checksum file can be generated is to keep the "generate single zip checksum" logic, but:

  • replace the "upload checksum file to GitHub" logic with "upload checksum file to intermediate location" logic (such as S3)
  • when the entire build process is finished, download & concatenate the checksum files, then upload the result to GitHub releases

Let me know if this idea is feasible, since I don't know what the full release process looks like.

@zcbenz

This comment has been minimized.

Copy link
Member

zcbenz commented Jul 26, 2016

Let me know if this idea is feasible, since I don't know what the full release process looks like.

It sounds great.

We upload the binaries by first waiting for CI machines to finish, and then run ./script/upload.py -p on the local machine to publish the release.

@malept

This comment has been minimized.

Copy link
Member Author

malept commented Jul 26, 2016

We upload the binaries by first waiting for CI machines to finish, and then run ./script/upload.py -p on then local machine to publish the release.

I assume there's some script/job manager that kicks off running ELECTRON_RELEASE=true script/cibuild on each CI machine?

@zcbenz

This comment has been minimized.

Copy link
Member

zcbenz commented Jul 26, 2016

I assume there's some script/job manager that kicks off running ELECTRON_RELEASE=true script/cibuild on each CI machine?

Yeah, the CI machines set ELECTRON_RELEASE=true when the commit message begins with Bump vx.x.x.

@malept

This comment has been minimized.

Copy link
Member Author

malept commented Jul 26, 2016

OK, so it sounds like it's a GitHub webhook that triggers all of the CI machines. Could the machine with the webhook somehow monitor (either via another hook or via polling) to see when all of the CI machines completed successfully?

@zcbenz

This comment has been minimized.

Copy link
Member

zcbenz commented Jul 26, 2016

It should be possible, but no one has done that.

@malept

This comment has been minimized.

Copy link
Member Author

malept commented Jul 26, 2016

I'd like to help with that, but as it currently stands, I can't move forward with this PR without knowing more about the release infrastructure and possibly being able to modify the code that provides the CI webhook.

@anaisbetts

This comment has been minimized.

Copy link
Contributor

anaisbetts commented Jul 26, 2016

The surf-download utility which is part of the surf-build npm package makes it dead-easy to download all of the releases from a GitHub Release (even if your project doesn't use Surf): https://github.com/surf-build/surf#surf-download

@malept

This comment has been minimized.

Copy link
Member Author

malept commented Jul 26, 2016

@paulcbetts I'm not sure how that helps here, unless you're implying that there should be a separate script that is run (manually?) when all of the CI machines have completed, to generate and upload the checksum file.

@anaisbetts

This comment has been minimized.

Copy link
Contributor

anaisbetts commented Jul 26, 2016

@malept Ya - the notion of trying to detect when the entire build finishes is probably p hard right now, you'll probably have to do this by-hand (or @zcbenz can add this to his release checklist and you can contribute the script that makes this a one-shot command).

@malept

This comment has been minimized.

Copy link
Member Author

malept commented Jul 26, 2016

@paulcbetts that's reasonable. I can rewrite this PR to turn this into a standalone script that can be run separately.

@zcbenz

This comment has been minimized.

Copy link
Member

zcbenz commented Jul 27, 2016

Our release infrastructure works like this:

  1. I push a version bump commit
  2. CI server runs build.py and upload.py
  3. I wait an hour for CI servers to finish
  4. I run upload.py -p on my machine

I think we can make the checksum generation automatic by:

  1. In upload.py upload the checksum files to intermediate place on S3
  2. In upload.py -p (which runs the code under args.publish_release), download the intermediate checksum files and merge them into a SHA256SUMS.txt, and then upload it

Downloading all the files in releases is usually a better idea, and we are already doing it for generating checksums for node headers (see upload-checksums.py). But the files of Electron are too large (mainly the symbols), it is not reasonable to download all of them on a local machine.

@malept

This comment has been minimized.

Copy link
Member Author

malept commented Jul 27, 2016

@zcbenz OK, that plan sounds reasonable. Let me know the S3 key prefix to put the intermediate files (the same place as the Node checksums?) and I can start modifying this PR accordingly.

@zcbenz

This comment has been minimized.

Copy link
Member

zcbenz commented Jul 27, 2016

Uploading to any key prefix should be fine, maybe we can upload to tmp/ so it would be easier when we want to cleanup files.

Generate and upload checksums for released ZIPs to GitHub
When generating an Electron release, create a `sha256sum`-compatible
file for each ZIP file, and upload them to the corresponding GitHub release.
This is primarily to confirm that the download of a given ZIP completed
successfully, as opposed to verifying that an Electron team member uploaded
the given ZIP files (which would require using a trusted GPG key).

@malept malept force-pushed the malept:dist-checksums branch from e6ab31a to 7f831df Aug 1, 2016

@malept malept force-pushed the malept:dist-checksums branch from b847e7c to 59de146 Aug 1, 2016

@malept

This comment has been minimized.

Copy link
Member Author

malept commented Aug 1, 2016

I've updated the PR with the suggestions in #6588 (comment). The script that merges the individual checksum files together from S3 uses the boto library instead of boto's scripts (as s3put does), because trying to parse the output of lss3 would have been too complicated.

@zcbenz

This comment has been minimized.

Copy link
Member

zcbenz commented Aug 1, 2016

Awesome, thanks!

@zcbenz zcbenz merged commit 8f75f10 into electron:master Aug 1, 2016

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@malept malept deleted the malept:dist-checksums branch Aug 1, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.