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

[WIP] Build app installer via GitHub actions. #901

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

adityamudgil2505
Copy link
Collaborator

@adityamudgil2505 adityamudgil2505 commented Mar 19, 2020

What's this PR do?
This will allow artifacts to upload to the GitHub package registry as draft release. This solves the Issue #406 .

Any background context you want to provide?
Earlier we are using Travis CI for macOS and Linux, and appveyor for windows testing. Now we are shifting this to GitHub actions which will also allow artifacts to upload to the GitHub package registry as a draft release.
We need to upgrade electron-builder version from 22.3.2 to 22.4.1 so that our windows pipeline will work properly.

You have tested this PR on:

  • Windows
  • Linux/Ubuntu
  • macOS

@adityamudgil2505
Copy link
Collaborator Author

@priyank-p @akashnimare Linux app is successfully built and the artifact is uploaded in both node versions.Check this link to see the logs and to download the uploaded artifact

But in case of Windows, the build was failed due to the following error Error link
and I found that it's a electron-builder version 22.3.2 problem from this electron-userland/electron-builder#4620 (comment)

And for mac will successfully build only if we add the environment variable

mac_certs: ${{ secrets.mac_certs }}
mac_certs_password: ${{ secrets.mac_certs_password }}

you can find the working of this in this link

I have one query regarding Linux build, the artifact upload is of size around 400MB and it contains following files after downloading, do we need all of these to build for Linux or only a few of them because its time taking to download all the files.
Screenshot 2020-03-20 at 7 40 13 PM

Copy link
Member

@priyank-p priyank-p left a comment

Choose a reason for hiding this comment

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

@adityamudgil2505 I posted a bunch of comments.

.github/workflows/ci_cd.yml Outdated Show resolved Hide resolved
.github/workflows/ci_cd.yml Outdated Show resolved Hide resolved
.github/workflows/ci_cd.yml Outdated Show resolved Hide resolved
.github/workflows/ci_cd.yml Outdated Show resolved Hide resolved
.github/workflows/ci_cd.yml Outdated Show resolved Hide resolved
.github/workflows/ci_cd.yml Outdated Show resolved Hide resolved
.github/workflows/ci_cd.yml Show resolved Hide resolved
.github/workflows/ci_cd.yml Outdated Show resolved Hide resolved
.github/workflows/ci_cd.yml Show resolved Hide resolved
.github/workflows/ci_cd.yml Outdated Show resolved Hide resolved
@priyank-p
Copy link
Member

@adityamudgil2505 For Linux (and for other platform), we do need all the files in dist.

Regarding the windows issue, you can try and downgrade the electron-build and see if it works. If it works. then add a commit changing the version and mention this issue in the commit message.

As for the macOS, once we have everything else working, I think we will push this to a branch in this repository and add the secrets to test if it works. If it works we will push it to master.

Finally, thanks for working on this issue.

@priyank-p priyank-p changed the title [WIP] Fix #406 Added GitHub actions support [WIP] Build app installer via GitHub actions. Mar 20, 2020
@adityamudgil2505
Copy link
Collaborator Author

@priyank-p I made all the changes. For node-version, there is no way for the latest version in the documentation. We need to set it manually actions/setup-node#61. But I found an open merge request actions/setup-node#104 which solves this issue.
After updating the version of Electron-builder from 22.3.2 to 22.4.1 and now the windows pipeline is running fine.
Link of Linux, Windows CI

Copy link
Member

@priyank-p priyank-p left a comment

Choose a reason for hiding this comment

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

@adityamudgil2505 This is looking great! I added some more comments. Also, please make sure the changes to electron-builder version is separated into its own separate preparatory commit. And other changes are squashed into their own coherent commits.

Checkout Zulip commit guidelines for detailed information on how to create coherent commits.

.github/workflows/ci_cd.yml Show resolved Hide resolved
.github/workflows/ci_cd.yml Outdated Show resolved Hide resolved
.github/workflows/ci_cd.yml Outdated Show resolved Hide resolved
.github/workflows/ci_cd.yml Outdated Show resolved Hide resolved
.github/workflows/ci_cd.yml Show resolved Hide resolved
@adityamudgil2505
Copy link
Collaborator Author

@priyank-p Please review.

package.json Outdated Show resolved Hide resolved
@zulipbot
Copy link
Member

Heads up @adityamudgil2505, we just merged some commits that conflict with the changes your made in this pull request! You can review this repository's recent commits to see where the conflicts occur. Please rebase your feature branch against the upstream/master branch and resolve your pull request's merge conflicts accordingly.

Base automatically changed from master to main January 22, 2021 19:22
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.

3 participants