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.

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

Compilation problem on Mac OS / clang #89

Closed
jorisvandenbossche opened this issue Jan 20, 2020 · 3 comments · Fixed by #91
Closed

Compilation problem on Mac OS / clang #89

jorisvandenbossche opened this issue Jan 20, 2020 · 3 comments · Fixed by #91

Comments

@jorisvandenbossche
Copy link
Member

See #83

With the current codebase, @sgillies and @vincentsarago report they get the following error:

duplicate symbol '_STRtreeType' in:
    build/temp.macosx-10.14-x86_64-3.7/src/lib.o
    build/temp.macosx-10.14-x86_64-3.7/src/strtree.o
ld: 1 duplicate symbol for architecture x86_64
clang: error: linker command failed with exit code 1 (use -v to see invocation)
error: command 'clang' failed with exit status 1

Removing an #include "strtree.h" solves this, but then results in some warnings (see #83). @sgillies suggests that the better solution would be to have one source file per extension module.

@jorisvandenbossche
Copy link
Member Author

I think the single source file is really the way to go.

@sgillies I think the main reason we split it up in several files, is for code organisation. It would become a really huge single C file if we put everything together. Are there no other solutions to this?

Or, we need to have multiple extension modules if we want to keep sources splitted?

@caspervdw
Copy link
Member

I am against splitting the source files. This will result in a huge file which is just hard to maintain. We just have to get the headers and includes right. I hope #91 fixes the issue.

@sgillies
Copy link

sgillies commented Jan 25, 2020

@caspervdw I agree about the benefits of small C source files. There is nothing wrong with this from a Python C extension standpoint.

As I see it, the root of the problem is src/lib.c and the way it includes all the other modules. If we want to keep small C modules, I think we must consider changing from the current state of one single extension module that has multiple sources (see https://github.com/pygeos/pygeos/blob/master/setup.py#L110-L114)

module_lib = Extension(
    "pygeos.lib",
    sources=["src/lib.c", "src/geos.c", "src/pygeom.c", "src/ufuncs.c", "src/coords.c", "src/strtree.c"],
    **get_geos_paths()
)

to multiple extension modules with a single source each:

module_libs = [
    Extension("pygeos._lib", sources=["src/lib.c"]),
    Extension("pygeos._geos", sources=["src/geos.c"]),
    Extension("pygeos._pygeom", sources=["src/pygeom.c"],
    ... # etc.
]

In my experience, this is the most "normal" way to make a package of extension modules, and makes simple C builds possible across platforms. In other words, I've found it to be the "happy path".

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants