Skip to content
Draft
Show file tree
Hide file tree
Changes from 13 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 24 additions & 0 deletions ports/odbccpp/portfile.cmake
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
vcpkg_from_github(
OUT_SOURCE_PATH SOURCE_PATH
REPO SAP/odbc-cpp-wrapper
REF "v${VERSION}"
SHA512 1c72fd203021104b37bd01b0db67cff587ca4f03f19dbd8a026fda4437a6d89fed168feb3d3788287df60d5e3cd3450797cd97153826e069fb3d242e7f136f74
HEAD_REF master
PATCHES
use-vcpkg-unixodbc.patch
)

vcpkg_cmake_configure(
SOURCE_PATH "${SOURCE_PATH}"
OPTIONS
-DCMAKE_DISABLE_FIND_PACKAGE_GTest=ON
-DCMAKE_DISABLE_FIND_PACKAGE_Doxygen=ON
)

vcpkg_cmake_install()

vcpkg_copy_pdbs()

file(REMOVE_RECURSE "${CURRENT_PACKAGES_DIR}/debug/include")

vcpkg_install_copyright(FILE_LIST "${SOURCE_PATH}/LICENSE")
32 changes: 32 additions & 0 deletions ports/odbccpp/use-vcpkg-unixodbc.patch
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -4,7 +4,17 @@ SET(CMAKE_CXX_STANDARD 11)

PROJECT(odbccpp)

-FIND_PACKAGE(ODBC REQUIRED)
+IF(UNIX)
+ FIND_PACKAGE(unofficial-unixodbc CONFIG)
+ IF(unixodbc_FOUND)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
+ IF(unixodbc_FOUND)
+ IF(unofficial-unixodbc_FOUND)

I haven't dug too deep into the problem here but I suspect that this is still falling back to the find_package(ODBC REQUIRED) call below which ends up searching for odbc_config and attempts to execute it.

+ SET(ODBC_TARGET unofficial::unixodbc::unixodbc)
+ ENDIF()
+ENDIF()
+IF(NOT ODBC_TARGET)
+ FIND_PACKAGE(ODBC REQUIRED)
+ SET(ODBC_TARGET ODBC::ODBC)
+ENDIF()
Comment on lines +8 to +16
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This must match the dependencies expressed in the manifests. I have more suggestions once the manifest dependency question is answered.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Good point, this is wrongfully adapted. Depending on the result of the other comment, this probably should be IF(LINUX OR APPLE).

Copy link
Copy Markdown
Contributor

@JavierMatosD JavierMatosD Mar 3, 2026

Choose a reason for hiding this comment

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

This must match the dependencies expressed in the manifests. I have more suggestions once the manifest dependency question is answered.

@dg0yt, is there anything else you want to suggest? Otherwise, this port looks good to go to me

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

IF(UNIX) still seems wrong to me. This would become an installation order dependency when unixodbc is extended to support bsd or ios or android.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The safest wiring would be an option which is controlled by the portfile. Here (project mode):

if(ODBCCPP_USE_UNIXODBC)
   FIND_PACKAGE(unofficial-unixodbc CONFIG REQUIRED)
   SET(ODBC_TARGET unofficial::unixodbc::unixodbc)
ELSE()
   ...

and in the portfile (vcpkg script mode):

vcpkg_list(SET options)
if(VCPKG_TARGET_IS_LINUX OR VCPKG_TARGET_IS_OSX)
    list(APPEND options -DODBCCPP_USE_UNIXODBC=ON)
endif()

vcpkg_cmake_configure(
    ...
    OPTIONS
       ${options}
    ...
)
  • I would probably create an alias ODBC::ODBC instead of replacing its use with a variable.
  • There is no installed CMake config. But unless using dynamic loading, there are conditional transitive usage requirements. User must duplicate the condition.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Thanks for the careful check. I added a config file to odbccpp, this is a good thing anyway. Also, I added your suggestions, I hope it fits now.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
+IF(NOT ODBC_TARGET)
+ FIND_PACKAGE(ODBC REQUIRED)
+ SET(ODBC_TARGET ODBC::ODBC)
+ENDIF()

I don't think this fallback is needed.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I changed it to an if else clause, but in general it is necessary for for Windows, I think. Hope this fits for you.

+
FIND_PACKAGE(Doxygen)
FIND_PACKAGE(GTest)

--- a/src/odbc/CMakeLists.txt
+++ b/src/odbc/CMakeLists.txt
@@ -51,7 +51,7 @@ ADD_LIBRARY(odbccpp ${odbccpp_sources})

TARGET_LINK_LIBRARIES(odbccpp
PUBLIC
- ODBC::ODBC
+ ${ODBC_TARGET}
)

IF(BUILD_SHARED_LIBS)
18 changes: 18 additions & 0 deletions ports/odbccpp/vcpkg.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
{
"name": "odbccpp",
"version": "1.5",
"description": "An object-oriented C++ wrapper of the ODBC API from SAP",
"homepage": "https://github.com/SAP/odbc-cpp-wrapper",
"license": "Apache-2.0",
"supports": "!uwp & !android",
"dependencies": [
{
"name": "unixodbc",
"platform": "linux | osx"
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

How is ODBC provided on other platforms which are not !windows?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Thanks for taking a look. In general I oriented on nanodbc and freetds, but I took the platform here from the supports config of unixodbc (https://github.com/microsoft/vcpkg/blob/b2f068faf45a3f04145bec0f52a66526ad590227/ports/unixodbc/vcpkg.json). For other platforms it must be provided by the system. Let me know if I miss something, I have no issue to change it to !windows if it makes sense.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Following unixodbc's supports is right.

},
{
"name": "vcpkg-cmake",
"host": true
}
]
}
4 changes: 4 additions & 0 deletions versions/baseline.json
Original file line number Diff line number Diff line change
Expand Up @@ -7104,6 +7104,10 @@
"baseline": "1.10.0",
"port-version": 0
},
"odbccpp": {
"baseline": "1.5",
"port-version": 0
},
"ode": {
"baseline": "0.16.6",
"port-version": 0
Expand Down
9 changes: 9 additions & 0 deletions versions/o-/odbccpp.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
{
"versions": [
{
"git-tree": "92baf39a2101d8db723433c665d308fe1dab9fc2",
"version": "1.5",
"port-version": 0
}
]
}
Loading