Skip to content
This repository was archived by the owner on Mar 3, 2023. It is now read-only.

Conversation

@mfonville
Copy link
Contributor

Addendum to PR #22015 to also fix the dependencies for RPM packages.
Some dependencies could be done through package names, but some had to be specified by its libraries because Fedora and OpenSuse etc don't stick to the same packagenames.

Does need testing/confirmation that it is correct.

@mfonville
Copy link
Contributor Author

@DeeDeeG Could you please test if this works for you on a bare RPM-based virtual machine?

@mfonville
Copy link
Contributor Author

@DeeDeeG did you already find some time to test these dependencies on your VM?

@DeeDeeG
Copy link
Contributor

DeeDeeG commented May 12, 2021

Hi @mfonville. I am not totally sure where to get a "minimal" Fedora environment that can also run the GUI app and confirm it really works. I can build this branch and see if it will install on Fedora Workstation (https://getfedora.org). That has a fair amount of stuff already on it. But it would be some sort of validation, at least.

@DeeDeeG
Copy link
Contributor

DeeDeeG commented May 13, 2021

Here's what I get when I try to install the built .RPM:

$ sudo dnf install ./out/atom.x86_64.rpm 
[ . . . ]
Error: 
 Problem: conflicting requests
  - nothing provides or needed by atom-dev-1.59.0.dev.94d1e7558-0.1.fc34.x86_64
  - nothing provides libX11-xcb.so.6()(64bit) needed by atom-dev-1.59.0.dev.94d1e7558-0.1.fc34.x86_64
  - nothing provides libcurl.so.3()(64bit) needed by atom-dev-1.59.0.dev.94d1e7558-0.1.fc34.x86_64
(try to add '--skip-broken' to skip uninstallable packages)

Edit to add: If you could give some insight how you found these, it would satisfy my curiosity, and hey maybe it would give me a hint of how better to test the end result. But anyway, I hope that info is helpful.

@DeeDeeG
Copy link
Contributor

DeeDeeG commented May 13, 2021

Ah, okay. I see that libcurl.so.3 or libcurl.so.4 is in the spec file now. So in theory it should be okay with either version. I looked up how to make that work and (after a while of looking) finally found this: https://serverfault.com/questions/299179/can-one-require-this-or-that-package-in-an-rpm-spec-file --> https://rpm.org/user_doc/boolean_dependencies.html.

So from reading that, I believe the logical or requires enclosing in parentheses: (option_a or option_b).

@mfonville
Copy link
Contributor Author

Thanks @DeeDeeG

I will add the parentheses. I determined the libraries by compiling atom on an a very bare install, and closely tracked which libraries where required in the linking process (adding them one by one).

With respect to the libX11-xcb.so.6 it is definitely libX11-xcb that was required, I think I made a mistake with the number during revising, I will fix that one too.

@mfonville
Copy link
Contributor Author

I now get a build error with rpmbuild on the CI. Apparently the parentheses don't work for (this version of rpmbuild) to specify our OR-statement. Will do some further reading on this

@mfonville
Copy link
Contributor Author

Apparently rpm is 4.12 in Ubuntu Xenial, and we do need 4.13 or higher :-/

Any newer version of Ubuntu than Xenial should fix this. Also I'd like to point out that Xenial lost support two weeks ago (April 2021, see https://wiki.ubuntu.com/Releases )

@sadick254 @darangi can we please upgrade the Ubuntu LTS we are building Atom on from Xenial to either Bionic or Focal?

@DeeDeeG
Copy link
Contributor

DeeDeeG commented May 13, 2021

I think it could be advantageous to see what it takes to get newer rpm on Xenial, rather than moving to Bionic quite yet. Due to compatibility reasons, explained below.


When you build on newer Ubuntu, you build C code against newer glibc(GNU C Library).

Stuff built for older glibc is runtime compatible with newer systems, but the reverse is not true; stuff built with newer glibc is not runtime compatible with older systems.

So the older the OS we build on, the broader compatibility. (And the older our compiler, so yeah it does probably slightly improve performance to build on newer OSes... There is a tradeoff. At least we are only one or two major versions behind for GCC/Clang.)

I think a couple of weeks might be a bit early, since a few stragglers might still be on Xenial. There is also the matter that different distros stabilize at different times (and lock in different glibc versions). I read a blog post indicating that Ubuntu 18.04 has too new a gibc for back compatibility with Debian 9, which is still a supported distro. (I lost the link or I would post it. Apparently ldd --version will reveal what glibc you have on your system.)

Though, theoretically, having a cut-off date where we move over to Bionic might be a good idea, so as to not put it off forever.

My two cents.

@mfonville
Copy link
Contributor Author

@DeeDeeG I have posted my response on that consideration here: #22416 (comment)

If there is another method of installing an updated rpm package that would also be a possibility. E.g. fetching the .deb from a newer Ubuntu release.

Does Atom btw have any telemetry data on which platform versions of the users? Because I wonder how many users of Atom would be on releases that are older than the latest stable/LTS release.

@DeeDeeG
Copy link
Contributor

DeeDeeG commented May 13, 2021

Does Atom btw have any telemetry data on which platform versions of the users? Because I wonder how many users of Atom would be on releases that are older than the latest stable/LTS release.

If this documentation is right, it looks like Atom's telemetry (only) includes OS info via the Chrome User Agent string.

Unfortunately that is not very fine-grained about what Linux you're using. Here's a Chromium UA string from Xenial in VirtualBox:

Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/90.0.4430.72 Safari/537.36

The bit that identifies the OS/system is just X11; Linux x86_64. As far as I can see, that could be any x86_64 Linux at all.

It is more useful on macOS and Windows, I believe. Chrome on macOS gives you the major_minor_patch version of the OS. Chrome on Windows apparently gives the NT version which you can translate to Windows 10, 8, 7, and so on.

@DeeDeeG
Copy link
Contributor

DeeDeeG commented May 13, 2021

If there is another method of installing an updated rpm package that would also be a possibility. E.g. fetching the .deb from a newer Ubuntu release.

Here is the list of URLs to the .debs that we would need, in order to get newer rpm (and its dependencies) installed on Xenial (in my testing in VirtualBox):

mirrors.kernel.org/ubuntu/pool/universe/r/rpm/debugedit_4.14.1+dfsg1-2_amd64.deb
security.ubuntu.com/ubuntu/pool/main/e/elfutils/libdw1_0.170-0.4ubuntu0.1_amd64.deb
security.ubuntu.com/ubuntu/pool/main/e/elfutils/libelf1_0.170-0.4ubuntu0.1_amd64.deb
mirrors.kernel.org/ubuntu/pool/main/x/xz-utils/liblzma5_5.2.2-1.3_amd64.deb
mirrors.kernel.org/ubuntu/pool/universe/r/rpm/librpm8_4.14.1+dfsg1-2_amd64.deb
mirrors.kernel.org/ubuntu/pool/universe/r/rpm/librpmbuild8_4.14.1+dfsg1-2_amd64.deb
mirrors.kernel.org/ubuntu/pool/universe/r/rpm/librpmio8_4.14.1+dfsg1-2_amd64.deb
mirrors.kernel.org/ubuntu/pool/universe/r/rpm/librpmsign8_4.14.1+dfsg1-2_amd64.deb
security.ubuntu.com/ubuntu/pool/main/libz/libzstd/libzstd1_1.3.3+dfsg-2ubuntu1.2_amd64.deb
mirrors.kernel.org/ubuntu/pool/universe/r/rpm/rpm2cpio_4.14.1+dfsg1-2_amd64.deb
mirrors.kernel.org/ubuntu/pool/universe/r/rpm/rpm_4.14.1+dfsg1-2_amd64.deb
mirrors.kernel.org/ubuntu/pool/universe/r/rpm/rpm-common_4.14.1+dfsg1-2_amd64.deb
Dependency tree (click to expand)

We could fetch these and install them before the build. (These are the set of rpm + dependencies from the Bionic repos, by the way).

Quick shell script to download and install these... (click to expand)
#!/bin/bash

mkdir bionic_rpm_debs && cd bionic_rpm_debs

for deb_URL in `cat path/to/deb_URLs.txt`
do
  echo "Downloading ${deb_URL}..."
  curl -L "${deb_URL}" -O
done

sudo apt install -y ./*.deb
cd ..

I'm looking into the files of them to check the lincenses; So far all are GPL, and their file sizes are small, so we could probably check them in to this repo. That secures against deprecation/archiving of the Bionic repos (at which point these URLs might go down).

Edit: I opened a PR targeting your branch that does this. See: mfonville#2

@DeeDeeG
Copy link
Contributor

DeeDeeG commented May 14, 2021

I get this message after installing the .rpm from my PR branch in the latest Fedora Docker image and trying to do atom --version:

/usr/share/atom-dev/atom: error while loading shared libraries: libXcomposite.so.1: cannot open shared object file: No such file or directory

Looks like libXcomposite.so.1 is missing.

More missing:

  • libXcursor.so.1
  • libXdamage.so.1

These are a bunch of X11 shared objects/libraries. Was there a package that used to pull these all in for us? I think an overall X11 package should do to pull all these in, if possible..

More:

  • libatk-1.0.so.0
  • libatk-bridge-2.0.so.0
    ...
  • libgtk-3.so.0
  • libdrm.so.2
  • libgbm.so.1

Then with all of those added, it worked. (Docker images are super super minimal. I wouldn't guess that most of this is realistically likely to be missing on a real desktop OS.)

@mfonville
Copy link
Contributor Author

Thanks for the input @DeeDeeG

When I was figuring out the dependencies I was doing it through Ubuntu/Debian, and these libraries were then pulled in by other 'higher' dependencies that I declared.
Apparently that doesn't work on Fedora, I will take a look again at how their dependencies are organised and which inter-dependencies there are between these libraries, so that with as small set as possible the correct (and minimal) set of dependencies are resolved.

@mfonville
Copy link
Contributor Author

mfonville commented May 14, 2021

I think we can solve it by requiring gtk3 and libgbm and drop the libX11-xcb

gtk3 will pull in all the libatk* dependencies and the libX*-libraries;
the libX*-libraries will pull in libX11, and libX11 will take care of the libxcb dependency (and by libxcb-dri3)
libatk* stuff will pull in any glib dependencies.

libgbm will take care of any libdrm dependencies.

I will try to apply these changes and update this PR

update: not in all distros the name libgbm is used as the "provides", I will take care of using the proper (library)naming to make it work with all major rpm-based distros.

update2: also we can drop libXtst.so.6 as it will be pulled in by the libatk* stuff.

@mfonville
Copy link
Contributor Author

@DeeDeeG can you test my latest changes?

@DeeDeeG
Copy link
Contributor

DeeDeeG commented May 14, 2021

Fedora ✅

Nice. Testing on the latest Fedora Docker image again. This installs, and I can do atom-dev --version and it "just works" ™️ .

Note: I even installed the old one + all the extra libs it needed, then reinstalled this over it, and it didn't add any new package requirements. So this updated list of dependencies is still pretty lean. 👍

openSUSE Leap ❌

I get this:

Problem: nothing provides libgcrypt needed by atom-dev-1.57.0.dev.fb2b2da7-0.1.x86_64

Although if I press i to ignore that during install, atom-dev --version works fine. Of course I'm not comprehensively testing the GUI app, just seeing if the CLI --version command will at least run...

Note: gtk3 isn't a real package on openSUSE. There is libgtk-3-0 instead. But it turns out libgtk-3-0 Provides: gtk3, so it "Just Works" ™️ . Hmm, maybe I've been presuming an issue about different package names, but some of them have good Provides: for compatibility with Fedora/RHEL/CentOS packages.

Now that I have thought of a way of testing openSUSE (that is: in Docker), we can properly test where package names might be usable on openSUSE, rather than shared object filenames. But obviously the .so dependencies do work, so no need to go in and re-do any of that if you don't feel like it.

@mfonville
Copy link
Contributor Author

mfonville commented May 14, 2021

I had been checking the support for openSUSE too when I made my changes :-)
So I had already checked on the "Provides" with gtk3 ;-)

I had not touched the libgcrypt but I see now that they indeed have 2 different names, I will update the PR again

PS: when making my PR I checked all my changes at https://pkgs.org/ to confirm compatibility with OpenSUSE, Fedora and Red Hat. That is why I had to work with .so to make some work, when they are the only common denominator between the distros.

@mfonville
Copy link
Contributor Author

Ok, instead of going completely to Bionic I now made this PR, to just upgrade rpm to the bionic version: #22433
If that PR can be merged first, than this PR should also be good to go if it all works on @DeeDeeG 's Docker images

@mfonville
Copy link
Contributor Author

@DeeDeeG Did you test the latest version at your OpenSUSE?

@DeeDeeG
Copy link
Contributor

DeeDeeG commented May 17, 2021

Oops, yes I think it worked. I can try again. (Juggling too many things, I think.)

@mfonville
Copy link
Contributor Author

If you could verify to be sure, please :-)

@DeeDeeG
Copy link
Contributor

DeeDeeG commented May 17, 2021

Wider testing this time. I didn't expect it would make a difference, but it actually did.

  • openSUSE Leap 15.2 (latest) ✔️
  • openSUSE Leap 15.3 (development preview) ❌
    • Missing libasound.so.2
  • openSUSE Tumbleweed (rolling) ❌
    • Missing libasound.so.2
  • Fedora 32 (oldest supported) ❌
    • Missing libasound.so.2 and libX11-xcb.so.1
  • Fedora 33 ❌
    • Missing libasound.so.2 and libX11-xcb.so.1
  • Fedora 34 (latest) ✔️
  • Fedora 35 (development preview) ✔️

Something should be done about libasound.so.2 and libX11-xcb.so.1.


It should be possible to include these dependencies as package names if you want, considering the following details (although I leave it to your judgment):

  • libasound.so.2 --> alsa-lib
    • alsa-lib on Fedora, libasound2 on openSUSE, but libasound2 has Provides: alsa-lib.
  • libX11-xcb.so.1 --> libX11-xcb
    • libX11-xcb on Fedora, libX11-xcb1 on openSUSE, but not missing on openSUSE so non-matching package names arguably irrelevant. The openSUSE package does not have Provides: for the Fedora package name in this case.

@mfonville
Copy link
Contributor Author

mfonville commented May 17, 2021

Pfff, they make it quite a puzzle.
I also checked and I agree with the alsa-lib.

but with the libX11-xcb.so.1 I don't know why you run into the error on Fedora. Because libX11-xcb.so.1 is specified in the Provides for all Fedora and OpenSUSE releases. Maybe some other mistake somewhere created this unresolved dependency?

Update: We don't even specify libX11-xcb.so.1, something else is going on, I will further check it out.

Update2: Neither do we specify libasound.so.2, I think this is generated somewhere?

Update3: AutoReqProv: no is specified, so no automatic dependencies should be added by rpmbuild. Could it be that maybe your Docker images are missing the repositories to find and install these libraries?

mfonville added 2 commits May 17, 2021 23:05
Require gtk3 and libgbm(library), drop their lower dependencies
@mfonville
Copy link
Contributor Author

I pushed now a rebase on master, that has the updated rpm from bionic, to see if it passes. @DeeDeeG maybe you can retry with the new artifacts that will be published by the CI.

@DeeDeeG
Copy link
Contributor

DeeDeeG commented May 18, 2021

maybe your Docker images are missing the repositories to find and install these libraries?

I can install the libraries individually with dnf install [package-name-here] or dnf install [so-filename-here]. I think the indirect dependencies just don't pull in libX11-xcb/libasound with the current setup on certain distros/releases. Something requires it on Fedora 34 but not Fedora 33 (I guess.) Trying to follow the exact chain of dependencies feels too hard in this case. It's like being lost in a maze.

I believe these need to be added to the spec file explicitly, or they will be missing... in older Fedora/newer openSUSE Docker images, anyway.

Maybe it's time to call it "good enough"? Recall that Atom requires a graphical desktop to be meaningfully usable. And as far as I know any graphical desktop will already have these last bits installed. That was likely true of most, if not all of these basic libraries we have been trying to include for the past several comments. The alternative I think is to add them to the spec file.

I pushed now a rebase on master, that has the updated rpm from bionic, to see if it passes. @DeeDeeG maybe you can retry with the new artifacts that will be published by the CI.

I see that CI runs for PRs do not publish .deb or .rpm packages. I made my own CI run manually: https://dev.azure.com/DeeDeeG/b/_build/results?buildId=1121

@mfonville
Copy link
Contributor Author

can install the libraries individually with dnf install [package-name-here] or dnf install [so-filename-here]. I think the indirect dependencies just don't pull in libX11-xcb/libasound with the current setup on certain distros/releases. Something requires it on Fedora 34 but not Fedora 33 (I guess.) Trying to follow the exact chain of dependencies feels too hard in this case. It's like being lost in a maze.

Ah, then I first misunderstood. It will add them to the spec (they are also specified for Debian, so it is logical that they are required in this spec too)

@DeeDeeG
Copy link
Contributor

DeeDeeG commented May 18, 2021

Here are my testing results and a description of the test method. Conclusion: looks good to me.

Results:

  • openSUSE Leap 15.2 (latest) ✔️
  • openSUSE Leap 15.3 (development preview) ✔️
    • Note for testing: The 15.3 development preview repos are a bit broken, but this isn't Atom's fault. Atom can still be installed, and atom-dev --version still works. See footnote: [1]
  • openSUSE Tumbleweed (rolling) ✔️
  • Fedora 32 (oldest supported) ✔️
  • Fedora 33 ✔️
  • Fedora 34 (latest) ✔️
  • Fedora 35 (development preview) ✔️

Test info:

  • Ubuntu 20.04 host with Docker installed
    • (sudo apt install docker.io, may require a system restart).
  • sudo docker run --rm -it fedora:32
    • (Runs a temporary container that is immediately deleted upon exiting it, with an interactive virtual terminal started in the container.)
    • To be repeated with each of these base images fedora:32, fedora:33, fedora:34, fedora:35, opensuse/leap:15.2, opensuse/leap:15.3, opensuse/tumbleweed:latest
  • Run (copy/paste) the following script in each container:
dnf install -y curl unzip || zypper install -y curl unzip # dnf == package manager in Fedora, zypper == package manager in openSUSE
curl -L 'https://dev.azure.com/DeeDeeG/7690c340-f92e-4cfc-838c-5f2c3651b584/_apis/build/builds/1122/artifacts?artifactName=atom.x86_64.rpm&api-version=6.0&%24format=zip' -o _atom.zip
unzip _atom.zip
dnf install -y ./atom.x86_64.rpm/atom.x86_64.rpm || zypper install -y --allow-unsigned-rpm ./atom.x86_64.rpm/atom.x86_64.rpm
atom-dev --version --no-sandbox
/usr/share/atom-dev/atom --version --no-sandbox
apm-dev --version
  • Type the exit command to exit the temporary Docker container when you are done (If you want, you may poke around and run commands first, for example to investigate if something went wrong.)

It passes if the atom-dev --version --no-sandbox and apm-dev --version commands print version output with no errors. For example:

# atom-dev --version --no-sandbox
Atom    : 1.59.0-dev-f55dcdae
Electron: 9.4.4
Chrome  : 83.0.4103.122
Node    : 12.14.1
# apm-dev --version
apm  2.6.2
npm  6.14.13
node 12.14.1 x64
atom 1.59.0-dev-f55dcdae
python 
git 2.31.1

Note: Testing apm-dev --version is probably redundant. Feel free to focus on just the output from atom-dev --version --no-sandbox. See footnote: [2]

Footnotes:

[1] On openSUSE Leap 15.3 development preview, the repos are slightly broken, but I do not think this is related to Atom whatsoever.

Problem: nothing provides /usr/bin/pidof needed by lsb-4.0.fake-lp153.1.1.x86_64
 Solution 1: do not install atom-dev-1.59.0.dev.f55dcdae-0.1.x86_64
 Solution 2: break lsb-4.0.fake-lp153.1.1.x86_64 by ignoring some of its dependencies

(To install Atom: Run zypper without -y so you can manually respond to this prompt. Type 2 and press Enter to continue with Atom installation despite the unrelated broken package. This package being not broken is openSUSE's responsibility, not ours. It will undoubtedly be fixed before openSUSE Leap 15.3 becomes a stable release.)

[2] Checking if apm-dev --version works appears to be redundant. I can report that it worked every time atom-dev --version --no-sandbox worked, and if I recall correctly it worked even when atom-dev errored due to missingshared objects. In my experience, apm-dev --version doesn't reveal anything useful for this testing scenario. But it is colorful and nice to look at, breaking up the monotony of testing a little bit, and makes it unmistakable at a glance that the script has fully completed. That's why I included it in the script. And it would be revealing if it didn't work, I suppose. I think it has easier requirements than Atom itself, though.

Copy link
Contributor

@DeeDeeG DeeDeeG left a comment

Choose a reason for hiding this comment

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

Looks good to me (installs properly in Fedora and openSUSE, without any missing dependencies as far as I can tell).

@mfonville
Copy link
Contributor Author

Thanks for all the testing @DeeDeeG
the /usr/bin/pidof is clearly something broken within OpenSUSE development version itself, and not Atom-related.

I am happy that all dependencies are now all good.

@sadick254 could you merge both this PR and #22015 ? Thank you!

Copy link
Contributor

@sadick254 sadick254 left a comment

Choose a reason for hiding this comment

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

Thank you @mfonville and @DeeDeeG for this. 👍

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants