benhoyt / scandir

Better directory iterator and faster os.walk(), now in the Python 3.5 stdlib
https://benhoyt.com/writings/scandir/
BSD 3-Clause "New" or "Revised" License
532 stars 68 forks source link

Compile error, "storage size of 'st' isn't known, Python 3.11, gcc 12.2.0. #141

Open ssteinerx opened 1 year ago

ssteinerx commented 1 year ago

NOTE: I'm really only wanting to extract the DirEntry struct for use in other C code.

$ gcc --version
gcc (Ubuntu 12.2.0-3ubuntu1) 12.2.0
Copyright (C) 2022 Free Software Foundation, Inc.

$ python setup.py build
running build
running build_py
running build_ext
building '_scandir' extension
gcc -Wsign-compare -DNDEBUG -g -fwrapv -O3 -Wall -fPIC -I/usr/local/include/python3.11 -c _scandir.c -o build/temp.linux-x86_64-cpython-311/_scandir.o
_scandir.c: In function ‘DirEntry_fetch_stat’:
_scandir.c:1018:28: error: storage size of ‘st’ isn’t known
 1018 |     struct _Py_stat_struct st;
      |                            ^~
_scandir.c:1018:28: warning: unused variable ‘st’ [-Wunused-variable]
_scandir.c:1070:1: warning: control reaches end of non-void function [-Wreturn-type]
 1070 | }
      | ^
/Caviar/WSSW-Projects/dirulator_django/submodules/scandir/setup.py:46: DeprecationWarning: The 'warn' function is deprecated, use 'warning' instead
  logging.warn("building %s failed with %s: %s", ext.name, info[0], info[1])
building _scandir failed with <class 'distutils.errors.CompileError'>: command '/usr/bin/gcc' failed with exit code 1

Python version in header is 3.11, _Py_stat_struct is defined in fileutils.h as it should be.

This use on line 1018 seems to be the only place _Py_stat_struct is used where it has to be allocated on the stack so the size must be known.

This really seems like a gcc bug to me; the compiler should be doing its pre-processor substitution long before worrying about the size of the struct it needs to allocate.

VSCode even offers to "inline macro" which produces the correct struct stat st; substitution shown below.

Filing a gcc bug with this complicated (no matter how obvious) test case is pointless and it'd probably disappear in a simpler case.

All I really want to do is use DirEntry in some other C code and I was starting by building this version before going in for surgery.

Replacing it with a direct struct stat "fixes" the problem (by avoiding it, will only work on *nix):

// On line 1018
    // struct _Py_stat_struct st;
    struct stat st;
benhoyt commented 1 year ago

Thanks for the report. It looks like in the main scandir code in the CPython repo this is now handled with a preprocessor if/else. See lines 491-501 here:

#ifdef MS_WINDOWS
#  define STAT win32_stat
#  define LSTAT win32_lstat
#  define FSTAT _Py_fstat_noraise
#  define STRUCT_STAT struct _Py_stat_struct
#else
#  define STAT stat
#  define LSTAT lstat
#  define FSTAT fstat
#  define STRUCT_STAT struct stat
#endif

You'd be welcome to submit a PR if you'd like. Seeing scandir is now included in Python 3.x itself, I personally don't have time/motivation to work on this repo much anymore. Either way, I'll leave this open for visibility.

cielavenir commented 1 year ago

I can see that MS_WINDOWS's _Py_stat_struct definition can be seen from python source code, not 3rdparty modules, so maybe redefining is better

cielavenir commented 1 year ago

attempted. (our company started Py3 migration this year~~)

benhoyt commented 1 year ago

@cielavenir, thanks for the PR, and I'll take a look at that separately. However, wouldn't a simpler way to address this in your Python 3 codebase be to just use scandir from the Python 3 stdlib? (It's been in the stdlib since as os.scandir since Python 3.5.)

If you still need Python 2 and Python 3 compatibility, you can do:

try:
    from os import scandir
except ImportError:
    from scandir import scandir   # fallback for Python < 3.5