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

Must reconsider gyp usage due to Python 2.0 deprecation in 2020 #207

Closed
ibc opened this issue Jul 18, 2018 · 28 comments
Closed

Must reconsider gyp usage due to Python 2.0 deprecation in 2020 #207

ibc opened this issue Jul 18, 2018 · 28 comments
Assignees
Milestone

Comments

@ibc
Copy link
Member

ibc commented Jul 18, 2018

Nobody is gonna add Python 3 support to gyp, so we are dead in 2020:

https://bugs.chromium.org/p/gyp/issues/detail?id=36

@saghul may you run away from ancient programming languages for a while and let's know your thoughts about this?

@saghul
Copy link
Contributor

saghul commented Jul 18, 2018

Don't stop believin': https://bugs.chromium.org/p/gyp/issues/detail?id=36#c44

@ibc
Copy link
Member Author

ibc commented Jul 18, 2018

Oh cool! Although I was more interested in moving to ninja plus the native node gyp (I don't mean node-gyp) approach.

@ibc
Copy link
Member Author

ibc commented Nov 6, 2018

No updates regarding gyp for Python 3.0. We all are gonna die.

@ibc
Copy link
Member Author

ibc commented Nov 19, 2018

As commented here:

The Node.js project is forking GYP, and the first item in the backlog is python3 support.
We already landed it for node-gyp nodejs/node-gyp#1335

You can follow the progress at https://github.com/refack/GYP which might soon move to the nodejs org.

BTW I do plan on integrating gyp.js into GYP to have it cross-runtime portable.

So there is hope :)

@ibc
Copy link
Member Author

ibc commented Dec 4, 2018

We all are gonna die.

@ibc
Copy link
Member Author

ibc commented Jan 22, 2019

Wow! now we have two GYP projects both compatible with Python 3!

However in both cases the commiter is Dirk Pranke... so I don't what to do. @saghul do you know anything else?

@ibc
Copy link
Member Author

ibc commented Jan 23, 2019

Ok, so I've installed master versions of both projects (the original one and the Node fork one) and run the configure.py file of mediasoup/worker:

  • The Node fork works with Python 2.

  • The Node fork miserably fails with Python 3:

python3 ./scripts/configure.py -R mediasoup-worker
Traceback (most recent call last):
  File "./scripts/configure.py", line 21, in <module>
    import gyp
  File "./deps/gyp/pylib/gyp/__init__.py", line 9, in <module>
    import gyp.input
  File "./deps/gyp/pylib/gyp/input.py", line 6, in <module>
    from compiler.ast import Const
ModuleNotFoundError: No module named 'compiler'
  • The original project works with Python 2.

  • The original project fails with Python 3 due to our configure.py syntax:

python3 ./scripts/configure.py -R mediasoup-worker
Traceback (most recent call last):
  File "./scripts/configure.py", line 85, in <module>
    (major, minor), is_clang = compiler_version()
  File "./scripts/configure.py", line 39, in compiler_version
    is_clang = 'clang' in proc.communicate()[0].split('\n')[0]
TypeError: a bytes-like object is required, not 'str'

I've also checked the configure.py script of the Nodejs project, which "seems" to be Python 3 (due to print() with parenthesis?) but the configure script clearly says that it requires Python 2. So no idea.

@saghul do you remember if you owe me any favor?

@saghul
Copy link
Contributor

saghul commented Jan 23, 2019

@ibc Do you have the py3 gyp update in a branch? I'll give it a go. The print() syntax can also be used in Python 2, so slight tweaks might be needed to your configure file. I can take a look.

@ibc
Copy link
Member Author

ibc commented Jan 23, 2019

Yes! Just do this:

  • Clone the repo
  • Checkout branch v3 (it contains the very latest gyp master branch)
  • Comment this check
  • cd worker/
  • Run make with PYTHON env variable pointing to python 2 or 3 to test them:
$ PYTHON=python2 make
$ PYTHON=/usr/local/garbage-stuff/python3 make

If you, for any reason, wanna try with the Node gyp fork (which miserably fails for me as shown above) just let me know. It's just about changing the corresponding repo in worker/scripts/get-dep.sh).

THANKS.

@saghul
Copy link
Contributor

saghul commented Jan 23, 2019

I think some more stuff is needed. With this change it starts to work:

diff --git a/worker/scripts/configure.py b/worker/scripts/configure.py
index fe086764..2aada38f 100755
--- a/worker/scripts/configure.py
+++ b/worker/scripts/configure.py
@@ -9,8 +9,8 @@ import sys
 
 version = sys.version_info[0]
 
-if version != 2:
-  raise RuntimeError('gyp requires Python 2, but this is Python ' + str(version) + ', ensure your python2 or python command points to the Python 2 executable')
+#if version != 2:
+#  raise RuntimeError('gyp requires Python 2, but this is Python ' + str(version) + ', ensure your python2 or python command points to the Python 2 executable')
 
 CC = os.environ.get('CC', 'cc')
 script_dir = os.path.dirname(__file__)
@@ -35,13 +35,13 @@ def host_arch():
 
 def compiler_version():
   proc = subprocess.Popen(CC.split() + ['--version'], stdout=subprocess.PIPE)
-  is_clang = 'clang' in proc.communicate()[0].split('\n')[0]
+  is_clang = b'clang' in proc.communicate()[0].split(b'\n')[0]
   proc = subprocess.Popen(CC.split() + ['-dumpversion'], stdout=subprocess.PIPE)
-  version = proc.communicate()[0].split('.')
+  version = proc.communicate()[0].split(b'.')
   mayor_version = int(version[:1][0])
   if is_clang == False and mayor_version >= 7:
       proc = subprocess.Popen(CC.split() + ['-dumpfullversion'], stdout=subprocess.PIPE)
-      version = proc.communicate()[0].split('.')
+      version = proc.communicate()[0].split(b'.')
   version = map(int, version[:2])
   version = tuple(version)
   return (version, is_clang)

But then fails to detect Xcode / CLT:

$ make
make Release
python3 ./scripts/configure.py -R mediasoup-worker
['-R', 'mediasoup-worker', '/Users/saghul/src/mediasoup/worker/mediasoup-worker.gyp', '-I', '/Users/saghul/src/mediasoup/worker/common.gypi', '--depth=.', '-f', 'make', '-Goutput_dir=/Users/saghul/src/mediasoup/worker/out', '--generator-output', '/Users/saghul/src/mediasoup/worker/out', '-Dgcc_version=42', '-Dclang=1', '-Dhost_arch=x64', '-Dtarget_arch=x64', '-Dopenssl_fips=', '-Dmediasoup_asan=false', '-Dnode_byteorder=little']
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
make[1]: *** [Release] Error 1
make: *** [default] Error 2

I'll take another look later.

@ibc
Copy link
Member Author

ibc commented Jan 23, 2019

Same here. It seems that /var/db/receipts/com.apple.pkg.DeveloperToolsCLI.plist should exist but it doesn't:

$ pkgutil --pkg-info=com.apple.pkg.DeveloperToolsCLI
No receipt for 'com.apple.pkg.DeveloperToolsCLI' found at '/'.

Installing/updating "Xcode components" to check it again. No, nothing.

@saghul
Copy link
Contributor

saghul commented Jan 23, 2019

I checked again. It's the same bytes / str problem inside some gyp checks. I don't think it's Python 3 ready yet.

@ibc
Copy link
Member Author

ibc commented Jan 23, 2019

I checked again. It's the same bytes / str problem inside some gyp checks. I don't think it's Python 3 ready yet.

Thanks. I've commented it in the gyp tracker: https://bugs.chromium.org/p/gyp/issues/detail?id=36#c58

Let's see how it evolves. We have time. Some weeks ago I thought we were to die and today it was almost fixed. Thanks! :)

@ibc
Copy link
Member Author

ibc commented Jan 23, 2019

BTW, I've open a Python2 console and replicated similar code to the one in gyp/pylib/gyp/xcode_emulation.py line 1434:

$ python
Python 2.7.10 (default, Aug 17 2018, 17:41:52)
[GCC 4.2.1 Compatible Apple LLVM 10.0.0 (clang-1000.0.42)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> import re
>>> import subprocess
>>> import sys
>>> STANDALONE_PKG_ID = "com.apple.pkg.DeveloperToolsCLILeo"
>>> cmdlist = ['/usr/sbin/pkgutil', '--pkg-info', STANDALONE_PKG_ID]
>>> job = subprocess.Popen(cmdlist, stdout=subprocess.PIPE)
>>> No receipt for 'com.apple.pkg.DeveloperToolsCLILeo' found at '/'.

So, it looks like the same error that happens when running the emediasoup configure.py with Python3, but note that I've used Python2!! No idea.

@saghul
Copy link
Contributor

saghul commented Jan 24, 2019

No, that's not the actual error. Your system doesn't have that package, that's the old name. The new names are com.apple.pkg.CLTools_Executables and com.apple.pkg.Xcode. But the problem is getting bytes back from the process output and then trying to split it with str.

@saghul
Copy link
Contributor

saghul commented Jan 24, 2019

With this patch compilation begins:

diff --git a/worker/deps/gyp/pylib/gyp/xcode_emulation.py b/worker/deps/gyp/pylib/gyp/xcode_emulation.py
index 4c875de3..ee03816d 100644
--- a/worker/deps/gyp/pylib/gyp/xcode_emulation.py
+++ b/worker/deps/gyp/pylib/gyp/xcode_emulation.py
@@ -526,7 +526,7 @@ class XcodeSettings(object):
       XcodeSettings._sdk_path_cache[sdk_root] = sdk_path
       if sdk_root:
         XcodeSettings._sdk_root_cache[sdk_path] = sdk_root
-    return XcodeSettings._sdk_path_cache[sdk_root]
+    return XcodeSettings._sdk_path_cache[sdk_root].decode()
 
   def _AppendPlatformVersionMinFlags(self, lst):
     self._Appendf(lst, 'MACOSX_DEPLOYMENT_TARGET', '-mmacosx-version-min=%s')
@@ -1413,7 +1413,7 @@ def XcodeVersion():
   version = version_list[0]
   build = version_list[-1]
   # Be careful to convert "4.2" to "0420":
-  version = version.split()[-1].replace('.', '')
+  version = version.split()[-1].decode().replace('.', '')
   version = (version + '0' * (3 - len(version))).zfill(4)
   if build:
     build = build.split()[-1]
@@ -1450,9 +1450,9 @@ def GetStdout(cmdlist):
   job = subprocess.Popen(cmdlist, stdout=subprocess.PIPE)
   out = job.communicate()[0]
   if job.returncode != 0:
-    sys.stderr.write(out + '\n')
+    sys.stderr.write(out + b'\n')
     raise GypError('Error %d running %s' % (job.returncode, cmdlist[0]))
-  return out.rstrip('\n')
+  return out.rstrip(b'\n')
 
 
 def MergeGlobalXcodeSettingsToSpec(global_dict, spec):
diff --git a/worker/scripts/configure.py b/worker/scripts/configure.py
index fe086764..2aada38f 100755
--- a/worker/scripts/configure.py
+++ b/worker/scripts/configure.py
@@ -9,8 +9,8 @@ import sys
 
 version = sys.version_info[0]
 
-if version != 2:
-  raise RuntimeError('gyp requires Python 2, but this is Python ' + str(version) + ', ensure your python2 or python command points to the Python 2 executable')
+#if version != 2:
+#  raise RuntimeError('gyp requires Python 2, but this is Python ' + str(version) + ', ensure your python2 or python command points to the Python 2 executable')
 
 CC = os.environ.get('CC', 'cc')
 script_dir = os.path.dirname(__file__)
@@ -35,13 +35,13 @@ def host_arch():
 
 def compiler_version():
   proc = subprocess.Popen(CC.split() + ['--version'], stdout=subprocess.PIPE)
-  is_clang = 'clang' in proc.communicate()[0].split('\n')[0]
+  is_clang = b'clang' in proc.communicate()[0].split(b'\n')[0]
   proc = subprocess.Popen(CC.split() + ['-dumpversion'], stdout=subprocess.PIPE)
-  version = proc.communicate()[0].split('.')
+  version = proc.communicate()[0].split(b'.')
   mayor_version = int(version[:1][0])
   if is_clang == False and mayor_version >= 7:
       proc = subprocess.Popen(CC.split() + ['-dumpfullversion'], stdout=subprocess.PIPE)
-      version = proc.communicate()[0].split('.')
+      version = proc.communicate()[0].split(b'.')
   version = map(int, version[:2])
   version = tuple(version)
   return (version, is_clang)

I have only tested it 10s on macOS though.

@ibc
Copy link
Member Author

ibc commented Jan 24, 2019

Oh! So, to clarify: you are making changes into xcode_emulation.py within gyp sources. Does it mean it looks like a bug in gyp?

@saghul
Copy link
Contributor

saghul commented Jan 24, 2019

Correct.

@ibc
Copy link
Member Author

ibc commented Jan 24, 2019

BTW confirmed that with your changes it works in both Python 2 and 3!

Ok, I'll comment that bug in https://bugs.chromium.org/p/gyp/issues/detail?id=36#c58

@ibc
Copy link
Member Author

ibc commented Jan 24, 2019

Done. Thanks, this is amazing!

@ibc ibc added this to the v3 updates milestone May 5, 2019
ibc added a commit that referenced this issue May 28, 2019
ibc added a commit that referenced this issue May 28, 2019
Note that gyp source code has been manually modified within the worker/deps/gyp
folder (credits to @saghul)
@ibc
Copy link
Member Author

ibc commented May 28, 2019

mediasoup 3.0.12 already supports Python 3: acf9417

However, tote that gyp source code has been manually modified (and the above path by @saghul
applied) within the worker/deps/gyp folder. So let's keep this issue open as a reminder.

gyp issue current status: the issue is still present. I've added a summary comment: https://bugs.chromium.org/p/gyp/issues/detail?id=36#c65

@ibc ibc removed the help wanted label May 29, 2019
@saghul
Copy link
Contributor

saghul commented May 29, 2019

🎉

@ibc
Copy link
Member Author

ibc commented May 29, 2019

@ibc ibc closed this as completed May 29, 2019
@ibc
Copy link
Member Author

ibc commented May 30, 2019

@saghul, there is a guy with this gyp error that I think may be related to Python 3 and some missing change. Basically this:

Traceback (most recent call last):
  File "./gyp-mac-tool", line 720, in <module>
    sys.exit(main(sys.argv[1:]))
  File "./gyp-mac-tool", line 30, in main
    exit_code = executor.Dispatch(args)
  File "./gyp-mac-tool", line 45, in Dispatch
    return getattr(self, method)(*args[1:])
  File "./gyp-mac-tool", line 274, in ExecFilterLibtool
    if not libtool_re.match(line) and not libtool_re5.match(line):
TypeError: cannot use a string pattern on a bytes-like object
make[1]: *** [/Users/username/Workspace/work/webrtc/node_modules/mediasoup/worker/out/Release/libopenssl.a] Error 1
make: *** [default] Error 2

The gyp-mac-tool file is autogenerated by gyp and it's just a copy of pylib/gyp/mac-tool.py. The line 274 is as follows:

    libtoolout = subprocess.Popen(cmd_list, stderr=subprocess.PIPE, env=env)
    _, err = libtoolout.communicate()
    for line in err.splitlines():
      if not libtool_re.match(line) and not libtool_re5.match(line):  # <--- line 274
        print(line, file=sys.stderr)

The are also other similar lines:

if ibtool_section_re.match(line):
elif not ibtool_re.match(line):

The environment is:

  • macOS: 10.14.5
  • mediasoup: 3.0.12
  • node: 10.15.0
  • python: 3.6.6
  • yarn: 1.16.0

Any magic to do here that you may identify?

@ibc ibc reopened this May 30, 2019
@philipnery
Copy link

philipnery commented May 30, 2019

As suggested in the thread, I've tried experimenting with make.

Since line returns a byte object, decoding it first actually resolves my issue with make. Decoding is necessary for Python 3.

for line in err.splitlines():
      decoded_line = line.decode("utf-8") 
      if not libtool_re.match(decoded_line) and not libtool_re5.match(decoded_line):
        print(decoded_line, file=sys.stderr)

@philipnery
Copy link

By the way, I've uploaded my change to gyp to make it more compatible with Python 3
https://chromium-review.googlesource.com/c/external/gyp/+/1639564

@ibc
Copy link
Member Author

ibc commented Jun 3, 2019

Thanks @philipnery, good work.

ibc added a commit that referenced this issue Jun 17, 2019
@ibc
Copy link
Member Author

ibc commented Jun 17, 2019

gyp dependency in mediasoup v3 has been updated to master, so let's consider this issue fixed. Thanks @philipnery!

@ibc ibc closed this as completed Jun 17, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

4 participants