-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Fix duckdb build failure #11059
base: main
Are you sure you want to change the base?
Fix duckdb build failure #11059
Conversation
✅ Deploy Preview for meta-velox canceled.
|
@@ -50,7 +50,7 @@ if(DEFINED ENV{INSTALL_PREFIX}) | |||
list(APPEND CMAKE_PREFIX_PATH "$ENV{INSTALL_PREFIX}") | |||
# Allow installed package headers to be picked up before brew/system package | |||
# headers | |||
include_directories(BEFORE "$ENV{INSTALL_PREFIX}/include") | |||
include_directories("$ENV{INSTALL_PREFIX}/include") |
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.
Please also remove the comment regarding the before
ordering above.
Just curious, since we already have CMAKE_PREFIX_PATH
to include $INSTALL_PREFIX
, do we need include_directories("$ENV{INSTALL_PREFIX}/include")
at all?
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.
Please also remove the comment regarding the
before
ordering above.Just curious, since we already have
CMAKE_PREFIX_PATH
to include$INSTALL_PREFIX
, do we needinclude_directories("$ENV{INSTALL_PREFIX}/include")
at all?
Good catch. I actually think include_directories("$ENV{INSTALL_PREFIX}/include")
is not needed. I will try it on a new repo tomorrow.
Upgrade FBOS dependencies to 2024.09.16.00 installs all dependencies in deps-install subfolder, and set the include directory to it in front of other include directories. This caused duckdb build failed with "error: use of undeclared identifier 'FMT_SNPRINTF'". This commit removes the BEFORE keyword to fix the problem.
Improve dependency install and build (#10920) installs all dependencies in
deps-install subfolder, and set the include directory to it in front of
other include directories. This caused duckdb build failed with "error:
use of undeclared identifier 'FMT_SNPRINTF'". This commit removes the
BEFORE keyword to fix the problem.
Fixes #11058