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

Update .deb package dependencies #19358

Merged
merged 3 commits into from May 23, 2019

Conversation

Projects
None yet
3 participants
@Arcanemagus
Copy link
Member

commented May 17, 2019

Requirements for Contributing a Bug Fix

Identify the Bug

First reported in atom/ci#90, it seems that we aren't reporting the correct dependencies list in our .deb package files.

Description of the Change

This PR:

  • Replaces the gconf2 and gconf-service dependencies with libgconf-2-4 (>= 3.2.5) | libgconf2-4
    • Originally added in f431bb6 as a blanket list from the webupd8 package of Atom at the time, these were likely mistaken attempts at the correct dependency.
    • electron-installer-debian lists libgconf-2-4 | libgconf2-4, we are using the slightly more restrictive version range from Chrome 64.0.3282.167-1 documented here.
  • Removes the libudev0 | libudev1 dependency
    • This was also added as part of f431bb6. Our current binary doesn't link against this so if it was used in the past it is no longer required.
  • Removes the libcap2 dependency
    • This was added in response to #7066, however no verification appears to have been done that this was actually necessary, and our current binary doesn't link against this library.

Alternate Designs

The first point there is a mistake that definitely should be requested. The latter two removals of libudev0 | libudev1 and libcap2 could simply be left in though as it's possible some other part requires them.

Possible Drawbacks

One of the removed dependencies may actually be required, leading to systems that don't have them installed from some other source having issues running Atom without manual installation of the dependencies first.

Verification Process

I manually ran through the dependencies that would be generated by electron-installer-debian for an Electron 2.x application and came up with the following list:

  • libgtk-3-0
  • libnotify4
  • libnss3
  • libxss1
  • libxtst6
  • xdg-utils
  • libgconf-2-4 | libgconf2-4
    • This is only required for Electron 2.x applications and should be removed once we migrate to Electron 3+
  • The following are required for Electron's trash implementation, but none appear to currently be listed as Atom dependencies
    • kde-cli-tools
    • kde-runtime
    • trash-cli
    • libglib2.0-bin
    • gvfs-bin

Excluding the dependencies listed in that for a generic Electron application we are left with:

  • libcurl3 | libcurl4
    • Needed for dugite, see #18201
  • git
    • Needed for APM, see #4668
  • libxkbfile1
  • libasound2 (>= 1.0.16)
  • libx11-xcb1 # From Chrome .deb, see #16812 (comment)
  • libgcrypt11 | libgcrypt20
    • The need for this is unclear, but it is a dependency of the binary so it should remain listed
    • Originally added in f431bb6 and expanded in 9b20239
  • python
    • This was Atom's first ever listed dependency... but with no reasoning attached. It's likely this is listed for APM.

After reviewing the list of dependencies and making the changes in this PR I ran through the build process manually in an atom/atom-docker-ci build instance and got a package with the following dependencies:

# dpkg -I out/atom-amd64.deb
 new Debian package, version 2.0.
 size 119913752 bytes: control archive=640 bytes.
     648 bytes,    12 lines      control
 Package: atom-dev
 Version: 1.39.0-dev-3945fd864
 Depends: git, libgconf-2-4 (>= 3.2.5) | libgconf2-4, libgtk-3-0 (>= 3.9.10), libgcrypt11 | libgcrypt20, libnotify4, libxtst6, libnss3 (>= 2:3.22), python, gvfs-bin, xdg-utils, libx11-xcb1, libxss1, libasound2 (>= 1.0.16), libxkbfile1, libcurl3 | libcurl4
 Recommends: lsb-release
 Suggests: libsecret-1-0, gir1.2-gnomekeyring-1.0
 Section: devel
 Priority: optional
 Architecture: amd64
 Installed-Size: 595808
 Maintainer: GitHub <atom@github.com>
 Description: A hackable text editor for the 21st Century.
  Atom is a free and open source text editor that is modern, approachable, and hackable to the core.

This has not been tested in a full Debian based linux distribution, only minimal ones within Docker!

Release Notes

N/A

Arcanemagus added some commits May 17, 2019

🐛 Replace gconf2 with libgconf-2-4
The `gconf2` and `gconf-service` dependencies originally were added in 
f431bb6 from the WebUpd8 version of the Atom .deb file. It looks like 
although they seemed to work for the most part, they were likely 
incorrect originally, and certainly are now.

This commit switches these over to the `libgconf-2-4 | libgconf2-4` 
dependency listed in electron-installer-debian and Chrome.
🔥 Remove libudev dependency
This is another dependency that came from the old WebUpd8 package in 
f431bb6, however this one doesn't seem like it was necessary in the 
first place, or if it wass the `atom` binary no longer requires it.
🔥 Remove libcap2
This was added at the request of a user in #7066 with no reasoning 
behind it as far as I can tell. Since the current Atom binary doesn't 
depend on this library it should be removed.
@50Wliu

This comment has been minimized.

Copy link
Member

commented May 20, 2019

  • This is only required for Electron 2.x applications and should be removed once we migrate to Electron 3+

We are now on Electron 3.

@Arcanemagus Arcanemagus force-pushed the la/deb-dependencies branch from 4073891 to 3945fd8 May 21, 2019

@Arcanemagus

This comment has been minimized.

Copy link
Member Author

commented May 21, 2019

Removed the removal of libgconf-2-4 as the Electron 3 upgrade was reverted.

@jasonrudolph
Copy link
Member

left a comment

@Arcanemagus: I can't claim expertise in Linux dependency management, but I've reviewed the pull request description and diff, and I'm comfortable moving forward with this change if you are. Feel free to merge. 👍

@Arcanemagus Arcanemagus merged commit 85dcc6f into master May 23, 2019

2 checks passed

Atom Pull Requests #20190517.9 succeeded
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details

@Arcanemagus Arcanemagus deleted the la/deb-dependencies branch May 23, 2019

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.