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

would crash if get_home() returns None #2300

Merged
merged 1 commit into from Sep 30, 2013
Merged

Conversation

mdboom
Copy link
Member

@mdboom mdboom commented Sep 27, 2013

See http://bugs.debian.org/719384 for original reports.

ATM get_home() would return None if none of the home directories current exists, even though e.g. HOME environment variable is set.
As a result os.path.join would spit out exception since it was not devised to have any opinion about None.

IMHO the logic here should be fixed and get_home should not return None even if a directory pointed to by HOME environment variable on a POSIX platform is defined

@SnarkBoojum
Copy link

I just had a look at the sources ; in lib/matplotlib/init.py:

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

    :see:  http://mail.python.org/pipermail/python-list/2005-February/325395.html
    """
    try:
        path = os.path.expanduser("~")
    except ImportError:
        # This happens on Google App Engine (pwd module is not present).
        pass
    else:
        if os.path.isdir(path):
            return path
    for evar in ('HOME', 'USERPROFILE', 'TMP'):
        path = os.environ.get(evar)
        if path is not None and os.path.isdir(path):
            return path
    return None

so it is looking into environment variables...

@mdboom
Copy link
Member

mdboom commented Aug 27, 2013

I agree that crashing strangely in os.path.join is bad, but what should it do if the home directory doesn't exist? Create it? I'm not sure that's the best idea either. If $HOME points to a non-existent directory, generally that means something is wrong (and we should probably raise a better exception, but I'm not sure blindly returning it is the answer).

@SnarkBoojum
Copy link

Ooohh... I had misread the original bug report... I had understood the code wasn't finding the home even though it was explicit... and now I see that $HOME points to something invalid!

@yarikoptic
Copy link
Author

Well I have filed a report instead of a PR because for me all the ambiguity where TMP could be the "home" is not clear. Imho $HOME is the home if it exists or not. On POSIX systems I guess HOME variable is always there... If this all was due to platform differences -- then code should reflect it. Now it just looks like a heuristic to me to get some directory to write to... then may be it should mktemp directory and return it as home if none among others was found

@pelson
Copy link
Member

pelson commented Aug 28, 2013

One very pertinent PR relating to this: #1824

@mdboom
Copy link
Member

mdboom commented Aug 28, 2013

Yeah, I think your suggestion to return mktemp in the failure case is the right one.

@WeatherGod
Copy link
Member

I have a very vague recollection of an issue reported once by a user that
did not have a writable filesystem available. Are we undoing a fix here?

@mdboom
Copy link
Member

mdboom commented Aug 28, 2013

@WeatherGod: #1824 is one such issue. I think what's happened here is I broke support for that when I moved to using XDG directories on Linux. It looks, on further reflection, like we need to go the longer route and handle the case where get_home returns None everywhere it is called.

mdboom added a commit that referenced this pull request Sep 30, 2013
would crash if get_home() returns None
@mdboom mdboom merged commit 1d51360 into matplotlib:v1.3.x Sep 30, 2013
@mdboom mdboom deleted the fix-get-home branch August 7, 2014 13:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants