Skip to content

Conversation

@ihnorton
Copy link
Member

  • add -I ${prefix_dir}/include so that internal includes can be found

  • add -x c++-header to silence warning

    warning: treating 'c-header' input as 'c++-header' when in C++ mode, this behavior is deprecated
    
  • add || true to some rm calls so they don't fail if the output has
    previously been removed

  • x-ref class CAPIHandle and start of C API restructuring TileDB#3306

- add `-I ${prefix_dir}/include` so that internal includes can be found
- add `-x c++-header` to silence warning
    ```
    warning: treating 'c-header' input as 'c++-header' when in C++ mode, this behavior is deprecated
    ```
- add `|| true` to some rm calls so they don't fail if the output has
  previously been removed

- x-ref TileDB-Inc/TileDB#3306
Copy link
Contributor

@DimitrisStaratzis DimitrisStaratzis left a comment

Choose a reason for hiding this comment

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

Thanks for the PR. The suggested changes work as expected but they fail to generate the FILTER constants . I am working on a fix.

Due to SWIG limitations it appears that we need to add swig typedefs for any tiledb_*_handle_t typedef using the new CAPIHandle infrastructure.
@ihnorton
Copy link
Member Author

ihnorton commented Aug 4, 2022

TileDB-Inc/TileDB#3414 moves a non-TileDB #include from the common.h file to tiledb.h so that it can be filtered out in the preprocessing steps. Otherwise SWIG generates wrappers for internal structs from stdint.h that we don't want (eg __darwin_mbstate_t).

Copy link
Contributor

@DimitrisStaratzis DimitrisStaratzis left a comment

Choose a reason for hiding this comment

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

Works as expected. Thank you! I will merge when 2.12.0 is out!

@DimitrisStaratzis DimitrisStaratzis merged commit 1ec9801 into master Oct 14, 2022
@DimitrisStaratzis DimitrisStaratzis deleted the ihn/fix-capihandle-swig branch October 14, 2022 10:16
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.

3 participants