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

Review/finalize Python approach for mid 2020 #20716

Merged
merged 2 commits into from Jun 6, 2020

Conversation

DeeDeeG
Copy link
Contributor

@DeeDeeG DeeDeeG commented May 8, 2020

Requirements for Contributing a Bug Fix

Identify the Bug

Addresses most of #20585

(Some small additional things are left to do, in order to adjust the repo for Python 3. See: #20585 (comment) for details.)

The following PR was rolled into this PR, so it should be automatically closed if this is merged:

Closes #20670

Description of the Change

This was a multi-part fix. Some of it has already been merged. This PR exists to review/finalize the approach taken. The goal is to ensure the Atom project supports Python 3, and can keep operating entirely without needing Python 2. That said, the approach taken here does maintain full backward compatibility with Python 2.

  • Upgrade apm to 2.5.0 #20703 already pulled in apm with newer node-gyp that supports Python 2 AND Python 3.
    • node-gyp is the only reason end users would need Python in order to use Atom/apm. It builds Atom packages that include native c/c++ code. gyp is written in Python, so running node-gyp requires a Python interpreter on the system.
  • script: Update npm to 6.14.4 for Python 3 support #20711 already updated npm (and its dependency node-gyp) in the script/ directory, to ensure Atom itself can be BUILT with either Python 2 or Python 3.
    • As mentioned before, this is fully backward-compatible; Atom can still be built using Python 2, no problem. The built Atom packages should be the same, regardless. This is about being able to build Atom after Python 2 becomes fully deprecated and developers update their machines to only having Python 3.
  • Commit 9b95473 drops the python dependency from the Debian/Ubuntu .deb package.
  • Commit e8b470c reverts Fix .deb install on ubuntu 20.04 #20406, which as it turns out took a misinformed approach.

Alternate Designs

The main, most-important thing that was accomplished with regard to Python and Atom, was updating to a newer version of node-gyp which supports both Python 3 and Python 2.

See: #20703 and atom/apm#887 for the implementation.

As far as I am concerned, and as far as I can tell, that part is not a controversial change, as there is no obvious downside, and as such there is no proposed alternate design for that part.


Everything else is at least an order of magnitude less important, IMO.

The subjective parts left to decide on are:

  • Have a python dependency in the .deb file?
    • This is different from the packaging for all other supported OSes, i.e. Fedora, macOS and Windows. It dates back to Remove lintian errors #3599.
    • If keeping a Python dependency in the .deb file, it should by Python 3 aware, i.e. python3 | python2 | python. (versionless python should be last, as the versionless python package is being deprecated in upstream Debian/Ubuntu.)
    • If keeping a Python dependency in the .deb file, should it be Depends: (hard requirement), Recommends: (optional but installed by defatult), or Suggests: (optional, not installed by default)?
    • Prior discussion is in the comments of Deb packaging: Support Python 3, move "python" to "Recommends:" #20670

Possible Drawbacks

For the core change, updating npm and node-gyp, there are no drawbacks that I am aware of, whatsoever. This is a minor version bump of npm, and a well-tested major version bump of node-gyp that has been working well so far in apm. The new version of node-gyp is included in the latest upstream releases of npm, so is likely to see wide usage, testing and attention for any bugs.

apm 2.5.0 also bundles newer Node 10.20.1, a minor version bump over Node 10.2.1. Again, this is a minor version bump of highly-used/tested upstream Node.js. Bugs or issues in the latest v10 Long-Term-Service version of Node are not expected, and in fact could reasonably be expected to include fixes for miscellaneous bugs.


As for the subjective situation with the .deb packaging, every available approach is somewhat arbitrary.

Users don't strictly need Python in order to use Atom/apm, unless they install one of a handful of Atom packages that include native code. See: #20406 (comment) for an investigation into this.

Putting Python in the .deb package's dependencies would purport to make sure everyone is prepared to install that handful of packages. (Conversely, removing the python dependency might be considered a "breaking change" for users of that handful of packages. But only in the rare circumstances where a user has no Python installed, which as far as I know is not possible on Debian/Ubuntu under a graphical desktop environment; Various Debian/Ubuntu desktop components depend on one version or other of Python. It's only possible to not have Python installed on headless server/docker variants of Debian/Ubuntu, I think.)

Furthermore: Other than the .deb package, no other official Atom package has Python as an install-time dependency. Atom users on other OSes have apparently been okay all this time without an install-time dependency on Python.

node-gyp has additional requirements that currently are NOT specified as install-time dependencies in any platform's packaging, particularly make, cc/gcc and c++/g++. So users without those installed need to read the node-gyp error messages they encounter, and understand the message well enough to realize that they must install a C compiler and a C++ compiler to their system.

As such, the Python dependency in the .deb file is not sufficient on its own to ensure Atom users can successfully build packages with node-gyp.

Admittedly, using the python3 | python2 | python dependency logic would work on all Debian/Ubuntu releases past and for the foreseeable future. But the question remains: Why go out of our way to ensure the user has Python when few users will need it, and the users who need it must also manually install certain dependencies like gcc and g++?

By the way, users of currently supported desktop-oriented builds of Debian/Ubuntu will have both Python 2 AND Python 3 installed, by default. Only minimal/headless server/Docker images of Debian/Ubuntu are at all likely to have neither Python 2 nor Python 3 installed.

Verification Process

Manually built apm and atom with every variation of these changes discussed. Checked whether the built atom .deb file could be installed on various distros. Checked whether the resulting apm command works, e.g. by running apm install tree-view. Saw that CI passes.

Release Notes

N/A

(Not user-facing? But this could work: "- Atom no longer requires Python in order to install on Debian/Ubuntu." "- Atom and apm now support Python 3.")

@DeeDeeG
Copy link
Contributor Author

DeeDeeG commented May 8, 2020

P.S.: For the record, this whole thing was kicked off after I posted an issue about not being able to install Atom on the Ubuntu 20.04 development branch. That issue was #20356. I have learned much since then, so while the discussion there might be of some historical/archival interest (maybe?) the commentary there is almost entirely outdated.

@DeeDeeG
Copy link
Contributor Author

DeeDeeG commented May 9, 2020

Timeline (background):

  • 2008: Python 3.0 released, to make breaking changes, remove cruft, and make going forward with a slightly better direction for Python possible.
  • From 2008 onward: Debian/Ubuntu gradually start porting software in their repositories over to Python 3.
  • 2012: After some time porting software like this, a plan is made to ship only Python 3 on the install media for Ubuntu, but still have Python 2 in the repos, for Ubuntu 12.10.
    • Meaning, prove the point that Ubuntu desktop can work without Python 2, save limited space on the install ISO by not including two major versions of Python.
    • The install media actually ended up shipping with both python 2 and python 3, in my testing, but this is the first of several attempts to partly or completely ditch Python 3 from an upcoming release of Ubuntu.
    • https://wiki.ubuntu.com/Python/FoundationsQPythonVersions
  • 2014: Python developers decide to keep maintaining Python 2 until 2020.
    • Source: 0
  • 2015-ish: Distros plan to start emphasizing Python 3 as their main Python version.
    • Source: 1
  • March 2018: Python developers clarify that January 1 2020 is the cutoff for Python 2 support.
    • Sources: 2 3
  • Early 2019: Debian and Ubuntu decide it's time to finally drop Python 2 support in their distros.
    • Sources on Ubuntu policy: 4 5 6 7; Sources on Debian policy: 8 9 10
  • November 17th, 2019: Upgrade npm apm#875 was posted, to investigate using newer npm in apm. Motivated by a desire for Python 3 compatibility, among various other improvements in node-gyp since the version apm had been shipping, in the node-gyp v3.x series.
    • Stalled for a while with broken tests. Debugging efforts picked up again on April 3rd 2020.
  • November 22nd, 2019: Somebody posted dependencies: atom still depends on python2 #20171, but this was closed as a duplicate of Upgrade npm apm#875
  • December 2019: End-of-Life for Python 2 officially extended until after April 2020.
    • Source: 11

Timeline (from the start of my involvement to this PR.)

  • January 24th 2020: Can't install the .deb package in Ubuntu 20.04 (Focal Fossa) development branch (due to changes in Ubuntu's python packaging) #20356 posted
    • Ubuntu packagers temporarily removed the versionless python package altogether from Ubuntu 20.04's repositories, to "smoke out" any packages still depending on it. This temporarily caused Atom to be uninstallable. They would eventually add a package python-is-python2 that "Provides" python. Meaning existing packages won't break. Atom can still be installed in Ubuntu 20.04 "Focal Fossa". But the versionless python package, and Python 2 are officially deprecated as of this release of Ubuntu.
  • February 12th: After some discussion, Fix .deb install on ubuntu 20.04 #20406 is posted
    • This PR simply changes the .deb package's Python dependency from python to python | python2. Allows apt to find the python2 package, and to consider the Python dependency satisfied, and allow Atom to install. This workaround was only applicable to Ubuntu 20.04 when said release was in active development, as the python-is-python2 package solves the issue this PR was designed to solve. Furthermore, apm is unable to find the python2 executable installed by the python2 package, so this PR introduced a corner case where apm can break. As of about a week before Ubuntu Focal Fossa had its final release, this PR was both unhelpful, AND could cause apm breakage, so in hindsight, this PR should be reverted.
    • This PR was merged on April 2nd.
  • April 1st: I opened Migrate Python dependency From Python 2 to Python 3 #20585 to track broader efforts/research for Atom to support Python 3.
  • April 26th: I posted Deb packaging: Support Python 3, move "python" to "Recommends:" #20670
    • PR was designed to allow installing Atom even if the versionless python package isn't available on the system (such as upcoming Debian/Ubuntu releases). Because apm looked for versionless python in a hard-coded way, the only way to install Atom without requiring the python packages in these upcoming releases would be to make Python optional or Drop the Python requirement. So that's the approach this PR took.
    • Was meant as a best-available workaround until Upgrade npm apm#875 could be finished.
  • April 29th: I posted Upgrade node gyp with fixes apm#887 after working to debug the test failures in Upgrade npm apm#875 for a while.
    • Debugged some more, reviewed, discussed... finished the work.
    • PR was merged on May 5th
  • May 5th: Atom 1.47.x beta series forked off with Fix .deb install on ubuntu 20.04 #20406 merged, and without Upgrade apm to 2.5.0 #20703 merged.
  • May 6th: Upgrade apm to 2.5.0 #20703 posted to use the updated apm / updated npm and node-gyp in Atom.
    • Merged May 7th.
  • May 7th: Updated Deb packaging: Support Python 3, move "python" to "Recommends:" #20670 with an approach that is Python-3-aware.
    • Proposed updates lead to a discussion that meandered to "do we need to have a Python dependency?" and "Where did the Python dependency even come from?" and whether just leaving Fix .deb install on ubuntu 20.04 #20406 as-is would work. Further discussion leads to a desire for a "wrap-up" PR that includes the decisions from that discussion (regarding dropping the Python dependency from the .deb package), and which encapsulates the whole "Python in Atom in 2020" saga for review.
  • May 7th again: Posted script: Update npm to 6.14.4 for Python 3 support #20711 to update the build scripts to a version of npm --> node-gyp that supports Python 3.
    • Backward compatible for building Atom with either Python 2 or Python 3. Built Atom packages should be the same, regardless of Python version used.
    • Merged May 12th.
  • May 8th: Posted this current PR in order to review the proposed approach, how we got here, etc.
  • May 12th: Updated this PR to reflect that script: Update npm to 6.14.4 for Python 3 support #20711 is merged. The diff for this PR is now limited to changing the Python dependency for the .deb package.

DeeDeeG added 2 commits May 12, 2020
This reverts commit 983bcd5, reversing
changes made to 6f2b863.
Python is only needed for apm --> npm --> node-gyp.
(For building Atom packages that include native C/C++ code.)

The rest of Atom/apm works 100%, even with no Python installed.

With Python 2 soon to be dropped from the Debian/Ubuntu repos,
having a hard dependency on `python` or `python2` is a problem.

None of the other OSes/platforms have an install-time requirement of
having Python on the system, so this is in line with Atom packaging
for the other platforms.
@DeeDeeG
Copy link
Contributor Author

DeeDeeG commented May 13, 2020

It occurs to me that while "there is no Python dependency in the Atom packaging for any other OS/platform besides Debian+Ubuntu"... That's mostly because there is no mechanism to do so on Windows or macOS.

In which case, whatever we decide should just be consistent across Ubuntu and Fedora, which both have package management systems with dependency resolution, and all that. (They both have pretty much identical systems with "Depends," "Recommends," and "Suggests," too.)

@lkashef
Copy link
Contributor

lkashef commented May 20, 2020

Thanks @DeeDeeG for the summary and the timeline, it's definitely helpful to have the whole context in one place.

I would not assume that an average user doesn't need to install native modules, as atom depends on multiple native modules and normally atom users installs a handful of packages (both native and non-native).

By the way, users of currently supported desktop-oriented builds of Debian/Ubuntu will have both Python 2 AND Python 3 installed, by default. Only minimal/headless server/Docker images of Debian/Ubuntu are at all likely to have neither Python 2 nor Python 3 installed.

So keeping the above in mind, I wanted to summarize the big picture to make sure we are on the same page.

I would assume the only affected users by dropping python as a required dependency, would be the ones with minimalist distros, but mainstream ones like Ubuntu, Fedora, etc should not be affected, since these OS-es come bundled with python, and apm 2.5 now supports up to python 3.

@DeeDeeG
Copy link
Contributor Author

DeeDeeG commented May 20, 2020

Basically correct, yes. Regarding this summary:

So keeping the above in mind, I wanted to summarize the big picture to make sure we are on the same page.

I would assume the only affected users by dropping python as a required dependency, would be the ones with minimalist distros, but mainstream ones like Ubuntu, Fedora, etc should not be affected, since these OS-es come bundled with python, and apm 2.5 now supports up to python 3.

@DeeDeeG
Copy link
Contributor Author

DeeDeeG commented May 20, 2020

More research and details (which confirm and reinforce the overall gist of the above summary), but I'm not sure if anyone needs this level of detail, so putting behind a "click to expand" thing:

More details (click to expand).
  • Regarding if end-users need Python:

I would not assume that an average user doesn't need to install native modules, as atom depends on multiple native modules [ . . . ]

These native modules are built and bundled with Atom. End-users get a warning message if they try to reinstall them manually. These modules end up as some sort of native compiled target during the Atom build process, and whatever form that takes, it doesn't require a Python executable after building; Python's only needed at build time. Since they're bundled with Atom, end-users should never need Python for these packages.

$ apm install tree-view
The tree-view package is bundled with Atom and should not be explicitly installed.
You can run `apm uninstall tree-view` to uninstall it and then the version bundled
with Atom will be used.
Installing tree-view to /home/[user]/.atom/packages ✓

But yeah, I agree some people do need Python to build native modules, like users who install the "terminal-in-Atom" type of packages.

  • Regarding Fedora:

Fedora isn't affected by this PR at all, since that OS uses .rpm packages. This proposed change only updates the .deb packaging. (Furthermore: Atom's .rpm package already doesn't require Python at install time, and apparently never has required Python at install time, based on the commit history. This PR is in line with the approach already taken for Fedora.)

  • Regarding what kind of Debian/Ubuntu would be affected:

I have only found / confirmed one edition each of Debian/Ubuntu that ships without Python, and that is the Docker images for "ubuntu" and "debian".

All the following that I tested DO come with Python:

  • Ubuntu Server (tested 20.04)
    • (This is a "headless server" environment I had guessed might not have Python, but it does have Python. It has Python 3.8.2)
  • Debian netinstall with everything deselected, including desktop environments and basic utilities (the most minimal version of Debian I am aware of, outside of Docker. Debian 10 tested.)
    • This is a very stripped down Debian, I am surprised it comes with Python. It has Python 3.7.3.
  • Ubuntu netinstall with no extras selected (18.04 tested)
    • Python 3.6.9
  • "minimal" spinoffs of Debian, such as BunsenLabs (Helium tested) or CrunchBang++(version 10.1 tested).
  • Regular desktop Debian/Ubuntu, of course (several versions tested)

[END OF EXPANDED DETAILS]

Briefly:

Native modules that Atom depends on are built and bundled during Atom's packaging process. End-users aren't supposed to reinstall or rebuild these; They get a warning message that they shouldn't do that, if they try. So, users shouldn't need Python for these packages.

This PR doesn't affect Atom's Fedora package (so it doesn't affect Fedora users), but it does coincidentally make Atom's Debian package match what Atom already is doing for Fedora. (No Python dependency).

From my testing, only extremely stripped-down environments like the tiny (~75 to 125 MB) Docker images of Debian/Ubuntu don't ship with Python. In fact it might exclusively be the Docker images; I can't find anything else that doesn't ship with Python.

@DeeDeeG
Copy link
Contributor Author

DeeDeeG commented May 31, 2020

I would like to see in the .deb package for Atom to do either:

  • Requires: python3 (>= 3.5) | python2 (>= 2.7) | python
  • or remove Python from Requires and let Atom install in e.g. Ubuntu Docker containers even without Python installed.

Edit: A reasonable deadline to do this by (getting it into the stable version of Atom) is October 22.

@lkashef lkashef merged commit e4b9c1e into atom:master Jun 6, 2020
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants