-
Notifications
You must be signed in to change notification settings - Fork 29.9k
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
./configure fails after Python version default changed #30129
Comments
@codebytere Could you try applying the changes from nodejs/node-gyp#1890 to |
You should still be able to control which Python I am interested in the outcome of Richard’s 1890 experiment above. Also, nodejs/node-gyp#1939 (merged) that really focuses on issues related to If you have recently upgraded to macOS Catalina then please provide a review of nodejs/node-gyp#1940. |
It was still fail after applying nodejs/node-gyp#1890 index 9a58df4782..610f90de21 100644
--- a/tools/gyp/pylib/gyp/xcode_emulation.py
+++ b/tools/gyp/pylib/gyp/xcode_emulation.py
@@ -20,6 +20,8 @@ import sys
import tempfile
from gyp.common import GypError
+PY3 = bytes != str
+
# Populated lazily by XcodeVersion, for efficiency, and to fix an issue when
# "xcodebuild" is called too quickly (it has been found to return incorrect
# version number).
@@ -1407,7 +1409,7 @@ def XcodeVersion():
except:
version = CLTVersion()
if version:
- version = re.match(r'(\d+\.\d+\.?\d*)', version).groups()[0]
+ version = ".".join(version.split(".")[:3])
else:
raise GypError("No Xcode or CLT version detected!")
# The CLT has no build information, so we return an empty string.
@@ -1453,6 +1455,8 @@ def GetStdoutQuiet(cmdlist):
job = subprocess.Popen(cmdlist, stdout=subprocess.PIPE,
stderr=subprocess.PIPE)
out = job.communicate()[0]
+ if PY3:
+ out = out.decode("utf-8")
if job.returncode != 0:
raise GypError('Error %d running %s' % (job.returncode, cmdlist[0]))
return out.rstrip('\n')
@@ -1463,6 +1467,8 @@ def GetStdout(cmdlist):
Raises |GypError| if the command return with a non-zero return code."""
job = subprocess.Popen(cmdlist, stdout=subprocess.PIPE)
out = job.communicate()[0]
+ if PY3:
+ out = out.decode("utf-8")
if job.returncode != 0:
sys.stderr.write(out + '\n') The error shows:
|
@ZYSzys Please try nodejs/node-gyp#1939 as well then. |
The code before nodejs/node-gyp#1932 might have been eating exceptions that are now appearing. This is safer in the long term but might cause hiccups in the near term. |
For some reason, that doesn't seem to work for me. Or at least it still reports finding Python 3.7.1, suggesting it's still using it (and getting me the same results). $ PYTHON=python2 ./configure
Node configure: Found Python 3.7.1...
INFO: Using floating patch "tools/icu/patches/64/source/common/putil.cpp" from "tools/icu"
INFO: Using floating patch "tools/icu/patches/64/source/i18n/dtptngen.cpp" from "tools/icu"
No receipt for 'com.apple.pkg.DeveloperToolsCLILeo' found at '/'.
No receipt for 'com.apple.pkg.DeveloperToolsCLI' found at '/'.
gyp: No Xcode or CLT version detected!
Error running GYP
$ To anticipate the obvious question: $ python2 --version
Python 2.7.15
$ |
After applying https://github.com/nodejs/node-gyp/pull/1890/commits/19398a4b0d88581e819b697445771a6e69759cd5.patch, I get similar results to those reported by @ZYSzys. Applying https://github.com/nodejs/node-gyp/pull/1939.patch on top of that does not apply cleanly, but the patch-merge-conflict to sort out is pretty straightforward I think, so I did that. Now File "./gyp-mac-tool", line 673
max_value_length = len(max(filelist.items(), key=lambda (k,v):len(v))[1])
^
SyntaxError: invalid syntax |
I was inaccurate when I wrote:
I should have instead written:
nodejs/node-gyp#1939 Is a rewrite of this logic and would be my preferred solution. Fixed in #30146, `lambda (k,v):len(v) is indeed a Python 3 syntax error which flake8 can spot automatically. #30143 reconfigures flake8 to find syntax errors even in our dependencies. |
As discussed in #30129 (comment), when we vendor in code, we own the Syntax Errors in that code. This PR adds The `.flake8` config file at the root of this repo puts blinders on the linting of our dependencies so this test disables that file before linting.
FWIW, |
FTR, Ben's comment above is the right way to force configure to use a specific python executable, |
As discussed in #30129 (comment), when we vendor in code, we own the Syntax Errors in that code. This PR adds The `.flake8` config file at the root of this repo puts blinders on the linting of our dependencies so this test disables that file before linting.
As discussed in #30129 (comment), when we vendor in code, we own the Syntax Errors in that code. This PR adds The `.flake8` config file at the root of this repo puts blinders on the linting of our dependencies so this test disables that file before linting. fixup: allow_failures until dependencies pass PR-URL: #30143 Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Jiawen Geng <[email protected]> Reviewed-By: Sam Roberts <[email protected]>
As discussed in #30129 (comment), when we vendor in code, we own the Syntax Errors in that code. This PR adds The `.flake8` config file at the root of this repo puts blinders on the linting of our dependencies so this test disables that file before linting. fixup: allow_failures until dependencies pass PR-URL: #30143 Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Jiawen Geng <[email protected]> Reviewed-By: Sam Roberts <[email protected]>
#30262 (comment) is correct. We have cleaned up the nodejs/node-gyp's gyp/pylib/gyp/xcode_emulation.py file especially in nodejs/node-gyp#1890 nodejs/node-gyp#1895 nodejs/node-gyp#1932 and nodejs/node-gyp#1939 but we have not brought those changes back to this repo's tools/gyp/pylib/gyp/xcode_emulation.py file which is required to fix this issue. PR on its way... |
As discussed in #30129 (comment), when we vendor in code, we own the Syntax Errors in that code. This PR adds The `.flake8` config file at the root of this repo puts blinders on the linting of our dependencies so this test disables that file before linting. fixup: allow_failures until dependencies pass PR-URL: #30143 Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Jiawen Geng <[email protected]> Reviewed-By: Sam Roberts <[email protected]>
As discussed in #30129 (comment), when we vendor in code, we own the Syntax Errors in that code. This PR adds The `.flake8` config file at the root of this repo puts blinders on the linting of our dependencies so this test disables that file before linting. fixup: allow_failures until dependencies pass PR-URL: #30143 Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Jiawen Geng <[email protected]> Reviewed-By: Sam Roberts <[email protected]>
As discussed in #30129 (comment), when we vendor in code, we own the Syntax Errors in that code. This PR adds The `.flake8` config file at the root of this repo puts blinders on the linting of our dependencies so this test disables that file before linting. fixup: allow_failures until dependencies pass PR-URL: #30143 Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Jiawen Geng <[email protected]> Reviewed-By: Sam Roberts <[email protected]>
As discussed in #30129 (comment), when we vendor in code, we own the Syntax Errors in that code. This PR adds The `.flake8` config file at the root of this repo puts blinders on the linting of our dependencies so this test disables that file before linting. fixup: allow_failures until dependencies pass PR-URL: #30143 Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Jiawen Geng <[email protected]> Reviewed-By: Sam Roberts <[email protected]>
PR-URL: #30272 Fixes: #30129 Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Shelley Vohr <[email protected]> Reviewed-By: Yongsheng Zhang <[email protected]> Reviewed-By: Daniel Bevenius <[email protected]> Reviewed-By: Michaël Zasso <[email protected]>
PR-URL: #30272 Fixes: #30129 Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Shelley Vohr <[email protected]> Reviewed-By: Yongsheng Zhang <[email protected]> Reviewed-By: Daniel Bevenius <[email protected]> Reviewed-By: Michaël Zasso <[email protected]>
master
Darwin
build
b2ccbb2 seems to have caused
./configure
to fail locally.Before:
After:
No changes were made to XCode, and i have command line tools on my machine already. Reverting this commit locally resolves the issue, but i'm not sure why it would have caused it to begin with?
cc @cclauss perhaps?
The text was updated successfully, but these errors were encountered: