-
Notifications
You must be signed in to change notification settings - Fork 20
Convert to C-API based extension #20
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
Merged
Merged
Changes from all commits
Commits
Show all changes
15 commits
Select commit
Hold shift + click to select a range
95c8c51
move to c-based extension (wip, still pending changes to the c-api up…
Maxxen 3e81001
cleanup symbols
Maxxen 99a05fc
fix warning, remove unused headers
Maxxen c7d9873
format
Maxxen 751936d
fixup headers
Maxxen b8c3f8d
add subnet overloads
Maxxen da7da7f
cleanup and add workflow
Maxxen ebeb6f8
make constant a define, format
Maxxen a1c256d
remove unused vcpkg
Maxxen 8545374
add deploy step
Maxxen 2963627
probe windows failure
Maxxen 584bbd0
dont use unicode literals
Maxxen f794aa4
remove another unicode literal
Maxxen dc1505d
skip windows test for now
Maxxen 93d416c
remove deploy for now
Maxxen File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,33 @@ | ||
| --- | ||
| BasedOnStyle: LLVM | ||
| SortIncludes: false | ||
| TabWidth: 4 | ||
| IndentWidth: 4 | ||
| ColumnLimit: 120 | ||
| AllowShortFunctionsOnASingleLine: false | ||
| --- | ||
| UseTab: ForIndentation | ||
| DerivePointerAlignment: false | ||
| PointerAlignment: Right | ||
| AlignConsecutiveMacros: true | ||
| AlignTrailingComments: true | ||
| AllowAllArgumentsOnNextLine: true | ||
| AllowAllConstructorInitializersOnNextLine: true | ||
| AllowAllParametersOfDeclarationOnNextLine: true | ||
| AlignAfterOpenBracket: Align | ||
| SpaceBeforeCpp11BracedList: true | ||
| SpaceBeforeCtorInitializerColon: true | ||
| SpaceBeforeInheritanceColon: true | ||
| SpacesInAngles: false | ||
| SpacesInCStyleCastParentheses: false | ||
| SpacesInConditionalStatement: false | ||
| AllowShortLambdasOnASingleLine: Inline | ||
| AllowShortLoopsOnASingleLine: false | ||
| AlwaysBreakTemplateDeclarations: Yes | ||
| IncludeBlocks: Regroup | ||
| Language: Cpp | ||
| AccessModifierOffset: -4 | ||
| --- | ||
| Language: Java | ||
| SpaceAfterCStyleCast: true | ||
| --- |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,55 @@ | ||
| Checks: '-*,clang-diagnostic-*,bugprone-*,performance-*,google-explicit-constructor,google-build-using-namespace,google-runtime-int,misc-definitions-in-headers,modernize-use-nullptr,modernize-use-override,-bugprone-macro-parentheses,readability-braces-around-statements,-bugprone-branch-clone,readability-identifier-naming,hicpp-exception-baseclass,misc-throw-by-value-catch-by-reference,-bugprone-signed-char-misuse,-bugprone-misplaced-widening-cast,-bugprone-sizeof-expression,-bugprone-easily-swappable-parameters,google-global-names-in-headers,llvm-header-guard,misc-definitions-in-headers,modernize-use-emplace,modernize-use-bool-literals,-performance-inefficient-string-concatenation,-performance-no-int-to-ptr,readability-container-size-empty,cppcoreguidelines-pro-type-cstyle-cast,-llvm-header-guard,-performance-enum-size,cppcoreguidelines-pro-type-const-cast,cppcoreguidelines-avoid-non-const-global-variables,cppcoreguidelines-interfaces-global-init,cppcoreguidelines-slicing,cppcoreguidelines-rvalue-reference-param-not-moved,cppcoreguidelines-virtual-class-destructor,-readability-identifier-naming,-bugprone-exception-escape,-bugprone-unused-local-non-trivial-variable,-bugprone-empty-catch,-misc-use-internal-linkage,-readability-static-definition-in-anonymous-namespace' | ||
| WarningsAsErrors: '*' | ||
| HeaderFilterRegex: 'src/include/duckdb/.*' | ||
| FormatStyle: none | ||
| CheckOptions: | ||
| - key: readability-identifier-naming.ClassCase | ||
| value: CamelCase | ||
| - key: readability-identifier-naming.EnumCase | ||
| value: CamelCase | ||
| - key: readability-identifier-naming.TypedefCase | ||
| value: lower_case | ||
| - key: readability-identifier-naming.TypedefSuffix | ||
| value: _t | ||
| - key: readability-identifier-naming.FunctionCase | ||
| value: CamelCase | ||
| - key: readability-identifier-naming.MemberCase | ||
| value: lower_case | ||
| - key: readability-identifier-naming.ParameterCase | ||
| value: lower_case | ||
| - key: readability-identifier-naming.ConstantCase | ||
| value: aNy_CasE | ||
| - key: readability-identifier-naming.ConstantParameterCase | ||
| value: lower_case | ||
| - key: readability-identifier-naming.NamespaceCase | ||
| value: lower_case | ||
| - key: readability-identifier-naming.MacroDefinitionCase | ||
| value: UPPER_CASE | ||
| - key: readability-identifier-naming.StaticConstantCase | ||
| value: UPPER_CASE | ||
| - key: readability-identifier-naming.ConstantMemberCase | ||
| value: aNy_CasE | ||
| - key: readability-identifier-naming.StaticVariableCase | ||
| value: UPPER_CASE | ||
| - key: readability-identifier-naming.ClassConstantCase | ||
| value: UPPER_CASE | ||
| - key: readability-identifier-naming.EnumConstantCase | ||
| value: UPPER_CASE | ||
| - key: readability-identifier-naming.ConstexprVariableCase | ||
| value: aNy_CasE | ||
| - key: readability-identifier-naming.StaticConstantCase | ||
| value: UPPER_CASE | ||
| - key: readability-identifier-naming.TemplateTemplateParameterCase | ||
| value: UPPER_CASE | ||
| - key: readability-identifier-naming.TypeTemplateParameterCase | ||
| value: UPPER_CASE | ||
| - key: readability-identifier-naming.VariableCase | ||
| value: lower_case | ||
| - key: modernize-use-emplace.SmartPointers | ||
| value: '::duckdb::shared_ptr;::duckdb::unique_ptr;::std::auto_ptr;::duckdb::weak_ptr' | ||
| - key: cppcoreguidelines-rvalue-reference-param-not-moved.IgnoreUnnamedParams | ||
| value: true | ||
| - key: misc-use-internal-linkage | ||
| value: true | ||
| - key: readability-static-definition-in-anonymous-namespace | ||
| value: true |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,2 +1,7 @@ | ||
| build/ | ||
| duckdb_unittest_tempdir/ | ||
| cmake_build | ||
| build | ||
| configure | ||
| .idea | ||
| duckdb_unittest_tempdir | ||
| /test/bin | ||
| venv |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,6 +1,3 @@ | ||
| [submodule "duckdb"] | ||
| path = duckdb | ||
| url = https://github.com/duckdb/duckdb.git | ||
| [submodule "extension-ci-tools"] | ||
| path = extension-ci-tools | ||
| url = https://github.com/duckdb/extension-ci-tools.git |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,21 +1,58 @@ | ||
| cmake_minimum_required(VERSION 3.5) | ||
| cmake_minimum_required(VERSION 3.5...3.29) | ||
| option(DUCKDB_WASM_EXTENSION "Whether compiling for Wasm target" OFF) | ||
|
|
||
| set(TARGET_NAME inet) | ||
| set(EXTENSION_NAME inet) | ||
| set(DUCKDB_EXTENSION_API_VERSION_UNSTABLE 1) | ||
| ### | ||
| # Configuration | ||
| ### | ||
| if(NOT DEFINED EXTENSION_NAME) | ||
| message(FATAL_ERROR "DuckDB extension name is required") | ||
| endif() | ||
| if (DEFINED TARGET_DUCKDB_VERSION_MAJOR) | ||
| add_definitions(-DDUCKDB_EXTENSION_API_VERSION_MAJOR=${TARGET_DUCKDB_VERSION_MAJOR}) | ||
| endif() | ||
| if (DEFINED TARGET_DUCKDB_VERSION_MINOR) | ||
| add_definitions(-DDUCKDB_EXTENSION_API_VERSION_MINOR=${TARGET_DUCKDB_VERSION_MINOR}) | ||
| endif() | ||
| if (DEFINED TARGET_DUCKDB_VERSION_PATCH) | ||
| add_definitions(-DDUCKDB_EXTENSION_API_VERSION_PATCH=${TARGET_DUCKDB_VERSION_PATCH}) | ||
| endif() | ||
|
|
||
| set(EXTENSION_NAME ${TARGET_NAME}_extension) | ||
| set(LOADABLE_EXTENSION_NAME ${TARGET_NAME}_loadable_extension) | ||
| add_definitions(-DDUCKDB_EXTENSION_NAME=${EXTENSION_NAME}) | ||
|
|
||
| project(${TARGET_NAME}) | ||
| include_directories(src/include) | ||
| if (DEFINED DUCKDB_EXTENSION_API_VERSION_UNSTABLE) | ||
| add_definitions(-DDUCKDB_EXTENSION_API_VERSION_UNSTABLE=${DUCKDB_EXTENSION_API_VERSION_UNSTABLE}) | ||
| endif() | ||
|
|
||
| set(EXTENSION_SOURCES src/inet_extension.cpp src/inet_functions.cpp | ||
| src/inet_escape_functions.cpp src/ipaddress.cpp) | ||
| ### | ||
| # Build | ||
| ### | ||
| project(${EXTENSION_NAME} LANGUAGES C) | ||
|
|
||
| build_static_extension(${TARGET_NAME} ${EXTENSION_SOURCES}) | ||
| build_loadable_extension(${TARGET_NAME} " " ${EXTENSION_SOURCES}) | ||
| # Create Extension library | ||
| set(EXTENSION_SOURCES | ||
| src/inet_extension.c | ||
| src/inet_html.c | ||
| src/inet_html_table.c | ||
| src/inet_ipaddress.c | ||
| ) | ||
|
|
||
| install( | ||
| TARGETS ${EXTENSION_NAME} | ||
| EXPORT "${DUCKDB_EXPORT_SET}" | ||
| LIBRARY DESTINATION "${INSTALL_LIB_DIR}" | ||
| ARCHIVE DESTINATION "${INSTALL_LIB_DIR}") | ||
| if (DUCKDB_WASM_EXTENSION) | ||
| add_library(${EXTENSION_NAME} STATIC ${EXTENSION_SOURCES}) | ||
| else() | ||
| add_library(${EXTENSION_NAME} SHARED ${EXTENSION_SOURCES}) | ||
| endif() | ||
|
|
||
| # Hide symbols | ||
| set(CMAKE_CXX_VISIBILITY_PRESET hidden) | ||
| set(CMAKE_C_VISIBILITY_PRESET hidden) | ||
| set(VISIBILITY_INLINES_HIDDEN ON) | ||
| set(CMAKE_C_FLAGS_RELEASE "${CMAKE_C_FLAGS_RELEASE} -s") | ||
| set(CMAKE_CXX_FLAGS_RELEASE "${CMAKE_CXX_FLAGS_RELEASE} -s") | ||
|
|
||
| # Include own headers | ||
| target_include_directories(${EXTENSION_NAME} PRIVATE src) | ||
|
|
||
| # Include DuckDB C API headers | ||
| target_include_directories(${EXTENSION_NAME} PRIVATE duckdb_capi) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,8 +1,32 @@ | ||
| .PHONY: clean clean_all | ||
|
|
||
| PROJ_DIR := $(dir $(abspath $(lastword $(MAKEFILE_LIST)))) | ||
|
|
||
| # Configuration of extension | ||
| EXT_NAME=inet | ||
| EXT_CONFIG=${PROJ_DIR}extension_config.cmake | ||
| # Main extension configuration | ||
| EXTENSION_NAME=inet | ||
|
|
||
| # Set to 1 to enable Unstable API (binaries will only work on TARGET_DUCKDB_VERSION, forwards compatibility will be broken) | ||
| # WARNING: When set to 1, the duckdb_extension.h from the TARGET_DUCKDB_VERSION must be used, using any other version of | ||
| # the header is unsafe. | ||
| USE_UNSTABLE_C_API=1 | ||
|
|
||
| # The DuckDB version to target | ||
| TARGET_DUCKDB_VERSION=d52dd4e3df | ||
|
|
||
| all: configure release | ||
|
|
||
| # Include makefiles from DuckDB | ||
| include extension-ci-tools/makefiles/c_api_extensions/base.Makefile | ||
| include extension-ci-tools/makefiles/c_api_extensions/c_cpp.Makefile | ||
|
|
||
| configure: venv platform extension_version | ||
|
|
||
| debug: build_extension_library_debug build_extension_with_metadata_debug | ||
| release: build_extension_library_release build_extension_with_metadata_release | ||
|
|
||
| test: test_debug | ||
| test_debug: test_extension_debug | ||
| test_release: test_extension_release | ||
|
|
||
| # Include the Makefile from extension-ci-tools | ||
| include extension-ci-tools/makefiles/duckdb_extension.Makefile | ||
| clean: clean_build clean_cmake | ||
| clean_all: clean clean_configure | ||
Submodule duckdb
deleted from
f99fed
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does it make sense to open an issue that lists all the C API functions that we need to stabilize to be able to switch inet to the stable C API?
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah we can do that - In this case I don't think inet actually needs any unstable changes, but there is technically a behavior change related to registering overloads (thats only available on upstream main), so I kept it as unstable just to signal that this won't work with any "stable" c-api build. Or rather, it won't work with all duckdb versions that provides the stable api, only those post v1.4.