Race condition in matplotlib mkdir()

[jdemeyer]

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

[leif]

Seen this before (i.e., similar), haven't we?

[jhpalmieri]

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...

[leif]

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.)

[leif]

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.

[leif]

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.

[leif]

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?

[jhpalmieri]

I've created a new spkg, link in the ticket description. I'm posting the diff here for review purposes.

[jhpalmieri]

(I also deleted src/build and a few random .pyc files.)

[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'.)

[leif]

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

[leif]

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())
 
 def add_png_flags(module):
-    try_pkgconfig(module, 'libpng', 'png')
+    try_pkgconfig(module, 'libpng12', 'png12')
     add_base_flags(module)
     add_numpy_flags(module)
     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

[jhpalmieri]

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.

[leif]

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.)

[leif]

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.)

[jhpalmieri]

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.

[leif]

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

[leif]

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.

[jhpalmieri]

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"

[leif]

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.

[leif]

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.

[jhpalmieri]

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.

[leif]

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".

[leif]

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.

[leif]

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.

[leif]

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.)

[jhpalmieri]

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.

[jhpalmieri]

patch for matplotlib spkg; for review only

[jhpalmieri]

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 whether pkg-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.

[leif]

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 whether pkg-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 libpngs 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.

[leif]

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.

[jhpalmieri]

I'm considering modifying SPKG.txt as follows:

SPKG.txt

@@ -40 +40 @@
 The patches are applied during the build process.
 
  * setupext.py: link to libpng12 instead of libpng to apparently avoid
-   some name clashes on OSX.  See Sage trac #4774.
+   some name clashes on OSX.  As of version 1.0.1.p0, this patch is
+   only applied on OS X, and it tests for the presence of
+   'pkg-config': if it is not installed, then it links to libpng12
+   instead of libpng; otherwise it does nothing.  See Sage trac #4774
+   and #11686.
 
  * __init__.py, finance.py, texmanager.py: instead of "if
    not.os.path.exists(DIR): os.mkdir(DIR)", do "try os.mkdir(DIR)" and
@@ -75 +79 @@
 === matplotlib 1.0.1.p0 (John Palmieri, 13 Aug 2011) ===
  * Patch __init__.py, finance.py, and texmanager.py to avoid race
    conditions when creating directories.  See Sage trac #11686
-   and #10159.  Also replaced the "src" directory with a freshly
-   downloaded copy of the vanilla source code.
+   and #10159.
+
+ * Updated the list of prerequisites in this file.
+ 
+ * Changed spkg-install so that the patches are now applied using
+   'patch'.
+
+ * The patch to setupext.py is now only applied on OS X.  The patch
+   itself tests whether pkg-config is installed: if so, it maintains
+   the status quo, and if not, it passes the argument 'libpng12' when
+   configuring then png package.
+
+ * Replaced the "src" directory with a freshly downloaded copy of
+   the vanilla source code.
 
 === matplotlib 1.0.1 (Ryan Grout, 10 Jan 2011) ===
  * 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.)

[leif]

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 K_N 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.

[jhpalmieri]

Okay, here's a new version. The only change from the previous one is recorded in the "SPKG.txt" patch.

[jhpalmieri]

for review only; this was applied on top of the other patch

[leif]

Replying to jhpalmieri:

Okay, here's a new version. The only change from the previous one is recorded in the "SPKG.txt" patch.

Ok.

[jdemeyer]

Haven't read my mail in the weekend and now I come back to see this. Nice work!

[jdemeyer]

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​

[jdemeyer]

Additional changes

[leif]

Replying to jdemeyer:

These changes are so trivial, I don't think they need review.

FWIW, they look ok to me.

[jdemeyer]

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.

Full log: ​http://build.sagemath.org/sage/builders/SUSE%20ES10-64%20%28iras%29/builds/161/steps/shell_1/logs/matplotlib/text

[leif]

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.)

[leif]

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).

[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, 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.

[leif]

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.

[jdemeyer]

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).

[leif]

Replying to leif:

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.

This is now #11696.

[jhpalmieri]

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?

[jhpalmieri]

Since Jeroen's package supersedes mine, there is no danger for me to overwrite my version. So here's a new spkg:

• ​http://sage.math.washington.edu/home/palmieri/SPKG/matplotlib-1.0.1.p0.spkg

I'm attaching the patch, too. If this approach is acceptable, I'll change the ticket description to point at this one.

[jdemeyer]

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).

[leif]

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.

[leif]

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.

[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.

the attached diff doesn't move the patch (to setupext.py btw., the commit message is wrong) from patches/Darwin/ to patches/.

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.

[jhpalmieri]

applied on top of jdemeyer's spkg

[leif]

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) from patches/Darwin/ to patches/.

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.

[leif]

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).

[jhpalmieri]

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.

[jdemeyer]

Is the new spkg now ready to be merged independently of #11696?

[jhpalmieri]

Yes, the new spkg is independent of #11696. (One of the consequences of the changes at #11696 is to make one of the patches in the new spkg unnecessary.)

[leif]

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.)

[johanbosman]

Log of a failed build

[johanbosman]

Can this ticket be reopened again? My build failed (see ​http://groups.google.com/group/sage-release/browse_thread/thread/6daea0d0dfef0562).

[leif]

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.

[leif]

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.)

[johanbosman]

Okay, we'll move the discussion somewhere else then. ;).

[leif]

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.)

[leif]

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. :) )

[johanbosman]

How do I install this?

[leif]

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 ....)

[johanbosman]

The point is that Sage did not build. Or is it okay for this purpose to use another version of Sage?

[leif]

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 should^TM 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)

[johanbosman]

Thanks for explaining. I'm trying to build it now. ;)

[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

Any suggestions?

[leif]

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.

[leif]

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 $?

?

[leif]

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.) ;-)

[leif]

Some follow-up, for upgrading to MPL 1.1.0: #11915