Skip to content

Commit

Permalink
Refactor EXTENSION_AUTOLOADING Make and CMake settings
Browse files Browse the repository at this point in the history
Previously, there were two independent settings ENABLE_EXTENSION_AUTOLOADING and ENABLE_EXTENSION_AUTOINSTALL, defaulting to 1
Now there is only one, EXTENSION_AUTOLOADING, that takes either 'none', 'load', or 'full'.

Currently default behaviour falls back to 'none', but emits a CMake warning. I think it would be
positive to move all our builds to specify explicitly the setting for autoloading.

At C++ level logic remains unchanged, and defaults also remains unchanged.
  • Loading branch information
carlopi committed Sep 20, 2023
1 parent 417c76c commit 2a33123
Show file tree
Hide file tree
Showing 7 changed files with 27 additions and 32 deletions.
3 changes: 1 addition & 2 deletions .github/actions/build_extensions/action.yml
Original file line number Diff line number Diff line change
Expand Up @@ -176,8 +176,7 @@ runs:
shell: bash
env:
EXTENSION_TESTS_ONLY: 1
ENABLE_EXTENSION_AUTOLOADING: 1
ENABLE_EXTENSION_AUTOINSTALL: 1
EXTENSION_AUTOLOADING: 'full'
GEN: ${{ inputs.ninja == 1 && 'ninja' || '' }}
USE_MERGED_VCPKG_MANIFEST: 1
run: |
Expand Down
11 changes: 4 additions & 7 deletions .github/workflows/Java.yml
Original file line number Diff line number Diff line change
Expand Up @@ -47,8 +47,7 @@ jobs:
image: quay.io/pypa/manylinux2014_x86_64
env:
GEN: ninja
ENABLE_EXTENSION_AUTOLOADING: 1
ENABLE_EXTENSION_AUTOINSTALL: 1
EXTENSION_AUTOLOADING: 'full'
BUILD_JDBC: 1
BUILD_JEMALLOC: 1
BUILD_JSON: 1
Expand Down Expand Up @@ -93,8 +92,7 @@ jobs:
env:
GEN: ninja
BUILD_JDBC: 1
ENABLE_EXTENSION_AUTOLOADING: 1
ENABLE_EXTENSION_AUTOINSTALL: 1
EXTENSION_AUTOLOADING: 'full'
OVERRIDE_JDBC_OS_ARCH: arm64
JAVA_HOME: ../../jdk8u345-b01
steps:
Expand Down Expand Up @@ -151,7 +149,7 @@ jobs:
run: >
python scripts/windows_ci.py
cmake -DCMAKE_BUILD_TYPE=Release -DCMAKE_GENERATOR_PLATFORM=x64 -DJDBC_DRIVER=1 -DBUILD_EXTENSIONS=json -DENABLE_EXTENSION_AUTOLOADING=1 -DENABLE_EXTENSION_AUTOINSTALL=1
cmake -DCMAKE_BUILD_TYPE=Release -DCMAKE_GENERATOR_PLATFORM=x64 -DJDBC_DRIVER=1 -DBUILD_EXTENSIONS=json -DEXTENSION_AUTOLOADING='full'
cmake --build . --config Release
- name: Java Tests
Expand All @@ -178,8 +176,7 @@ jobs:
BUILD_JDBC: 1
BUILD_JSON: 1
OSX_BUILD_UNIVERSAL: 1
ENABLE_EXTENSION_AUTOLOADING: 1
ENABLE_EXTENSION_AUTOINSTALL: 1
EXTENSION_AUTOLOADING: 'full'
steps:
- uses: actions/checkout@v3
with:
Expand Down
6 changes: 2 additions & 4 deletions .github/workflows/LinuxRelease.yml
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,7 @@ jobs:
container: quay.io/pypa/manylinux2014_x86_64
env:
EXTENSION_CONFIGS: '${GITHUB_WORKSPACE}/.github/config/bundled_extensions.cmake'
ENABLE_EXTENSION_AUTOLOADING: 1
ENABLE_EXTENSION_AUTOINSTALL: 1
EXTENSION_AUTOLOADING: 'full'
GEN: ninja
BUILD_BENCHMARK: 1
BUILD_ODBC: 1
Expand Down Expand Up @@ -116,8 +115,7 @@ jobs:
container: ubuntu:18.04
env:
EXTENSION_CONFIGS: '${GITHUB_WORKSPACE}/.github/config/bundled_extensions.cmake'
ENABLE_EXTENSION_AUTOLOADING: 1
ENABLE_EXTENSION_AUTOINSTALL: 1
EXTENSION_AUTOLOADING: 'install,all'
GEN: ninja
BUILD_BENCHMARK: 1
TREAT_WARNINGS_AS_ERRORS: 1
Expand Down
8 changes: 3 additions & 5 deletions .github/workflows/OSX.yml
Original file line number Diff line number Diff line change
Expand Up @@ -78,8 +78,7 @@ jobs:
needs: xcode-debug
env:
EXTENSION_CONFIGS: '${GITHUB_WORKSPACE}/.github/config/bundled_extensions.cmake'
ENABLE_EXTENSION_AUTOLOADING: 1
ENABLE_EXTENSION_AUTOINSTALL: 1
EXTENSION_AUTOLOADING: 'full'
BUILD_ODBC: 1
OSX_BUILD_UNIVERSAL: 1
GEN: ninja
Expand Down Expand Up @@ -270,8 +269,7 @@ jobs:
shell: bash
env:
EXTENSION_TESTS_ONLY: 1
ENABLE_EXTENSION_AUTOLOADING: 1
ENABLE_EXTENSION_AUTOINSTALL: 1
EXTENSION_AUTOLOADING: 'full'
run: |
rm -rf build/release
make
Expand All @@ -285,4 +283,4 @@ jobs:
run: |
python3 scripts/get_test_list.py --file-contains 'require ' --list '"*.test"' > test.list
python3 scripts/get_test_list.py --file-contains 'require-env LOCAL_EXTENSION_REPO' --list '"*.test"' >> test.list
python3 scripts/run_tests_one_by_one.py ./build/release/test/unittest '-f test.list'
python3 scripts/run_tests_one_by_one.py ./build/release/test/unittest '-f test.list'
4 changes: 2 additions & 2 deletions .github/workflows/Windows.yml
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ jobs:
shell: bash
run: |
python scripts/windows_ci.py
cmake -DCMAKE_BUILD_TYPE=Release -DCMAKE_GENERATOR_PLATFORM=x64 -DENABLE_EXTENSION_AUTOLOADING=1 -DENABLE_EXTENSION_AUTOINSTALL=1 -DDUCKDB_EXTENSION_CONFIGS="${GITHUB_WORKSPACE}/.github/config/bundled_extensions.cmake" -DBUILD_ODBC_DRIVER=1 -DDISABLE_UNITY=1
cmake -DCMAKE_BUILD_TYPE=Release -DCMAKE_GENERATOR_PLATFORM=x64 -DEXTENSION_AUTOLOADING='full' -DDUCKDB_EXTENSION_CONFIGS="${GITHUB_WORKSPACE}/.github/config/bundled_extensions.cmake" -DBUILD_ODBC_DRIVER=1 -DDISABLE_UNITY=1
cmake --build . --config Release
- name: Test
Expand Down Expand Up @@ -232,4 +232,4 @@ jobs:
s3_id: ${{ secrets.S3_ID }}
s3_key: ${{ secrets.S3_KEY }}
signing_pk: ${{ secrets.DUCKDB_EXTENSION_SIGNING_PK }}
unittest_script: python3 scripts/run_tests_one_by_one.py ./build/release/test/Release/unittest.exe
unittest_script: python3 scripts/run_tests_one_by_one.py ./build/release/test/Release/unittest.exe
18 changes: 11 additions & 7 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -124,8 +124,7 @@ endif()

option(FORCE_WARN_UNUSED "Unused code objects lead to compiler warnings." FALSE)

option(ENABLE_EXTENSION_AUTOLOADING "Enable extension auto-loading by default." FALSE)
option(ENABLE_EXTENSION_AUTOINSTAll "Enable extension auto-installing by default." FALSE)
option(EXTENSION_AUTOLOADING "Enable extension auto-loading by default." "missing")
option(EXTENSION_TESTS_ONLY "Only load the tests for extensions, don't actually build them; useful for testing loadable extensions" FALSE)
option(WASM_LOADABLE_EXTENSIONS "WebAssembly build with loadable extensions." FALSE)
option(ENABLE_SANITIZER "Enable address sanitizer." TRUE)
Expand Down Expand Up @@ -212,11 +211,16 @@ if(${EXPLICIT_EXCEPTIONS})
set(CXX_EXTRA "${CXX_EXTRA} -fexceptions")
endif()

if (ENABLE_EXTENSION_AUTOLOADING)
add_definitions(-DDUCKDB_EXTENSION_AUTOLOAD_DEFAULT=1)
endif()
if (ENABLE_EXTENSION_AUTOINSTALL)
add_definitions(-DDUCKDB_EXTENSION_AUTOINSTALL_DEFAULT=${ENABLE_EXTENSION_AUTOINSTALL})

if (EXTENSION_AUTOLOADING STREQUAL "none")
add_definitions(-DDUCKDB_EXTENSION_AUTOLOAD_DEFAULT=0 -DDUCKDB_EXTENSION_AUTOINSTALL_DEFAULT=0)
elseif (EXTENSION_AUTOLOADING STREQUAL "full")
add_definitions(-DDUCKDB_EXTENSION_AUTOLOAD_DEFAULT=1 -DDUCKDB_EXTENSION_AUTOINSTALL_DEFAULT=1)
elseif (EXTENSION_AUTOLOADING STREQUAL "load")
add_definitions(-DDUCKDB_EXTENSION_AUTOLOAD_DEFAULT=1 -DDUCKDB_EXTENSION_AUTOINSTALL_DEFAULT=0)
else()
message(WARNING "Value for EXTENSION_AUTOLOADING not provided or not matching one of none|load|full, provide one to the build system")

Check warning on line 222 in CMakeLists.txt

View workflow job for this annotation

GitHub Actions / Linux Extensions (ggc4)

Value for EXTENSION_AUTOLOADING not provided or not matching one of

Check warning on line 222 in CMakeLists.txt

View workflow job for this annotation

GitHub Actions / Linux Extensions (ggc4)

Value for EXTENSION_AUTOLOADING not provided or not matching one of
add_definitions(-DDUCKDB_EXTENSION_AUTOLOAD_DEFAULT=0 -DDUCKDB_EXTENSION_AUTOINSTALL_DEFAULT=0)
endif()

option(OSX_BUILD_UNIVERSAL "Build both architectures on OSX and create a single binary containing both." FALSE)
Expand Down
9 changes: 4 additions & 5 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -75,11 +75,10 @@ endif
ifeq (${DISABLE_BUILTIN_EXTENSIONS}, 1)
CMAKE_VARS:=${CMAKE_VARS} -DDISABLE_BUILTIN_EXTENSIONS=1
endif
ifneq (${ENABLE_EXTENSION_AUTOLOADING}, "")
CMAKE_VARS:=${CMAKE_VARS} -DENABLE_EXTENSION_AUTOLOADING=${ENABLE_EXTENSION_AUTOLOADING}
endif
ifneq (${ENABLE_EXTENSION_AUTOINSTALL}, "")
CMAKE_VARS:=${CMAKE_VARS} -DENABLE_EXTENSION_AUTOINSTALL=${ENABLE_EXTENSION_AUTOINSTALL}
ifneq (${EXTENSION_AUTOLOADING}, "")
CMAKE_VARS:=${CMAKE_VARS} -DEXTENSION_AUTOLOADING=${EXTENSION_AUTOLOADING}
else
CMAKE_VARS:=${CMAKE_VARS} -DEXTENSION_AUTOLOADING='default'}
endif
ifeq (${BUILD_BENCHMARK}, 1)
CMAKE_VARS:=${CMAKE_VARS} -DBUILD_BENCHMARKS=1
Expand Down

0 comments on commit 2a33123

Please sign in to comment.