| Submitter | Yuya Nishihara |
|---|---|
| Date | March 25, 2017, 10:38 a.m. |
| Message ID | <05dc34258f021523bc1d.1490438331@mimosa> |
| Download | mbox | patch |
| Permalink | /patch/19666/ |
| State | Accepted |
| Headers | show |
Comments
The series makes the CPython code cleaner. Marked as prereviewed. I was making "getfstype" a pure C function, so it's possible to be reused directly for non-cpython code (as a side effect, it does not have Python error handling). The new "describefstype" provides enough support if we want a pure C implementation later. There won't be much duplicated code. Thanks! Excerpts from Yuya Nishihara's message of 2017-03-25 19:38:51 +0900: > # HG changeset patch > # User Yuya Nishihara <yuya@tcha.org> > # Date 1490430323 -32400 > # Sat Mar 25 17:25:23 2017 +0900 > # Node ID 05dc34258f021523bc1de5c403f671fa4d1b9905 > # Parent 69a0abd9c61f0dae12d60b4f6fd4ada8288bd451 > statfs: make getfstype() raise OSError > > It's better for getfstype() function to not suppress an error. Callers can > handle it as necessary. Now "hg debugfsinfo" will report OSError. > > diff --git a/mercurial/osutil.c b/mercurial/osutil.c > --- a/mercurial/osutil.c > +++ b/mercurial/osutil.c > @@ -1090,7 +1090,7 @@ static PyObject *getfstype(PyObject *sel > memset(&buf, 0, sizeof(buf)); > r = statfs(path, &buf); > if (r != 0) > - Py_RETURN_NONE; > + return PyErr_SetFromErrno(PyExc_OSError); > return Py_BuildValue("s", describefstype(&buf)); > } > #endif /* defined(HAVE_LINUX_STATFS) || defined(HAVE_BSD_STATFS) */ > diff --git a/mercurial/util.py b/mercurial/util.py > --- a/mercurial/util.py > +++ b/mercurial/util.py > @@ -1089,7 +1089,10 @@ def copyfile(src, dest, hardlink=False, > if hardlink: > # Hardlinks are problematic on CIFS (issue4546), do not allow hardlinks > # unless we are confident that dest is on a whitelisted filesystem. > - fstype = getfstype(os.path.dirname(dest)) > + try: > + fstype = getfstype(os.path.dirname(dest)) > + except OSError: > + fstype = None > if fstype not in _hardlinkfswhitelist: > hardlink = False > if hardlink: > @@ -1372,7 +1375,7 @@ def fspath(name, root): > def getfstype(dirpath): > '''Get the filesystem type name from a directory (best-effort) > > - Returns None if we are unsure, or errors like ENOENT, EPERM happen. > + Returns None if we are unsure. Raises OSError on ENOENT, EPERM, etc. > ''' > return getattr(osutil, 'getfstype', lambda x: None)(dirpath) > > diff --git a/tests/hghave.py b/tests/hghave.py > --- a/tests/hghave.py > +++ b/tests/hghave.py > @@ -349,7 +349,10 @@ def has_hardlink(): > @check("hardlink-whitelisted", "hardlinks on whitelisted filesystems") > def has_hardlink_whitelisted(): > from mercurial import util > - fstype = util.getfstype('.') > + try: > + fstype = util.getfstype('.') > + except OSError: > + return False > return fstype in util._hardlinkfswhitelist > > @check("rmcwd", "can remove current working directory")
On Sat, Mar 25, 2017 at 07:38:51PM +0900, Yuya Nishihara wrote: > # HG changeset patch > # User Yuya Nishihara <yuya@tcha.org> > # Date 1490430323 -32400 > # Sat Mar 25 17:25:23 2017 +0900 > # Node ID 05dc34258f021523bc1de5c403f671fa4d1b9905 > # Parent 69a0abd9c61f0dae12d60b4f6fd4ada8288bd451 > statfs: make getfstype() raise OSError Queued, thanks > > It's better for getfstype() function to not suppress an error. Callers can > handle it as necessary. Now "hg debugfsinfo" will report OSError. > > diff --git a/mercurial/osutil.c b/mercurial/osutil.c > --- a/mercurial/osutil.c > +++ b/mercurial/osutil.c > @@ -1090,7 +1090,7 @@ static PyObject *getfstype(PyObject *sel > memset(&buf, 0, sizeof(buf)); > r = statfs(path, &buf); > if (r != 0) > - Py_RETURN_NONE; > + return PyErr_SetFromErrno(PyExc_OSError); > return Py_BuildValue("s", describefstype(&buf)); > } > #endif /* defined(HAVE_LINUX_STATFS) || defined(HAVE_BSD_STATFS) */ > diff --git a/mercurial/util.py b/mercurial/util.py > --- a/mercurial/util.py > +++ b/mercurial/util.py > @@ -1089,7 +1089,10 @@ def copyfile(src, dest, hardlink=False, > if hardlink: > # Hardlinks are problematic on CIFS (issue4546), do not allow hardlinks > # unless we are confident that dest is on a whitelisted filesystem. > - fstype = getfstype(os.path.dirname(dest)) > + try: > + fstype = getfstype(os.path.dirname(dest)) > + except OSError: > + fstype = None > if fstype not in _hardlinkfswhitelist: > hardlink = False > if hardlink: > @@ -1372,7 +1375,7 @@ def fspath(name, root): > def getfstype(dirpath): > '''Get the filesystem type name from a directory (best-effort) > > - Returns None if we are unsure, or errors like ENOENT, EPERM happen. > + Returns None if we are unsure. Raises OSError on ENOENT, EPERM, etc. > ''' > return getattr(osutil, 'getfstype', lambda x: None)(dirpath) > > diff --git a/tests/hghave.py b/tests/hghave.py > --- a/tests/hghave.py > +++ b/tests/hghave.py > @@ -349,7 +349,10 @@ def has_hardlink(): > @check("hardlink-whitelisted", "hardlinks on whitelisted filesystems") > def has_hardlink_whitelisted(): > from mercurial import util > - fstype = util.getfstype('.') > + try: > + fstype = util.getfstype('.') > + except OSError: > + return False > return fstype in util._hardlinkfswhitelist > > @check("rmcwd", "can remove current working directory") > _______________________________________________ > Mercurial-devel mailing list > Mercurial-devel@mercurial-scm.org > https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Patch
diff --git a/mercurial/osutil.c b/mercurial/osutil.c --- a/mercurial/osutil.c +++ b/mercurial/osutil.c @@ -1090,7 +1090,7 @@ static PyObject *getfstype(PyObject *sel memset(&buf, 0, sizeof(buf)); r = statfs(path, &buf); if (r != 0) - Py_RETURN_NONE; + return PyErr_SetFromErrno(PyExc_OSError); return Py_BuildValue("s", describefstype(&buf)); } #endif /* defined(HAVE_LINUX_STATFS) || defined(HAVE_BSD_STATFS) */ diff --git a/mercurial/util.py b/mercurial/util.py --- a/mercurial/util.py +++ b/mercurial/util.py @@ -1089,7 +1089,10 @@ def copyfile(src, dest, hardlink=False, if hardlink: # Hardlinks are problematic on CIFS (issue4546), do not allow hardlinks # unless we are confident that dest is on a whitelisted filesystem. - fstype = getfstype(os.path.dirname(dest)) + try: + fstype = getfstype(os.path.dirname(dest)) + except OSError: + fstype = None if fstype not in _hardlinkfswhitelist: hardlink = False if hardlink: @@ -1372,7 +1375,7 @@ def fspath(name, root): def getfstype(dirpath): '''Get the filesystem type name from a directory (best-effort) - Returns None if we are unsure, or errors like ENOENT, EPERM happen. + Returns None if we are unsure. Raises OSError on ENOENT, EPERM, etc. ''' return getattr(osutil, 'getfstype', lambda x: None)(dirpath) diff --git a/tests/hghave.py b/tests/hghave.py --- a/tests/hghave.py +++ b/tests/hghave.py @@ -349,7 +349,10 @@ def has_hardlink(): @check("hardlink-whitelisted", "hardlinks on whitelisted filesystems") def has_hardlink_whitelisted(): from mercurial import util - fstype = util.getfstype('.') + try: + fstype = util.getfstype('.') + except OSError: + return False return fstype in util._hardlinkfswhitelist @check("rmcwd", "can remove current working directory")