#11686 closed defect (fixed)
Race condition in matplotlib mkdir()
Reported by: | jdemeyer | Owned by: | tbd |
---|---|---|---|
Priority: | minor | Milestone: | sage-4.7.2 |
Component: | packages: standard | Keywords: | MPL Errno 17 libpng |
Cc: | jhpalmieri, jason | Merged in: | sage-4.7.2.alpha2 |
Authors: | John Palmieri | Reviewers: | Leif Leonhardy |
Report Upstream: | N/A | Work issues: | |
Branch: | Commit: | ||
Dependencies: | Stopgaps: |
Description (last modified by )
I regularly see this error when testing on the buildbot:
sage -t -long -force_lib devel/sage/doc/de/tutorial/tour_functions.rst ********************************************************************** File "/Users/buildbot/build/sage/bsd-1/bsd_64_upgrade/build/sage-4.5.3-4.7.1/devel/sage-main/doc/de/tutorial/tour_functions.rst", line 24: sage: plot(f, 0, 2) Exception raised: Traceback (most recent call last): File "/Users/buildbot/build/sage/bsd-1/bsd_64_upgrade/build/sage-4.5.3-4.7.1/local/bin/ncadoctest.py", line 1231, in run_one_test self.run_one_example(test, example, filename, compileflags) File "/Users/buildbot/build/sage/bsd-1/bsd_64_upgrade/build/sage-4.5.3-4.7.1/local/bin/sagedoctest.py", line 38, in run_one_example OrigDocTestRunner.run_one_example(self, test, example, filename, compileflags) File "/Users/buildbot/build/sage/bsd-1/bsd_64_upgrade/build/sage-4.5.3-4.7.1/local/bin/ncadoctest.py", line 1172, in run_one_example compileflags, 1) in test.globs File "<doctest __main__.example_0[5]>", line 1, in <module> plot(f, Integer(0), Integer(2))###line 24: sage: plot(f, 0, 2) File "/Users/buildbot/build/sage/bsd-1/bsd_64_upgrade/build/sage-4.5.3-4.7.1/local/lib/python/site-packages/sage/misc/displayhook.py", line 174, in displayhook print_obj(sys.stdout, obj) File "/Users/buildbot/build/sage/bsd-1/bsd_64_upgrade/build/sage-4.5.3-4.7.1/local/lib/python/site-packages/sage/misc/displayhook.py", line 142, in print_obj print >>out_stream, `obj` File "sage_object.pyx", line 154, in sage.structure.sage_object.SageObject.__repr__ (sage/structure/sage_object.c:1463) File "/Users/buildbot/build/sage/bsd-1/bsd_64_upgrade/build/sage-4.5.3-4.7.1/local/lib/python/site-packages/sage/plot/plot.py", line 1081, in _repr_ self.show() File "/Users/buildbot/build/sage/bsd-1/bsd_64_upgrade/build/sage-4.5.3-4.7.1/local/lib/python/site-packages/sage/misc/decorators.py", line 426, in wrapper return func(*args, **kwds) File "/Users/buildbot/build/sage/bsd-1/bsd_64_upgrade/build/sage-4.5.3-4.7.1/local/lib/python/site-packages/sage/plot/plot.py", line 1723, in show self.save(DOCTEST_MODE_FILE, **kwds) File "/Users/buildbot/build/sage/bsd-1/bsd_64_upgrade/build/sage-4.5.3-4.7.1/local/lib/python/site-packages/sage/misc/decorators.py", line 426, in wrapper return func(*args, **kwds) File "/Users/buildbot/build/sage/bsd-1/bsd_64_upgrade/build/sage-4.5.3-4.7.1/local/lib/python/site-packages/sage/plot/plot.py", line 2453, in save figure = self.matplotlib(**options) File "/Users/buildbot/build/sage/bsd-1/bsd_64_upgrade/build/sage-4.5.3-4.7.1/local/lib/python/site-packages/sage/plot/plot.py", line 1930, in matplotlib from matplotlib.figure import Figure, figaspect File "/Users/buildbot/build/sage/bsd-1/bsd_64_upgrade/build/sage-4.5.3-4.7.1/local/lib/python/site-packages/matplotlib/figure.py", line 18, in <module> from axes import Axes, SubplotBase, subplot_class_factory File "/Users/buildbot/build/sage/bsd-1/bsd_64_upgrade/build/sage-4.5.3-4.7.1/local/lib/python/site-packages/matplotlib/axes.py", line 18, in <module> import matplotlib.contour as mcontour File "/Users/buildbot/build/sage/bsd-1/bsd_64_upgrade/build/sage-4.5.3-4.7.1/local/lib/python/site-packages/matplotlib/contour.py", line 21, in <module> import matplotlib.texmanager as texmanager File "/Users/buildbot/build/sage/bsd-1/bsd_64_upgrade/build/sage-4.5.3-4.7.1/local/lib/python/site-packages/matplotlib/texmanager.py", line 72, in <module> class TexManager: File "/Users/buildbot/build/sage/bsd-1/bsd_64_upgrade/build/sage-4.5.3-4.7.1/local/lib/python/site-packages/matplotlib/texmanager.py", line 92, in TexManager os.mkdir(texcache) OSError: [Errno 17] File exists: '/tmp/dot_sage.WboXjWq0ow/matplotlib-1.0.1/tex.cache'
The problem looks to be the following race condition in the matplotlib code:
if not os.path.exists(texcache): os.mkdir(texcache)
New spkg available at
http://sage.math.washington.edu/home/palmieri/SPKG/matplotlib-1.0.1.p0.spkg
Attachments (5)
Change History (79)
comment:1 Changed 10 years ago by
- Cc jhpalmieri added
comment:2 follow-up: ↓ 4 Changed 10 years ago by
Looks like the same issue (with the same fix) as in #10159. I reported this to the matplotlib mailing list in October 2010, but no response.
In fact, what happened to the patches from #10159 which dealt with this issue? Leif has noted on #10588 that those patches seem to have been lost in the update to version 1.0.1...
comment:3 Changed 10 years ago by
Replying to leif:
Seen this before (i.e., similar), haven't we?
This is a regression introduced by #10588, previously fixed in John's spkg from #10159, because the spkg of the former had not been based on the latter (and the bug hasn't [yet] been fixed upstream, at least not in 1.0.1).
Also, the current spkg is full of crap in src/build/
. (The whole directory can be deleted.)
comment:4 in reply to: ↑ 2 ; follow-up: ↓ 6 Changed 10 years ago by
Replying to jhpalmieri:
In fact, what happened to the patches from #10159 which dealt with this issue? Leif has noted on #10588 that those patches seem to have been lost in the update to version 1.0.1...
It's different to what I first thought; see my previous comment... ;-)
With a bit of luck your patches from #10159 still apply seamlessly to the current upstream version; haven't tested that yet.
comment:5 Changed 10 years ago by
Just checked: John's 1.0.0.p0 spkg has been in Sage 4.6.1 and 4.6.2.alpha0, then the one from #10588 got merged into Sage 4.6.2.alpha1.
comment:6 in reply to: ↑ 4 Changed 10 years ago by
Replying to leif:
With a bit of luck your patches from #10159 still apply seamlessly to the current upstream version; haven't tested that yet.
Hmmm, a little work has to be done:
~/Sage/spkgs/matplotlib-1.0.1.p1/patches$ patch __init__.py < __init__.py.patch patching file __init__.py Hunk #1 succeeded at 103 with fuzz 2. ~/Sage/spkgs/matplotlib-1.0.1.p1/patches$ patch finance.py < finance.py.patch patching file finance.py Hunk #1 FAILED at 4. Hunk #2 succeeded at 173 (offset 4 lines). 1 out of 2 hunks FAILED -- saving rejects to file finance.py.rej ~/Sage/spkgs/matplotlib-1.0.1.p1/patches$ patch texmanager.py < texmanager.py.patch patching file texmanager.py $
John, are you going to do this?
comment:7 Changed 10 years ago by
- Description modified (diff)
- Status changed from new to needs_review
I've created a new spkg, link in the ticket description. I'm posting the diff here for review purposes.
comment:8 Changed 10 years ago by
(I also deleted src/build
and a few random .pyc
files.)
comment:9 follow-up: ↓ 10 Changed 10 years ago by
(Actually, I didn't delete 'src/build', I just dropped in a freshly downloaded vanilla source as the 'src' directory, in case the previous version was corrupted in other ways than just the presence of 'build'.)
comment:10 in reply to: ↑ 9 Changed 10 years ago by
Replying to jhpalmieri:
(Actually, I didn't delete 'src/build', I just dropped in a freshly downloaded vanilla source as the 'src' directory, in case the previous version was corrupted in other ways than just the presence of 'build'.)
Yep, the upstream sources of Ryan's and your spkg differ:
Hundreds of instances of
diff -r matplotlib-1.0.1/src/lib/pytz/tzfile.py matplotlib-1.0.1.p0/src/lib/pytz/tzfile.py 1c1 < #!/usr/bin/env python2 --- > #!/usr/bin/env python
and also
diff -r matplotlib-1.0.1/src/setup.py matplotlib-1.0.1.p0/src/setup.py 21c21 < rc = dict({'backend':'GTKAgg', 'numerix':'numpy'}) --- > rc = {'backend':'Agg'} Only in matplotlib-1.0.1/src/: setupext.pyc Only in matplotlib-1.0.1.p0/src/src: backend_agg.cpp Only in matplotlib-1.0.1.p0/src/src: backend_gdk.c Only in matplotlib-1.0.1.p0/src/src: image.cpp Only in matplotlib-1.0.1.p0/src/src: path.cpp
comment:11 Changed 10 years ago by
Is this normal?
OPTIONAL BACKEND DEPENDENCIES libpng: found, but unknown version (no pkg-config)
I do have pkg-config
installed, and $SAGE_ROOT/local/lib/pkgconfig/libpng12.pc
(target of a symbolic link libpng.pc
) looks like this:
... Name: libpng Description: Loads and saves PNG files Version: 1.2.35 Libs: -L${libdir} -lpng12 ...
So maybe either pkg-config
or MPL is confused by the "wrong" Name:
, since we patch MPL's setupext.py
:
-
src/setupext.py
538 538 module.include_dirs.append(numpy.get_include()) 539 539 540 540 def add_png_flags(module): 541 try_pkgconfig(module, 'libpng ', 'png')541 try_pkgconfig(module, 'libpng12', 'png12') 542 542 add_base_flags(module) 543 543 add_numpy_flags(module) 544 544 module.libraries.append('z')
Probably this patch should only be applied on MacOS X, since
* setupext.py: link to libpng12 instead of libpng to apparently avoid some name clashes on OSX.
(Currently running doctests, so haven't checked if not applying the patch on Linux changes anything w.r.t. that.)
We should add freetype (and now also GNU patch) to the Dependencies:
REQUIRED DEPENDENCIES numpy: 1.5.1 freetype2: 9.16.3
== Dependencies == * python * numpy
comment:12 follow-up: ↓ 16 Changed 10 years ago by
On my OS X box, I see the same thing with the old 1.0.1 and with the new one:
REQUIRED DEPENDENCIES numpy: 1.5.1 freetype2: found, but unknown version (no pkg-config) OPTIONAL BACKEND DEPENDENCIES libpng: found, but unknown version (no pkg-config)
I see that on linux, or at least on sage.math, it reports the correct version for these.
I can
- apply the patch to
setupext.py
only on Darwin. - apply the patch always, but add an
if ... then ... else
so it only does something different on Darwin.
Opinions?
I'll certainly add freetype and patch to the dependencies.
comment:13 Changed 10 years ago by
Impressive:
$ du -h matplotlib-1.0.1*.spkg
12M matplotlib-1.0.1.p0.spkg
19M matplotlib-1.0.1.spkg
So the Sage tarball will get a bit smaller again.
(It'll get even smaller if someone someday removes the JSMath fonts and other obsolete files from SageNB's Mercurial repository; I'm not sure if we even ship duplicates in other packages.)
comment:14 Changed 10 years ago by
The version MPL reports for freetype2 clearly comes from Sage's .pc
file, since my system one refers to a different one; also PKG_CONFIG_PATH
is "properly" set to (just) Sage's pkg-config
directory.
Is PKG_CONFIG_PATH
set on sage.math outside the Sage environment? (We only set it to Sage's if it is empty, i.e. don't prepend Sage's in case it is already set.)
comment:15 Changed 10 years ago by
I'm confused now. On sage.math, if I look at the install log from the old matplotlib-1.0.1 (during a fresh Sage build), it says
OPTIONAL BACKEND DEPENDENCIES libpng: 1.2.35
But if I do "sage -f ..." on the same spkg, I get this in its install log:
OPTIONAL BACKEND DEPENDENCIES libpng: found, but unknown version (no pkg-config)
And of course, in this old spkg, this was the only patch installed, and it was installed regardless of the OS. I don't know what's going on, but I'm posting a new spkg which only applies the patch on Darwin. I hope this improves things on non-Darwin systems, or at least doesn't hurt anything on such systems.
comment:16 in reply to: ↑ 12 ; follow-up: ↓ 18 Changed 10 years ago by
Replying to jhpalmieri:
I can
- apply the patch to
setupext.py
only on Darwin.
- apply the patch always, but add an
if ... then ... else
so it only does something different on Darwin.
Opinions?
I think modifying the patch is easier, but do what you prefer.
Note that the change should only be made if the OS is Darwin and pkg-config
is not installed, since Sage's libpng*.pc
contains
Libs: -L${libdir} -lpng12
comment:17 Changed 10 years ago by
I should perhaps open another ticket for changing sage-env
to always add Sage's pkg-config
directory to PKG_CONFIG_PATH
, since otherwise wrong libraries may get used.
comment:18 in reply to: ↑ 16 ; follow-up: ↓ 19 Changed 10 years ago by
Replying to leif:
Note that the change should only be made if the OS is Darwin and
pkg-config
is not installed
Here's a version which does this. I'm not a shell script expert; is this a good way to accomplish it?
if [ "$UNAME" = "Darwin" ]; then command -v pkg-config || patch -p1 < "$patch"
comment:19 in reply to: ↑ 18 Changed 10 years ago by
Replying to jhpalmieri:
Here's a version which does this. I'm not a shell script expert; is this a good way to accomplish it?
if [ "$UNAME" = "Darwin" ]; then command -v pkg-config || patch -p1 < "$patch"
I see you've redirected the output of command
to /dev/null
in the actual patch.
IMHO a bit more readable would be
if [ "$UNAME" = Darwin ] && ! command -v pkg-config >/dev/null; then patch -p1 < "$patch" # Apply only on Darwin if pkg-config isn't installed fi
but for simplicity (if you're not going to change the patch) I would move the specific patch to patches/Darwin/
and do the following:
# Apply patches for all platforms. See SPKG.txt for why and what. for patch in ../patches/*.patch; do patch -p1 <"$patch" if [ $? -ne 0 ]; then echo >&2 "Error applying '$patch'" exit 1 fi done # Apply patch(es) for Darwin. if [ "$UNAME" = Darwin ]; then for patch in ../patches/Darwin/*.patch; do patch -p1 <"$patch" if [ $? -ne 0 ]; then echo >&2 "Error applying '$patch'" exit 1 fi done fi
(The latter would have to be changed from a loop to an if
-statement depending on command -v ...
if we only apply the single patch and don't test for pkg-config
in the patch itself, i.e., from Python.)
Having the patch in a separate directory it's also immediately clear that it is platform-specific; unfortunately we here have another precondition.
comment:20 Changed 10 years ago by
For whatever reason, I still get
... NOTE: Set SAGE_MATPLOTLIB_GUI to anything but 'no' to try to build the Matplotlib GUI. Not building any matplotlib graphical backends. patching file lib/matplotlib/__init__.py patching file lib/matplotlib/finance.py patching file lib/matplotlib/texmanager.py basedirlist is: ['/home/leif/Sage/sage-4.7.1.rc2/local'] ============================================================================ BUILDING MATPLOTLIB matplotlib: 1.0.1 python: 2.6.4 (r264, Aug 11 2011, 12:43:02) [GCC 4.5.1] platform: linux2 REQUIRED DEPENDENCIES numpy: 1.5.1 freetype2: 9.16.3 OPTIONAL BACKEND DEPENDENCIES libpng: found, but unknown version (no pkg-config) ...
i.e. an "unknown" libpng version, even without the patch to setupext.py
.
comment:21 Changed 10 years ago by
I think that using the vanilla source is best when possible, so I'm only going to apply the patch on Darwin. However, it's possible that someone will install (or uninstall) pkg-config and not recompile Sage, so I've put the test for that inside the patch.
comment:22 follow-up: ↓ 23 Changed 10 years ago by
Haha:
~/Sage/sage-4.7.1.rc2$ env PKG_CONFIG_PATH=/foo ./sage -f ~/Sage/spkgs/matplotlib-1.0.1.p0.spkg Force installing /home/leif/Sage/spkgs/matplotlib-1.0.1.p0.spkg ... NOTE: Set SAGE_MATPLOTLIB_GUI to anything but 'no' to try to build the Matplotlib GUI. Not building any matplotlib graphical backends. patching file lib/matplotlib/__init__.py patching file lib/matplotlib/finance.py patching file lib/matplotlib/texmanager.py basedirlist is: ['/home/leif/Sage/sage-4.7.1.rc2/local'] ============================================================================ BUILDING MATPLOTLIB matplotlib: 1.0.1 python: 2.6.4 (r264, Aug 11 2011, 12:43:02) [GCC 4.5.1] platform: linux2 REQUIRED DEPENDENCIES numpy: 1.5.1 freetype2: 9.22.3 OPTIONAL BACKEND DEPENDENCIES libpng: 1.2.42 ...
So somehow Sage's libpng*.pc
appears to be "broken".
comment:23 in reply to: ↑ 22 ; follow-up: ↓ 24 Changed 10 years ago by
Replying to leif:
So somehow Sage's
libpng*.pc
appears to be "broken".
Yep, apparently by sage-location
, during the build.
If I reinstall libpng12
, and then MPL, everything is fine.
The offending line was
SAGE_ROOT=${SAGE_ROOT}
SAGE_ROOT=/the/appropriate/absolute/path
works. I have no idea why pkg-config
doesn't take $SAGE_ROOT
from the environment if it isn't defined in the .pc
file itself; it IMHO used to -- last year, but meanwhile doesn't ... 8/
I.e.,
prefix=${SAGE_ROOT}
does no longer work unless SAGE_ROOT
is also defined in the .pc
file itself.
We may have to really wrap pkg-config
in $SAGE_ROOT/local/bin
with
#!/bin/sh if [ -z "$SAGE_ROOT" ]; then echo >&2 "Error: $0: SAGE_ROOT not defined." exit 1 fi exec /usr/bin/env PATH=`echo $PATH | sed -e "s|$SAGE_ROOT/local/bin:||"` \ pkg-config --define-variable=SAGE_ROOT="$SAGE_ROOT" "$@"
(Or just call the system's pkg-config
without --define-variable ...
if SAGE_ROOT
is undefined.)
Defining SAGE_ORIG_PATH
in sage-env
wouldn't be a bad idea either.
comment:24 in reply to: ↑ 23 Changed 10 years ago by
- Reviewers set to Leif Leonhardy
Replying to leif:
Replying to leif:
So somehow Sage's
libpng*.pc
appears to be "broken".
[...]
prefix=${SAGE_ROOT}
does no longer work unless
SAGE_ROOT
is also defined in the.pc
file itself.
It never did that way; I used $${SAGE_ROOT}
.
We may have to really wrap
pkg-config
in$SAGE_ROOT/local/bin
[...]
We don't have to, and shouldn't do so (see e.g. this comment at #10202).
So what's the current status of this ticket?
Is the attached diff (and the spkg) still current, or did you / do you plan to make further changes to spkg-install
(w.r.t. to applying the patches) and the setupext.py
patch?
I think the pkg-config
issues (on Linux) have shown to be rather unrelated to this ticket (and MPL itself), so the patch to setupext.py
may not have to only be applied on MacOS X (if pkg-config
is installed), though limiting its application shouldn't hurt either.
If you want to leave the code as is, I'll re-review and test it now. The only thing I noticed is that SPKG.txt
isn't very explicit regarding the patch to setupext.py
, i.e. what it currently exactly does and that it's indeed only applied on Darwin. I'd consider this a minor issue though.
comment:25 Changed 10 years ago by
Oh, and the Changelog entry could mention that you refreshed the upstream sources. (They apparently actually changed the #!/usr/bin/env python2
, unless Ryan had previously modified the sources.)
comment:26 Changed 10 years ago by
I'll post one more version, updating SPKG.txt and the commit message, saying that the upstream sources have been refreshed. I don't really know the purpose of the setupext.py patch, so I don't feel qualified to modify what SPKG.txt says. If you have suggestions, I'd be happy to hear them. For now, I added a trac number to the comment in SPKG.txt (that trac number was in the original patch from #4774, but has since disappeared...).
The new spkg is in place, so once I've attached the most recent patch to this ticket, it will be ready for review.
comment:27 follow-up: ↓ 28 Changed 10 years ago by
Summarizing:
- this ticket includes most of the patches from #10159 (the missing parts are because I got tired, so I didn't fix the fact that some lines in SPKG.txt have lengths > 80).
- in SPKG.txt, the prerequisites have been updated.
- the patches now use "patch" for their application.
- the patches are divided into ones specifically for Darwin, and ones for all platforms.
- the Darwin patch passes 'libpng12' or 'libpng' to
try_pkgconfig
, depending on whetherpkg-config
is installed, instead of just always passing 'libpng12'. - as part of the patches from #10159, the installation procedure prints Sage's setting of the variable MPLCONFIGDIR.
- the 'src' directory has been replaced with a vanilla source, downloaded 13-Aug-2011.
comment:28 in reply to: ↑ 27 ; follow-up: ↓ 29 Changed 10 years ago by
Replying to jhpalmieri:
Summarizing:
[...]
- in SPKG.txt, the prerequisites have been updated.
- the patches now use "patch" for their application.
- the patches are divided into ones specifically for Darwin, and ones for all platforms.
- the Darwin patch passes 'libpng12' or 'libpng' to
try_pkgconfig
, depending on whetherpkg-config
is installed, instead of just always passing 'libpng12'.
(May also read "... instead of just always passing 'libpng' "; it is at least not clear to what "instead" refers to, i.e. unmodifed code or unconditional application of the patch.)
- as part of the patches from #10159, the installation procedure prints Sage's setting of the variable MPLCONFIGDIR.
[...]
I would have just added the above to the Changelog entry. :)
I don't really know the purpose of the setupext.py patch, so I don't feel qualified to modify what SPKG.txt says. If you have suggestions, I'd be happy to hear them.
I rather meant noting that the patch is only applied on Darwin, and that it only changes the behaviour if pkg-config
is not installed.
W.r.t. why this is necessary:
The assumption is that when pkg-config
is installed, it will always pick up Sage's libpng.pc
(which is a symbolic link to libpng12.pc
), so MPL will always be linked to the correct, i.e. Sage's libpng
, regardless of whether "libpng"
or "libpng12"
is used in try_pkgconfig()
.
If pkg-config
is not available, and you pass just "libpng"
(without 12
) to try_pkgconfig()
, MPL might pick up some random system libpng
(without the 12
), which can lead to name clashes and other errors because two different libpng
s could be linked into Sage.
So it should be pretty safe to just always pass "libpng12"
, no matter what operating system and whether pkg-config
is installed or not -- Sorry for the previous noise...
(On the other hand, it's also not bad to only apply such patches on [potentially] affected systems; if Sage's libpng12
one day changes to, say, libpng13
, we'd have to modify MPL for all systems to work. If we only used "libpng12"
on e.g. Darwin instead, only Darwin would break on such an upgrade.)
Is the following still current (for version 1.x)?
... config.add_section('gui_support') for backend in ('gtk', 'gtkagg', 'tkagg', 'wxagg', 'macosx', 'windowing'): config.set('gui_support', backend, graphical_backend) ...
$ hg log -v make-setup-config.py
changeset: 37:0aef07ceab83
user: Jason Grout <jason-sage@creativetrax.com>
date: Fri Jun 11 12:06:36 2010 -0500
...
(That's the only entry, corresponding to matplotlib-0.99.3-svn8415.spkg
.)
I haven't tried to (attempt to) build any graphical backends.
comment:29 in reply to: ↑ 28 Changed 10 years ago by
- Cc jason added
- Status changed from needs_review to needs_info
Replying to leif:
Is the following still current (for version 1.x)?
... config.add_section('gui_support') for backend in ('gtk', 'gtkagg', 'tkagg', 'wxagg', 'macosx', 'windowing'): config.set('gui_support', backend, graphical_backend) ...
[...]
$ hg log -v make-setup-config.py
changeset: 37:0aef07ceab83
user: Jason Grout <jason-sage@creativetrax.com>
date: Fri Jun 11 12:06:36 2010 -0500
...
(That's the only entry, corresponding to
matplotlib-0.99.3-svn8415.spkg
.)
I haven't tried to (attempt to) build any graphical backends.
At least setting SAGE_MATPLOTLIB_GUI=yes
has no real effect on my system, because of at least some missing components:
... OPTIONAL BACKEND DEPENDENCIES libpng: 1.2.35 Tkinter: no * TKAgg requires Tkinter wxPython: no * wxPython not found Gtk+: no * Building for Gtk+ requires pygtk; you must be able * to "import gtk" in your build/install environment Mac OS X native: no Qt: no Qt4: no Cairo: no
Perhaps one should test this on other systems as well, though not really on topic of this ticket, i.e., I think if the list of graphics backends needs to be updated, this should be done on a follow-up, but we should IMHO check this while we're at it.
Otherwise positive review for the current spkg.
comment:30 follow-up: ↓ 31 Changed 10 years ago by
I'm considering modifying SPKG.txt as follows:
-
SPKG.txt
diff --git a/SPKG.txt b/SPKG.txt
a b The following patches are in the patches 40 40 The patches are applied during the build process. 41 41 42 42 * setupext.py: link to libpng12 instead of libpng to apparently avoid 43 some name clashes on OSX. See Sage trac #4774. 43 some name clashes on OSX. As of version 1.0.1.p0, this patch is 44 only applied on OS X, and it tests for the presence of 45 'pkg-config': if it is not installed, then it links to libpng12 46 instead of libpng; otherwise it does nothing. See Sage trac #4774 47 and #11686. 44 48 45 49 * __init__.py, finance.py, texmanager.py: instead of "if 46 50 not.os.path.exists(DIR): os.mkdir(DIR)", do "try os.mkdir(DIR)" and … … The patches are applied during the build 75 79 === matplotlib 1.0.1.p0 (John Palmieri, 13 Aug 2011) === 76 80 * Patch __init__.py, finance.py, and texmanager.py to avoid race 77 81 conditions when creating directories. See Sage trac #11686 78 and #10159. Also replaced the "src" directory with a freshly 79 downloaded copy of the vanilla source code. 82 and #10159. 83 84 * Updated the list of prerequisites in this file. 85 86 * Changed spkg-install so that the patches are now applied using 87 'patch'. 88 89 * The patch to setupext.py is now only applied on OS X. The patch 90 itself tests whether pkg-config is installed: if so, it maintains 91 the status quo, and if not, it passes the argument 'libpng12' when 92 configuring then png package. 93 94 * Replaced the "src" directory with a freshly downloaded copy of 95 the vanilla source code. 80 96 81 97 === matplotlib 1.0.1 (Ryan Grout, 10 Jan 2011) === 82 98 * Update to matplotlib 1.0.1. Fixes a handful of annoying bugs in SAGE.
Re SAGE_MATPLOTLIB_GUI
: I'm not sure what to check, so I'm hoping we can do this on a follow-up ticket. ("We" meaning someone else, actually.)
comment:31 in reply to: ↑ 30 Changed 10 years ago by
Replying to jhpalmieri:
I'm considering modifying SPKG.txt as follows ...
s/OSX/OS X/
s/then png package/the png package/
s/otherwise it does nothing/otherwise doesn't change the behavio[u]r/
I'd also say explicitly searches for and uses libpng12 instead of links to ... in the description of the patch to setupext.py
.
Re
SAGE_MATPLOTLIB_GUI
: I'm not sure what to check, so I'm hoping we can do this on a follow-up ticket. ("We" meaning someone else, actually.)
Well, one would have to check MPL's docs / release notes / changelog, and also have to install [at least] a lot of Python crap stuff from within the Sage installation, on KN different machines for each of the N platforms supported by Sage, and test each of the M supported graphical backend variants on these... ;-)
For the moment, step one should be sufficient, and Jason or $SOMEONE_ELSE
might already know whether anything changed.
comment:32 follow-up: ↓ 33 Changed 10 years ago by
- Status changed from needs_info to needs_review
Okay, here's a new version. The only change from the previous one is recorded in the "SPKG.txt" patch.
comment:33 in reply to: ↑ 32 Changed 10 years ago by
- Status changed from needs_review to positive_review
Replying to jhpalmieri:
Okay, here's a new version. The only change from the previous one is recorded in the "SPKG.txt" patch.
Ok.
comment:34 Changed 10 years ago by
Haven't read my mail in the weekend and now I come back to see this. Nice work!
comment:35 follow-up: ↓ 36 Changed 10 years ago by
- Description modified (diff)
New version which only does some cleanup in SPKG.txt
: http://boxen.math.washington.edu/home/jdemeyer/spkg/matplotlib-1.0.1.p0.spkg
These changes are so trivial, I don't think they need review.
Changes: matplotlib-jdemeyer.patch
comment:36 in reply to: ↑ 35 Changed 10 years ago by
Replying to jdemeyer:
These changes are so trivial, I don't think they need review.
FWIW, they look ok to me.
comment:37 follow-up: ↓ 38 Changed 10 years ago by
- Status changed from positive_review to needs_work
This fails on the buildbot machine iras
:
Warning: Attempted to overwrite SAGE_ROOT environment variable matplotlib-1.0.1.p0 Machine: Linux iras 2.6.16.46-0.12-default #1 SMP Thu May 17 14:00:09 UTC 2007 ia64 ia64 ia64 GNU/Linux [...] ============================================================================ BUILDING MATPLOTLIB matplotlib: 1.0.1 python: 2.6.4 (r264:75706, Aug 16 2011, 18:26:56) [GCC 4.6.1] platform: linux2 REQUIRED DEPENDENCIES numpy: 1.5.1 freetype2: found, but unknown version (no pkg-config) OPTIONAL BACKEND DEPENDENCIES libpng: found, but unknown version (no pkg-config) Gtk+: no * Building for Gtk+ requires pygtk; you must be able * to "import gtk" in your build/install environment Qt: no Qt4: no Cairo: no OPTIONAL DATE/TIMEZONE DEPENDENCIES datetime: present, version unknown dateutil: matplotlib will provide pytz: matplotlib will provide adding pytz OPTIONAL USETEX DEPENDENCIES dvipng: no ghostscript: 8.15.3 latex: no [...] building 'matplotlib._png' extension gcc -fno-strict-aliasing -g -O2 -DNDEBUG -g -O3 -Wall -Wstrict-prototypes -fPIC -DPY_ARRAY_UNIQUE_SYMBOL=MPL_ARRAY_API -DPYCXX_ISO_CPP_LIB=1 -I/home/buildbot/build/sage/iras-1/iras_full/build/sage-4.7.2.alpha1/local/include -I. -I/home/buildbot/build/sage/iras-1/iras_full/build/sage-4.7.2.alpha1/local/lib/python2.6/site-packages/numpy/core/include -I. -I/home/buildbot/build/sage/iras-1/iras_full/build/sage-4.7.2.alpha1/local/include/python2.6 -c src/_png.cpp -o build/temp.linux-ia64-2.6/src/_png.o cc1plus: warning: command line option ‘-Wstrict-prototypes’ is valid for Ada/C/ObjC but not for C++ [enabled by default] src/_png.cpp: In function ‘void init_png()’: src/_png.cpp:485:25: warning: variable ‘_png’ set but not used [-Wunused-but-set-variable] g++ -pthread -shared build/temp.linux-ia64-2.6/src/_png.o build/temp.linux-ia64-2.6/src/mplutils.o build/temp.linux-ia64-2.6/CXX/cxxsupport.o build/temp.linux-ia64-2.6/CXX/cxx_extensions.o build/temp.linux-ia64-2.6/CXX/IndirectPythonInterface.o build/temp.linux-ia64-2.6/CXX/cxxextensions.o -L/home/buildbot/build/sage/iras-1/iras_full/build/sage-4.7.2.alpha1/local/lib -L/home/buildbot/build/sage/iras-1/iras_full/build/sage-4.7.2.alpha1/local/lib -lpng -lz -lstdc++ -lm -lpython2.6 -o build/lib.linux-ia64-2.6/matplotlib/_png.so /usr/local/binutils-2.21/ia64-Linux-suse-gcc-4.5.1/bin/ld: cannot find -lpng collect2: ld returned 1 exit status error: command 'g++' failed with exit status 1 Error building matplotlib package.
comment:38 in reply to: ↑ 37 Changed 10 years ago by
Replying to jdemeyer:
This fails on the buildbot machine
iras
:
... OPTIONAL BACKEND DEPENDENCIES libpng: found, but unknown version (no pkg-config) ... g++ -pthread -shared build/temp.linux-ia64-2.6/src/_png.o build/temp.linux-ia64-2.6/src/mplutils.o build/temp.linux-ia64-2.6/CXX/cxxsupport.o build/temp.linux-ia64-2.6/CXX/cxx_extensions.o build/temp.linux-ia64-2.6/CXX/IndirectPythonInterface.o build/temp.linux-ia64-2.6/CXX/cxxextensions.o -L/home/buildbot/build/sage/iras-1/iras_full/build/sage-4.7.2.alpha1/local/lib -L/home/buildbot/build/sage/iras-1/iras_full/build/sage-4.7.2.alpha1/local/lib -lpng -lz -lstdc++ -lm -lpython2.6 -o build/lib.linux-ia64-2.6/matplotlib/_png.so /usr/local/binutils-2.21/ia64-Linux-suse-gcc-4.5.1/bin/ld: cannot find -lpng
Read the ticket.
This is unrelated to the MPL spkg, but due to a messed-up libpng12.pc
file.
If you first reinstall the libpng12
spkg, and then MPL, all should work fine.
(Or fix local/lib/pkgconfig/libpng12.pc
manually, most probably by deleting the line SAGE_ROOT=${SAGE_ROOT}
. Another cause can be having PKG_CONFIG_PATH
set outside the Sage environment, which is a bug in sage-env
, cf. also #11687.)
comment:39 follow-up: ↓ 40 Changed 10 years ago by
P.S.: You can also try
$ ./sage -sh -c "pkg-config --libs libpng"
on iras, which should either raise an error, or give the wrong library, unless you fix the .pc
file (and/or unset PKG_CONFIG_PATH
in the "global" environment).
comment:40 in reply to: ↑ 39 ; follow-up: ↓ 41 Changed 10 years ago by
Replying to leif:
This is unrelated to the MPL spkg, but due to a messed-up
libpng12.pc
file.
I don't have time to care where the bug is, only that the old spkg worked and the new one not.
If this new matplotlib spkg exposes a bug in the libpng spkg, then the new matplotlib spkg cannot be merged without fixing the libpng bug.
buildbot@iras sage-4.7.2.alpha1$ ./sage -sh -c "pkg-config --libs libpng" Starting subshell with Sage environment variables set. Be sure to exit when you are done and do not do anything with other copies of Sage! Bypassing shell configuration files ... bash: pkg-config: command not found Exited Sage subshell.
comment:41 in reply to: ↑ 40 ; follow-up: ↓ 42 Changed 10 years ago by
Replying to jdemeyer:
Replying to leif:
This is unrelated to the MPL spkg, but due to a messed-up
libpng12.pc
file.I don't have time to care where the bug is,
It should be obvious from a first glance that we ran into similar and discussed this on the ticket here.
only that the old spkg worked and the new one not.
That's simply not true, or rather an incident. Reinstalling the old spkg won't work either, at least not on all systems.
If this new matplotlib spkg exposes a bug in the libpng spkg, then the new matplotlib spkg cannot be merged without fixing the libpng bug.
Part of the bug is caused by sage-location
(and eventually also sage-env
). See e.g. ticket:10202:26 for how we should IMHO fix .pc
files w.r.t. relocation.
Nevertheless, the libpng12
spkg could likely create symbolic links from local/lib/libpng.{a,la,so}
, then MPL would also find it and link to it without the help of pkg-config
.
pkg-config
btw. should be installed on all supported platforms except Darwin.
comment:42 in reply to: ↑ 41 Changed 10 years ago by
Replying to leif:
pkg-config
btw. should be installed on all supported platforms except Darwin.
I don't think that pkg-config
is part of Sage's prerequisites. This machine without pkg-config
did manage to build earlier versions of Sage, so this new matplotlib spkg is a regression (even if the problem is not with matplotlib itself).
comment:43 Changed 10 years ago by
comment:44 Changed 10 years ago by
- Work issues set to Either check for pkg-config on other platforms as well, or make the ticket depend on #11696.
comment:45 follow-up: ↓ 47 Changed 10 years ago by
- Status changed from needs_work to needs_info
Sorry, I should have tested this on skynet. I created a new spkg (not publicly available yet) which patches setupext.py
on all platforms, not just Darwin. I tried many of the skynet machines and on all of them but iras, both the old and the new spkg seem to work fine. On iras, as Jeroen reports, the old one fails. The new one works, though.
So I think we should apply this patch on all platforms. That's the old behavior, after all. Shall I post a new spkg?
comment:46 Changed 10 years ago by
- Status changed from needs_info to needs_review
Since Jeroen's package supersedes mine, there is no danger for me to overwrite my version. So here's a new spkg:
I'm attaching the patch, too. If this approach is acceptable, I'll change the ticket description to point at this one.
comment:47 in reply to: ↑ 45 Changed 10 years ago by
Replying to jhpalmieri:
Sorry, I should have tested this on skynet.
No need to apologize. There is a reason we test all potential Sage releases on skynet. We certainly do not want to burden all Sage developers with testing on skynet (most of them probably don't have access to skynet anyway).
comment:48 follow-up: ↓ 49 Changed 10 years ago by
Replying to jhpalmieri:
So I think we should apply this patch on all platforms.
But IMHO only if pkg-config
is not available.
That's the old behavior, after all.
Which is a bit odd, as pointed out earlier.
comment:49 in reply to: ↑ 48 ; follow-up: ↓ 50 Changed 10 years ago by
Replying to leif:
Replying to jhpalmieri:
So I think we should apply this patch on all platforms.
But IMHO only if
pkg-config
is not available.
Looks as if you do that or similar (by checking for pkg-config
in the patch itself), though the attached diff doesn't move the patch (to setupext.py
btw., the commit message is wrong) from patches/Darwin/
to patches/
.
Note that I've also made a new libpng
spkg at #11696, which needs testing especially on Darwin.
comment:50 in reply to: ↑ 49 ; follow-up: ↓ 51 Changed 10 years ago by
Replying to leif:
(by checking for pkg-config in the patch itself)
As I noted earlier, someone might install pkg-config without recompiling Sage or this spkg, so it makes sense to me to test this at run-time, not just at installation.
the attached diff doesn't move the patch (to
setupext.py
btw., the commit message is wrong) frompatches/Darwin/
topatches/
.
It actually does, but it's hard to see in the patch: it's right after the header for "spkg-install".
I'll fix the commit message, and I'll look at #11696 eventually. It's easy enough for me to test on Darwin.
comment:51 in reply to: ↑ 50 ; follow-up: ↓ 52 Changed 10 years ago by
- Keywords MPL Errno 14 libpng added
- Status changed from needs_review to positive_review
Replying to jhpalmieri:
Replying to leif:
(by checking for pkg-config in the patch itself)
As I noted earlier, someone might install pkg-config without recompiling Sage or this spkg, so it makes sense to me to test this at run-time, not just at installation.
Well, the code is only executed when MPL gets installed, and the file itself doesn't get installed anywhere AFAIK.
the attached diff doesn't move the patch (to
setupext.py
btw., the commit message is wrong) frompatches/Darwin/
topatches/
.It actually does, but it's hard to see in the patch: it's right after the header for "spkg-install".
I see. Also the spkg is ok, so I'll give it positive review again, also based on your comment that it works on iras (which I would expect it to anyway).
I'll fix the commit message, and I'll look at #11696 eventually. It's easy enough for me to test on Darwin.
Perhaps on some ancient Darwin systems as well if you can.
comment:52 in reply to: ↑ 51 Changed 10 years ago by
Replying to leif:
Perhaps on some ancient Darwin systems as well if you can.
And perhaps also on iras by first installing the new libpng spkg and afterwards your old matplotlib spkg (the one that only applies the patch on Darwin).
comment:53 Changed 10 years ago by
- Keywords 17 added; 14 removed
comment:54 Changed 10 years ago by
- Description modified (diff)
I'll leave the old SPKG around for a while, in case anyone needs it. I also just made a version which doesn't patch setupext.py at all, to help test #11696.
comment:55 follow-up: ↓ 57 Changed 10 years ago by
Is the new spkg now ready to be merged independently of #11696?
comment:56 Changed 10 years ago by
comment:57 in reply to: ↑ 55 Changed 10 years ago by
- Work issues Either check for pkg-config on other platforms as well, or make the ticket depend on #11696. deleted
Replying to jdemeyer:
Is the new spkg now ready to be merged independently of #11696?
Yes. I just forgot to delete the work issues when giving it positive review again.
I think it doesn't hurt if #11696 also (partially) fixes this in a different way, too, though, and the spkg there contains other important changes to the libpng spkg. (Currently, the latter needs work though, i.e. I have to update the spkg there, but as said, the MPL one here can be merged regardless of that.)
comment:58 Changed 10 years ago by
- Merged in set to sage-4.7.2.alpha2
- Resolution set to fixed
- Status changed from positive_review to closed
comment:59 follow-up: ↓ 60 Changed 10 years ago by
Can this ticket be reopened again? My build failed (see http://groups.google.com/group/sage-release/browse_thread/thread/6daea0d0dfef0562).
comment:60 in reply to: ↑ 59 ; follow-up: ↓ 61 Changed 10 years ago by
Replying to johanbosman:
Can this ticket be reopened again? My build failed (see http://groups.google.com/group/sage-release/browse_thread/thread/6daea0d0dfef0562).
Cf. my comment here. (You could try building with my current libpng spkg there, though it needs work for other constellations.)
I'd prefer opening a follow-up ticket, as this ticket has already been merged; haven't looked at your build log yet, though.
comment:61 in reply to: ↑ 60 Changed 10 years ago by
Replying to leif:
I'd prefer opening a follow-up ticket, as this ticket has already been merged; haven't looked at your build log yet, though.
(And this ticket was intended to just fix a regression, because the first MPL 1.0.1 spkg had accidentally been based on an obsolete one; therefore its title.)
comment:62 Changed 10 years ago by
Okay, we'll move the discussion somewhere else then. ;).
comment:63 Changed 10 years ago by
Apparently MPL has problems using your pkg-config
, for whatever reason...:
... REQUIRED DEPENDENCIES numpy: 1.5.1 freetype2: found, but unknown version (no pkg-config) OPTIONAL BACKEND DEPENDENCIES libpng: found, but unknown version (no pkg-config) ...
The patched setupext.py
in contrast seems to find it, and therefore disables the work-around for when pkg-config
is not installed.
(The note "(no pkg-config)" also appears if pkg-config
is installed, but unable to read the .pc
file, e.g. because of duplicate definitions of SAGE_ROOT
in it.)
comment:64 Changed 10 years ago by
Johan, please install this spkg (which provides some debugging output, not trying to fix anything yet), and inspect the top of the install log (only up to "[Edit setup.cfg to suppress the above messages]" if it fails, which I expect it to).
Thanks.
(I think I now know where the problem lies. :) )
comment:65 follow-up: ↓ 66 Changed 10 years ago by
How do I install this?
comment:66 in reply to: ↑ 65 Changed 10 years ago by
Replying to johanbosman:
How do I install this?
For example by typing
$ ./sage -f http://sage.math.washington.edu/home/leif/Sage/spkgs/matplotlib-1.0.1.p0-debug.spkg
in your SAGE_ROOT
directory. (Or download it manually with your favourite program and pass the local filename to ./sage -f ...
.)
comment:67 follow-up: ↓ 68 Changed 10 years ago by
The point is that Sage did not build. Or is it okay for this purpose to use another version of Sage?
comment:68 in reply to: ↑ 67 Changed 10 years ago by
Replying to johanbosman:
The point is that Sage did not build. Or is it okay for this purpose to use another version of Sage?
This AFAIK shouldn't matter (that Sage didn't build), but I'm not totally sure.
Since I assume previous versions of Sage did build for you on that machine (with an older MPL spkg), I'd use the Sage installation where it failed (i.e., Sage 4.7.2.alpha2), since the error might (at least in theory) originate from something else.
To be on the safe side, copy the downloaded package to $SAGE_ROOT/spkg/standard/
and (re)run make
. Then Sage shouldTM pick up the new spkg with debugging. The log is then spkg/logs/matplotlib-1.0.1.p0-debug.log
.
(In case Sage doesn't pick up the right one, which I don't think, you could (re)move the original MPL spkg from there. A matter of file modification times, so you could equally just touch
the new spkg.)
To see in advance which version Sage would install, do
$ (cd $SAGE_ROOT/spkg/standard/ && ./newest_version matplotlib)
comment:69 Changed 10 years ago by
Thanks for explaining. I'm trying to build it now. ;)
comment:70 follow-up: ↓ 71 Changed 10 years ago by
Hmm, I tried to upload the top of the build log, but I got an error message:
IOError: [Errno 28] No space left on device
Any suggestions?
comment:71 in reply to: ↑ 70 Changed 10 years ago by
Replying to johanbosman:
Hmm, I tried to upload the top of the build log, but I got an error message:
IOError: [Errno 28] No space left on device
To trac?
I didn't think of the (full) MPL log (which you attached) being that large btw.
Any suggestions?
Try replacing the already attached one with the newer, smaller one. Requires it having the same filename of course.
Or upload it onto sage.math
and provide a link. If it's not too long, you could also quote it here in a comment (in a "code block").
Or mail it to me (notdotreallyatonlinedotde, replacing the obvious), then I can upload it to sage.math
.
comment:72 follow-up: ↓ 73 Changed 10 years ago by
Hahaha, my guess was right:
... has_pkgconfig(): trying 'pkg-config --help'... has_pkgconfig(): command exited with 34304. Output is: Usage: pkg-config [OPTION...] --version output version of pkg-config --modversion output version for package --atleast-pkgconfig-version=VERSION sh: line 1: 16589 Abort trap pkg-config --help has_pkgconfig(): returning False. ... add_png_flags(): Sage: 'pkg-config' is in PATH, not trying to use work-around. ...
Johan, can you give the output of
$ pkg-config --version ; echo $? $ pkg-config --help ; echo $?
?
comment:73 in reply to: ↑ 72 Changed 10 years ago by
Replying to leif:
Hahaha, my guess was right:
... has_pkgconfig(): trying 'pkg-config --help'... has_pkgconfig(): command exited with 34304. ... has_pkgconfig(): returning False. ... add_png_flags(): Sage: 'pkg-config' is in PATH, not trying to use work-around. ...
For the record, I have a (preliminary) MPL 1.0.1.p1 spkg fixing this, which Johan already tested.
(Cf. http://groups.google.com/group/sage-release/msg/49a4ded0bc168976.)
I'll open a follow-up for that one at some uncertain point in the future... (Remind me of that in case nothing should happen for a while.) ;-)
comment:74 Changed 10 years ago by
Some follow-up, for upgrading to MPL 1.1.0: #11915
Seen this before (i.e., similar), haven't we?