8000 Support environments without a home dir or writable file system by mgiuca-google · Pull Request #1824 · matplotlib/matplotlib · GitHub
[go: up one dir, main page]

Skip to content

Support environments without a home dir or writable file system #1824

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

Merged
merged 17 commits into from
Apr 16, 2013
Merged
Changes from 1 commit
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
8083f25
shutil.move: Guard against IOError and print warnings instead of cras…
mgiuca-google Mar 6, 2013
a697a26
matplotlib: _is_writable_dir uses os.access instead of TemporaryFile.
mgiuca-google Mar 6, 2013
4d65400
Matplotlib now works when the user has no home directory.
mgiuca-google Mar 6, 2013
21921a3
get_configdir returns None if tempfile.gettempdir() is not available.
mgiuca-google Mar 6, 2013
4987dcd
Deal with all cases where get_configdir might return None.
mgiuca-google Mar 6, 2013
64c797b
font_manager: Gracefully handle the case of there being no config dir.
mgiuca-google Mar 6, 2013
1dbd6de
texmanager: Gracefully handle the case of there being no config dir u…
mgiuca-google Mar 6, 2013
1adfc85
finance: Gracefully handle the case of there being no config dir.
mgiuca-google Mar 8, 2013
941efd4
Fix formatting and other misc code tweaks.
mgiuca-google Mar 17, 2013
ca6cd19
matplotlib.get_home: Removing catch-all except blocks.
mgiuca-google Mar 18, 2013
cc8cd1b
matplotlib, texmanager: Change WARNING prints into real warnings.
mgiuca-google Mar 18, 2013
f01ebe1
matplotlib, texmanager: Only print the rename message if it actually …
mgiuca-google Mar 18, 2013
018ce26
finance: Fixed caching when cachename is supplied.
mgiuca-google Mar 18, 2013
6a4f1e7
matplotlib: Use cbook.mkdirs instead of os.makedirs.
mgiuca-google Mar 18, 2013
4f55a27
matplotlib: Remove catch for OSError.
mgiuca-google Mar 18, 2013
81639a1
matplotlib: _is_writable_dir checks that it is a directory.
mgiuca-google Mar 18, 2013
8335773
matplotlib: _is_writable_dir tests with os.access and TemporaryFile.
mgiuca-google Mar 18, 2013
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Matplotlib now works when the user has no home directory.
matplotlib.get_home now returns None if there is no home directory, instead of
raising RuntimeError. Updated code in several places to handle this gracefully.

matplotlib.get_configdir now returns a temporary directory if there is no home
directory, instead of raising RuntimeError.
  • Loading branch information
mgiuca-google committed Apr 16, 2013
commit 4d654004c6b0eca84c6ed1a0353d51c2220f8213
84 changes: 45 additions & 39 deletions lib/matplotlib/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -470,26 +470,25 @@ def checkdep_usetex(s):

def _get_home():
"""Find user's home directory if possible.
Otherwise raise error.
Otherwise, returns None.

:see: http://mail.python.org/pipermail/python-list/2005-February/263921.html
:see: http://mail.python.org/pipermail/python-list/2005-February/325395.html
Copy link
Member

Choose a reason for hiding this comment

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

👍

"""
path=''
try:
path=os.path.expanduser("~")
path = os.path.expanduser("~")
except:
Copy link
Member

Choose a reason for hiding this comment

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

I'd rather remove this bare except. Would you mind adding the specific exception(s) types that might occur?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK. Well I hope I didn't miss any... but the documentation for both os.path.expanduser and pwd do not mention any exceptions. I dug through the code and it doesn't look like any exceptions are possible on normal (non-App-Engine) platforms. (pwd.getpwnam can raise KeyError, but that is caught in expanduser.) So I think we could actually remove that try-except entirely, except on App Engine, the pwd module is sadly not present, so calling expanduser raises ImportError. I'm changing the code to explicitly catch ImportError with a comment that this is for App Engine. I hope this is okay.

pass
if not os.path.isdir(path):
for evar in ('HOME', 'USERPROFILE', 'TMP'):
try:
path = os.environ[evar]
if os.path.isdir(path):
break
except: pass
if path:
return path
else:
raise RuntimeError('please define environment variable $HOME')
if os.path.isdir(path):
return path
for evar in ('HOME', 'USERPROFILE', 'TMP'):
try:
Copy link
Member

Choose a reason for hiding this comment

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

The try except seems superfluous to me, I'd change this to:

path = os.environ.get(evar)
if path is not None and os.path.isdir(path):
    return path

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

path = os.environ[evar]
if os.path.isdir(path):
return path
except:
pass
return None


def _create_tmp_config_dir():
< 10000 tool-tip id="tooltip-d9d2d84b-e15a-4db8-92e7-f76a6d6d6e7a" for="expand-link-40-diff-47339c58ad4d3bf37dfc5302e1c27245645ed38258d9b146a3f4d47274ef054d" popover="manual" data-direction="ne" data-type="label" data-view-component="true" class="sr-only position-absolute">Expand All @@ -513,12 +512,14 @@ def _get_configdir():
"""
Return the string representing the configuration directory.

Default is HOME/.matplotlib. You can override this with the
MPLCONFIGDIR environment variable. If the default is not
writable, and MPLCONFIGDIR is not set, then
tempfile.gettempdir() is used to provide a directory in
which a matplotlib subdirectory is created as the configuration
directory.
The directory is chosen as follows:

1. If the MPLCONFIGDIR environment variable is supplied, choose that. Else,
choose the '.matplotlib' subdirectory of the user's home directory (and
create it if necessary).
2. If the chosen directory exists and is writable, use that as the
configuration directory.
3. Create a temporary directory, and use it as the configuration directory.
"""

configdir = os.environ.get('MPLCONFIGDIR')
Expand All @@ -530,18 +531,21 @@ def _get_configdir():
return configdir

h = get_home()
p = os.path.join(get_home(), '.matplotlib')
if h is not None:
p = os.path.join(h, '.matplotlib')

if os.path.exists(p):
if not _is_writable_dir(p):
return _create_tmp_config_dir()
else:
if not _is_writable_dir(h):
return _create_tmp_config_dir()
from matplotlib.cbook import mkdirs
mkdirs(p)
if os.path.exists(p):
if not _is_writable_dir(p):
return _create_tmp_config_dir()
else:
if not _is_writable_dir(h):
return _create_tmp_config_dir()
from matplotlib.cbook import mkdirs
mkdirs(p)
Copy link
Member

Choose a reason for hiding this comment

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

There must be a subtlety that I'm missing here - I can't see the difference between cbook.mkdirs and os.makedirs. Any ideas?

Copy link
Member

Choose a reason for hiding this comment

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

The difference is that cbook.mkdirs doesn't complain if the directory already exists.

Copy link
Member

Choose a reason for hiding this comment

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

cbook.mkdirs() is even safer than os.makedirs(). Imagine two processes, one needing to make A/B, and the other needing to make A/C. There is a race condition in os.makedirs() where one of the two processes will fail to make their directory because the exception was thrown while dealing with the parent directory.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So why does it use os.makedirs above? (It's a little bit above the context shown in this patch, but the first block of get_configdir calls os.makedirs, whereas the second block calls cbook.mkdirs.) Can we change it so it consistently calls cbook.mkdirs?

Copy link
Member

Choose a reason for hiding this comment

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

Yes -- let's change this to cbook.mkdirs in both cases.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.


return p

return p
return _create_tmp_config_dir()
get_configdir = verbose.wrap('CONFIGDIR=%s', _get_configdir, always=False)


Expand Down Expand Up @@ -643,18 +647,20 @@ def matplotlib_fname():
print("WARNING: File could not be renamed: %s" % e, file=sys.stderr)
Copy link
Member

Choose a reason for hiding this comment

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

Could you turn this into a real warning.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done (and the rest).


home = get_home()
oldname = os.path.join( home, '.matplotlibrc')
if os.path.exists(oldname):
configdir = get_configdir()
newname = os.path.join(configdir, 'matplotlibrc')
print("""\
if home:
oldname = os.path.join( home, '.matplotlibrc')
Copy link
Member

Choose a reason for hiding this comment

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

Please remove the space before home.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done (and the rest).

if os.path.exists(oldname):
configdir = get_configdir()
newname = os.path.join(configdir, 'matplotlibrc')
print("""\
WARNING: Old rc filename "%s" found and renamed to
new default rc file name "%s"."""%(oldname, newname), file=sys.stderr)

try:
shutil.move(oldname, newname)
except IOError as e:
print("WARNING: File could not be renamed: %s" % e, file=sys.stderr)
try:
shutil.move(oldname, newname)
except IOError as e:
print("WARNING: File could not be renamed: %s" % e,
file=sys.stderr)


fname = os.path.join( os.getcwd(), 'matplotlibrc')
Copy link
Member

Choose a reason for hiding this comment

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

Would you mind removing a newline before this line & getting rid of the space before os.getcwd.

Expand Down
0