Skip to content

Conversation

Mytherin
Copy link
Collaborator

Follow-up fix from #16396 - thanks suvayu for all of the work on figuring this out.

@Mytherin Mytherin changed the title Align DUCKDB_API definition between duckdb.h and winapi.hpp Align DUCKDB_API definition between duckdb.h and winapi.hpp for MinGW Feb 25, 2025
@Mytherin
Copy link
Collaborator Author

I've launched a nightly test run here - https://github.com/Mytherin/duckdb/actions/runs/13518555521

@duckdb-draftbot duckdb-draftbot marked this pull request as draft February 25, 2025 11:30
@Mytherin
Copy link
Collaborator Author

Turns out the reason this changed is that DUCKDB_API cannot be used in MinGW for inlined functions - which happens in a number of places in the codebase. It seems to me like the correct fix here is to remove DUCKDB_API from those calls instead of disabling DUCKDB_API for the MinGW platform entirely. I'll give that a go.

@suvayu
Copy link

suvayu commented Feb 25, 2025

I was looking at the man page for x86_64-w64-mingw32-g++, I came across this:

   -fno-keep-inline-dllexport
       This is a more fine-grained version of -fkeep-inline-functions, which applies
       only to functions that are declared using the "dllexport" attribute or declspec.

   -fkeep-inline-functions
       In C, emit "static" functions that are declared "inline" into the object file,
       even if the function has been inlined into all of its callers.  This switch does
       not affect functions using the "extern inline" extension in GNU C90.  In C++,
       emit any and all inline functions into the object file.

Maybe adding one of these options to the compile commands of the C-API files with inlined exported functions is also another way to approach this?

@Mytherin Mytherin marked this pull request as ready for review February 25, 2025 16:02
@duckdb-draftbot duckdb-draftbot marked this pull request as draft February 25, 2025 16:27
@Mytherin Mytherin marked this pull request as ready for review February 25, 2025 17:24
@Mytherin Mytherin changed the title Align DUCKDB_API definition between duckdb.h and winapi.hpp for MinGW Rename DUCKDB_API to DUCKDB_C_API for duckdb.h Feb 25, 2025
@Mytherin
Copy link
Collaborator Author

After struggling with this for a bit I've solved it in a different manner - by switching the name of the definition in duckdb.h to DUCKDB_C_API

Copy link

@suvayu suvayu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't realise until now the two macros are meant for different things! This seems like a reasonable solution to me (in my non-expert opinion 😃).

I tested this PR with my build script, followed by a check of the symbols, everything is fixed.

@Mytherin Mytherin merged commit b6d9eab into duckdb:v1.2-histrionicus Feb 26, 2025
50 checks passed
krlmlr added a commit to duckdb/duckdb-r that referenced this pull request Mar 7, 2025
Rename `DUCKDB_API` to `DUCKDB_C_API` for `duckdb.h` (duckdb/duckdb#16397)
@Mytherin Mytherin deleted the mingwdeclspec branch April 2, 2025 09:24
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 this pull request may close these issues.

2 participants