This is an archive of the discontinued Mercurial Phabricator instance.

Differential D1020

largefiles: do not use platform.system()
ClosedPublic

Authored by quark on Oct 11 2017, 8:57 PM.

Details

Reviewers
ryanmce
Group Reviewers
hg-reviewers
Commits
rHGbb6544b1c56e: largefiles: do not use platform.system()
Summary

See the previous patch for the reason.

Diff Detail

Repository
rHG Mercurial
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

quark created this revision.Oct 11 2017, 8:57 PM
ryanmce requested changes to this revision.Oct 12 2017, 10:56 AM
ryanmce added a subscriber: ryanmce.
ryanmce added inline comments.
hgext/largefiles/lfutil.py
82

why is this not a pycompat.osname check? It doesn't make sense to abort below with pycompat.osname if that might still be posix.

To be clear, your patch isn't making this worse, but it's exposing something sketchy. Either we need the Abort below to include the sysplatform in its output or we need this check to be against pycombat.osname.

I suspect we want to change the abort below, in a separate patch.

This revision now requires changes to proceed.Oct 12 2017, 10:56 AM
quark added inline comments.Oct 12 2017, 11:20 AM
hgext/largefiles/lfutil.py
82

pycompat.osname returns posix on OS X.

quark requested review of this revision.Oct 12 2017, 4:11 PM

I changed it in a later patch.

Closed by commit rHGbb6544b1c56e: largefiles: do not use platform.system() (authored by quark). · Explain WhyOct 13 2017, 10:06 AM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathPackages
Mhgext/largefiles/lfutil.py (3 lines)

Diff 2679

hgext/largefiles/lfutil.py

# Copyright 2009-2010 Gregory P. Ward # Copyright 2009-2010 Gregory P. Ward
# Copyright 2009-2010 Intelerad Medical Systems Incorporated # Copyright 2009-2010 Intelerad Medical Systems Incorporated
# Copyright 2010-2011 Fog Creek Software # Copyright 2010-2011 Fog Creek Software
# Copyright 2010-2011 Unity Technologies # Copyright 2010-2011 Unity Technologies
# #
# This software may be used and distributed according to the terms of the # This software may be used and distributed according to the terms of the
# GNU General Public License version 2 or any later version. # GNU General Public License version 2 or any later version.
'''largefiles utility code: must not import other modules in this package.''' '''largefiles utility code: must not import other modules in this package.'''
from __future__ import absolute_import from __future__ import absolute_import
import copy import copy
import hashlib import hashlib
import os import os
import platform
import stat import stat
from mercurial.i18n import _ from mercurial.i18n import _
from mercurial import ( from mercurial import (
dirstate, dirstate,
encoding, encoding,
error, error,
path = ui.configpath(longname, 'usercache', None) path = ui.configpath(longname, 'usercache', None)
if path: if path:
return path return path
if pycompat.osname == 'nt': if pycompat.osname == 'nt':
appdata = encoding.environ.get('LOCALAPPDATA',\ appdata = encoding.environ.get('LOCALAPPDATA',\
encoding.environ.get('APPDATA')) encoding.environ.get('APPDATA'))
if appdata: if appdata:
return os.path.join(appdata, longname) return os.path.join(appdata, longname)
elif platform.system() == 'Darwin': elif pycompat.sysplatform == 'darwin':
ryanmceUnsubmitted
Not Done

why is this not a pycompat.osname check? It doesn't make sense to abort below with pycompat.osname if that might still be posix.

To be clear, your patch isn't making this worse, but it's exposing something sketchy. Either we need the Abort below to include the sysplatform in its output or we need this check to be against pycombat.osname.

I suspect we want to change the abort below, in a separate patch.

ryanmce: why is this not a pycompat.osname check? It doesn't make sense to abort below with pycompat.
quarkAuthorUnsubmitted
Not Done

pycompat.osname returns posix on OS X.

quark: `pycompat.osname` returns `posix` on OS X.
home = encoding.environ.get('HOME') home = encoding.environ.get('HOME')
if home: if home:
return os.path.join(home, 'Library', 'Caches', longname) return os.path.join(home, 'Library', 'Caches', longname)
elif pycompat.osname == 'posix': elif pycompat.osname == 'posix':
path = encoding.environ.get('XDG_CACHE_HOME') path = encoding.environ.get('XDG_CACHE_HOME')
if path: if path:
return os.path.join(path, longname) return os.path.join(path, longname)
home = encoding.environ.get('HOME') home = encoding.environ.get('HOME')