You are not logged in.

Pages: 1

#1 2024-02-23 01:09:20

djnz00
Member
Registered: 2024-02-23
Posts: 7

PKGBUILD review: scylla-cpp-driver

I packaged up the C++ client library for ScyllaDB. There is no equivalent package in pacman or AUR.

pkgname=scylla-cpp-driver
pkgver=2.16.2b
pkgrel=1
pkgdesc='The real-time big data database that is API-compatible with Apache Cassandra and Amazon DynamoDB'
arch=('x86_64')
url="http://www.scylladb.com/"
license=('AGPL')
provides=('scylla-cpp-driver')
conflicts=('scylla-cpp-driver')
depends=('gcc-libs' 'libuv' 'openssl' 'zlib')
makedepends=('cmake' 'gcc')

source=(patch.txt https://github.com/scylladb/cpp-driver/archive/refs/tags/${pkgver}.tar.gz)
sha256sums=('b667b4d2b4ec42b4557fb0ed7f62aba7d7474da24dc5dbff8f772d658d1940de'
            '25f89d4676f9c9ec8e889c2213d2b3e1e87a71e59c16f66fdd180807bb11b052')

prepare() {
    mv "cpp-driver-${pkgver}"/* .
    patch -b -p1 <patch.txt
}

build() {
    FLAGS="-fdebug-prefix-map=${srcdir}=/usr/src -ffile-prefix-map=${srcdir}=/usr/src" cmake "-DCMAKE_CXX_FLAGS=${FLAGS}" "-DCMAKE_C_FLAGS=${FLAGS}" -DCMAKE_INSTALL_RPATH=/usr/lib -B build && make -C build
}

package() {
    cmake --install build --prefix "${pkgdir}/usr"
}

patch.txt

diff -r 1b428df41915 connection_pool.cpp
--- a/src/connection_pool.cpp	Fri Feb 09 11:16:33 2024 -0400
+++ b/src/connection_pool.cpp	Fri Feb 09 11:31:42 2024 -0400
@@ -98,7 +98,7 @@
       if (connections_by_shard_[connection->shard_id()].size() < num_connections_per_shard_) {
         add_connection(PooledConnection::Ptr(new PooledConnection(this, connection)));
       } else {
-        host_->add_unpooled_connection(std::move(connection));
+        host_->add_unpooled_connection(connection);
       }
     }
   }

Offline

#2 2024-02-23 02:05:21

Scimmia
Fellow
Registered: 2012-09-01
Posts: 11,614

Re: PKGBUILD review: scylla-cpp-driver

"AGPL" is no longer a valid license specifier. See https://wiki.archlinux.org/title/PKGBUILD#license
So this provides scylla-cpp-driver, meaning that it can be used in place of the scylla-cpp-driver package? Remove the provides.
Wait, it also conflicts with scylla-cpp-driver, so it can't be installed at the same time as itself? Remove the conflicts.
gcc is part of base-devel and should not be in the makedepends array.
The upstream source needs renamed, ${pkgver}.tar.gz is too generic when used with $SRCDEST. See the warning at https://wiki.archlinux.org/title/PKGBUILD#source
Don't move the source, just cd into that dir.
What's with the -b on patch? Doesn't make much sense when used in a PKGBUILD.
Why set a /usr/lib rpath?
You generally set the prefix during the build, then set DESTDIR on the install. Setting prefix on the install can do strange things. For an example: https://wiki.archlinux.org/title/CMake_ … e_packages
What's up with the flags? Does that clobber Arch's CFLAGS/CXXFLAGS?

Last edited by Scimmia (2024-02-23 02:13:00)

Offline

#3 2024-02-23 08:11:17

loqs
Member
Registered: 2014-03-06
Posts: 17,483

Re: PKGBUILD review: scylla-cpp-driver

Why rename https://github.com/scylladb/cpp-driver/ … 398d3.diff to patch.txt removing the git commit ID and making the name generic?  Also providing the source of a patch in a comment and its purpose helps with understanding.  Using https://github.com/scylladb/cpp-driver/ … 98d3.patch instead of the diff to keep the upstream commit message also helps with understanding.
Edit:
The build below addresses all comments.  Instead of applying a patch for the compiler warning that is converted to an error by -Werror it removes the use of -Werror.  license is changed to Apache-2.0 based on the README.md#license.

pkgname=scylla-cpp-driver
_pkgname=cpp-driver
pkgver=2.16.2b
pkgrel=1
pkgdesc='The real-time big data database that is API-compatible with Apache Cassandra and Amazon DynamoDB'
arch=('x86_64')
url="http://www.scylladb.com/"
license=('Apache-2.0')
depends=('gcc-libs' 'libuv' 'openssl' 'zlib')
makedepends=('cmake')

source=(https://github.com/scylladb/$_pkgname/archive/$pkgver/$_pkgname-$pkgver.tar.gz)
sha256sums=('25f89d4676f9c9ec8e889c2213d2b3e1e87a71e59c16f66fdd180807bb11b052')

prepare() {
    sed -i 's/ -Werror//' $_pkgname-$pkgver/src/CMakeLists.txt
}

build() {
    cmake -B build -S $_pkgname-$pkgver \
        -DCMAKE_BUILD_TYPE='None' \
        -DCMAKE_INSTALL_PREFIX='/usr' \
        -Wno-dev
    cmake --build build
}

package() {
    DESTDIR="$pkgdir" cmake --install build
}

Last edited by loqs (2024-02-23 09:34:04)

Offline

#4 2024-02-29 22:06:58

djnz00
Member
Registered: 2024-02-23
Posts: 7

Re: PKGBUILD review: scylla-cpp-driver

FLAGS="-fdebug-prefix-map=${srcdir}=/usr/src -ffile-prefix-map=${srcdir}=/usr/src" is necessary to avoid leaking the package builder's full build directory path into the generated package binary, which typically leaks the home directory name, and that in turn risks inadvertently leaking an identity.

cmake "-DCMAKE_CXX_FLAGS=${FLAGS}" "-DCMAKE_C_FLAGS=${FLAGS}"appends flags to the default, does not substitute those otherwise used.

"-DCMAKE_INSTALL_RPATH=/usr/lib" - without this, applications built using this library will read the wrong rpath from the built .so (and the home directory will get leaked into the binary again)

removing -Werror ... since the source is intended to be built with -Werror, particularly for production builds, this seems inferior to patching the warning, which indicates an authentic quality problem with the code.

agree about sourcing the patch from github, I wasn't aware that existed.

Offline

#5 2024-02-29 22:08:44

djnz00
Member
Registered: 2024-02-23
Posts: 7

Re: PKGBUILD review: scylla-cpp-driver

... also agree about provides and conflicts, assuming that they're redundant, I got those from copying a different PKGBUILD

Offline

#6 2024-02-29 22:27:21

djnz00
Member
Registered: 2024-02-23
Posts: 7

Re: PKGBUILD review: scylla-cpp-driver

having said all that I am still getting this warning from makepkg: "==> WARNING: Package contains reference to $srcdir", I'll try and integrate a fix into your version

Offline

#7 2024-02-29 23:02:28

djnz00
Member
Registered: 2024-02-23
Posts: 7

Re: PKGBUILD review: scylla-cpp-driver

here's my merge of your version with mine, including fetching the patch from github:

pkgname=scylla-cpp-driver
_pkgname=cpp-driver
_patch=ae17b0ff32e70bac5b6a2bc1516237e71c0398d3
pkgver=2.16.2b
pkgrel=1
pkgdesc='The real-time big data database that is API-compatible with Apache Cassandra and Amazon DynamoDB'
arch=('x86_64')
url="http://www.scylladb.com/"
license=('Apache-2.0')
depends=('gcc-libs' 'libuv' 'openssl' 'zlib')
makedepends=('cmake' 'gcc')

source=(https://github.com/scylladb/${_pkgname}/commit/${_patch}.diff https://github.com/scylladb/{$_pkgname}/archive/refs/tags/${pkgver}.tar.gz)
sha256sums=('4e39dc2a333a19f5d8be8be4b4ebabb6ec16a7c9af29e324e6938b5e80cd2513'
            '25f89d4676f9c9ec8e889c2213d2b3e1e87a71e59c16f66fdd180807bb11b052')

prepare() {
    cd "${_pkgname}-${pkgver}"
    patch -p1 <../${_patch}.diff
}

build() {
    # prevent leaking srcdir into package binaries
    FLAGS="-fdebug-prefix-map=${srcdir}=/usr/src"
    FLAGS="${FLAGS} -ffile-prefix-map=${srcdir}=/usr/src"
    cmake -B build -S "${_pkgname}-${pkgver}" \
        -DCMAKE_BUILD_TYPE='None' \
        -DCMAKE_INSTALL_PREFIX='/usr' \
	-DCMAKE_CXX_FLAGS="${FLAGS}" \
	-DCMAKE_C_FLAGS="${FLAGS}" \
	-DCMAKE_INSTALL_RPATH=/usr/lib \
	-Wno-dev &&
    cmake --build build
}

package() {
    DESTDIR="$pkgdir" cmake --install build
}

Offline

#8 2024-03-01 08:51:37

loqs
Member
Registered: 2014-03-06
Posts: 17,483

Re: PKGBUILD review: scylla-cpp-driver

djnz00 wrote:

FLAGS="-fdebug-prefix-map=${srcdir}=/usr/src -ffile-prefix-map=${srcdir}=/usr/src" is necessary to avoid leaking the package builder's full build directory path into the generated package binary, which typically leaks the home directory name, and that in turn risks inadvertently leaking an identity.

Not if you use the debug option which is now enabled by default.  https://gitlab.archlinux.org/pacman/pac … .sh.in#L32 Also if the packager builds use devtools their home directory will not be used so will not be recorded.

djnz00 wrote:
loqs wrote:

cmake "-DCMAKE_CXX_FLAGS=${FLAGS}" "-DCMAKE_C_FLAGS=${FLAGS}"appends flags to the default, does not substitute those otherwise used.

Your updated PKGBUILD is still overriding CFLAGS and CXXFLAGS by not appending.

djnz00 wrote:

"-DCMAKE_INSTALL_RPATH=/usr/lib" - without this, applications built using this library will read the wrong rpath from the built .so (and the home directory will get leaked into the binary again)

CMake_package_guidelines#Removing_insecure_RPATH_references_from_binaries What is the point to setting RPATH to /usr/lib which is in the dynamic linkers default search path?

Offline

#9 2024-03-02 01:33:31

djnz00
Member
Registered: 2024-02-23
Posts: 7

Re: PKGBUILD review: scylla-cpp-driver

thanks for pointing out the makepkg.conf debug option, as you suggested it was unset but the .pacnew had it as set (along with lto); so that enabled removing all customization of CFLAGS and CXXFLAGS...

@loqs - looks good to you now?

pkgname=scylla-cpp-driver
_pkgname=cpp-driver
_patch=ae17b0ff32e70bac5b6a2bc1516237e71c0398d3
pkgver=2.16.2b
pkgrel=1
pkgdesc='The real-time big data database that is API-compatible with Apache Cassandra and Amazon DynamoDB'
arch=('x86_64')
url="http://www.scylladb.com/"
license=('Apache-2.0')
depends=('gcc-libs' 'libuv' 'openssl' 'zlib')
makedepends=('cmake' 'gcc')

source=(https://github.com/scylladb/${_pkgname}/commit/${_patch}.diff https://github.com/scylladb/{$_pkgname}/archive/refs/tags/${pkgver}.tar.gz)
sha256sums=('4e39dc2a333a19f5d8be8be4b4ebabb6ec16a7c9af29e324e6938b5e80cd2513'
            '25f89d4676f9c9ec8e889c2213d2b3e1e87a71e59c16f66fdd180807bb11b052')

prepare() {
    cd "${_pkgname}-${pkgver}"
    patch -p1 <../${_patch}.diff
}

build() {
    cmake -B build -S "${_pkgname}-${pkgver}" \
        -DCMAKE_BUILD_TYPE='None' \
        -DCMAKE_INSTALL_PREFIX='/usr' \
	-Wno-dev &&
    cmake --build build
}

package() {
    DESTDIR="$pkgdir" cmake --install build
}

Offline

#10 2024-03-02 02:03:17

Scimmia
Fellow
Registered: 2012-09-01
Posts: 11,614

Re: PKGBUILD review: scylla-cpp-driver

Getting closer. You still have issues with makedepends and with renaming the source file (see earlier post).

Offline

#11 2024-03-02 19:49:27

djnz00
Member
Registered: 2024-02-23
Posts: 7

Re: PKGBUILD review: scylla-cpp-driver

apart from removing gcc from makedepends, I don't see any renaming of the source file?

Offline

#12 2024-03-02 20:15:13

progandy
Member
Registered: 2012-05-17
Posts: 5,205

Re: PKGBUILD review: scylla-cpp-driver

You should add a renaming for the tar.gz:

Scimmia wrote:

The upstream source needs renamed, ${pkgver}.tar.gz is too generic when used with $SRCDEST. See the warning at https://wiki.archlinux.org/title/PKGBUILD#source

| alias CUTF='LANG=en_XX.UTF-8@POSIX ' |

Offline

#13 2024-03-02 20:57:51

loqs
Member
Registered: 2014-03-06
Posts: 17,483

Re: PKGBUILD review: scylla-cpp-driver

progandy wrote:

You should add a renaming for the tar.gz

Or let github do it:

source=(https://github.com/scylladb/$_pkgname/archive/$pkgver/$_pkgname-$pkgver.tar.gz)

Offline

Pages: 1

Board footer

Powered by FluxBB