Fix some ctypes errors #507
Conversation
|
If I'm correct this attempts to address the trac issue 3384. |
Yes. Sorry I forgot to mention it. |
|
For me, on mac with clang, only the addition of It would be good if you could see what flags, or combinations thereof, causes actual improvements for you. |
|
Might it be an idea to sync this to upstream? https://github.com/davidjamesca/ctypesgen/blob/master/ctypesgen/parser/preprocessor.py#L110 |
I think so, when we found a good solution for this. It seems to me different platforms need different defines. |
|
I have a very similar problem with Gentoo, by the way. No need to be on Mac to see this kind of errors. |
|
@lbartoletti no, it doesn't. I still get stuff like : |
|
Adding |
|
It is my understanding these errors show on all platforms in various degree (see e.g. build logs on https://github.com/OSGeo/grass/actions), but they are all different and seemingly need a separate set of flags. |
|
It sounds like you need to figure out what C or C++ standard the code is following, and stick with that in the compiler flags. The code is too exotic for its own good, and too reliant on special features. |
This is unrelated issue. Upgrade/downgrade your Python. No such problem with python 3.7.9 on ~AMD64. |
|
I'm using Python 3.7.7. You are saying this is an issue with specific versions of python ? Do you have a bug report to point to ? @marisn |
|
So, I tried with Python 3.8.2, and I get the exact same |
Sorry, my bad. I'm getting old and my memory isn't perfect any more. It is bug #435 Check for any extra proj .so files in your system. |
|
Thanks. That was it. I somehow had both PROJ 7.0.1 and PROJ 6.3.2... |
|
The I did some further research into this. I do not think the "official" version of ctypesgen has changed that much in regard to this issue. Seems to me, it remains to tackle this through flags suited for different platforms. With a simplified quantification of builds of the ctypes module with following code: cd [grass-dir]
./configure [...]
make
# go to ctypes dir and remove compiled files
cd lib/python/ctypes
rm -rf OBJ*
# make changes in source- and/or make-files and compile ctypes module again
make &> make.log
# count number of lines starting with "Error:"
cat make.log | grep "^Error:*" | wc -lI could reduce number of lines starting with "Error:" from 24547 to 551! Maybe changes along the following could be a way to address this (note I'm not sure how to correctly identify BDS systems): diff --git a/lib/python/ctypes/Makefile b/lib/python/ctypes/Makefile
index f30c84011..9b1b88f8b 100644
--- a/lib/python/ctypes/Makefile
+++ b/lib/python/ctypes/Makefile
@@ -56,14 +56,18 @@ proj_INC = $(PROJINC)
vector_INC = $(VECT_INC) $(VECT_CFLAGS)
vedit_INC = $(VECT_INC) $(VECT_CFLAGS)
-MAC_FLAGS = ""
+CTYPE_CFLAGS = -D__GLIBC_HAVE_LONG_LONG
ifneq ($(findstring darwin,$(ARCH)),)
-MAC_FLAGS = "-D_Nullable="
+CTYPE_CFLAGS += -D_Nullable= \"-D__attribute__(x)=\" -Drestrict= -D_Nonnull= -Dint8_t=char -DCF_INLINE= -D_Null_unspecified= -D__DARWIN_OS_INLINE= -DUC_INLINE=
+else
+ifneq ($(findstring freebsd,$(ARCH)),)
+CTYPE_CFLAGS += \"-D__attribute__(x)=\" \"-D__aligned(x)=\" -D_Noreturn= -D__GNUCLIKE_BUILTIN_VARARGS -D__GNUCLIKE_BUILTIN_STDARG
+endif
endif
SED = sed
CTYPESGEN = ./ctypesgen.py
-CTYPESFLAGS = --cpp "$(CC) -E $(CPPFLAGS) $(LFS_CFLAGS) $(MAC_FLAGS) $(EXTRA_CFLAGS) $(NLS_CFLAGS) $(DEFS) $(EXTRA_INC) $(INC) -D__GLIBC_HAVE_LONG_LONG"
+CTYPESFLAGS = --cpp "$(CC) -E $(CPPFLAGS) $(LFS_CFLAGS) $(CTYPE_CFLAGS) $(EXTRA_CFLAGS) $(NLS_CFLAGS) $(DEFS) $(EXTRA_INC) $(INC)"
EXTRA_CLEAN_FILES := $(wildcard ctypesgencore/*.pyc) $(wildcard ctypesgencore/*/*.pyc)
ifneq ($(MINGW),)
|
There was a problem hiding this comment.
@lbartoletti
The ctypesgen package in main branch has been updated to community version of https://github.com/ctypesgen/ctypesgen. The patches needed for this to work with GRASS at all are listed in https://github.com/OSGeo/grass/blob/main/python/libgrass_interface_generator/README.md.
I believe __asm__ and __attribute__ definitions are now already dealt with. Please check out a build log from main branch.
I'm now actively contributing to incorporate these patches (in some way or another) upstreams. Ideally, real bugs (not annoying warnings) should be addressed over there [update: and should be patched in GRASS until it's fixed upstreams].
To silence some of these warnings, however, is best achieved through the Makefile, and preferably conditioned by platform.
|
The relevant code is now in |
@lbartoletti would you mind to rebase your PR? |
lbartoletti
force-pushed
the
fix_ctypes_bsd
branch
from
654a5da to
a7a3aed
Compare
There was a problem hiding this comment.
The change looks okay, although I'm unsure about the bool there.
A ctypes related tests fails on Ubuntu, so that's suspicious (I didn't try to restart it). The Windows test fails, but perhaps for some unrelated reason. Maybe rebase/merge will help there.
There is a conflict with main (again, sorry) and it seems some things are now already fixed one main.
|
If there are any "defines" that are general and/or fixes real bugs they should be addressed to upstream ctypesgen. Otherwise I'm reluctant to add them at this point just to silence build log “Errors”, which are just failure to parse (mostly) non-standard C syntax (e.g. GNU C extensions). Some notes:
Side note: it is not necessary to modify the ctypesgen source code (i.e. preprocessor.py) to add any “define”, the same is achieved by adding to In short: even though I really appreciate your effort @lbartoletti, I think this PR should be closed if none of the additions fixes real bugs (as the case was with e.g. #456). |
Yes, this PR tends to reduce noise, but it was only a draft and I never succeeded to correct the xxintrin.h errors |
I have been working for a while on a solution for parsing attributes and similar causes of error message. It's not easy, to say the least... |
This PR reduces error messages on BSD-like systems (FreeBSD and MacOs).
I am still stuck on xxintrin.h.
Only tested on my local machine (FreeBSD 12 amd64).
Still as a WIP for now.