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

build: use bin path override if no python is found in PATH #16241

Conversation

bradleythughes
Copy link
Member

On systems with no "python" in the PATH, e.g. when building with poudriere on FreeBSD, we should always create a python symlink in get_bin_override(). Otherwise, configure fails with the following error:

Traceback (most recent call last):
  File "./configure", line 1461, in <module>
    bin_override = get_bin_override()
  File "./configure", line 1360, in get_bin_override
    if os.path.realpath(which('python')) == os.path.realpath(sys.executable):
  File "/usr/local/lib/python2.7/posixpath.py", line 375, in realpath
    path, ok = _joinrealpath('', filename, {})
  File "/usr/local/lib/python2.7/posixpath.py", line 381, in _joinrealpath
    if isabs(rest):
  File "/usr/local/lib/python2.7/posixpath.py", line 54, in isabs
    return s.startswith('/')
AttributeError: 'NoneType' object has no attribute 'startswith'
Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • commit message follows commit guidelines
Affected core subsystem(s)

build

@nodejs-github-bot nodejs-github-bot added the build Issues and PRs related to build files or the CI. label Oct 16, 2017
@bradleythughes
Copy link
Member Author

This happens with the recently released v8.7.0 as well, so if it's possible to land this on v8.x, I'd be very appreciative. :)

@gibfahn
Copy link
Member

gibfahn commented Oct 16, 2017

Wow, never occurred to me that there were systems/configurations without a python defined.

cc/ @forivall @addaleax @bengl @bnoordhuis @nodejs/python , PTAL

@gireeshpunathil
Copy link
Member

I am sorry, but not sure I understand - On systems with no "python" in the PATH , the stack trace that you listed is not possible right? as that trace itself is a python manifestation?

@forivall
Copy link
Contributor

I'm guessing that it means systems where there's a python2 and maybe python3 but no python, or that python is just installed somewhere not on the path.

@bradleythughes
Copy link
Member Author

That's correct. There is python2.7 and python2 in the path, but no python.

@gireeshpunathil
Copy link
Member

thanks @forivall and @bradleythughes. Can you also please have a look at #16058 and #14737 and see that this is supplementing / complimenting to those? I am also trying to relate these.

@bradleythughes
Copy link
Member Author

@gireeshpunathil Yes, I found both of those PRs when I hit the configure error shown above with the v8.7.0 release. This PR addresses a specific situation not covered by the other two, so yes, I would say this PR is both complimentary and supplementary. 🙂

Copy link
Member

@bnoordhuis bnoordhuis left a comment

Choose a reason for hiding this comment

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

LGTM modulo style nit.

configure Outdated Show resolved Hide resolved
Copy link
Contributor

@thefourtheye thefourtheye left a comment

Choose a reason for hiding this comment

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

LGTM with @bnoordhuis's comment.

On systems with no "python" in the PATH, e.g. FreeBSD, we should always
create a python symlink in get_bin_override(). Otherwise, configure
fails with the following error:

Traceback (most recent call last):
  File "./configure", line 1461, in <module>
    bin_override = get_bin_override()
  File "./configure", line 1360, in get_bin_override
    if os.path.realpath(which('python')) == os.path.realpath(sys.executable):
  File "/usr/local/lib/python2.7/posixpath.py", line 375, in realpath
    path, ok = _joinrealpath('', filename, {})
  File "/usr/local/lib/python2.7/posixpath.py", line 381, in _joinrealpath
    if isabs(rest):
  File "/usr/local/lib/python2.7/posixpath.py", line 54, in isabs
    return s.startswith('/')
AttributeError: 'NoneType' object has no attribute 'startswith'
@bradleythughes bradleythughes force-pushed the build-use-bin-override-if-no-python branch from b165063 to 0f1872c Compare October 17, 2017 19:47
@gireeshpunathil
Copy link
Member

@gireeshpunathil
Copy link
Member

More CI failures than I can comprehend, running it once again:
CI: https://ci.nodejs.org/job/node-test-pull-request/10897/

@refack
Copy link
Contributor

refack commented Oct 22, 2017

Landed in 02a5267

Thanks!

@refack refack closed this Oct 22, 2017
refack pushed a commit that referenced this pull request Oct 22, 2017
On systems with no "python" in the PATH, e.g. FreeBSD, we should always
create a python symlink in get_bin_override().

PR-URL: #16241
Reviewed-By: Gibson Fahnestock <[email protected]>
Reviewed-By: Gireesh Punathil <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Sakthipriyan Vairamani <[email protected]>
Reviewed-By: Refael Ackermann <[email protected]>
MylesBorins pushed a commit that referenced this pull request Oct 23, 2017
On systems with no "python" in the PATH, e.g. FreeBSD, we should always
create a python symlink in get_bin_override().

PR-URL: #16241
Reviewed-By: Gibson Fahnestock <[email protected]>
Reviewed-By: Gireesh Punathil <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Sakthipriyan Vairamani <[email protected]>
Reviewed-By: Refael Ackermann <[email protected]>
addaleax pushed a commit to ayojs/ayo that referenced this pull request Oct 26, 2017
On systems with no "python" in the PATH, e.g. FreeBSD, we should always
create a python symlink in get_bin_override().

PR-URL: nodejs/node#16241
Reviewed-By: Gibson Fahnestock <[email protected]>
Reviewed-By: Gireesh Punathil <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Sakthipriyan Vairamani <[email protected]>
Reviewed-By: Refael Ackermann <[email protected]>
@MylesBorins
Copy link
Contributor

can we get a backport to v6.x?

@refack
Copy link
Contributor

refack commented Nov 14, 2017

Backporting now.

@MylesBorins
Copy link
Contributor

@refack NVM... lands cleanly with #16058

MylesBorins pushed a commit that referenced this pull request Nov 14, 2017
On systems with no "python" in the PATH, e.g. FreeBSD, we should always
create a python symlink in get_bin_override().

PR-URL: #16241
Reviewed-By: Gibson Fahnestock <[email protected]>
Reviewed-By: Gireesh Punathil <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Sakthipriyan Vairamani <[email protected]>
Reviewed-By: Refael Ackermann <[email protected]>
@refack refack removed their assignment Nov 14, 2017
@MylesBorins MylesBorins mentioned this pull request Nov 21, 2017
MylesBorins pushed a commit that referenced this pull request Nov 21, 2017
On systems with no "python" in the PATH, e.g. FreeBSD, we should always
create a python symlink in get_bin_override().

PR-URL: #16241
Reviewed-By: Gibson Fahnestock <[email protected]>
Reviewed-By: Gireesh Punathil <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Sakthipriyan Vairamani <[email protected]>
Reviewed-By: Refael Ackermann <[email protected]>
MylesBorins pushed a commit that referenced this pull request Nov 28, 2017
On systems with no "python" in the PATH, e.g. FreeBSD, we should always
create a python symlink in get_bin_override().

PR-URL: #16241
Reviewed-By: Gibson Fahnestock <[email protected]>
Reviewed-By: Gireesh Punathil <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Sakthipriyan Vairamani <[email protected]>
Reviewed-By: Refael Ackermann <[email protected]>
addaleax pushed a commit to ayojs/ayo that referenced this pull request Dec 7, 2017
On systems with no "python" in the PATH, e.g. FreeBSD, we should always
create a python symlink in get_bin_override().

PR-URL: nodejs/node#16241
Reviewed-By: Gibson Fahnestock <[email protected]>
Reviewed-By: Gireesh Punathil <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Sakthipriyan Vairamani <[email protected]>
Reviewed-By: Refael Ackermann <[email protected]>
@bradleythughes bradleythughes deleted the build-use-bin-override-if-no-python branch September 30, 2020 21:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build Issues and PRs related to build files or the CI.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants