Opened 6 months ago

Closed 4 weeks ago

Last modified 4 weeks ago

#5488 closed enhancement (complete)

Replace tools/cql_config with pkg-config from cpp-driver

Reported by: tomek Owned by: fdupont
Priority: medium Milestone: Kea1.4-final
Component: build system Version: git
Keywords: Cc:
CVSS Scoring: Parent Tickets:
Sensitive: no Defect Severity: N/A
Sub-Project: DHCP Feature Depending on Ticket:
Estimated Difficulty: 0 Add Hours to Ticket: 0
Total Hours: 0 Internal?: no

Description

When Cassandra was first integrated, cpp-driver didn't provide any way to use their package, so cql_config script was developed to return compilation and linker options.

Usage of cql_config is inconvenient, because it requires modifying tools/cql_config_defines.sh script. As of cpp-driver-2.8.0, it now provides .pc files that can be used by pkg-config. It should be possible to use pkg-config to determine the paths.

Subtickets

Attachments (1)

cpp-driver.rb (479 bytes) - added by fdupont 7 weeks ago.
last version of brew formula for cpp diver

Download all attachments as: .zip

Change History (19)

comment:1 Changed 5 months ago by razvan.becheriu

  • Owner set to razvan.becheriu
  • Status changed from new to accepted

comment:2 Changed 5 months ago by fdupont

Please review and merge #5494 before and check it still configures after #5488. Note I don't believe there is a brew definition for cpp-driver. I can create one if needed.

comment:3 Changed 4 months ago by fdupont

As this becomes more and more important I propose 3 things:

-1- increase the priority of this ticket to the highest Cassandra ticket (as Cassandra tickets depends on this one IMHO nobody should object).
-2- update configure.ac to use pkg-config on macOS.
-3- someone (e.g. you) checks it is an improvement on supported systems where cpp-driver is packaged (i.e. some Linuxes?)

comment:4 Changed 4 months ago by fdupont

  • Priority changed from medium to high

Bump the priority of it before working on Cassandra code becomes a nightmare. BTW there are cpp-driver packages only for CentIOS and Ubuntu in a not official repository.

comment:5 Changed 4 months ago by razvan.becheriu

Fixed google benchmarks with source:
--with-benchmark-source
Modified configure.ac to use pkg-config by default when enabling cassandra backend
(--with-cql)
Added support for statically linking with cassandra cpp driver
--enable-cql-static-link

Note:
to compile with static cpp-driver, the driver must be compiled with:
cmake .. -DCASS_BUILD_STATIC=ON -DCMAKE_CXX_FLAGS="-fPIC"

If you want to build the packages to use with apt or yum, please add "-DCMAKE_CXX_FLAGS='-fPIC'" to the cmake command by modifying the lines in:

packaging/debian/rules
dh_auto_configure -- -DCMAKE_BUILD_TYPE=RELEASE -DCASS_BUILD_STATIC=ON -DCMAKE_CXX_FLAGS='-fPIC' -DCASS_INSTALL_PKG_CONFIG=OFF -DCMAKE_INSTALL_LIBDIR=/usr/lib

packaging/homebrew/cassandra-cpp-driver.rb
system "cmake", "-DCMAKE_BUILD_TYPE=RELEASE", "-DCASS_BUILD_STATIC=ON", "-DCMAKE_CXX_FLAGS='-fPIC'", "-DCASS_INSTALL_PKG_CONFIG=OFF", "-DCMAKE_INSTALL_PREFIX:PATH=#{prefix}", "-DCMAKE_INSTALL_LIBDIR=#{lib}", ".."

You can find the sources in PR71:
https://github.com/isc-projects/kea/pull/71

Please test for MacOSX

Regards

Last edited 4 months ago by razvan.becheriu (previous) (diff)

comment:6 Changed 4 months ago by razvan.becheriu

  • Owner changed from razvan.becheriu to fdupont
  • Status changed from accepted to reviewing

Changed 7 weeks ago by fdupont

last version of brew formula for cpp diver

comment:7 Changed 7 weeks ago by fdupont

  • Owner changed from fdupont to UnAssigned

Ported and fixed PR71. Ready for review.
I am attaching the formula for brew on macOS.

comment:8 Changed 6 weeks ago by tomek

  • Priority changed from high to medium

comment:9 Changed 5 weeks ago by tmark

  • Owner changed from UnAssigned to tmark

comment:10 Changed 5 weeks ago by tmark

  • Owner changed from tmark to fdupont

I found a syntax in configure.ac which I corrected, so please pull.

I did test it under Centos 7 building with both tools/cql_config and pkg-config and it seems to work fine with the syntax correction. I did not test the static build.

I believe you'll need to update the developer's guide instructions on building to mention pkg-config support and you'll need a ChangeLog? entry.

comment:11 Changed 5 weeks ago by fdupont

BTW there is strictly no issue to move this to 1.4-final...

comment:12 Changed 5 weeks ago by tomek

  • Milestone changed from Kea1.4 to Kea1.4-final

Remaining tickets that did not make it into beta are moved to final.

comment:13 Changed 4 weeks ago by fdupont

Updated the installation document for the case a package is available in the target system.
Please look at it...
Note that after merging I'll push the brew cpp-driver file on the branch and export it to GitHub.

comment:14 Changed 4 weeks ago by fdupont

  • Owner changed from fdupont to UnAssigned

comment:15 Changed 4 weeks ago by tmark

  • Owner changed from UnAssigned to fdupont

I made a couple minor wording changes, so pull first. Go ahead and merge it.

comment:16 Changed 4 weeks ago by fdupont

  • Resolution set to complete
  • Status changed from reviewing to closed

Merged and closed. BTW I closed too the PR71 and pushed the branch on GitHub after attaching the macOS brew file for cpp-driver.

comment:17 follow-up: Changed 4 weeks ago by razvan.becheriu

Hi,

I have updated the PR with the requested changes, but it seems that the final merge did not include these changes.
fdupont asked to rename the parameter from '--enable-cql-static-link' to '--enable-cql-static-lib'.
These changes have not been merged.
Any reason for that? Is it a mistake?

Thanks,
Razvan

comment:18 in reply to: ↑ 17 Changed 4 weeks ago by fdupont

Replying to razvan.becheriu:

I have updated the PR with the requested changes, but it seems that the final merge did not include these changes.
fdupont asked to rename the parameter from '--enable-cql-static-link' to '--enable-cql-static-lib'.
These changes have not been merged.
Any reason for that? Is it a mistake?

=> no reason. It is (was!) a mistake.

Note: See TracTickets for help on using tickets.