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 additional dependencies for linux #16812

Merged
merged 2 commits into from Mar 5, 2018

Conversation

Projects
None yet
4 participants
@ungb
Contributor

ungb commented Feb 21, 2018

From @Arcanemagus:

I can confirm from building a minimal install for CI purposes that libxss1 and libasound2 are required. The base image that I am using there has libx11-xcb1 and libxkbfile1 installed automatically already, still checking that those aren't included transitively from another dependency.

Note that this fixes:
#4114 (Supposedly fixed in f431bb6, but at minimum libasound2 was reported there but missed)
#13591 (Unstated, but supposedly an Ubuntu 16.04 install of some sort)
#16659 (Some minimal Ubuntu base image)

/cc @Arcanemagus

Bryant Ung
@daviwil

This comment has been minimized.

Member

daviwil commented Feb 21, 2018

Any idea what changes caused these dependencies to be needed?

@Arcanemagus

This comment has been minimized.

Contributor

Arcanemagus commented Feb 21, 2018

I think they have always been needed, it's just they are installed by default in most Desktop distributions.

I can confirm from building a minimal install for CI purposes that libxss1 and libasound2 are required. The base image that I am using there has libx11-xcb1 and libxkbfile1 installed automatically already, still checking that those aren't included transitively from another dependency.

Note that this fixes:
#4114 (Supposedly fixed in f431bb6, but at minimum libasound2 was reported there but missed)
#13591 (Unstated, but supposedly an Ubuntu 16.04 install of some sort)
#16659 (Some minimal Ubuntu base image)

@Arcanemagus

This comment has been minimized.

Contributor

Arcanemagus commented Feb 21, 2018

Ah, looks like I'm getting libx11-xcb1 and libxkbfile1 from bringing in xvfb.

@Arcanemagus

This comment has been minimized.

Contributor

Arcanemagus commented Feb 21, 2018

Note that this is the list from the Chrome 64.0.3282.167-1 .deb:

Depends: ca-certificates, fonts-liberation, gconf-service, libappindicator1, libasound2 (>= 1.0.16), libatk-bridge2.0-0 (>= 2.5.3), libatk1.0-0 (>= 1.12.4), libc6 (>= 2.15), libcairo2 (>= 1.6.0), libcups2 (>= 1.4.0), libdbus-1-3 (>= 1.1.4), libexpat1 (>= 2.0.1), libfontconfig1 (>= 2.11), libgcc1 (>= 1:3.0), libgconf-2-4 (>= 3.2.5), libgdk-pixbuf2.0-0 (>= 2.22.0), libglib2.0-0 (>= 2.28.0), libgtk-3-0 (>= 3.9.10), libnspr4 (>= 2:4.9-2~), libnss3 (>= 2:3.22), libpango-1.0-0 (>= 1.14.0), libpangocairo-1.0-0 (>= 1.14.0), libstdc++6 (>= 4.6), libx11-6 (>= 2:1.4.99.1), libx11-xcb1, libxcb1 (>= 1.6), libxcomposite1 (>= 1:0.3-1), libxcursor1 (>> 1.1.2), libxdamage1 (>= 1:1.1), libxext6, libxfixes3, libxi6 (>= 2:1.2.99.4), libxrandr2 (>= 2:1.2.99.3), libxrender1, libxss1, libxtst6, lsb-release, wget, xdg-utils (>= 1.0.2)

Note that it includes the following:

libxss1
libx11-xcb1
libasound2 (>= 1.0.16)

Since Atom runs on Electron, which is based on Chrome, I'd say that's a pretty good indicator that these are needed 😉.

Edit: libxkbfile1 isn't in the list from Chrome, so I'd say that's the only one that there is any question on.

@daviwil

This comment has been minimized.

Member

daviwil commented Feb 22, 2018

Thanks a lot for all the background details! This all sounds great to me and the changes look fine.

@Ben3eeE

This comment has been minimized.

Member

Ben3eeE commented Feb 22, 2018

Edit: libxkbfile1 isn't in the list from Chrome, so I'd say that's the only one that there is any question on.

atom-keymap and keyboard-layout use libxkbfile since Atom 1.12 or 1.13 https://github.com/atom/atom-keymap/pull/152/files and https://github.com/atom/keyboard-layout/pull/18/files

@@ -1,6 +1,6 @@
Package: <%= appFileName %>
Version: <%= version %>
Depends: git, gconf2, gconf-service, libgtk2.0-0, libudev0 | libudev1, libgcrypt11 | libgcrypt20, libnotify4, libxtst6, libnss3, python, gvfs-bin, xdg-utils, libcap2
Depends: git, gconf2, gconf-service, libgtk2.0-0, libudev0 | libudev1, libgcrypt11 | libgcrypt20, libnotify4, libxtst6, libnss3, python, gvfs-bin, xdg-utils, libcap2, libx11-xcb1, libxss1, libasound2, libxkbfile1

This comment has been minimized.

@Arcanemagus

Arcanemagus Feb 22, 2018

Contributor

I would change the libasound2 dependency to have the same minimum version as the Chrome package: libasound2 (>= 1.0.16).

This comment has been minimized.

@Arcanemagus

Arcanemagus Feb 22, 2018

Contributor

On that note, you may want to add a minimum version to the following:

  • libnss3 -> libnss3 (>= 2:3.22)
Bryant Ung
@ungb

This comment has been minimized.

Contributor

ungb commented Feb 22, 2018

I've made the changes requested. Probably won't have much more time this week to test this. I didn't see any issues on ubuntu stock when testing on there, but of course, these deps are already on the system. I wanted to test this on the docker scenario but didn't know how to put the .deb file in a place where you can wget it.

This isn't super urgent but I'll try to get around to it next week.

@Arcanemagus

This comment has been minimized.

Contributor

Arcanemagus commented Feb 22, 2018

You can test it locally using COPY in the Dockerfile (or by firing up a minimal VM 😆).

@Arcanemagus

This comment has been minimized.

Contributor

Arcanemagus commented Feb 22, 2018

Looks like Travis is having some network issues.

Thanks for getting this put up btw!

@Arcanemagus Arcanemagus referenced this pull request Mar 2, 2018

Merged

Update to CircleCI 2.0 #82

@daviwil

This comment has been minimized.

Member

daviwil commented Mar 5, 2018

Restarting the TravisCI build to see if we can get this merged :)

@daviwil daviwil merged commit 2756c65 into master Mar 5, 2018

3 checks passed

ci/circleci Your tests passed on CircleCI!
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@daviwil daviwil deleted the ungb-deb-dependency branch Mar 5, 2018

@Arcanemagus Arcanemagus added the linux label Mar 23, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment