Closed
Bug 783727
Opened 12 years ago
Closed 12 years ago
Add psutil Python package to mozilla-central
Categories
(Firefox Build System :: General, defect)
Firefox Build System
General
Tracking
(Not tracked)
RESOLVED
FIXED
mozilla18
People
(Reporter: gps, Assigned: gps)
References
Details
Attachments
(6 files, 13 obsolete files)
607.69 KB,
patch
|
glandium
:
review+
|
Details | Diff | Splinter Review |
1.47 KB,
patch
|
Details | Diff | Splinter Review | |
2.10 KB,
patch
|
glandium
:
review+
|
Details | Diff | Splinter Review |
2.36 KB,
patch
|
glandium
:
review+
|
Details | Diff | Splinter Review |
1.48 KB,
patch
|
Details | Diff | Splinter Review | |
4.03 KB,
patch
|
ted
:
review+
|
Details | Diff | Splinter Review |
https://code.google.com/p/psutil/wiki/Documentation I'm salivating over the possibilities. One-line memory usage? Yeah, that would have been nice when implementing linker memory measuring. I think it would be cool to spin up a background thread during builds, test runs, etc that measured process resource utilization and report that as part of the build. Anyway, this is an awesome tool. Let's put it to use. Try at https://tbpl.mozilla.org/?tree=Try&rev=3d105a18481c I suspect the Python developer headers (required for compiling) aren't on the buildbot machines. Will file releng bugs if we want to do this.
Assignee | ||
Comment 1•12 years ago
|
||
Surprisingly, the Try build only failed on the ARM 7 builders \o/
Comment 2•12 years ago
|
||
Please make it optional, and enable it in the mozconfigs, it would be bad to start requiring python development files just to build firefox.
Assignee | ||
Comment 3•12 years ago
|
||
(In reply to Mike Hommey [:glandium] from comment #2) > Please make it optional, and enable it in the mozconfigs, it would be bad to > start requiring python development files just to build firefox. Yeah, I can do that. Although, I'd really like to get this running on buildbot. Let's just say I have my build splendid branch recording CPU, I/O, and memory metrics every second during builds. It can also correlate that to tiers and directories in the make traversal :D I just need a decent UI. Maybe I'll use the HTTP server built-in to Python's standard library along with some nifty JS graphing library... I don't feel like writing configure magic to detect Python headers. The Python packaging tools have their own magic for this and I don't think it's worth duplicating. I think I'll just add a capability to our virtualenv populator that tries to install packages. If it fails, it continues instead of bailing. You OK with this approach?
Comment 4•12 years ago
|
||
Magic things are bad. Either you want the feature and it has to fail when it can't build, or you don't, and you certainly don't expect it to be enabled just because you happen to have the python headers installed. So I'm more in favor of a configure arg or just an AC_SUBST, depending how discoverable you want it to be. Then add the required ac_add_options or export to mozconfigs.
Comment 5•12 years ago
|
||
With this in mozilla-central, we should assess what can be done with mozbase/mozprocess with this module and possibly depend on it there
Assignee | ||
Comment 6•12 years ago
|
||
(In reply to Jeff Hammel [:jhammel] from comment #5) > With this in mozilla-central, we should assess what can be done with > mozbase/mozprocess with this module and possibly depend on it there psutil.POpen() wraps subprocess.POpen() and returns a psutil.Process, which has all the lovely APIs for querying state.
Assignee | ||
Comment 7•12 years ago
|
||
I suck at autoconf. This is the best I could come up with. I know it isn't good enough to add to the tree. Do you think you could take it to the next level for me? I also threw in the virtualenv changes so you can see where I'm going with this. This assumes psutil is at python/psutil. So, if you want to test, you'll need to uncompress a psutil archive there or hack up packages.txt.
Attachment #657182 -
Flags: feedback?(mh+mozilla)
Comment 8•12 years ago
|
||
Comment on attachment 657182 [details] [diff] [review] alpha autoconf magic Review of attachment 657182 [details] [diff] [review]: ----------------------------------------------------------------- That's not very much worse than anything else in the tree. But it will fail to do the right thing when cross-compiling. ::: configure.in @@ +3089,5 @@ > + > +for flag in $_PYTHON_LIBS; do > + case $flag in > + -lpython*) > + python_lib=`echo $flag | sed 's/^-l//'` You're not using that variable. (and it lacks an underscore at the beginning)
Attachment #657182 -
Flags: feedback?(mh+mozilla)
Comment 9•12 years ago
|
||
(In reply to Mike Hommey [:glandium] from comment #8) > That's not very much worse than anything else in the tree. But it will fail > to do the right thing when cross-compiling. Note that there aren't autoconf macros to correctly handle the cross-compiling case. Modifying ac_compile is the closest to something that can work without too much hassle.
Comment 10•12 years ago
|
||
Something like: _save_ac_compile=${ac_compile} ac_compile='${HOST_CC-cc} -c $HOST_CFLAGS $HOST_CPPFLAGS conftest.$ac_ext 1>&5' ... do the test ... ac_compile=${_save_ac_compile}
Assignee | ||
Comment 11•12 years ago
|
||
This plays with fire. See discussion in bug 659881. I think this is better for everybody. By using the version from the actual Python, we should ensure maximum compatibility. In other words, we default to something that should work instead of defaulting to something that might work. If systems (e.g. buildbot hosts) want to force a version, they can do so via a mozconfig file. We /may/ need to do this as part of this patch. I'll let a try build fail first.
Assignee: nobody → gps
Attachment #657182 -
Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #658344 -
Flags: review?(mh+mozilla)
Assignee | ||
Comment 12•12 years ago
|
||
Pretty straightforward, I think.
Attachment #658345 -
Flags: review?(mh+mozilla)
Assignee | ||
Comment 13•12 years ago
|
||
I just added the archive from upstream unaltered.
Attachment #658346 -
Flags: review?(mh+mozilla)
Assignee | ||
Comment 14•12 years ago
|
||
One packages.txt line does the compilation. The other one sets up the pth file. Therefore, no egg-info directories. Try at http://tbpl.mozilla.org/?tree=Try&rev=f1b128f26923
Attachment #658347 -
Flags: review?(mh+mozilla)
Comment 15•12 years ago
|
||
Comment on attachment 658344 [details] [diff] [review] Part 1: Change how MACOSX_DEPLOYMENT_TARGET is set, v1 Review of attachment 658344 [details] [diff] [review]: ----------------------------------------------------------------- I'm not very fond of the idea, because it enforces MACOSX_DEPLOYMENT_TARGET to what python was built against, which is completely irrelevant to what we are trying to build. Moreover, if you *do* need to build for an older osx version than what python was built for, you'd now be screwed, instead of being able to build what you want. In essence, the problem is that we don't have a different MACOSX_DEPLOYMENT_TARGET for host and target builds. We do want a specific version for target builds, and don't really care for host builds. Host builds are where we'd want MACOSX_DEPLOYMENT_TARGET not to be set, and are what the virtualenv population are. I think MACOSX_DEPLOYMENT_TARGET is better kept as it currently is. ::: configure.in @@ +900,2 @@ > MOZ_ARG_ENABLE_STRING(macos-target, > + [ --enable-macos-target=VER (default=$osx_deployment_target_system) These kind of things don't actually work. The help string of a MOZ_ARG_ENABLE_STRING is placed at the beginning of configure, while the variable assignment happens much later.
Attachment #658344 -
Flags: review?(mh+mozilla) → review-
Comment 16•12 years ago
|
||
Comment on attachment 658345 [details] [diff] [review] Part 2: Detect Python SDK in configure, v1 Review of attachment 658345 [details] [diff] [review]: ----------------------------------------------------------------- ::: configure.in @@ +3109,5 @@ > + > + _SAVE_CFLAGS="$CFLAGS" > + _SAVE_LDFLAGS="$LDFLAGS" > + CFLAGS="$CFLAGS $_PYTHON_CFLAGS" > + LDFLAGS="$LDFLAGS $_PYTHON_LDFLAGS" Please use HOST_CFLAGS/HOST_LDFLAGS, and set ac_link to '${HOST_CC-cc} -o conftest${ac_exeext} $HOST_CFLAGS $HOST_CPPFLAGS $HOST_LDFLAGS conftest.$ac_ext $LIBS 1>&5'
Attachment #658345 -
Flags: review?(mh+mozilla) → feedback+
Updated•12 years ago
|
Attachment #658346 -
Flags: review?(mh+mozilla) → review+
Comment 17•12 years ago
|
||
Comment on attachment 658347 [details] [diff] [review] Part 4: Add psutil to virtualenv, v1 Review of attachment 658347 [details] [diff] [review]: ----------------------------------------------------------------- ::: .gitignore @@ +49,5 @@ > + > +# Python virtualenv artifacts. > +python/psutil/.*.so > +python/psutil/.*.o > +python/psutil/.*.dll What is virtualenv placing there exactly? Can we do force it not to do that, or to do it elsewhere? why no .obj?
Assignee | ||
Comment 18•12 years ago
|
||
When you build C extensions, it will perform the original compilation in a build/ directory. On OS X, it produces the files: python/psutil//build python/psutil//build/lib.macosx-10.8-x86_64-2.7 python/psutil//build/lib.macosx-10.8-x86_64-2.7/_psutil_osx.so python/psutil//build/lib.macosx-10.8-x86_64-2.7/_psutil_posix.so python/psutil//build/temp.macosx-10.8-x86_64-2.7 python/psutil//build/temp.macosx-10.8-x86_64-2.7/psutil python/psutil//build/temp.macosx-10.8-x86_64-2.7/psutil/_psutil_common.o python/psutil//build/temp.macosx-10.8-x86_64-2.7/psutil/_psutil_osx.o python/psutil//build/temp.macosx-10.8-x86_64-2.7/psutil/_psutil_posix.o python/psutil//build/temp.macosx-10.8-x86_64-2.7/psutil/arch python/psutil//build/temp.macosx-10.8-x86_64-2.7/psutil/arch/osx python/psutil//build/temp.macosx-10.8-x86_64-2.7/psutil/arch/osx/process_info.o You'll notice that the packages.txt line has --inplace defined. This causes the build to copy the .so into the same directory as setup.py, yielding: python/psutil/_psutil_osx.so python/psutil/_psutil_posix.so This enables you to import things since python/psutil is added as part of a .pth. Ideally these things would be installed to the virtualenv to not pollute srcdir. Unfortunately, I don't believe there is an "install just the C extensions" command for setuptools. I suppose we could hack something into our virtualenv population logic if we really wanted to. But, I think this might be somewhat hacky and fragile.
Assignee | ||
Comment 19•12 years ago
|
||
OK. No more mucking around with the global MACOSX_DEPLOYMENT_TARGET. Instead, we now set this variable to the same as Python's only when populating the virtualenv. Keep in mind this change only affects Python packages with C extensions. Currently we have none. After all parts of this bug land, we'll have psutil. And even that will be optional. One would think that the old behavior of not setting MACOSX_DEPLOYMENT_TARGET would work. However, on my machine (which has Python 2.7.3 built from source and Python's MACOSX_DEPLOYMENT_TARGET set to 10.8), I get the following error when building psutil through configure: Populating Python virtualenv running build_ext building '_psutil_osx' extension /usr/local/llvm/bin/clang -DNDEBUG -g -fwrapv -O3 -Wall -Wstrict-prototypes -fcolor-diagnostics -DXP_UNIX -DXP_MACOSX -DNO_X11 -I/usr/local/bin/../Cellar/python/2.7.3/include/python2.7 -c psutil/_psutil_osx.c -o build/temp.macosx-10.8-x86_64-2.7/psutil/_psutil_osx.o Traceback (most recent call last): File "/Users/gps/src/services-central/python/psutil/setup.py", line 159, in <module> main() File "/Users/gps/src/services-central/python/psutil/setup.py", line 156, in main setup(**setup_args) File "/usr/local/Cellar/python/2.7.3/lib/python2.7/distutils/core.py", line 152, in setup dist.run_commands() File "/usr/local/Cellar/python/2.7.3/lib/python2.7/distutils/dist.py", line 953, in run_commands self.run_command(cmd) File "/usr/local/Cellar/python/2.7.3/lib/python2.7/distutils/dist.py", line 972, in run_command cmd_obj.run() File "/Users/gps/src/services-central/obj-ff-dbg/_virtualenv/lib/python2.7/site-packages/setuptools-0.6c11-py2.7.egg/setuptools/command/build_ext.py", line 46, in run File "/usr/local/Cellar/python/2.7.3/lib/python2.7/distutils/command/build_ext.py", line 339, in run self.build_extensions() File "/usr/local/Cellar/python/2.7.3/lib/python2.7/distutils/command/build_ext.py", line 448, in build_extensions self.build_extension(ext) File "/Users/gps/src/services-central/obj-ff-dbg/_virtualenv/lib/python2.7/site-packages/setuptools-0.6c11-py2.7.egg/setuptools/command/build_ext.py", line 175, in build_extension File "/usr/local/Cellar/python/2.7.3/lib/python2.7/distutils/command/build_ext.py", line 498, in build_extension depends=ext.depends) File "/usr/local/Cellar/python/2.7.3/lib/python2.7/distutils/ccompiler.py", line 572, in compile self._compile(obj, src, ext, cc_args, extra_postargs, pp_opts) File "/usr/local/Cellar/python/2.7.3/lib/python2.7/distutils/unixccompiler.py", line 178, in _compile extra_postargs) File "/usr/local/Cellar/python/2.7.3/lib/python2.7/distutils/ccompiler.py", line 873, in spawn spawn(cmd, dry_run=self.dry_run) File "/usr/local/Cellar/python/2.7.3/lib/python2.7/distutils/spawn.py", line 34, in spawn _spawn_posix(cmd, search_path, dry_run=dry_run) File "/usr/local/Cellar/python/2.7.3/lib/python2.7/distutils/spawn.py", line 122, in _spawn_posix if _cfg_target_split > [int(x) for x in cur_target.split('.')]: ValueError: invalid literal for int() with base 10: '' Traceback (most recent call last): File "/Users/gps/src/services-central/build/virtualenv/populate_virtualenv.py", line 85, in <module> populate_virtualenv(sys.argv[1], sys.argv[2], have_c) File "/Users/gps/src/services-central/build/virtualenv/populate_virtualenv.py", line 61, in populate_virtualenv handle_package(package) File "/Users/gps/src/services-central/build/virtualenv/populate_virtualenv.py", line 58, in handle_package handle_package(package[1:]) File "/Users/gps/src/services-central/build/virtualenv/populate_virtualenv.py", line 49, in handle_package package[2:]) File "/Users/gps/src/services-central/build/virtualenv/populate_virtualenv.py", line 77, in call_setup raise Exception('Error installing package: %s' % directory) Exception: Error installing package: /Users/gps/src/services-central/python/psutil *** Fix above errors and then restart with "make -f client.mk build" I can't not set MACOSX_DEPLOYMENT_TARGET as part of the command line because I get: Populating Python virtualenv running build_ext building '_psutil_osx' extension creating build creating build/temp.macosx-10.8-x86_64-2.7 creating build/temp.macosx-10.8-x86_64-2.7/psutil creating build/temp.macosx-10.8-x86_64-2.7/psutil/arch creating build/temp.macosx-10.8-x86_64-2.7/psutil/arch/osx /usr/local/llvm/bin/clang -DNDEBUG -g -fwrapv -O3 -Wall -Wstrict-prototypes -fcolor-diagnostics -DXP_UNIX -DXP_MACOSX -DNO_X11 -I/usr/local/bin/../Cellar/python/2.7.3/include/python2.7 -c psutil/_psutil_osx.c -o build/temp.macosx-10.8-x86_64-2.7/psutil/_psutil_osx.o error: $MACOSX_DEPLOYMENT_TARGET mismatch: now "10.6" but "10.8" during configure Traceback (most recent call last): File "/Users/gps/src/services-central/build/virtualenv/populate_virtualenv.py", line 85, in <module> populate_virtualenv(sys.argv[1], sys.argv[2], have_c) File "/Users/gps/src/services-central/build/virtualenv/populate_virtualenv.py", line 61, in populate_virtualenv handle_package(package) File "/Users/gps/src/services-central/build/virtualenv/populate_virtualenv.py", line 58, in handle_package handle_package(package[1:]) File "/Users/gps/src/services-central/build/virtualenv/populate_virtualenv.py", line 49, in handle_package package[2:]) File "/Users/gps/src/services-central/build/virtualenv/populate_virtualenv.py", line 77, in call_setup raise Exception('Error installing package: %s' % directory) Exception: Error installing package: /Users/gps/src/services-central/python/psutil *** Fix above errors and then restart with "make -f client.mk build" I suppose we could hold off exporting MACOSX_DEPLOYMENT_TARGET. But, that seems like more pain than it's worth.
Attachment #658344 -
Attachment is obsolete: true
Attachment #658523 -
Flags: review?(mh+mozilla)
Assignee | ||
Comment 20•12 years ago
|
||
Incorporated feedback. Parts 3 and 4 are identical to previous (I still need to investigate .obj on Windows and will add a new patch for part 4 if I need to add version control ignores). Try at https://tbpl.mozilla.org/?tree=Try&rev=7c78aeeb75f9
Attachment #658345 -
Attachment is obsolete: true
Attachment #658524 -
Flags: review?(mh+mozilla)
Assignee | ||
Comment 21•12 years ago
|
||
As part of the latest try run, I discovered that sysconfig is only available in Python 2.7. I'm going to change part 1 to be dependent on Python 2.7. Since the build system should soon require 2.7, I think this will be acceptable. Anyway, I'm sure there is a more efficient way to do this. But, I'm not sure if autoconf has a built-in split or string parsing without invoking extra shells.
Attachment #658561 -
Flags: review?(mh+mozilla)
Assignee | ||
Comment 22•12 years ago
|
||
Comment on attachment 658523 [details] [diff] [review] Part 1: Set MACOSX_DEPLOYMENT_TARGET when building virtualenv, v2 sysconfig is only present in Python 2.7. Will need some additional work here.
Attachment #658523 -
Flags: review?(mh+mozilla)
Assignee | ||
Comment 23•12 years ago
|
||
Now conditional on Python 2.7. I suppose there is a way to get at this information with Python 2.6 and below. But, presumably builds will soon require 2.7, so I'm not too worried about supporting it.
Attachment #658523 -
Attachment is obsolete: true
Attachment #658569 -
Flags: review?(mh+mozilla)
Assignee | ||
Comment 24•12 years ago
|
||
Now passing whether to build c extensions as an argument not an environment variable. Note that C extension support is still failing on try due to compile failures in AC_TRY_LINK. This may not be the final version of this patch.
Attachment #658524 -
Attachment is obsolete: true
Attachment #658524 -
Flags: review?(mh+mozilla)
Attachment #658570 -
Flags: review?(mh+mozilla)
Assignee | ||
Comment 25•12 years ago
|
||
Python 2.5 doesn't have namedtuple. So, use index access.
Attachment #658561 -
Attachment is obsolete: true
Attachment #658561 -
Flags: review?(mh+mozilla)
Attachment #658590 -
Flags: review?(mh+mozilla)
Assignee | ||
Comment 26•12 years ago
|
||
I should probably test these things before submitting a patch for review. Silly square brackets and autoconf.
Attachment #658590 -
Attachment is obsolete: true
Attachment #658590 -
Flags: review?(mh+mozilla)
Attachment #658594 -
Flags: review?(mh+mozilla)
Assignee | ||
Comment 27•12 years ago
|
||
Comment on attachment 658594 [details] [diff] [review] Part 0: Capture Python version in configure, v3 Review of attachment 658594 [details] [diff] [review]: ----------------------------------------------------------------- ::: configure.in @@ +1796,5 @@ > changequote(,) > +python_version_major=`$PYTHON -c 'import sys; print sys.version_info[0]'` > +python_version_minor=`$PYTHON -c 'import sys; print sys.version_info[1]'` > +python_version_micro=`$PYTHON -c 'import sys; print sys.version_info[2]'` > +-changequote([,]) Ignore the leading -. It just isn't my day.
Assignee | ||
Comment 28•12 years ago
|
||
It turns out that Python on the OS X builders is broken (bug 788705). And, there doesn't appear to be a python-config.exe on Windows. So, I just reimplemented python-config minimally in a standalone .py file. I initially tried to inline it in configure.in, but the escaping, etc got too ugly for my tastes. With all these patches applied, this is how things fare: * OS X - works locally and on build infra \o/ * Linux - works locally. Build infra fails to detect Python SDK because of linking issue. Not sure if the .so is missing or what. The Linux builders are running Python 2.5, so I expect them to break. * Windows - Does not work locally nor on the build infra. It fails to detect the Python SDK. As part of investigating this, I pushed a Try build that tries to build psutil by ignoring the result of Python SDK detection: https://tbpl.mozilla.org/?tree=Try&rev=0bf33ffa65f6 Ignore the reds on that build because I put an "exit 1" at the bottom of configure to save try resources. And, I also added extra debugging which throws off the log parsers. If you dig into the logs, you'll see that psutil managed to build for Linux, Android Armv6, Android opt, Android debug, and all the Windows builders! However, Armv7a ICS opt and debug both failed because Python.h couldn't be found. I'll have to file a bug for that. Basically, this means that the "Python SDK" detection in this patch is wrong. Unfortunately, I have no clue how to fix it short of trying to build the extension and ignoring (rather than aborting) if it fails. There is just so much magic happening inside distutils and I think trying to duplicate that inside configure is just asking for trouble. Anyway, the try for this patch series modulo the configure abort is at https://tbpl.mozilla.org/?tree=Try&rev=ecfe65f305da. As I stated earlier, only OS X should work.
Attachment #658570 -
Attachment is obsolete: true
Attachment #658570 -
Flags: review?(mh+mozilla)
Attachment #658711 -
Flags: review?(mh+mozilla)
Assignee | ||
Comment 29•12 years ago
|
||
I pushed a Try that changes the behavior to try to build extensions and silently ignores failures. It /should/ work on all the builders except Armv7a (because they don't have the Python development files). http://tbpl.mozilla.org/?tree=Try&rev=da2d5f60dad1 I also filed bug 788908 to track installing the Python development files on the Armv7a machines. Arguably, we may want a single "action" in packages.txt to denote build_ext + .pth so they are done as an atomic unit. In the case of psutil, |import psutil| will fail with an ImportError because the main psutil module tries to import the binary extension. This fails if this is missing. So, while psutil's .py files are in the virtualenv, we ultimately fail.
Assignee | ||
Comment 30•12 years ago
|
||
Adding people from the Python brain trust who may be able to help here. Comments 19 and 28 have the context. Basically, what's the best way to test whether we can compile Python C extensions without going through setup.py for an actual package?
Comment 31•12 years ago
|
||
Comment on attachment 658569 [details] [diff] [review] Part 1: Set MACOSX_DEPLOYMENT_TARGET when building virtualenv, v3 Review of attachment 658569 [details] [diff] [review]: ----------------------------------------------------------------- I don't know how likely it is that future versions of virtualenv would end up breaking with an empty MACOSX_DEPLOYMENT_TARGET, but we might as well use the same thing for it.
Attachment #658569 -
Flags: review?(mh+mozilla) → review+
Comment 32•12 years ago
|
||
Comment on attachment 658594 [details] [diff] [review] Part 0: Capture Python version in configure, v3 Review of attachment 658594 [details] [diff] [review]: ----------------------------------------------------------------- ::: configure.in @@ +1796,4 @@ > changequote(,) > +python_version_major=`$PYTHON -c 'import sys; print sys.version_info[0]'` > +python_version_minor=`$PYTHON -c 'import sys; print sys.version_info[1]'` > +python_version_micro=`$PYTHON -c 'import sys; print sys.version_info[2]'` Something like the following should work, and only run python once: read python_version_major python_version_minor python_version_micro <<EOF `$PYTHON -c 'import sys; print sys.version_info[0],sys.version_info[1],sys.version_info[2]'` EOF @@ +1799,5 @@ > +python_version_micro=`$PYTHON -c 'import sys; print sys.version_info[2]'` > +-changequote([,]) > + > +if test $python_version_major -ne $PYTHON_VERSION_MAJOR; then > + AC_MSG_ERROR([Cannot build on Python $python_version_major.]) Please use the same error message as below.
Attachment #658594 -
Flags: review?(mh+mozilla) → review+
Comment 33•12 years ago
|
||
Comment on attachment 658711 [details] [diff] [review] Part 2: Detect Python SDK in configure, v4 Review of attachment 658711 [details] [diff] [review]: ----------------------------------------------------------------- More eyes on python_extensions_flags.py can't hurt.
Attachment #658711 -
Flags: review?(ted.mielczarek)
Attachment #658711 -
Flags: review?(mh+mozilla)
Attachment #658711 -
Flags: review+
Comment 34•12 years ago
|
||
(In reply to Gregory Szorc [:gps] from comment #30) > Adding people from the Python brain trust who may be able to help here. > Comments 19 and 28 have the context. > > Basically, what's the best way to test whether we can compile Python C > extensions without going through setup.py for an actual package? I am not sure to understand this question, but running through setup.py is mandatory because that's how the distutils machinery initializes its compiler to create the extension -- plus people do things sometimes in setup.py that affect compilation. So I would say: you can't test if you can compile a Python C extension until you try The simplest way to try it is to do: $ python setup.py build_ext -i and see if you get your .so files in the tree alongside the .c files
Assignee | ||
Comment 35•12 years ago
|
||
(In reply to Tarek Ziadé (:tarek) from comment #34) > I am not sure to understand this question, but running through setup.py is > mandatory because that's how the distutils machinery initializes its > compiler to create the extension -- plus people do things sometimes in > setup.py that affect compilation. I want to test whether we can compile *any* Python C detection. This typically boils down to trying to compile the most minimal Python C extension possible (just a call to Py_Initialize or similar). Unfortunately, I'm getting hung up because I don't know how to extract the right compiler flags to use. |python-config --cflags --ldflags| is giving me something that doesn't actually work. Or, in the case of Windows, AFAICT there is no python-config, so I'm not sure how exactly to obtain the compiler/linker arguments to use. I do know that on these same machines, building an extension (psutil) through setup.py does work. And, that setup.py is simply populating different variables depending on the platform - nothing too crazy. So, I guess my question is whether I can easily have Python try to compile a C extensions without setting up a dummy directory tree containing extension files. Is there a "compile this C file with the default compiler settings" API somewhere in the bowels of distutils?
Status: ASSIGNED → NEW
Assignee | ||
Updated•12 years ago
|
Status: NEW → ASSIGNED
Assignee | ||
Comment 36•12 years ago
|
||
This is an alternate version of part 2. It adds "optional" and "optional-skipnext" actions to the virtualenv manifest. Since we are having difficulty detecting generically whether we can build C extensions in configure, I figure we should just try to build C extensions and not abort if they fail. Since this actually works to build psutil on the buildbot hosts, I prefer this version. I would still like generic "can we build Python C extensions" detection in configure. But, until there is a robust way to do that, I think this is our best option. Try at https://tbpl.mozilla.org/?tree=Try&rev=9846f5f53b1c
Attachment #658952 -
Flags: review?(mh+mozilla)
Assignee | ||
Comment 37•12 years ago
|
||
Python's behavior on variable assignment and closures gets me all the time. Kinda wish Python had explicit variable declaration... New try at https://tbpl.mozilla.org/?tree=Try&rev=0829d13ecaa1
Attachment #658952 -
Attachment is obsolete: true
Attachment #658952 -
Flags: review?(mh+mozilla)
Attachment #658953 -
Flags: review?(mh+mozilla)
Assignee | ||
Comment 38•12 years ago
|
||
This is part of my preferred "b track" for this bug. It uses optional-skipnext to install psutil. Since the last patch, I also updated the VCS ignore files to catch things properly, including on Windows.
Attachment #658956 -
Flags: review?(mh+mozilla)
Assignee | ||
Comment 39•12 years ago
|
||
More Python scoping BS. Apparently you need Python 3 to declare a non-global variable as non-local. The workaround is to use a mutable type. Head asplode. Try at http://tbpl.mozilla.org/?tree=Try&rev=fdac65e18a97
Attachment #658953 -
Attachment is obsolete: true
Attachment #658953 -
Flags: review?(mh+mozilla)
Attachment #658961 -
Flags: review?(mh+mozilla)
Assignee | ||
Comment 40•12 years ago
|
||
I found a bug in psutil's reporting of I/O wait on OS X: https://code.google.com/p/psutil/issues/detail?id=323. I'll see about upstreaming the fix and adding the fix to this bug.
Comment 41•12 years ago
|
||
Comment on attachment 658961 [details] [diff] [review] Part 2b: Add optional virtualenv population actions, v3 Review of attachment 658961 [details] [diff] [review]: ----------------------------------------------------------------- ::: build/virtualenv/populate_virtualenv.py @@ +28,5 @@ > + "optional" field is stripped then the remaining line is processed > + like normal. e.g. "optional:setup.py:python/foo:built_ext:-i" > + > + optional-skipnext -- This is like optional except the action immediately > + following this one in the manifest is skipped if this action fails. I think i'd be more comfortable if the marker was on the next action. That is, make the action optional depending on the result of the previous. Ted, what do you think?
Attachment #658961 -
Flags: review?(ted.mielczarek)
Attachment #658961 -
Flags: review?(mh+mozilla)
Attachment #658961 -
Flags: feedback-
Comment 42•12 years ago
|
||
Comment on attachment 658956 [details] [diff] [review] Part 4b: Add psutil to virtualenv, v1 Resetting, pending discussion on part 2b.
Attachment #658956 -
Flags: review?(mh+mozilla)
Comment 43•12 years ago
|
||
Comment on attachment 658347 [details] [diff] [review] Part 4: Add psutil to virtualenv, v1 This is replaced by part 4b, right?
Attachment #658347 -
Flags: review?(mh+mozilla)
Comment 44•12 years ago
|
||
> + optional-skipnext -- This is like optional except the action immediately
> + following this one in the manifest is skipped if this action fails.
I can't say I'm a fan. I generally feel when domain specific languages are making one-offs like this semantic, its time to rethink a bit
Assignee | ||
Comment 45•12 years ago
|
||
(In reply to Jeff Hammel [:jhammel] from comment #44) > > + optional-skipnext -- This is like optional except the action immediately > > + following this one in the manifest is skipped if this action fails. > > I can't say I'm a fan. I generally feel when domain specific languages are > making one-offs like this semantic, its time to rethink a bit I don't like it either. FWIW, I invented packages.txt because it was less evil than a Makefile and was super simple. It has since grown features beyond what I originally envisioned. In the context of the new world, I think it should be rewritten to use Preprocessor.py or similar. But, I think that is the domain of a follow-up bug. (In reply to Mike Hommey [:glandium] from comment #43) > Comment on attachment 658347 [details] [diff] [review] > Part 4: Add psutil to virtualenv, v1 > > This is replaced by part 4b, right? Yes.
Comment 46•12 years ago
|
||
Comment on attachment 658961 [details] [diff] [review] Part 2b: Add optional virtualenv population actions, v3 Review of attachment 658961 [details] [diff] [review]: ----------------------------------------------------------------- ::: build/virtualenv/populate_virtualenv.py @@ +28,5 @@ > + "optional" field is stripped then the remaining line is processed > + like normal. e.g. "optional:setup.py:python/foo:built_ext:-i" > + > + optional-skipnext -- This is like optional except the action immediately > + following this one in the manifest is skipped if this action fails. Yeah, I don't like this either. I'm fine with "optional", but "optional-skipnext" is a bit too much for me.
Attachment #658961 -
Flags: review?(ted.mielczarek) → review-
Assignee | ||
Comment 47•12 years ago
|
||
No skipnext. I propose the compromise be to make both the build_ext and .pth actions for psutil optional. We can file a follow-up bug to convert packages.txt to use Preprocess.py where I imagine we'll solve this by having sections composed of multiple actions. If an action in a section fails, that section is aborted. Until that is ready, psutil will have a .pth created if build_ext fails. However, |import psutil| will raise ImportError because it won't be able to load the compiled extension file since it is missing. If we do it properly, it will still raise an ImportError but it will be because there is no psutil package in site-packages because the .pth is missing. The end result is the same, so I think this is an acceptable compromise.
Attachment #658711 -
Attachment is obsolete: true
Attachment #658961 -
Attachment is obsolete: true
Attachment #658711 -
Flags: review?(ted.mielczarek)
Assignee | ||
Comment 48•12 years ago
|
||
That last patch was completely busted due to my stupidity.
Attachment #661348 -
Attachment is obsolete: true
Attachment #661353 -
Flags: review?(ted.mielczarek)
Updated•12 years ago
|
Attachment #661353 -
Flags: review?(ted.mielczarek) → review+
Assignee | ||
Comment 49•12 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/e9352b9ec6f8 https://hg.mozilla.org/integration/mozilla-inbound/rev/980d552c34ac https://hg.mozilla.org/integration/mozilla-inbound/rev/faf1c66154f4 https://hg.mozilla.org/integration/mozilla-inbound/rev/11374d8585f6 https://hg.mozilla.org/integration/mozilla-inbound/rev/d5720a3513ca
Target Milestone: --- → mozilla18
Comment 50•12 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/e9352b9ec6f8 https://hg.mozilla.org/mozilla-central/rev/980d552c34ac https://hg.mozilla.org/mozilla-central/rev/faf1c66154f4 https://hg.mozilla.org/mozilla-central/rev/11374d8585f6 https://hg.mozilla.org/mozilla-central/rev/d5720a3513ca
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Updated•6 years ago
|
Product: Core → Firefox Build System
You need to log in
before you can comment on or make changes to this bug.
Description
•