Skip to content

Conversation

arjenpdevries
Copy link

This pull request modifies CMakeLists.txt based on comparison with the HTTPFS extension, to solve a building problem reported in issue #89

CMakeLists.txt Outdated
@@ -53,7 +57,8 @@ add_definitions(-DUI_EXTENSION_GIT_SHA="${UI_EXTENSION_GIT_SHA}")
build_static_extension(${TARGET_NAME} ${EXTENSION_SOURCES})
build_loadable_extension(${TARGET_NAME} " " ${EXTENSION_SOURCES})

target_link_libraries(${EXTENSION_NAME} OpenSSL::SSL OpenSSL::Crypto)
target_link_libraries(ui_loadable_extension ${OPENSSL_LIBRARIES})
Copy link
Collaborator

Choose a reason for hiding this comment

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

We're not building the ui_loadable_extension so we should remove this

Copy link
Author

Choose a reason for hiding this comment

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

Hmmm. That's not true, because of the line above this: build_loadable_extension(${TARGET_NAME} " " ${EXTENSION_SOURCES}).

And if I remove the line for target_link_libraries(ui_extension) my setting of building and running the extension still succeeds.

arjenpdevries and others added 2 commits March 26, 2025 16:58
removed unnecessary static libs directive.
@arjenpdevries
Copy link
Author

arjenpdevries commented Mar 26, 2025

So I have additionally tried with both target_link_libraries(ui_extension) and target_link_libraries(ui_loadable_extension) and either in isolation, and the only success is with having at least ui_loadable_extension in the instructions.

As discussed in the review, maybe I did misunderstand that ui could be an extension that was meant to always be linked in; but this now does work with loading the extension as well, i.e., during the build ui is defined as a module only loaded on demand:

-- Extensions linked into DuckDB: [core_functions, parquet, jemalloc]
-- Extensions built but not linked: [httpfs, ui, fts]

Example of running it after building with these changes:

[arjen@apc extension]$ ../duckdb -unsigned
v1.2.1-dev648 cd0d0da9b1
Enter ".help" for usage hints.
D install 'ui/ui.duckdb_extension';
D load ui;
D call start_ui();
┌──────────────────────────────────────┐
│                result                │
│               varchar                │
├──────────────────────────────────────┤
│ UI started at http://localhost:4213/ │
└──────────────────────────────────────┘

@arjenpdevries arjenpdevries requested a review from Y-- March 26, 2025 22:09
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