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

[wip] Improve ability to detect Python 2 during build #16885

Closed
wants to merge 3 commits into from

Conversation

smashwilson
Copy link
Contributor

@smashwilson smashwilson commented Mar 5, 2018

Description of the Change

Use @atom/dowsing-rod to aggressively detect python 2 installations and pass it to node-gyp subprocesses. We won't be able to get away from requiring python 2 entirely, but we can at least accept a python 2 path from npm config get python or ${PYTHON}.

Without this in place, even with ${PYTHON} set to a python2 install, the build for git-utils fails with a python 2/3 compatibility problem. It turns out that the Makefile generated by node-gyp sometimes executes a file with a #!/usr/bin/env python shebang line, so unless the python2 binary is actually discoverable on the ${PATH} as "python", the build fails.

Alternate Designs

The clearest alternative is to actually fix node-gyp at the source, by porting gyp to Python 3 or JavaScript. That would require getting a patch accepted into gyp, then waiting for it to be included in a node-gyp release, then a node release, then upgrade our build to use that node version.

Why Should This Be In Core?

It's where the build is.

I'm also going to create a parallel pull request for apm to improve our Python detection during apm builds (atom/apm#775).

Benefits

Developers who:

  • Use homebrew on MacOS and have the python formula installed.
  • Use Arch Linux or other distributions that use Python 3 as their default python package.

...will be able to build Atom without having to break things on their system that rely on Python 3 being the default. We'll also have a nice message if Python 2 cannot be found.

Possible Drawbacks

Verification Process

The ultimate test is to run script/build with Python 3 on the PATH.

Applicable Issues

@Arcanemagus
Copy link
Contributor

Re: Alternate designs: nodejs/node-gyp#1337

@smashwilson
Copy link
Contributor Author

There's also this: nodejs/node-gyp#960

@smashwilson
Copy link
Contributor Author

I believe (hope?) that this has been obsoleted by new npm/node-gyp releases.

@smashwilson smashwilson closed this Feb 1, 2021
@smashwilson smashwilson deleted the aw-python2 branch February 1, 2021 14:19
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.

None yet

2 participants