Patchwork [1,of,3] setup: use a more strict way to test BSD or OSX's statfs

login
register
mail settings
Submitter Jun Wu
Date March 24, 2017, 5:32 a.m.
Message ID <597a29c947fe2b9f9ac0.1490333572@localhost.localdomain>
Download mbox | patch
Permalink /patch/19622/
State Accepted
Headers show

Comments

Jun Wu - March 24, 2017, 5:32 a.m.
# HG changeset patch
# User Jun Wu <quark@fb.com>
# Date 1490332536 25200
#      Thu Mar 23 22:15:36 2017 -0700
# Node ID 597a29c947fe2b9f9ac0a6a03cf710ab9f69757c
# Parent  2c02bb7fd7fc1212029dc903527e35a9efb7dbe1
# Available At https://bitbucket.org/quark-zju/hg-draft
#              hg pull https://bitbucket.org/quark-zju/hg-draft -r 597a29c947fe
setup: use a more strict way to test BSD or OSX's statfs

We want to use the `f_fstypename` field to get the filesystem type. Test it
directly. The new macro HAVE_BSD_STATFS implys the old HAVE_SYS_MOUNT_H and
HAVE_SYS_PARAM_H. So the latter ones are removed.
David Soria Parra - March 24, 2017, 5:01 p.m.
On Thu, Mar 23, 2017 at 10:32:52PM -0700, Jun Wu wrote:
> # HG changeset patch
> # User Jun Wu <quark@fb.com>
> # Date 1490332536 25200
> #      Thu Mar 23 22:15:36 2017 -0700
> # Node ID 597a29c947fe2b9f9ac0a6a03cf710ab9f69757c
> # Parent  2c02bb7fd7fc1212029dc903527e35a9efb7dbe1
> # Available At https://bitbucket.org/quark-zju/hg-draft
> #              hg pull https://bitbucket.org/quark-zju/hg-draft -r 597a29c947fe
> setup: use a more strict way to test BSD or OSX's statfs
> 
> We want to use the `f_fstypename` field to get the filesystem type. Test it
> directly. The new macro HAVE_BSD_STATFS implys the old HAVE_SYS_MOUNT_H and
> HAVE_SYS_PARAM_H. So the latter ones are removed.
> 

I think the series is good. I wonder if it makes sense to have a more generic
hasmember() function in setup.py. I have a patch for that as I was working on
ZFS and UFS detection myself.
Jun Wu - March 24, 2017, 5:05 p.m.
I think "cancompile" is cleaner - you can check headers, functions, fields
together.

The current code will actually break if the system has "statfs" but does not
have a "struct statfs" in the header files we checked. So I plan to make the
linux statfs check explicit, by removing checking for "statfs" function,
linux headers, and adding a "cancompile" check for Linux.

Excerpts from David Soria Parra's message of 2017-03-24 10:01:14 -0700:
> On Thu, Mar 23, 2017 at 10:32:52PM -0700, Jun Wu wrote:
> > # HG changeset patch
> > # User Jun Wu <quark@fb.com>
> > # Date 1490332536 25200
> > #      Thu Mar 23 22:15:36 2017 -0700
> > # Node ID 597a29c947fe2b9f9ac0a6a03cf710ab9f69757c
> > # Parent  2c02bb7fd7fc1212029dc903527e35a9efb7dbe1
> > # Available At https://bitbucket.org/quark-zju/hg-draft 
> > #              hg pull https://bitbucket.org/quark-zju/hg-draft  -r 597a29c947fe
> > setup: use a more strict way to test BSD or OSX's statfs
> > 
> > We want to use the `f_fstypename` field to get the filesystem type. Test it
> > directly. The new macro HAVE_BSD_STATFS implys the old HAVE_SYS_MOUNT_H and
> > HAVE_SYS_PARAM_H. So the latter ones are removed.
> > 
> 
> I think the series is good. I wonder if it makes sense to have a more generic
> hasmember() function in setup.py. I have a patch for that as I was working on
> ZFS and UFS detection myself.

Patch

diff --git a/setup.py b/setup.py
--- a/setup.py
+++ b/setup.py
@@ -597,6 +597,4 @@  for plat, func in [('bsd', 'setproctitle
 
 for plat, header in [
-    ('bsd|darwin|linux', 'sys/mount.h'),
-    ('bsd|darwin|linux', 'sys/param.h'),
     ('linux', 'linux/magic.h'),
     ('linux', 'sys/vfs.h'),
@@ -606,4 +604,14 @@  for plat, header in [
         osutil_cflags.append('-DHAVE_%s' % macro)
 
+for plat, macro, code in [
+    ('bsd|darwin', 'BSD_STATFS', '''
+     #include <sys/param.h>
+     #include <sys/mount.h>
+     int main() { struct statfs s; return sizeof(s.f_fstypename); }
+     '''),
+]:
+    if re.search(plat, sys.platform) and cancompile(new_compiler(), code):
+        osutil_cflags.append('-DHAVE_%s' % macro)
+
 if sys.platform == 'darwin':
     osutil_ldflags += ['-framework', 'ApplicationServices']