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

Add rpm package creation support #1694

Merged
merged 4 commits into from
Sep 19, 2019
Merged

Conversation

hicom150
Copy link
Contributor

@hicom150 hicom150 commented Sep 15, 2019

What is the purpose of this pull request? (put an "X" next to item)

[ ] Documentation update
[ ] Bug fix
[x] New feature
[ ] Other, please explain:

What changes did you make? (Give an overview)

Given that we have recently added electron-installer-debian it is straightforward to add electron-installer-redhat and add rpm file creation support.

Is there anything you'd like reviewers to focus on?

It would be nice if someone with Fedora could check if the generated package works as expected 😅

Related:

@Borewit
Copy link
Member

Borewit commented Sep 15, 2019

I would expect that this command only builds the rpm, but the target platform is still all

npm run package linux ---platform=rpm

Copy link
Member

@Borewit Borewit left a comment

Choose a reason for hiding this comment

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

  • Update README needs to describe the rpm package target.

  • Succeed to build rpm

  • Succeed to install rpm

  • Succeed run installed wetorrent-desktop, via Gnome desktop

bin/package.js Show resolved Hide resolved
@feross
Copy link
Member

feross commented Sep 15, 2019

@Borewit npm run package linux ---platform=rpm

The command to use is npm run package -- linux --package=rpm.

Copy link
Member

@feross feross left a comment

Choose a reason for hiding this comment

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

This change generally looks good to me. I'd like to wait to merge this until after my PR #1696 is merged. Then, I'd like this to be updated to support building an .rpm for the arm64 architecture as well. You can just copy how it's done for .deb.

@hicom150
Copy link
Contributor Author

Done! 👍

Copy link
Member

@feross feross left a comment

Choose a reason for hiding this comment

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

LGTM. We should test the built files on Red Hat / CentOS before merging this.

@feross
Copy link
Member

feross commented Sep 17, 2019

I tried building this and ran into these issues:

  • The Homebrew system dep is called rpm, not rpmbuild. Changing this allowed me to build successfully.

  • When double-clicking on the .rpm file in CentOS, I get the following error:

Screen Shot 2019-09-17 at 2 57 44 PM

@hicom150
Copy link
Contributor Author

Thank you @feross for the feedback 😉

The Homebrew system dep is called rpm, not rpmbuild. Changing this allowed me to build successfully.

Nice catch, changed! 👍

When double-clicking on the .rpm file in CentOS, I get the following error

Could you try with yum install to check what could be the error cause?

In the other hand, I also try to install in a VM with Fedora 30 and get a "incompatible architecture" error. So I can guess that for RedHat based distributions we have to set destArch='x86_64' and for Debian based ones destArch='amd64'.

I commit this last change so we can test that this assumption is right for all the different distributions 😅

bin/package.js Show resolved Hide resolved
bin/package.js Show resolved Hide resolved
@feross
Copy link
Member

feross commented Sep 18, 2019

Ahh, I was getting a "Cannot find a valid baseurl for repo: base/7/x86_64" error which I think means yum couldn't reach the internet. Weird that it needs to do that to install a local .rpm file. Anyway, I resolved that. Now I get:

Loaded plugins: fastestmirror, langpacks
Examining /home/feross/Desktop/webtorrent-desktop-0.21.0-1.x86_64.rpm: webtorrent-desktop-0.21.0-1.x86_64
Marking /home/feross/Desktop/webtorrent-desktop-0.21.0-1.x86_64.rpm to be installed
Resolving Dependencies
--> Running transaction check
---> Package webtorrent-desktop.x86_64 0:0.21.0-1 will be installed
--> Processing Dependency: (kde-cli-tools or kde-runtime or trash-cli or glib2 or gvfs-client) for package: webtorrent-desktop-0.21.0-1.x86_64
Loading mirror speeds from cached hostfile
 * base: mirror.sjc02.svwh.net
 * extras: mirror.sjc02.svwh.net
 * updates: mirror.sjc02.svwh.net
base                                                     | 3.6 kB     00:00     
extras                                                   | 2.9 kB     00:00     
updates                                                  | 2.9 kB     00:00     
updates/7/x86_64/primary_db                                | 1.1 MB   00:00     
--> Processing Dependency: libXScrnSaver for package: webtorrent-desktop-0.21.0-1.x86_64
--> Running transaction check
---> Package libXScrnSaver.x86_64 0:1.2.2-6.1.el7 will be installed
---> Package webtorrent-desktop.x86_64 0:0.21.0-1 will be installed
--> Processing Dependency: (kde-cli-tools or kde-runtime or trash-cli or glib2 or gvfs-client) for package: webtorrent-desktop-0.21.0-1.x86_64
--> Finished Dependency Resolution
Error: Package: webtorrent-desktop-0.21.0-1.x86_64 (/webtorrent-desktop-0.21.0-1.x86_64)
           Requires: (kde-cli-tools or kde-runtime or trash-cli or glib2 or gvfs-client)
 You could try using --skip-broken to work around the problem
 You could try running: rpm -Va --nofiles --nodigest

Is this something we can resolve with how we produce the .rpm file?

@hicom150
Copy link
Contributor Author

After some investigation, I found that electron-installer-redhat has the requirement of having rpm > 4.13 and that is why we can install successfully in Fedora and fails in CentOS 😓

I don't know what next steps should we take... Any suggestions?

@mathiasvr
Copy link
Contributor

We could just make a notice that it is a requirement to have rpm > 4.13 to use the installer?
I don't think it's worth it to try and support older versions unless someone would like to do it.

I don't know much about snap packages, but is there any chance that would work as an alternative?

@feross
Copy link
Member

feross commented Sep 19, 2019

We could just make a notice that it is a requirement to have rpm > 4.13 to use the installer?

Yeah, this sounds good to me. Right now there's no .rpm support at all, so this is an improvement :)

We should probably make a page with detailed installation options for all the different OSes. Even on Mac, it's possible to install via Homebrew casks, and there's lots of options on Linux. I made an issue about this: #1706

@feross feross merged commit 88e7167 into webtorrent:master Sep 19, 2019
@hicom150 hicom150 deleted the add_package_rpm branch September 19, 2019 19:57
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