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

Fix .deb install on ubuntu 20.04 #20406

Merged
merged 3 commits into from Apr 2, 2020
Merged

Fix .deb install on ubuntu 20.04 #20406

merged 3 commits into from Apr 2, 2020

Conversation

zorn-v
Copy link
Contributor

@zorn-v zorn-v commented Feb 12, 2020

Fix #20356

@mfonville
Copy link
Contributor

@mfonville mfonville commented Mar 9, 2020

With Ubuntu 20.04 release schedule in a month, wouldn't it be appropriate to merge this for a next release, so that all Ubuntu users have a sane upgrade path?

@DeeDeeG
Copy link
Contributor

@DeeDeeG DeeDeeG commented Mar 23, 2020

Exactly one month until Focal Fossa final release. See the release schedule for details:

https://wiki.ubuntu.com/FocalFossa/ReleaseSchedule

As it stands, atom cannot be installed on Ubuntu 20.04 (Focal Fossa). This PR would be an appropriate and easy solution, IMO.

@aminya
Copy link
Contributor

@aminya aminya commented Mar 27, 2020

Could you push a dummy commit? Apparently, CI build hasn't been triggered.

aminya
aminya approved these changes Mar 28, 2020
Copy link
Contributor

@aminya aminya left a comment

Windows x64 is failed because of the timeout, so there is no issue in terms of the tests.

@DeeDeeG
Copy link
Contributor

@DeeDeeG DeeDeeG commented Apr 1, 2020

CC-ing some active maintainers of this repo: @darangi @lkashef @Aerijo.

There is a packaging issue for Atom against the upcoming Ubuntu 20.04 LTS release. That will come out in just over three weeks. (The issue prevents installing Atom in Ubuntu 20.04.)

Please consider this small (just a few bytes of diff) Pull Request to fix the situation. Thank you.

@lkashef
Copy link
Contributor

@lkashef lkashef commented Apr 1, 2020

Thanks @zorn-v for your contribution 🙇‍♂️

The CI failed test seems to be unrelated and flaky so I will just give it a re-run and merge this in 👍

Thanks @aminya and @DeeDeeG for the support and the heads up.

@DeeDeeG
Copy link
Contributor

@DeeDeeG DeeDeeG commented Apr 1, 2020

Update April 17th: This comment turned out not to be true!

  • python-is-python3 does not Provide the python package. So it's irrelevant to atom.
  • atom includes apm, which currently bundles an old version of node-gyp that only supports Python 2, which must be on the PATH as python. Newer node-gyp should be specified at the apm repo to allow Python 2/Python 3 to both be supported.

Another heads-up: In Ubuntu 20.04, it will soon be possible to install a package "python-is-python3" which satisfies the python deb dependency with Python 3.x. In previous Ubuntu releases, running versionless python generally implied running Python 2.x

So, in theory (that is, if the python code Atom uses isn't Python 3 compatible), continuing to include the generic python as a valid dependency could break that part of Atom for users with python-is-python3 installed.

(Edit: Whereas removing the generic python as a dependency -- and only depending on python2 -- might break distros earlier than 19.04 Disco Dingo, which appears to be the first Ubuntu release that had a python2 package.) (Edit: Depending on simply python3 -- and dropping versionless python -- would be compatible with distros at least as far back as Ubuntu 16.04 Xenial Xerus.)

For what it's worth: according to this comment, Python is only used for node-gyp. A long-term solution to this Python 2.x/3.x ambiguity would be to confirm that node-gyp is compatible with Python 3... Or update the way Atom is packaged so the python (and/or python2/python3) dependency/ies can be dropped entirely from Atom's deb package.

Edit: Recent node-gyp does appear to support Python 3: https://github.com/nodejs/node-gyp#on-unix

I am not an expert with regard to python, node, etc. or how Atom is packaged for debian/Ubuntu. Just thought I should mention.

In summary: For the time-being, this PR solves the problem for the vast majority of users. Those who deliberately install python-is-python3 might or might not break node-gyp as bundled with Atom. (I don't know/understand this for sure, just interpreting the comment I linked above.) I believe this PR is a reasonable step forward, and much better than doing nothing. (The difference between most users being able to use Atom on Ubuntu 20.04, vs no-one on Ubuntu 20.04 without hacky workarounds).

@DeeDeeG
Copy link
Contributor

@DeeDeeG DeeDeeG commented Apr 1, 2020

So, eventually, it may make sense to switch the dependency from python to python3, and not use an "either-is-allowed" approach as in this PR with python | python2.

For now, removing the generic python dependency would drop support for debian Jessie.

(Jessie has versionless python but no python2 package (python2 in Jessie's package archives returns a 404 not found error...), and Jessie has python3 that is too old (3.4; node-gyp requires Python 2.7 for Python 2, or 3.5 and newer for Python 3).

(I suppose that if node-gyp can be un-bundled from this deb package, then perhaps the issue can be handled by dropping this package's dependency on any sort of python or python2 or pythonr3.)

Edit to add: Jessie will be EOL/not supported after the end of June 2020: https://www.debian.org/releases/jessie/

@zorn-v
Copy link
Contributor Author

@zorn-v zorn-v commented Apr 1, 2020

which satisfies the python deb dependency with Python 3.x

No, it is just symlink. There is no "Provides" section in that package, only "Breaks" and "Replaces".
You can check with apt show python-is-python3

Description: symlinks /usr/bin/python to python3
In Ubuntu, all python packages use explicit python3 or python2
interpreter and do not use unversioned /usr/bin/python at all. Some
third-party code is now predominantly python3 based, yet may use
/usr/bin/python.
.
This is a convenience package which ships a symlink to point
the /usr/bin/python interpreter at the current default python3. It may
improve compatibility with other modern systems, whilst breaking some
obsolete or 3rd-party software.

But anyway removing python2 from deps is good.

@DeeDeeG
Copy link
Contributor

@DeeDeeG DeeDeeG commented Apr 1, 2020

@zorn-v python-is-python3 will provide/satisfy the python package soon: https://lists.ubuntu.com/archives/ubuntu-devel-discuss/2020-March/018615.html

[EDIT: Hmm... On closer reading, Mathias only confirmed that python-is-python2 will have Provides: python. Maybe python-is-python3 won't. That would make sense, now that I think about it. It's still a little unclear.]

In my opinion, the python dependency should stay somewhere in the "Depends:" at least until Debian Jessie is end-of-life, which will be at the end of June.

There is a way to do this with no python2 dependency. I suppose it could be python | python3 (>= 3.5.0) | python2 to be most flexible. Or just python | python3 (>= 3.5.0) in order to prefer Python 3.x, and begin to phase out support for Python 2.x.

@mfonville
Copy link
Contributor

@mfonville mfonville commented Apr 1, 2020

I think upgrading from Python 2 to Python 3 for Atom's dependencies should be decoupled from fixing the current Python 2 dpkg dependency administration.

So I'd propose to merge in as soon as possible the change to the python | python2 dependency. That still uses Python 2.

And make a new PR that does all necessary testing whether Atom works with Python 3. If that all works fine, you could have until June 2020:
python | python3 (>= 3.5.0) to keep support Debian Jessie with Python 2
and after June 2020 you could change it to:
python3 (>= 3.5.0) (at that time all supported Ubuntu and Debian releases have Python3.5+)

@DeeDeeG
Copy link
Contributor

@DeeDeeG DeeDeeG commented Apr 1, 2020

@mfonville Yes, 100%. I just thought mentioning it here would ensure people who already dealt with this PR (who have the background and context to understand the work left to be done) are aware of the things on the TODO list for, say, a couple months from now.

IMO this can/probably should be just merged as-is and any follow-up work is icing on the cake. (I defer judgment on the actual decision to maintainers of this repo, as always.)

I volunteer to open said follow-up PR if it helps the maintainers here feel comfortable and ready to merge this. Thanks again.

@zorn-v

This comment has been minimized.

@lkashef
Copy link
Contributor

@lkashef lkashef commented Apr 2, 2020

Thanks @DeeDeeG and @mfonville for the extra context, it was very insightful for sure.

I will go ahead and merge this PR as it seems the most simple solution to support Ubuntu 20.04 while maintaining backward compatibility.


About migrating to python3, @DeeDeeG if you would like to take a shot at that and mention me to help push it through, that would definitely be helpful.


One last thing, keep in mind that the PR will take time to go from Nightly to Beta then to Stable, so until this PR takes it's cycle, I would recommend using Atom Nightly with Ubuntu 20.04.

@lkashef lkashef merged commit 983bcd5 into atom:master Apr 2, 2020
1 check passed
@DeeDeeG
Copy link
Contributor

@DeeDeeG DeeDeeG commented Apr 2, 2020

@lkashef Thanks for merging!

Re: Migrating to python3 I think this is good as-is now that this PR is merged (i.e. should work with all currently active/supported Debian/Ubuntu distros plus Ubuntu 20.04, out in three weeks), and IMO migrating the dependency to pure Python 3 would be premature until at least when Debian Jessie is not supported anymore (June this year).

Also: In hindsight, the python-is-python3 thing looks like it would not be a problem.

Longer explanation: As far as I understand it, we only need Python for node-gyp (according to that one comment?? I want to do more research to confirm that.) and I now know that node-gyp supports both Python 2 and Python 3.)

So it doesn't matter if python is ambiguous because both meanings would work.

I'm interested in doing the research to confirm all the ducks are in a row going forward, but I admit I'm learning some of this info as I go. I would plan to post any updates in the follow-up issue I opened: #20585

@mfonville
Copy link
Contributor

@mfonville mfonville commented Apr 12, 2020

@lkashef
With the release of Ubuntu 20.04 within 10 days from now, and the cycle still not through for even a beta release.
I'd propose this trivial patch for inclusion in a 1.45.1 release if that is possible. Otherwise users get during the running of do-release-upgrade an automatic removal of atom.

@zorn-v

This comment has been minimized.

@DeeDeeG
Copy link
Contributor

@DeeDeeG DeeDeeG commented Apr 12, 2020

No harm in backporting this to atom 1.45.x IMO, if possible.

That said, there's a chance it will be a somewhat moot point. atom depending on simply python might pull in python-is-python2.

I find this difficult to test for now, waiting on Ubuntu package maintainers update python-is-python2 to have Provides: python... And they are putting that off until the last minute to track down packages that are broken by versionless python being deprecated.

So maybe there is no action needed, maybe not. I will try to update here as early as said update to pythin-is-python2 happens.

(Backporting this PR is still a safe/no-downsides and reasonable step though, IMO.)

Copy link

@seng84 seng84 left a comment

Google acc

@seng84

This comment has been minimized.

@lkashef
Copy link
Contributor

@lkashef lkashef commented Apr 17, 2020

Thanks @DeeDeeG, highly appreciate you digging deep into this.

@lkashef
Copy link
Contributor

@lkashef lkashef commented Apr 17, 2020

@lkashef
With the release of Ubuntu 20.04 within 10 days from now, and the cycle still not through for even a beta release.
I'd propose this trivial patch for inclusion in a 1.45.1 release if that is possible. Otherwise users get during the running of do-release-upgrade an automatic removal of atom.

@mfonville Thanks for the heads up, although as per @DeeDeeG's comment, we are not sure if the release will break Atom on Ubuntu 20.04, did they release any updates on the matter?

This kinda of information would be helpful to make a decision.

On a different note, Atom Nightly should work on Ubuntu 20.04 until we decide on the patch.

@mfonville
Copy link
Contributor

@mfonville mfonville commented Apr 17, 2020

@lkashef Yesterday Ubuntu published the python-is-python2 package ( https://packages.ubuntu.com/focal/python-is-python2)
so luckily, as mentioned by @DeeDeeG Atom is install-able again when upgrading to 20.04.
One thing to check for this patch (@DeeDeeG maybe you can check?) if node-gyp is correctly using the python2 env and not python?

@lkashef
Copy link
Contributor

@lkashef lkashef commented Apr 17, 2020

Thanks for the update @mfonville!

That's great news and would definitely buy us some time to make sure we resolve this across different versions.

Feel free to mention me, in case the situation escalated or new information came up.

@DeeDeeG
Copy link
Contributor

@DeeDeeG DeeDeeG commented Apr 17, 2020

Tested this, you can just do apt install atom now, even on a fresh install of Ubuntu 20.04.

The "Provides: python" bit means this PR wasn't strictly necessary, with hindsight.

My apologies. On the other hand, it just works™ now, so that's good news.


Ongoing effort to switch to Python 3 entirely should still be looked at, because the next stable Debian (whenever they decide to release Debian Bullseye) and Ubuntu 20.10 plan on not shipping Python 2.

I posted an issue to track that whole thing here: #20585

As I found out, the first order of business would be this PR over at the apm repo: atom/apm#875 (apm bundles a version of npm, which depends on node-gyp. Need a newer node-gyp to support Python 3. Should be back-compatible with Python 2 as well, so could ship on today's distros no problem.)

That PR sorts out the main Python usage in atom. There is a little more to do to get CI passing over there, and probably some follow-up work at this repo, but that PR would be the core thing IMO. I am not familiar with the tests over there, so if anyone knows what to do for that PR to pass CI I think that would be very helpful!)

@DeeDeeG
Copy link
Contributor

@DeeDeeG DeeDeeG commented Apr 17, 2020

@mfonville the version of node-gyp used in atom at the moment specifically uses python if I recall correctly. But if you install atom in Focal you get the python-is-python2 package, which depends on python2 and symlinks /user/bin/python --> /usr/bin/python2. So node-gyp is none the wiser about it. It should "just work." (People seem to be reading these issues/PR threads a lot so I wanted to lay that all out.)

As mentioned in the comment just above, newer node-gyp that supports Python 3 would be gotten through this PR: atom/apm#875

@DeeDeeG
Copy link
Contributor

@DeeDeeG DeeDeeG commented Apr 17, 2020

@lkashef I just realized this PR should probably be reverted. 😬

If one does sudo apt install python2 and sudo apt install atom-nightly... they can end up with python2 installed, but no symlink from python to python2.

Then apm-nightly install [package] fails because it can't find the executable python.

So, basically the user needs to have python symlinked to python2... The dependencies from before this PR would now work better than the current python | python2. If we just depend on versionless python, apt pulls in python-is-python2, problem solved.)

Once again, apologies for getting this wrong.

(How things are in Ubuntu development branch isn't necessarily how it will be in stable release. I got confused about what the eventual situation would be in final release. And I only recently found the way to test python support, by running apm install [package].)

@zorn-v
Copy link
Contributor Author

@zorn-v zorn-v commented Apr 18, 2020

One problem (then become others) that atom can not make "patch release"

@zorn-v
Copy link
Contributor Author

@zorn-v zorn-v commented Apr 18, 2020

And, yeah. Problem was known sins february

@DeeDeeG
Copy link
Contributor

@DeeDeeG DeeDeeG commented Apr 18, 2020

@zorn-v thank you for being involved in making a fix. But the criticisms of atom here are not helpful or called for. The decision whether to release an update of a piece of software is the maintainers' decision alone. It turns out in this case, this did need some time to make sure it was the right thing to do. With adequate research and testing, I found out this PR actually wasn't needed all along. (I regret repeatedly saying it was needed, when In hindsight I didn't understand the situation enough.)

If this PR gets reverted before the next stable release, there will be no need for a patch update anyhow.

Please stop criticising the atom maintainers and their actions, or insisting they should make a patch release.

Edit to clarify: I'm not affiliated with the atom project. I'm not speaking for the atom maintainers. Those are just my personal thoughts as an open-source contributor in general.

@DeeDeeG
Copy link
Contributor

@DeeDeeG DeeDeeG commented Apr 18, 2020

The sources of my confusion here was down to a few things:

  • I didn't understand that python-is-python2 would eventually "Provide" versionless python, and that this had been disabled only temporarily. That situation was not very clear to me without more digging and reaching out to folks from Ubuntu.
  • I didn't understand the behavior of apt clearly enough to see that by having "Provides: python", python-is-python2 would be auto-installed when trying to install atom.
  • I initially hadn't identified a way to test actual python support in apm, so I was basing whether this PR "solved the problem" simply by whether it allowed you to install the atom package.

With all those things being clearer to me now, I can see this PR was unnecessary, and opens up a possibility of apm install not working anymore, so it should be reverted.

@DeeDeeG
Copy link
Contributor

@DeeDeeG DeeDeeG commented Apr 19, 2020

Update: Not many packages require node-gyp in order to install. (And that's the only time I believe Atom uses Python: during package installs that use node-gyp.) So the negative impact of this PR looks pretty limited.

Testing details (click to expand):

To test this, I have tried installing many packages in atom. (Via "settings -> install"). I tested this with an "unsupported" version of Python on my system (Python 3). Only a few packages failed to install due to wrong Python version:

(There are almost definitely some more packages than this that need node-gyp. But I installed what felt like hundreds of packages, and these are the only ones out of those that failed due to my system having the wrong Python version. The point being, most packages don't need node-gyp and would therefore be unaffected by this PR.)

(Other than the ones listed above, I only found tree-view, and some other packages bundled with Atom, which similarly fail to install, but these are bundled with Atom and aren't intended to be manually installed or upgraded by end-users. So I don't think they count. These can't be reinstalled or upgraded from within the Atom UI, but they can be reinstalled on the command-line with apm.)

If someone knows another package that requires to be built with node-gyp at install time, (other than packages bundled with Atom such as tree-view), please let me know.

(My setup for testing this is Ubuntu 20.04, using Atom stable 1.45.0 patched to include this PR. I also tried with Atom nightly 1.47.0, which had the same results. python is symlinked to python3 on my machine. I got equivalent results trying this briefly on Windows with Atom 1.45.0 and no Python installed whatsoever.)

[End of testing details]

So maybe Atom doesn't strictly need a Python dependency at all. Not having such a requirement would bring the Debian/Ubuntu (.deb) package for Atom in line with the Windows and Fedora (.rpm) packaging. And I haven't had time to confirm this yet, but I think the macOS packaging doesn't require Python either. (It probably relies on the fact macOS ships Python 2 out of the box, but that's almost off-topic level of detail.)

Regarding whether to revert or keep this PR, it seems like pretty low-stakes either way, IMO. But reverting it would ensure the packages I listed above (in "Testing details") can always be built and installed with Atom/apm... On Debian/Ubuntu anyhow.

(See also the snap package of Atom... which has no apparent Python requirement to install. It's equivalent to the Windows, .rpm or macOS packaging.)


Summary and recommendation from me: This PR effectively gives the option of running Atom with broken Python support, which has surprisingly low impact on using Atom. I'd suggest making Python an optional dependency, or dropping the dependency altogether... Or just reverting to before this PR, which returns to requiring working Python support to install Atom on Ubuntu. (Requiring Python support that then can't be used (Requires: python2) seems kinda silly.)

Making Python an optional dependency (or dropping the dependency altogether) is more future-proof than going back to the way it was before this PR.

@zorn-v
Copy link
Contributor Author

@zorn-v zorn-v commented Apr 20, 2020

@DeeDeeG
My criticisms is something like "agony", caused by "deading" of my favorite editor.
You investigations is great, but is somebody will take them into account ?

@zorn-v
Copy link
Contributor Author

@zorn-v zorn-v commented Apr 20, 2020

If I was deb packager of atom I assign @DeeDeeG surely for it 😄

@lkashef
Copy link
Contributor

@lkashef lkashef commented Apr 21, 2020

@DeeDeeG your research has been awesome! thanks again! I am currently working on other stuff but I will definitely, take a deep dive into the comments.

DeeDeeG added a commit to DeeDeeG/atom that referenced this issue May 8, 2020
This reverts commit 983bcd5, reversing
changes made to 6f2b863.
DeeDeeG added a commit to DeeDeeG/atom that referenced this issue May 12, 2020
This reverts commit 983bcd5, reversing
changes made to 6f2b863.
anwarnafa
Copy link

@anwarnafa anwarnafa commented on 7b59451 Aug 13, 2020

Choose a reason for hiding this comment

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

وااااو

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
7 participants