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.

Sign up for GitHub

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

Jump to bottom

Override executable name on Linux #371

Merged
merged 1 commit into from
Aug 22, 2018
Merged

Override executable name on Linux #371

merged 1 commit into from
Aug 22, 2018

Conversation

pronebird
Copy link
Contributor

@pronebird pronebird commented Aug 22, 2018 •
edited by faern
Loading

GitHub issue: electron-userland/electron-builder#1895

Describe what this PR changes. Why this is wanted. And, if needed, how it does it.

Git checklist:

What

This PR overrides the name of executable produced on Linux.

Why

Because electron-builder defaults the executable name on Linux to the name field from package.json, as opposed to using productName as per documentation. This results into app being called desktop which is undesired.

How

Via electron-builder.yml configuration.

Testing

I would appreciate if you could test it on Linux. I'll do the same on Windows/macOS to verify we don't have regressions in how executables are called there after migration to workspaces.

Please git checkout and build this branch to verify that executable name is correct.

git fetch
git checkout fix-executable-name-on-linux
./build.sh --dev-build

This change is Reviewable

faern
faern approved these changes Aug 22, 2018
View reviewed changes
Copy link
Member

@faern faern left a comment

Choose a reason for hiding this comment

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

Tested and works on Linux.

Reviewed 1 of 1 files at r1.
Reviewable status: 0 of 1 files reviewed, all discussions resolved (waiting on @jvff and @faern)

faern
faern approved these changes Aug 22, 2018
View reviewed changes
Copy link
Member

@faern faern left a comment

Choose a reason for hiding this comment

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

:lgtm:

Reviewable status: 0 of 1 files reviewed, all discussions resolved (waiting on @jvff and @faern)

jvff
jvff suggested changes Aug 22, 2018
View reviewed changes
Copy link
Contributor

@jvff jvff left a comment

Choose a reason for hiding this comment

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

Reviewed 1 of 1 files at r1.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @pronebird)

a discussion (no related file):
The RPM package still seems to use the desktop name. I saw this through the command:

$ rpm -qp dist/MullvadVPN-2018.2_x86_64.rpm                                                                                                                                                                                                                                                                                                                                                
desktop-2018.2.0-1.x86_64

@pronebird pronebird force-pushed the fix-executable-name-on-linux branch from ca56cdb to ce5686f Compare August 22, 2018 11:26
pronebird
pronebird commented Aug 22, 2018
View reviewed changes
Copy link
Contributor Author

@pronebird pronebird 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: all files reviewed, 1 unresolved discussion (waiting on @pronebird)

a discussion (no related file):

Previously, jvff (Janito Vaqueiro Ferreira Filho) wrote…

The RPM package still seems to use the desktop name. I saw this through the command:

$ rpm -qp dist/MullvadVPN-2018.2_x86_64.rpm                                                                                                                                                                                                                                                                                                                                                
desktop-2018.2.0-1.x86_64

😿

@pronebird pronebird force-pushed the fix-executable-name-on-linux branch from 094cd89 to 4ed10db Compare August 22, 2018 14:08
pronebird
pronebird commented Aug 22, 2018
View reviewed changes
Copy link
Contributor Author

@pronebird pronebird 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 1 files reviewed, 1 unresolved discussion (waiting on @jvff, @faern, and @pronebird)

a discussion (no related file):

Previously, pronebird (Andrei Mihailov) wrote…

😿

Alright, instead I override extraMetadata now which is basically merged with package.json:

{ ...packageJson, ...extraMetadata }

Wish me luck.

jvff
jvff approved these changes Aug 22, 2018
View reviewed changes
Copy link
Contributor

@jvff jvff left a comment

Choose a reason for hiding this comment

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

Reviewed 1 of 1 files at r2.
Reviewable status: 0 of 1 files reviewed, all discussions resolved (waiting on @faern and @jvff)

pronebird
pronebird commented Aug 22, 2018
View reviewed changes
Copy link
Contributor Author

@pronebird pronebird 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 1 files reviewed, all discussions resolved (waiting on @faern and @jvff)

a discussion (no related file):

Previously, pronebird (Andrei Mihailov) wrote…

Alright, instead I override extraMetadata now which is basically merged with package.json:

{ ...packageJson, ...extraMetadata }

Wish me luck.

$ rpm -qp MullvadVPN-2018.2-dev-4ed10dbb_x86_64.rpm 
mullvad-vpn-2018.2.0_dev_4ed10dbb-1.x86_64

jvff
jvff approved these changes Aug 22, 2018
View reviewed changes
Copy link
Contributor

@jvff jvff 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 1 files reviewed, all discussions resolved (waiting on @faern and @jvff)

pronebird
pronebird commented Aug 22, 2018
View reviewed changes
Copy link
Contributor Author

@pronebird pronebird 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 1 files reviewed, all discussions resolved (waiting on @faern and @jvff)

a discussion (no related file):

Previously, pronebird (Andrei Mihailov) wrote…
$ rpm -qp MullvadVPN-2018.2-dev-4ed10dbb_x86_64.rpm 
mullvad-vpn-2018.2.0_dev_4ed10dbb-1.x86_64

 dpkg --info MullvadVPN-2018.2-dev-4ed10dbb_amd64.deb 
 new Debian package, version 2.0.
 size 50324754 bytes: control archive=3529 bytes.
      43 bytes,     1 lines      conffiles            
     416 bytes,    13 lines      control              
    6501 bytes,    93 lines      md5sums              
     107 bytes,     4 lines   *  postinst             #!/usr/bin/env
     921 bytes,    40 lines   *  postrm               #!/usr/bin/env
 Package: mullvad-vpn
 Version: 2018.2.0-dev-4ed10dbb
 License: GPL-3.0
 Vendor: Mullvad VPN <[email protected]>
 Architecture: amd64
 Maintainer: Mullvad VPN <[email protected]>
 Installed-Size: 182195
 Depends: gconf2, gconf-service, libnotify4, libappindicator1, libxtst6, libnss3, libxss1
 Section: default
 Priority: extra
 Homepage: https://github.com/mullvad/mullvadvpn-app#readme
 Description: 
   Mullvad VPN client

jvff
jvff approved these changes Aug 22, 2018
View reviewed changes
Copy link
Contributor

@jvff jvff 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 1 files reviewed, all discussions resolved (waiting on @faern and @jvff)

@pronebird pronebird force-pushed the fix-executable-name-on-linux branch from 4ed10db to d4c4283 Compare August 22, 2018 14:27
jvff
jvff approved these changes Aug 22, 2018
View reviewed changes
Copy link
Contributor

@jvff jvff 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 1 files reviewed, all discussions resolved (waiting on @faern and @jvff)

faern
faern approved these changes Aug 22, 2018
View reviewed changes
Copy link
Member

@faern faern left a comment

Choose a reason for hiding this comment

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

:lgtm:

Reviewed 1 of 1 files at r2.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved

@pronebird pronebird force-pushed the fix-executable-name-on-linux branch from d4c4283 to 9ae25d6 Compare August 22, 2018 14:42
faern
faern approved these changes Aug 22, 2018
View reviewed changes
Copy link
Member

@faern faern 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: :shipit: complete! all files reviewed, all discussions resolved

@pronebird pronebird merged commit 9ae25d6 into master Aug 22, 2018
@pronebird pronebird deleted the fix-executable-name-on-linux branch August 22, 2018 15:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@faern faern faern approved these changes

@jvff jvff jvff approved these changes

Assignees

@faern faern

@jvff jvff

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@pronebird @faern @jvff