matplotlib: avoid race conditions when creating (config) directories

[John Palmieri]

This is a followup to #6235. You can find a new spkg at

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

and I've attached the mercurial patch from the old version to the new, for reference. There are two changes: record in the SPKG.txt file the change from #6235 about MPLCONFIGDIR, so that in future upgrades, we don't screw up how that variable is set. Also, patch texmanager.py so that it creates the config dir in a way less likely to cause problems. (See #8677 for a similar situation; the solution was taken from there.)

[John Palmieri]

A note about the patch: it is a little large because I had to add the file patches/texmanager.py to the repository. Also, in addition to the important changes, it also changes various parts of SPKG.txt so that the lines are shorter than 80 characters.

[John Palmieri]

I just reported this upstream. If I get any feedback, I'll report it here.

[Leif Leonhardy]

Apart from the typo in the commit message, and I'm not sure if I like assert there, I'm ok with the patch, and the spkg is clean.

Works for me as advertised, so positive review.

(Tested on Ubuntu 10.04 x86_64, Core2 Quad. I also deleted the MPL config dir and then ran

$ ./sage -tp N -long devel/sage/sage/plot/ # N={32,64} (though only 49 files)

Each time all doctests passed. Perhaps others should stress-[doc]test it on other platforms like MacOS X, too.)

[John Palmieri]

Replying to leif:

Apart from the typo in the commit message

If you mean the spurious apostrophe, I've removed it.

, and I'm not sure if I like assert

I just took that from #8677.

Works for me as advertised, so positive review.

Great, thanks!

[Leif Leonhardy]

Oh, I just noticed the patch does not (exactly) what the ticket advertises... (In fact, the patch addresses the race condition in the creation of the TeX cache.)

There are still some other instances of if not os.path.exists(foo): mkdir(foo) and similar, so it's a bit surprising I did not run into any race condition.

John, are you willing to fix these, too?

Unless you meanwhile have Sun's grep installed, you can find them by e.g.

$ grep -B5 -A2  mkdir `find src -name \*.py`

(Giving about two false positives; also not all are equally important I think.)

[Leif Leonhardy]

I tried the same multiple times with DOT_SAGE set to some NFS-mounted filesystem, made that busy by another machine, but still don't get any errors...

[John Palmieri]

Okay, I've fixed some more of these. I think that only the files in src/lib/matplotlib get installed, so I haven't bothered with the other ones. I also ignored the function "mkdirs" in cbook.py. This is only used in sphinxext/plot_directive.py, so I don't think it should affect us. Also, the function could perhaps just be replaced with a call to ​os.makedirs, which is already recursive and allows you to set the mode. Maybe the matplotlib people are worried about the presence of ".." in the pathname? Anyway, I don't have a good way to test any changes to that part of the code, so I'm going to leave it alone.

[John Palmieri]

Mercurial patch for matplotlib spkg; for reference only

[Leif Leonhardy]

Ok.

Also, raise is more adequate there, since one cannot assert the only possible OSError is EEXIST, so it's less confusing now.

(I still can't force errors.)