Navigation Menu

Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MPLCONFIGDIR tries to be created in read-only home #832

Merged
merged 2 commits into from Aug 25, 2012

Conversation

mdboom
Copy link
Member

@mdboom mdboom commented Aug 23, 2012

Hi,

I just ran into an issue after upgrading matplotlib on a server that generates graphs. The stacktrace I'm getting is:

Traceback (most recent call last):
  File "/srv/wiki/env/scripts/gen_open_tickets.py", line 5, in <module>
    import matplotlib
  File "/usr/lib/pymodules/python2.6/matplotlib/__init__.py", line 711, in <module>
    rcParams = rc_params()
  File "/usr/lib/pymodules/python2.6/matplotlib/__init__.py", line 629, in rc_params
    fname = matplotlib_fname()
  File "/usr/lib/pymodules/python2.6/matplotlib/__init__.py", line 567, in matplotlib_fname
    fname = os.path.join(get_configdir(), 'matplotlibrc')
  File "/usr/lib/pymodules/python2.6/matplotlib/__init__.py", line 240, in wrapper
    ret = func(*args, **kwargs)
  File "/usr/lib/pymodules/python2.6/matplotlib/__init__.py", line 439, in _get_configdir
    raise RuntimeError("Failed to create %s/.matplotlib; consider setting MPLCONFIGDIR to a writable directory for matplotlib configuration data"%h)
RuntimeError: Failed to create /var/www/.matplotlib; consider setting MPLCONFIGDIR to a writable directory for matplotlib configuration data

Obviously, I am getting this because my user has no write permission in its home folder. I've seen other people having this issue, too: http://www.mail-archive.com/matplotlib-users@lists.sourceforge.net/msg08089.html and while setting MPLCONFIGDIR to /tmp certainly would fix this issue, I wonder why it needs to be created in the first place? Like almost all other *NIX systems I think matplotlib should not fail if a resource doesn't exist. Possibly warn. It'll work anyway, right?

What do you think? Can we make it a warning? I could probably come up with a pull request if noone else has time.

@JensRantil
Copy link
Author

Oh, also it seems like this has been "broken" since 2010-08-01 if that helps. Source: http://matplotlib.sourceforge.net/_static/CHANGELOG

@pelson pelson mentioned this pull request Jun 5, 2012
@pelson
Copy link
Member

pelson commented Aug 19, 2012

The linked issue has nothing to do with this issue. @JensRantil it seems reasonable to turn it into a warning, but does that make everything else work?

@mdboom
Copy link
Member

mdboom commented Aug 19, 2012

We need write permissions on that directory to build the font cache, among other things. So it doesn't actually work to continue.

In that case, it should probably automatically switch to using a temp directory that is then cleared atexit.

@JensRantil
Copy link
Author

@pelson wrote:

The linked issue has nothing to do with this issue. @JensRantil it seems reasonable to turn it into a warning, but does that make everything else work?

I never tried. However, as @mdboom wrote, it seems that it wouldn't fix the issue. Createing a temporary folder in temp directory (and making a warning about it) sounds like a reasonable solution.

@ghost ghost assigned mdboom Aug 21, 2012
@mdboom
Copy link
Member

mdboom commented Aug 23, 2012

I'm going through the implementation here which isn't so bad, but I have to say, fixing this doesn't feel right to me. Creating the font cache entails considerable startup overhead, that should only need to happen the first time. Creating a temp directory and then destroying that after each run is going to cause performance issues, and I'm worried this bug will just become "why is the performance of running matplotlib in my webserver so bad?" and the response is "set an MPLCONFIGDIR to a writable directory", which they would have known up front had this just raised an exception as it currently does. I think the analogy to other UNIX tools is fine, but choose the right analogy -- does apache do anything useful without config files and a cache directory? I think I'm strongly leaning to treating this as WONTFIX, but I'd appreciate the feedback of other old-timers: @efiring, @ddale, etc.

@efiring
Copy link
Member

efiring commented Aug 23, 2012

I agree with @mdboom that making the temp directory and deleting it each time is not a good solution. WONTFIX is a reasonable option. Possible alternative: at the point where now an exception is raised, try one more way of finding a persistent temporary directory in which to put .matplotlib: tempfile.gettempdir(). On linux this is /tmp, which is usually writeable; on OS X 10.7 it appears to be a directory created for the user and persistent at least from one shell invocation to another; I have no idea what it is on Windows. In any case, this provides at least the possibility of being able to write a reusable cache in common circumstances. I think a warning would still be appropriate if this gettempdir() fallback is used, and an exception if even that fails--but that should be rare.

@mdboom
Copy link
Member

mdboom commented Aug 23, 2012

Ah -- I was thinking it would be a rotating temp directory as returned from tempfile.mkdtmp(), but I see how if it had a consistent name maybe that's better. I know that's usually considered bad practice for security reasons, because a malicious process can guess the name of the temp dir and inject things into it, but I wonder if that's a problem in practice as it's no more guessable than the home directory is.

@efiring
Copy link
Member

efiring commented Aug 23, 2012

I think that what I am suggesting is standard practice. Looking in /tmp on my vmware virtual machine, for example, I find

drwx------ 2 efiring efiring 4096 Aug 23 08:14 vmware-efiring
drwx------ 2 root    root    4096 Aug 23 08:15 vmware-root
efiring@manini3:~/work/programs/py/mpl/matplotlib$ ll /tmp/vmware-efiring/
total 8
-rw-r--r-- 1 efiring efiring 4534 Aug 23 08:14 apploader-1943.log

so the practice is to use /tmp as a place to put a user-only directory with a predictable name, persistent until the next reboot. I don't see any reason why we can't do the same thing. It looks like that is what tempfile.gettempdir() is for.

@mdboom
Copy link
Member

mdboom commented Aug 23, 2012

I've attached a PR that I think addresses this in the way @efiring described.

def _create_tmp_config_dir():
"""
If the config directory can not be created, create a temporary
directory that is destroyed atexit.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But now it is not destroyed atexit, correct?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes -- forgot to update the docstring.

@efiring efiring merged commit 41a6242 into matplotlib:master Aug 25, 2012
@efiring
Copy link
Member

efiring commented Aug 25, 2012

I added a change to a docstring and to the MPLCONFIGDIR faq entry. Undoubtedly other polishing is possible, but I thought it better to get this merged and move on; we still have a daunting number of pull requests.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants