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.

Sign up for GitHub

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

Jump to bottom

_scandir extension doesn't compile on Solaris 11 #29

Closed
jc0n opened this issue Jul 4, 2014 · 16 comments
Closed

_scandir extension doesn't compile on Solaris 11 #29

jc0n opened this issue Jul 4, 2014 · 16 comments

Comments

@jc0n
Copy link

jc0n commented Jul 4, 2014

gcc -m64 -fno-strict-aliasing -g -O2 -DNDEBUG -g -O3 -Wall -Wstrict-prototypes -fPIC -I/usr/local/include/python2.7 -c _scandir.c -o build/temp.solaris-2.11-i86pc.64bit-2.7/_scandir.o
_scandir.c: In function '_fi_next':
_scandir.c:484:60: error: 'struct dirent' has no member named 'd_type'
_scandir.c:486:60: error: 'struct dirent' has no member named 'd_type'
_scandir.c:488:1: warning: control reaches end of non-void function
error: command 'gcc' failed with exit status 1

See http://stackoverflow.com/questions/2197918/cross-platform-way-of-testing-whether-a-file-is-a-directory
@jc0n jc0n changed the title _scandir extension doesn't compile on Solaris _scandir extension doesn't compile on Solaris 11 Jul 4, 2014
@benhoyt
Copy link
Owner

benhoyt commented Jul 5, 2014

Interesting. Well, scandir won't gain us much on Solaris in that case. Do you know of a compiler macro to test whether the dirent struct has d_type? If so, we can return d_type as DT_UNKNOWN.

@htoothrot
Copy link
Contributor

glibc defines _DIRENT_HAVE_D_TYPE, but there are other platforms that have d_type that don't define this (BSD/OS X).

Qt5 uses: #elif defined(_DIRENT_HAVE_D_TYPE) || defined(Q_OS_BSD4)
(https://github.com/qtproject/qtbase/blob/dev/src/corelib/io/qfilesystemengine.cpp#L333)

(I suppose you could also check for one of the other defs like DT_UNKNOWN being defined.)

@benhoyt
Copy link
Owner

benhoyt commented Jul 8, 2014

Hmm, I'm not sure the best combination of macros. Yeah, I think checking for DT_UNKNOWN makes a lot of sense -- I have no way of checking on Solaris, can you confirm DT_UNKNOWN is not defined on Solaris?

@htoothrot
Copy link
Contributor

I checked Solaris 11.1, DT_UNKNOWN is not defined. (I will keep the VM available for further testing.)

Do the python devs have anything to say about these kinds of checks being in the codebase?

@benhoyt
Copy link
Owner

benhoyt commented May 15, 2015

I've heavily changed _scandir.c to update it to the version of C code used in Python 3.5. Feel free to check if this is still an issue, otherwise I'll close.

@mverrilli
Copy link

Hi, I believe the issue is that Solaris does not support d_type in dirent. See this for an example of how to shim it. http://blueslugs.com/2010/06/13/adirent-ch-adding-d_type-to-struct-dirent-on-opensolaris/

Wouldn't scandir still be useful due to it being a generator (if I understand it properly) for those trying to read a huge amount of files in a single directory?

@benhoyt
Copy link
Owner

benhoyt commented May 17, 2015

Got it, thanks. _scandir.c already has a HAVE_DIRENT_D_TYPE macro that's defined on any non-Windows system (see here). Somewhat tellingly, I wrote this comment just above that, /* May need a better way of determining this */. :-)

In CPython that macro is set in the configure script via a small C program that uses dirent.d_type -- if the script compiles, it's assumed we have d_type and the macro is it. We can't really do this here (or at least I'd rather not).

Per the comment above, what about defining HAVE_DIRENT_D_TYPE if it's not Windows and DT_UNKNOWN is defined? Like so:

#if !defined(MS_WINDOWS) && defined(DT_UNKNOWN)
#define HAVE_DIRENT_D_TYPE 1
#endif

@mverrilli
Copy link

Yep, that will fix it. Was just looking at that bit of code and came here to ask about it.

@mverrilli
Copy link

Also, assuming that is the exact code change you make, it compiled and the tests were successful on SunOS 5.10 Generic_150400-14 sun4v sparc sun4v:

test_traversal (test_walk.TestWalk) ... ok
test_symlink_to_directory (test_walk.TestWalkSymlink) ... ok
test_symlink_to_file (test_walk.TestWalkSymlink) ... ok
test_basic (test_scandir.TestScandirGeneric) ... ok
test_bytes (test_scandir.TestScandirGeneric) ... ok
test_dir_entry (test_scandir.TestScandirGeneric) ... ok
test_file_attributes (test_scandir.TestScandirGeneric) ... skipped 'st_file_attributes not supported'
test_path (test_scandir.TestScandirGeneric) ... ok
test_returns_iter (test_scandir.TestScandirGeneric) ... ok
test_stat (test_scandir.TestScandirGeneric) ... ok  
test_symlink (test_scandir.TestScandirGeneric) ... ok
test_unicode (test_scandir.TestScandirGeneric) ... ok

----------------------------------------------------------------------
Ran 12 tests in 0.020s

@benhoyt
Copy link
Owner

benhoyt commented May 17, 2015

Thanks. It compiling now is good, but unfortunately it's not actually being used yet. I have too tight a test on sys.platform in scandir.py. Two questions:

  1. Could you please send me the value of sys.platform on this system? Is it 'sunos5'?
  2. Please change this line in scandir.py:
elif sys.platform.startswith(('linux', 'darwin')) or 'bsd' in sys.platform:

to include your SunOS system (either include 'sonos5' in the startswith list or just change it to elif True: for now) and rerun the tests and send the output? The ctypes version will not work as the dirent struct there is wrong (you may have to comment that out), but at least you should see test results for TestScandirC.

@mverrilli
Copy link

Ohh, Okie doke. I have to leave soon but I can get back on this later to try and help. Lots of errors.

>>> sys.platform
'sunos5'

and:

$ python test/run_tests.py
test_traversal (test_walk.TestWalk) ... FAIL
test_symlink_to_directory (test_walk.TestWalkSymlink) ... ERROR
test_symlink_to_file (test_walk.TestWalkSymlink) ... ERROR
test_basic (test_scandir.TestScandirGeneric) ... ok
test_bytes (test_scandir.TestScandirGeneric) ... ok
test_dir_entry (test_scandir.TestScandirGeneric) ... ok
test_file_attributes (test_scandir.TestScandirGeneric) ... skipped 'st_file_attributes not supported'
test_path (test_scandir.TestScandirGeneric) ... ok
test_returns_iter (test_scandir.TestScandirGeneric) ... ok
test_stat (test_scandir.TestScandirGeneric) ... ok
test_symlink (test_scandir.TestScandirGeneric) ... ok
test_unicode (test_scandir.TestScandirGeneric) ... ok
test_basic (test_scandir.TestScandirPython) ... ERROR
test_bytes (test_scandir.TestScandirPython) ... ERROR
test_dir_entry (test_scandir.TestScandirPython) ... ERROR
test_file_attributes (test_scandir.TestScandirPython) ... skipped 'st_file_attributes not supported'
test_path (test_scandir.TestScandirPython) ... ERROR
test_returns_iter (test_scandir.TestScandirPython) ... ERROR
test_stat (test_scandir.TestScandirPython) ... ERROR
test_symlink (test_scandir.TestScandirPython) ... ERROR
test_unicode (test_scandir.TestScandirPython) ... ERROR

======================================================================
ERROR: test_symlink_to_directory (test_walk.TestWalkSymlink)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/mv182q/scandir/x/scandir-master/test/test_walk.py", line 185, in test_symlink_to_directory
    dirs = sorted(output[0][1])
IndexError: list index out of range

======================================================================
ERROR: test_symlink_to_file (test_walk.TestWalkSymlink)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/mv182q/scandir/x/scandir-master/test/test_walk.py", line 160, in test_symlink_to_file
    dirs = sorted(output[0][1])
IndexError: list index out of range

======================================================================
ERROR: test_basic (test_scandir.TestScandirPython)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/mv182q/scandir/x/scandir-master/test/test_scandir.py", line 109, in test_basic
    entries = sorted(self.scandir_func(TEST_PATH), key=lambda e: e.name)
  File "/home/mv182q/scandir/x/scandir-master/test/../scandir.py", line 551, in scandir_python
    raise posix_error(path)
OSError: [Errno 0] Error 0: '/home/mv182q/scandir/x/scandir-master/test/testdir'

======================================================================
ERROR: test_bytes (test_scandir.TestScandirPython)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/mv182q/scandir/x/scandir-master/test/test_scandir.py", line 210, in test_bytes
    entries = [e for e in self.scandir_func(path) if e.name.startswith(b'unicod')]
  File "/home/mv182q/scandir/x/scandir-master/test/../scandir.py", line 551, in scandir_python
    raise posix_error(path)
OSError: [Errno 0] Error 0: '/home/mv182q/scandir/x/scandir-master/test/testdir/subdir'

======================================================================
ERROR: test_dir_entry (test_scandir.TestScandirPython)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/mv182q/scandir/x/scandir-master/test/test_scandir.py", line 117, in test_dir_entry
    entries = dict((e.name, e) for e in self.scandir_func(TEST_PATH))
  File "/home/mv182q/scandir/x/scandir-master/test/test_scandir.py", line 117, in <genexpr>
    entries = dict((e.name, e) for e in self.scandir_func(TEST_PATH))
  File "/home/mv182q/scandir/x/scandir-master/test/../scandir.py", line 551, in scandir_python
    raise posix_error(path)
OSError: [Errno 0] Error 0: '/home/mv182q/scandir/x/scandir-master/test/testdir'

======================================================================
ERROR: test_path (test_scandir.TestScandirPython)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/mv182q/scandir/x/scandir-master/test/test_scandir.py", line 169, in test_path
    entries = sorted(self.scandir_func(TEST_PATH), key=lambda e: e.name)
  File "/home/mv182q/scandir/x/scandir-master/test/../scandir.py", line 551, in scandir_python
    raise posix_error(path)
OSError: [Errno 0] Error 0: '/home/mv182q/scandir/x/scandir-master/test/testdir'

======================================================================
ERROR: test_returns_iter (test_scandir.TestScandirPython)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/mv182q/scandir/x/scandir-master/test/test_scandir.py", line 141, in test_returns_iter
    entry = next(it)
  File "/home/mv182q/scandir/x/scandir-master/test/../scandir.py", line 551, in scandir_python
    raise posix_error(path)
OSError: [Errno 0] Error 0: '/home/mv182q/scandir/x/scandir-master/test/testdir'

======================================================================
ERROR: test_stat (test_scandir.TestScandirPython)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/mv182q/scandir/x/scandir-master/test/test_scandir.py", line 129, in test_stat
    entries = list(self.scandir_func(TEST_PATH))
  File "/home/mv182q/scandir/x/scandir-master/test/../scandir.py", line 551, in scandir_python
    raise posix_error(path)
OSError: [Errno 0] Error 0: '/home/mv182q/scandir/x/scandir-master/test/testdir'

======================================================================
ERROR: test_symlink (test_scandir.TestScandirPython)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/mv182q/scandir/x/scandir-master/test/test_scandir.py", line 180, in test_symlink
    key=lambda e: e.name)
  File "/home/mv182q/scandir/x/scandir-master/test/../scandir.py", line 551, in scandir_python
    raise posix_error(path)
OSError: [Errno 0] Error 0: '/home/mv182q/scandir/x/scandir-master/test/testdir/linkdir'

======================================================================
ERROR: test_unicode (test_scandir.TestScandirPython)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/mv182q/scandir/x/scandir-master/test/test_scandir.py", line 228, in test_unicode
    entries = [e for e in self.scandir_func(path) if e.name.startswith('unicod')]
  File "/home/mv182q/scandir/x/scandir-master/test/../scandir.py", line 551, in scandir_python
    raise posix_error(path)
OSError: [Errno 0] Error 0: u'/home/mv182q/scandir/x/scandir-master/test/testdir/subdir'

======================================================================
FAIL: test_traversal (test_walk.TestWalk)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/mv182q/scandir/x/scandir-master/test/test_walk.py", line 60, in test_traversal
    self.assertEqual(len(all), 4)
AssertionError: 0 != 4

----------------------------------------------------------------------
Ran 21 tests in 0.019s

FAILED (failures=1, errors=10, skipped=2)

@benhoyt
Copy link
Owner

benhoyt commented May 17, 2015

Okay, thanks. scandir_python definitely won't work (as it expects d_type) so TestScandirPython will fail (actually, I kind of expected it to crash).

However, what I would have expected is to see TestScandirC in the test output. The fact that it's not means scandir.py can't import the C module. I expect that if you run Python and try import _scandir you'll get some kind of ImportError. Can you check and try do determine why?

In any case, I'm not sure whether it's worth spending too much time on Solaris/SunOS, as there's going to be no performance improvement for scandir without d_type. So maybe it's okay to just let it use scandir_generic?

@mverrilli
Copy link

Sorry I was in a hurry and didn't do the test right with _scandir. _scandir loads. Here is the test, all scandir_python fails on the readdir_r call.

$ python run_tests.py
test_traversal (test_walk.TestWalk) ... ok
test_symlink_to_directory (test_walk.TestWalkSymlink) ... ok
test_symlink_to_file (test_walk.TestWalkSymlink) ... ok
test_basic (test_scandir.TestScandirC) ... ok
test_bytes (test_scandir.TestScandirC) ... ok
test_dir_entry (test_scandir.TestScandirC) ... ok
test_file_attributes (test_scandir.TestScandirC) ... skipped 'st_file_attributes not supported'
test_path (test_scandir.TestScandirC) ... ok
test_returns_iter (test_scandir.TestScandirC) ... ok
test_stat (test_scandir.TestScandirC) ... ok
test_symlink (test_scandir.TestScandirC) ... ok
test_unicode (test_scandir.TestScandirC) ... ok
test_basic (test_scandir.TestScandirGeneric) ... ok
test_bytes (test_scandir.TestScandirGeneric) ... ok
test_dir_entry (test_scandir.TestScandirGeneric) ... ok
test_file_attributes (test_scandir.TestScandirGeneric) ... skipped 'st_file_attributes not supported'
test_path (test_scandir.TestScandirGeneric) ... ok
test_returns_iter (test_scandir.TestScandirGeneric) ... ok
test_stat (test_scandir.TestScandirGeneric) ... ok
test_symlink (test_scandir.TestScandirGeneric) ... ok
test_unicode (test_scandir.TestScandirGeneric) ... ok
test_basic (test_scandir.TestScandirPython) ... ERROR
test_bytes (test_scandir.TestScandirPython) ... ERROR
test_dir_entry (test_scandir.TestScandirPython) ... ERROR
test_file_attributes (test_scandir.TestScandirPython) ... skipped 'st_file_attributes not supported'
test_path (test_scandir.TestScandirPython) ... ERROR
test_returns_iter (test_scandir.TestScandirPython) ... ERROR
test_stat (test_scandir.TestScandirPython) ... ERROR
test_symlink (test_scandir.TestScandirPython) ... ERROR
test_unicode (test_scandir.TestScandirPython) ... ERROR

======================================================================
ERROR: test_basic (test_scandir.TestScandirPython)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/mv182q/scandir/x/scandir-master/test/test_scandir.py", line 109, in test_basic
    entries = sorted(self.scandir_func(TEST_PATH), key=lambda e: e.name)
  File "/home/mv182q/scandir/x/scandir-master/test/../scandir.py", line 551, in scandir_python
    raise posix_error(path)
OSError: [Errno 0] Error 0: '/home/mv182q/scandir/x/scandir-master/test/testdir'

======================================================================
ERROR: test_bytes (test_scandir.TestScandirPython)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/mv182q/scandir/x/scandir-master/test/test_scandir.py", line 210, in test_bytes
    entries = [e for e in self.scandir_func(path) if e.name.startswith(b'unicod')]
  File "/home/mv182q/scandir/x/scandir-master/test/../scandir.py", line 551, in scandir_python
    raise posix_error(path)
OSError: [Errno 0] Error 0: '/home/mv182q/scandir/x/scandir-master/test/testdir/subdir'

======================================================================
ERROR: test_dir_entry (test_scandir.TestScandirPython)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/mv182q/scandir/x/scandir-master/test/test_scandir.py", line 117, in test_dir_entry
    entries = dict((e.name, e) for e in self.scandir_func(TEST_PATH))
  File "/home/mv182q/scandir/x/scandir-master/test/test_scandir.py", line 117, in <genexpr>
    entries = dict((e.name, e) for e in self.scandir_func(TEST_PATH))
  File "/home/mv182q/scandir/x/scandir-master/test/../scandir.py", line 551, in scandir_python
    raise posix_error(path)
OSError: [Errno 0] Error 0: '/home/mv182q/scandir/x/scandir-master/test/testdir'

======================================================================
ERROR: test_path (test_scandir.TestScandirPython)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/mv182q/scandir/x/scandir-master/test/test_scandir.py", line 169, in test_path
    entries = sorted(self.scandir_func(TEST_PATH), key=lambda e: e.name)
  File "/home/mv182q/scandir/x/scandir-master/test/../scandir.py", line 551, in scandir_python
    raise posix_error(path)
OSError: [Errno 0] Error 0: '/home/mv182q/scandir/x/scandir-master/test/testdir'

======================================================================
ERROR: test_returns_iter (test_scandir.TestScandirPython)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/mv182q/scandir/x/scandir-master/test/test_scandir.py", line 141, in test_returns_iter
    entry = next(it)
  File "/home/mv182q/scandir/x/scandir-master/test/../scandir.py", line 551, in scandir_python
    raise posix_error(path)
OSError: [Errno 0] Error 0: '/home/mv182q/scandir/x/scandir-master/test/testdir'

======================================================================
ERROR: test_stat (test_scandir.TestScandirPython)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/mv182q/scandir/x/scandir-master/test/test_scandir.py", line 129, in test_stat
    entries = list(self.scandir_func(TEST_PATH))
  File "/home/mv182q/scandir/x/scandir-master/test/../scandir.py", line 551, in scandir_python
    raise posix_error(path)
OSError: [Errno 0] Error 0: '/home/mv182q/scandir/x/scandir-master/test/testdir'

======================================================================
ERROR: test_symlink (test_scandir.TestScandirPython)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/mv182q/scandir/x/scandir-master/test/test_scandir.py", line 180, in test_symlink
    key=lambda e: e.name)
  File "/home/mv182q/scandir/x/scandir-master/test/../scandir.py", line 551, in scandir_python
    raise posix_error(path)
OSError: [Errno 0] Error 0: '/home/mv182q/scandir/x/scandir-master/test/testdir/linkdir'

======================================================================
ERROR: test_unicode (test_scandir.TestScandirPython)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/mv182q/scandir/x/scandir-master/test/test_scandir.py", line 228, in test_unicode
    entries = [e for e in self.scandir_func(path) if e.name.startswith('unicod')]
  File "/home/mv182q/scandir/x/scandir-master/test/../scandir.py", line 551, in scandir_python
    raise posix_error(path)
OSError: [Errno 0] Error 0: u'/home/mv182q/scandir/x/scandir-master/test/testdir/subdir'

----------------------------------------------------------------------
Ran 30 tests in 0.025s

FAILED (errors=8, skipped=3)

@mverrilli
Copy link

Also with regards to performance gains... there is one big advantage to using scandir on Solaris and that is.. if you are trying to read a directory with millions of files in it. os.listdir will read the entire directory while scandir iterates and only calls getdents as needed. This is my very unfortunate situation.

I did a truss on this and confirmed it using _scandir vs listdir. I only saw a single stat call which I assume was making sure it was a directory or something that we were scanning. Looked pretty efficient to me. I know that if I had to recurse into directories, I would incur a stat call on every file... but that's no different than where I am already at today, so it is a draw on this part.

Do you agree? Or am I missing something else important? I'm just going to use _scandir for my purposes, but I'm happy to help get the scandir_python working if you want.

@benhoyt
Copy link
Owner

benhoyt commented May 18, 2015

Yeah, that's a good point. I'm glad TestScandirC works fine. I'll commit a fix so scandir_python isn't defined and TestScandirPython doesn't run (which is expected to be broken due to d_type missing).

benhoyt added a commit that referenced this issue May 19, 2015
@benhoyt
Enable C extension module to compile on systems without d_type (like …
612a61c 
…Solaris).

Don't try to use scandir_python on systems without d_type (Solaris).
#29
@benhoyt
Copy link
Owner

benhoyt commented May 19, 2015

Thanks, @mverrilli. I've just commited 612a61c which should fix this and allow _scandir.c to compile and scandir to use that and work on Solaris.

@benhoyt benhoyt closed this as completed May 19, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@htoothrot @jc0n @mverrilli @benhoyt