Submitter | Matt Harbison |
---|---|
Date | Dec. 28, 2017, 11:31 p.m. |
Message ID | <452b23148c906aca4765.1514503898@Envy> |
Download | mbox | patch |
Permalink | /patch/26483/ |
State | Accepted |
Headers | show |
Comments
On Thu, 28 Dec 2017 18:31:38 -0500, Matt Harbison wrote: > # HG changeset patch > # User Matt Harbison <matt_harbison@yahoo.com> > # Date 1514442941 18000 > # Thu Dec 28 01:35:41 2017 -0500 > # Node ID 452b23148c906aca4765d568263d9e3615cce228 > # Parent 95a9be56c3bb614fc1334a4cc8ec669f9eb7bc5a > cext: implement osutil.getfstype() on Windows > > This will allow NTFS to be added to the hardlink whitelist, and resume creating > hardlinks in transactions (which was disabled globally in 07a92bbd02e5; see also > e5ce49a30146). I'll wait until this is accepted before implementing the pure > version. Perhaps we don't need a cext version if we'll reimplement it with ctypes. > + fullpath = calloc(size, sizeof(fullpath[0])); > + volume = calloc(size, sizeof(volume[0])); Nit: no need to zero-fill? > + if (!fullpath || !volume) { > + PyErr_SetString(PyExc_MemoryError, "out of memory"); > + goto bail; Nit: PyErr_NoMemory() is preferred in our code. > +bail: > + if (fullpath) > + free(fullpath); > + if (volume) > + free(volume); Nit: free(NULL) is valid.
> On Dec 30, 2017, at 2:13 AM, Yuya Nishihara <yuya@tcha.org> wrote: > >> On Thu, 28 Dec 2017 18:31:38 -0500, Matt Harbison wrote: >> # HG changeset patch >> # User Matt Harbison <matt_harbison@yahoo.com> >> # Date 1514442941 18000 >> # Thu Dec 28 01:35:41 2017 -0500 >> # Node ID 452b23148c906aca4765d568263d9e3615cce228 >> # Parent 95a9be56c3bb614fc1334a4cc8ec669f9eb7bc5a >> cext: implement osutil.getfstype() on Windows >> >> This will allow NTFS to be added to the hardlink whitelist, and resume creating >> hardlinks in transactions (which was disabled globally in 07a92bbd02e5; see also >> e5ce49a30146). I'll wait until this is accepted before implementing the pure >> version. > > Perhaps we don't need a cext version if we'll reimplement it with ctypes. So move the existing util.getfstype to posix, and then “util.getfstype = platform.getfstype”? Works for me. Do you want to drop this from hg-committed? >> + fullpath = calloc(size, sizeof(fullpath[0])); >> + volume = calloc(size, sizeof(volume[0])); > > Nit: no need to zero-fill? > >> + if (!fullpath || !volume) { >> + PyErr_SetString(PyExc_MemoryError, "out of memory"); >> + goto bail; > > Nit: PyErr_NoMemory() is preferred in our code. > >> +bail: >> + if (fullpath) >> + free(fullpath); >> + if (volume) >> + free(volume); > > Nit: free(NULL) is valid.
On Sat, 30 Dec 2017 11:19:56 -0500, Matt Harbison wrote: > > On Dec 30, 2017, at 2:13 AM, Yuya Nishihara <yuya@tcha.org> wrote: > >> On Thu, 28 Dec 2017 18:31:38 -0500, Matt Harbison wrote: > >> # HG changeset patch > >> # User Matt Harbison <matt_harbison@yahoo.com> > >> # Date 1514442941 18000 > >> # Thu Dec 28 01:35:41 2017 -0500 > >> # Node ID 452b23148c906aca4765d568263d9e3615cce228 > >> # Parent 95a9be56c3bb614fc1334a4cc8ec669f9eb7bc5a > >> cext: implement osutil.getfstype() on Windows > >> > >> This will allow NTFS to be added to the hardlink whitelist, and resume creating > >> hardlinks in transactions (which was disabled globally in 07a92bbd02e5; see also > >> e5ce49a30146). I'll wait until this is accepted before implementing the pure > >> version. > > > > Perhaps we don't need a cext version if we'll reimplement it with ctypes. > > So move the existing util.getfstype to posix, and then “util.getfstype = platform.getfstype”? Yep, that's how win32api interface has been evolved. > Do you want to drop this from hg-committed? Pruned 4be2befbec25 and a210a1a19734.
Patch
diff --git a/mercurial/cext/osutil.c b/mercurial/cext/osutil.c --- a/mercurial/cext/osutil.c +++ b/mercurial/cext/osutil.c @@ -1268,6 +1268,84 @@ PyMem_Free(name); return file_obj; } + +static PyObject *getfstype(PyObject *self, PyObject *args) +/* given a directory path, return filesystem type name (best-effort) */ +{ + const char *path = NULL; + char *fullpath = NULL; + char *volume = NULL; + DWORD size = 0; + char fstype[MAX_PATH + 1]; + + if (!PyArg_ParseTuple(args, "s", &path)) + return NULL; + + size = GetFullPathName(path, 0, NULL, NULL); + + if (size == 0) { + PyErr_SetFromWindowsErrWithFilename(GetLastError(), path); + return NULL; + } + + fullpath = calloc(size, sizeof(fullpath[0])); + volume = calloc(size, sizeof(volume[0])); + + if (!fullpath || !volume) { + PyErr_SetString(PyExc_MemoryError, "out of memory"); + goto bail; + } + + if (!GetFullPathName(path, size, fullpath, NULL)) { + PyErr_SetFromWindowsErrWithFilename(GetLastError(), path); + goto bail; + } + + if (!GetVolumePathName(fullpath, volume, size)) { + PyErr_SetFromWindowsErrWithFilename(GetLastError(), path); + goto bail; + } + + /* Even though the documentation says SMB doesn't support volume management + * functions, passing an SMB path to GetVolumeInformation() returns 'NTFS'. + * So bail unless this is known to _not_ be a network drive. */ + switch (GetDriveType(volume)) { + case DRIVE_FIXED: + case DRIVE_REMOVABLE: + case DRIVE_CDROM: + case DRIVE_RAMDISK: + break; + + case DRIVE_REMOTE: + free(fullpath); + free(volume); + return Py_BuildValue("s", "cifs"); + + case DRIVE_UNKNOWN: + case DRIVE_NO_ROOT_DIR: + default: + free(fullpath); + free(volume); + Py_RETURN_NONE; + } + + if (!GetVolumeInformation(volume, NULL, 0, NULL, NULL, NULL, fstype, + sizeof(fstype))) { + PyErr_SetFromWindowsErrWithFilename(GetLastError(), volume); + goto bail; + } + + free(fullpath); + free(volume); + return Py_BuildValue("s", fstype); + +bail: + if (fullpath) + free(fullpath); + if (volume) + free(volume); + return NULL; +} #endif #ifdef __APPLE__ @@ -1307,13 +1385,13 @@ {"setprocname", (PyCFunction)setprocname, METH_VARARGS, "set process title (best-effort)\n"}, #endif -#if defined(HAVE_BSD_STATFS) || defined(HAVE_LINUX_STATFS) + {"unblocksignal", (PyCFunction)unblocksignal, METH_VARARGS, + "change signal mask to unblock a given signal\n"}, +#endif /* ndef _WIN32 */ +#if defined(HAVE_BSD_STATFS) || defined(HAVE_LINUX_STATFS) || defined(_WIN32) {"getfstype", (PyCFunction)getfstype, METH_VARARGS, "get filesystem type (best-effort)\n"}, #endif - {"unblocksignal", (PyCFunction)unblocksignal, METH_VARARGS, - "change signal mask to unblock a given signal\n"}, -#endif /* ndef _WIN32 */ #ifdef __APPLE__ { "isgui", (PyCFunction)isgui, METH_NOARGS, @@ -1323,7 +1401,7 @@ {NULL, NULL} }; -static const int version = 2; +static const int version = 3; #ifdef IS_PY3K static struct PyModuleDef osutil_module = { diff --git a/mercurial/policy.py b/mercurial/policy.py --- a/mercurial/policy.py +++ b/mercurial/policy.py @@ -74,7 +74,7 @@ (r'cext', r'bdiff'): 1, (r'cext', r'diffhelpers'): 1, (r'cext', r'mpatch'): 1, - (r'cext', r'osutil'): 2, + (r'cext', r'osutil'): 3, (r'cext', r'parsers'): 4, }