Support rpm and deb packaging #122

syuu1228 · 2024-04-16T10:50:49Z

Add build_rpm.sh and build_deb.sh build scripts to produce rpm & deb package.

closes scylladb/scylla-pkg#3714

closes #121

piodul · 2024-04-16T12:49:27Z

Just to be clear, the failing clippy check has nothing to do with this change - we are using the newest version of the clippy linter and apparently new lints were introduced which our code does not satisfy.

It would probably be nice to fix them first to have green CI, but I personally don't think it's required.

Opened an issue about it: #123

wprzytula · 2024-04-16T14:37:41Z

I've successfully packaged the driver into RPM and installed it on my Fedora 39. Congratulations, @syuu1228! 🚀

README.md

dist/debian/changelog.template

SCYLLA-VERSION-GEN

dist/debian/build_deb.sh

dist/redhat/build_rpm.sh

dist/redhat/scylla-cpp-rust-driver.spec

dist/debian/debian/control

dist/debian/debian/rules

dist/redhat/scylla-cpp-rust-driver.spec

roydahan · 2024-05-01T13:51:54Z

@syuu1228 where are we standing with this one?
Can you please address/reply the comments?

syuu1228 · 2024-05-02T03:16:13Z

Fixed codes mentioned on comments, also bash coding style fixed (by following ShellCheck rules)

syuu1228 · 2024-05-02T04:44:55Z

Just realized that generating pkgconfig files on deb was not implemented yet, fixed.

roydahan · 2024-05-09T11:52:27Z

We would really want to merge and release 0.1.0 by the end of next week.
So, whoever should review/re-review and/or address comments, please prioritze.

mykaul · 2024-05-09T13:24:27Z

dist/common/pkgconfig/scylla_cpp_driver.pc.in

+Version: @version@
+Libs: -L${libdir} -lscylla_cpp_driver
+Cflags: -I${includedir}/scylladb
+URL: https://github.com/scylladb/cpp-rust-driver


Do we need to add a license line here as well?

ping @syuu1228

Well, I don't find license directive on pkg-config manual, what does it means?

Lorak-mmk

I've built the rpm package and compared it to our cpp-driver rpm package (https://github.com/scylladb/cpp-driver/releases/download/2.16.2b/scylla-cpp-driver-2.16.2b-1.el7.x86_64.rpm ). I have few questions (sorry if those are obvious, I don't have much experience in packaging).
cpp-driver provides the following files:

/usr/lib64/libscylla-cpp-driver.so
/usr/lib64/libscylla-cpp-driver.so.2
/usr/lib64/libscylla-cpp-driver.so.2.16.2-b
/usr/share/doc/scylla-cpp-driver-2.16.2b
/usr/share/doc/scylla-cpp-driver-2.16.2b/LICENSE.txt
/usr/share/doc/scylla-cpp-driver-2.16.2b/README.md

while cpp-rust-driver provides:

/usr/lib/.build-id
/usr/lib/.build-id/8a
/usr/lib/.build-id/8a/ad7ba567e0fd997cb3aa7c1819083466bfa71e
/usr/lib64/libscylla_cpp_driver.so
/usr/share/doc/scylla-cpp-rust-driver
/usr/share/doc/scylla-cpp-rust-driver/LICENSE
/usr/share/doc/scylla-cpp-rust-driver/README.md

What is the .build-id folder?
Why do we only have .so without version number in the name? cpp-driver provides, as you can see, .so.<major-version> and .so.<full-version>, which are symlinks to main .so. Why is that?
cpp-rust-driver has dashes in .so name while cpp-driver has underscores. Is it going to be a problem for a user looking for a drop-in replacement?

One more thing: apart from the commit with packaging there are 2 more commits. First one claims to update Rust Driver to newest version, but updates it to a commit from August of last year. Unless those 2 commits are necessary (and I don't see why would they be) please stick to the main goal of this PR which is packaging and drop those 2 commits. We'll update Rust Driver in a separate PR.

wprzytula · 2024-05-10T13:06:45Z

One more thing: apart from the commit with packaging there are 2 more commits. First one claims to update Rust Driver to newest version, but updates it to a commit from August of last year. Unless those 2 commits are necessary (and I don't see why would they be) please stick to the main goal of this PR which is packaging and drop those 2 commits. We'll update Rust Driver in a separate PR.

@Lorak-mmk These two commits are both mine, and they are already merged to master; I have no idea why they are displayed here.

Lorak-mmk · 2024-05-10T13:26:26Z

One more thing: apart from the commit with packaging there are 2 more commits. First one claims to update Rust Driver to newest version, but updates it to a commit from August of last year. Unless those 2 commits are necessary (and I don't see why would they be) please stick to the main goal of this PR which is packaging and drop those 2 commits. We'll update Rust Driver in a separate PR.

@Lorak-mmk These two commits are both mine, and they are already merged to master; I have no idea why they are displayed here.

Rebasing a PR on master will probably fix this

wprzytula · 2024-05-10T13:42:27Z

cpp-rust-driver has dashes in .so name while cpp-driver has underscores. Is it going to be a problem for a user looking for a drop-in replacement?

Good catch! That's probably a very important question.

syuu1228 · 2024-05-28T19:04:28Z

I found that versioning.sh will causes mv: 'target/release/libscylla_cpp_driver.a' and 'target/release/libscylla-cpp-driver_static.a' are the same file error when we build twice on same profile.
This is not occur on build_rpm.sh and build_deb.sh since both are running in temporary container, but if we build the library manually or calling versioning.sh from CMake will be problematic.

To avoid the error, stop calling mv and use cp --remove-destination to remove the destination file first.
Note: ln is already safe since -f option does same behavior (remove the destination file first).

Lorak-mmk · 2024-05-28T19:27:16Z

README.md

+
+To build deb package, run the following command:
+```shell
+./dist/redhat/build_deb.sh --target mantic


redhat -> debian

Lorak-mmk · 2024-05-28T19:38:54Z

You said you fixed the issue but the snippet you posted still has dashes instead of underscores - why?

I think it's opposite, it should be replace underscores with dashes, since we want to rename libscylla_cpp_driver.so.X.Y.Z to libscylla-cpp-driver.so.X.Y.Z. But I found that I did not renamed static library yet, so I implemented it and pushed new version.

Oh, that's right. In my initial comment about it I posted correct file lists and then mistakenly wrote "cpp-rust-driver has dashes in .so name while cpp-driver has underscores" while it was the other way around.
After bulding the package I see those files inside:

/usr/lib/.build-id
/usr/lib/.build-id/61
/usr/lib/.build-id/61/48a6727fa1f90104938a95ad2845314910309a
/usr/lib64/libscylla-cpp-driver.so.2
/usr/lib64/libscylla-cpp-driver.so.2.16.1
/usr/share/doc/scylla-cpp-rust-driver
/usr/share/doc/scylla-cpp-rust-driver/LICENSE
/usr/share/doc/scylla-cpp-rust-driver/README.md

/usr/lib64/libscylla-cpp-driver.so is still missing compared to cpp-driver.

Another small issue: after building the package with ./dist/redhat/build_rpm.sh --target fedora-39-x86_64 a version file is created in main folder of repo.
I don't fully understand why, I thought that the build was done in some sort of container.
Anyways, it should be either removed or added to .gitignore.

What is the .build-id folder?

It is the standard place to store debuginfo on newer distribution.
Both RedHat variants and Debian variants use same place.

I don't see this folder in .deb packages. Is this expected?
Oh wait, I actually see it in dbgsym deb, I assume this is expected.

Another question, hopefully last one: In dev packages (both rpm and deb) I don't see versioned SOs (.so.2, .so.2.16.1), only the normal one (.so). Is this expected?

syuu1228 · 2024-05-31T02:14:21Z

Add a workaround patch for Ubuntu 24.04 build, since Ubuntu 24.04 version of patchelf breaks libscylla-cpp-driver.so for some reason (probably a bug on patchelf).
It will cause error when stripping debug symbol since the ELF header is corrupted.

The error message is like this:

        readelf -W --section-headers debian/libscylla-cpp-driver-dev/usr/lib/x86_64-linux-gnu/libscylla-cpp-driver_static.a | sed -n '/^ *\[[ 0-9]*]/s/ *\[[ 0-9]*\] *//p' | awk 'BEGIN {rv=1} /^NULL/ {next} $1 ~ /^.(text|data|(preinit|init|fini)_array$)/ {if ($5 !~ /^0+$/) rv=0} END { exit rv}'
f132a9c6e662bd19d130b1e3ee434af824abbfe0
readelf: Error: no .dynamic section in the dynamic segment
        install -m0755 -d debian/.debhelper/libscylla-cpp-driver0/dbgsym-root/usr/lib/debug/.build-id/f1
        objcopy --only-keep-debug --compress-debug-sections debian/libscylla-cpp-driver0/usr/lib/x86_64-linux-gnu/libscylla-cpp-driver.so.2.16.1 debian/.debhelper/libscylla-cpp-driver0/dbgsym-root/usr/lib/debug/.build-id/f1/32a9c6e662bd19d130b1e3ee434af824abbfe0.debug
objcopy: warning: debian/libscylla-cpp-driver0/usr/lib/x86_64-linux-gnu/libscylla-cpp-driver.so.2.16.1 has a corrupt string table index
objcopy: unable to modify 'debian/libscylla-cpp-driver0/usr/lib/x86_64-linux-gnu/libscylla-cpp-driver.so.2.16.1' due to errors
dh_strip: error: objcopy --only-keep-debug --compress-debug-sections debian/libscylla-cpp-driver0/usr/lib/x86_64-linux-gnu/libscylla-cpp-driver.so.2.16.1 debian/.debhelper/libscylla-cpp-driver0/dbgsym-root/usr/lib/debug/.build-id/f1/32a9c6e662bd19d130b1e3ee434af824abbfe0.debug returned exit code 1
dh_strip: error: Aborting due to earlier error
make: *** [debian/rules:15: binary] Error 25
dpkg-buildpackage: error: fakeroot debian/rules binary subprocess returned exit status 2
debuild: fatal error at line 1184:
dpkg-buildpackage -us -uc -ui failed

This can be avoid by using Ubuntu 23.10's patchelf, the patch do that in versioning.sh.
(This should be temporary solution but I not find other way for now)

syuu1228 · 2024-05-31T03:01:11Z

/usr/lib64/libscylla-cpp-driver.so is still missing compared to cpp-driver.

Sorry I forgot to describe what is this.
On both RedHat and Debian, library packages does not include libXXX.so file on main package, it will installed in -devel (or -dev in Debian) package.

Debian Policy Manual says:
The development package should contain a symlink for the associated shared library without a version number. For example, the libgdbm-dev package should include a symlink from /usr/lib/libgdbm.so to libgdbm.so.3.0.0. This symlink is needed by the linker (ld) when compiling packages, as it will only look for libgdbm.so when compiling dynamically.
https://www.debian.org/doc/debian-policy/ch-sharedlibs.html#development-files

Seems like cpp-driver packaging libscylla-cpp-driver.so in main package, but unless the difference causes compatibility problem, I think we should keep current packaging since it looks correct.

syuu1228 · 2024-05-31T03:08:33Z

Another question, hopefully last one: In dev packages (both rpm and deb) I don't see versioned SOs (.so.2, .so.2.16.1), only the normal one (.so). Is this expected?

I think this is same topic as I described above, -dev package will contain libXXX.so, and main package will contain libXXX.so.1.2.3 and libXXX.so.1.

Lorak-mmk · 2024-05-31T12:37:39Z

Ok, so the cpp-driver is in the wrong here IIUC.
If so, please fix the last tiny issue I mentioned (version file, which should be .gitignore'd / removed after build / not created) and we can merge.

syuu1228 · 2024-06-01T03:28:10Z

I just found that we actually can fix SONAME using RUSTFLAGS, even we build Rust in Cargo:
https://developers.redhat.com/articles/2022/09/05/how-create-c-binding-rust-library#fixing_the_soname
I think this is correct approach than paching ELF header, and it also should fix Ubuntu 24.04 patchelf issue, so pushed new version to do that.

syuu1228 · 2024-06-01T03:30:42Z

If so, please fix the last tiny issue I mentioned (version file, which should be .gitignore'd / removed after build / not created) and we can merge.

Pushed new version to add version to .gitignore.

Lorak-mmk · 2024-06-01T11:53:11Z

dist/redhat/scylla-cpp-rust-driver.spec

+if [ -z $RUSTFLAGS ]; then
+    RUSTFLAGS="-Copt-level=3 -Cdebuginfo=2 -Ccodegen-units=1 -Cstrip=none -Cforce-frame-pointers=yes --cap-lints=warn"
+fi


Either I didn't notice it before or it wasn't here before. All those parameters (apart from --cap-lints=warn and -Cforce-frame-pointers=yes) are set in packaging profile in Cargo.toml, why set them again here?

This is new thing, added at #122 (comment)

Since I intended to apply RedHat's default RUSTFLAGS to build because it can be safer to build rpm, and manually set RUSTFLAGS only on older versions of RedHat, and the parameters are taken from /usr/lib/rpm/macros.
But yes, we aleady have these parameters to Cargo.toml, maybe we shouldn't use distro default RUSTFLAGS at all.

To avoid applying these duplicated parameters, we probably should not use distro default RUSTFLAGS and force apply RUSTFLAGS like this (without [ -z $RUSTFLAGS ] condition since we want overwrite):
RUSTFLAGS="-Cforce-frame-pointers=yes --cap-lints=warn"

Lorak-mmk · 2024-06-01T11:53:58Z

dist/debian/debian/rules

+SCYLLA_VERSION := $(shell cat version | awk -F'-' '{print $1}' | sed 's/-/~/')
+
+VERSION_MAJOR := $(shell sed -n -e 's/^#define CASS_VERSION_MAJOR \(.*\)/\1/p' include/cassandra.h)
+RUSTFLAGS := -C debuginfo=2 --cap-lints warn -C linker=x86_64-linux-gnu-gcc -C link-arg=-Wl,-Bsymbolic-functions -C link-arg=-Wl,-z,relro -Clink-arg=-Wl,-soname=libscylla-cpp-driver.so.$(VERSION_MAJOR)


Same here, -C debuginfo=2 probably shouldn't be set here

The reason why these parameters are applying duplicated parameters is same, this RUSTFLAGS is taken from Debian package build system.
After dropping duplicated parameters like above, it will be:
RUSTFLAGS := --cap-lints warn -C linker=x86_64-linux-gnu-gcc -C link-arg=-Wl,-Bsymbolic-functions -C link-arg=-Wl,-z,relro -Clink-arg=-Wl,-soname=libscylla-cpp-driver.so.$(VERSION_MAJOR)

Add build_rpm.sh and build_deb.sh build scripts to produce rpm & deb package. closes scylladb/scylla-pkg#3714 closes scylladb#121 Signed-off-by: Takuya ASADA <syuu@scylladb.com>

syuu1228 · 2024-06-03T09:44:53Z

@Lorak-mmk Pushed new version to change RUSTFLAGS on both rpm & deb.

roydahan · 2024-06-03T13:01:53Z

@syuu1228 can you please check why CI fails?

On Ubuntu, we need to run "apt-get update" prior to "apt-get install", otherwise it does not gurantee the package database is up to date. If we don't do that, "apt-get install" with outdated database may cause error something like: Err:9 mirror+file:/etc/apt/apt-mirrors.txt jammy-updates/main amd64 libc6-dbg amd64 2.35-0ubuntu3.7 404 Not Found [IP: 52.252.75.106 80] E: Failed to fetch mirror+file:/etc/apt/apt-mirrors.txt/pool/main/g/glibc/libc6-dbg_2.35-0ubuntu3.7_amd64.deb 404 Not Found [IP: 52.252.75.106 80] E: Unable to fetch some archives, maybe run apt-get update or try with --fix-missing? Fetched 504 kB in 0s (1279 kB/s) To fix this, add "apt-get update" on the begging of "Setup environment". Related scylladb#122

On Ubuntu, we need to run "apt-get update" prior to "apt-get install", otherwise it does not gurantee the package database is up to date. If we don't do that, "apt-get install" with outdated database may cause error something like: Err:9 mirror+file:/etc/apt/apt-mirrors.txt jammy-updates/main amd64 libc6-dbg amd64 2.35-0ubuntu3.7 404 Not Found [IP: 52.252.75.106 80] E: Failed to fetch mirror+file:/etc/apt/apt-mirrors.txt/pool/main/g/glibc/libc6-dbg_2.35-0ubuntu3.7_amd64.deb 404 Not Found [IP: 52.252.75.106 80] E: Unable to fetch some archives, maybe run apt-get update or try with --fix-missing? Fetched 504 kB in 0s (1279 kB/s) To fix this, add "apt-get update" on the begging of scripts. Related scylladb#122

On Ubuntu, we need to run "apt-get update" prior to "apt-get install", otherwise it does not gurantee the package database is up to date. If we don't do that, "apt-get install" with outdated database may cause error something like: Err:9 mirror+file:/etc/apt/apt-mirrors.txt jammy-updates/main amd64 libc6-dbg amd64 2.35-0ubuntu3.7 404 Not Found [IP: 52.252.75.106 80] E: Failed to fetch mirror+file:/etc/apt/apt-mirrors.txt/pool/main/g/glibc/libc6-dbg_2.35-0ubuntu3.7_amd64.deb 404 Not Found [IP: 52.252.75.106 80] E: Unable to fetch some archives, maybe run apt-get update or try with --fix-missing? Fetched 504 kB in 0s (1279 kB/s) To fix this, add "apt-get update" on the begging of "Setup environment". Related scylladb#122

syuu1228 · 2024-06-03T14:21:45Z

@roydahan This is a bug on .github/workflows, sent fix at #127

On Ubuntu, we need to run "apt-get update" prior to "apt-get install", otherwise it does not gurantee the package database is up to date. If we don't do that, "apt-get install" with outdated database may cause error something like: Err:9 mirror+file:/etc/apt/apt-mirrors.txt jammy-updates/main amd64 libc6-dbg amd64 2.35-0ubuntu3.7 404 Not Found [IP: 52.252.75.106 80] E: Failed to fetch mirror+file:/etc/apt/apt-mirrors.txt/pool/main/g/glibc/libc6-dbg_2.35-0ubuntu3.7_amd64.deb 404 Not Found [IP: 52.252.75.106 80] E: Unable to fetch some archives, maybe run apt-get update or try with --fix-missing? Fetched 504 kB in 0s (1279 kB/s) To fix this, add "apt-get update" on the begging of "Setup environment". Related #122

roydahan · 2024-06-03T15:26:22Z

@wprzytula would you want the honor to merge?
Once we merge, we need to officially build and release 0.1.0.

wprzytula · 2024-06-03T16:11:13Z

@wprzytula would you want the honor to merge? Once we merge, we need to officially build and release 0.1.0.

Gladly!

yaronkaikov requested review from roydahan and avelanarius April 16, 2024 11:30

roydahan requested review from piodul and wprzytula April 16, 2024 11:38

Lorak-mmk self-requested a review April 16, 2024 12:11

piodul mentioned this pull request Apr 16, 2024

The code does not adhere to clippy 1.77.2 lints #123

Closed

wprzytula requested changes Apr 16, 2024

View reviewed changes

Gor027 mentioned this pull request Apr 29, 2024

Add RPM/DEB packaging #88

Closed

5 tasks

Lorak-mmk requested changes Apr 29, 2024

View reviewed changes

dist/debian/debian/rules Outdated Show resolved Hide resolved

dist/redhat/scylla-cpp-rust-driver.spec Outdated Show resolved Hide resolved

dist/redhat/scylla-cpp-rust-driver.spec Outdated Show resolved Hide resolved

dist/redhat/scylla-cpp-rust-driver.spec Outdated Show resolved Hide resolved

syuu1228 force-pushed the rpm_deb_packaging branch 2 times, most recently from 0e977f3 to 6a174f8 Compare May 2, 2024 01:14

syuu1228 force-pushed the rpm_deb_packaging branch from 6a174f8 to 5036de6 Compare May 2, 2024 04:40

roydahan requested review from Lorak-mmk and wprzytula May 6, 2024 00:40

roydahan assigned syuu1228, Lorak-mmk and wprzytula May 9, 2024

mykaul reviewed May 9, 2024

View reviewed changes

Lorak-mmk reviewed May 10, 2024

View reviewed changes

syuu1228 force-pushed the rpm_deb_packaging branch 2 times, most recently from f552e9c to 1d7b516 Compare May 15, 2024 04:44

syuu1228 force-pushed the rpm_deb_packaging branch from 0d245b4 to 788f9f9 Compare May 28, 2024 18:51

Lorak-mmk requested changes May 28, 2024

View reviewed changes

syuu1228 force-pushed the rpm_deb_packaging branch 2 times, most recently from ede74e8 to 1b96858 Compare May 31, 2024 02:06

syuu1228 force-pushed the rpm_deb_packaging branch from 1b96858 to bd1782e Compare June 1, 2024 03:23

syuu1228 force-pushed the rpm_deb_packaging branch from bd1782e to 06ceaab Compare June 1, 2024 03:29

Lorak-mmk requested changes Jun 1, 2024

View reviewed changes

Support rpm and deb packaging

58f1408

Add build_rpm.sh and build_deb.sh build scripts to produce rpm & deb package. closes scylladb/scylla-pkg#3714 closes scylladb#121 Signed-off-by: Takuya ASADA <syuu@scylladb.com>

syuu1228 force-pushed the rpm_deb_packaging branch from 06ceaab to 58f1408 Compare June 3, 2024 09:43

Lorak-mmk approved these changes Jun 3, 2024

View reviewed changes

syuu1228 mentioned this pull request Jun 3, 2024

CI: Fix error on apt-get install #127

Merged

Lorak-mmk mentioned this pull request Jun 3, 2024

Add install capabilities to top-level CMake config #101

Closed

5 tasks

wprzytula merged commit 879c6ab into scylladb:master Jun 3, 2024
3 of 5 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support rpm and deb packaging #122

Support rpm and deb packaging #122

syuu1228 commented Apr 16, 2024 •

edited by yaronkaikov

Loading

piodul commented Apr 16, 2024

wprzytula commented Apr 16, 2024

roydahan commented May 1, 2024

syuu1228 commented May 2, 2024

syuu1228 commented May 2, 2024

roydahan commented May 9, 2024

mykaul May 9, 2024

wprzytula May 21, 2024

syuu1228 May 22, 2024

Lorak-mmk left a comment

wprzytula commented May 10, 2024

Lorak-mmk commented May 10, 2024

wprzytula commented May 10, 2024

syuu1228 commented May 28, 2024

Lorak-mmk May 28, 2024

Lorak-mmk commented May 28, 2024

syuu1228 commented May 31, 2024

syuu1228 commented May 31, 2024

syuu1228 commented May 31, 2024

Lorak-mmk commented May 31, 2024

syuu1228 commented Jun 1, 2024

syuu1228 commented Jun 1, 2024

Lorak-mmk Jun 1, 2024

syuu1228 Jun 3, 2024

Lorak-mmk Jun 1, 2024

syuu1228 Jun 3, 2024

syuu1228 commented Jun 3, 2024

roydahan commented Jun 3, 2024

syuu1228 commented Jun 3, 2024

roydahan commented Jun 3, 2024

wprzytula commented Jun 3, 2024

Support rpm and deb packaging #122

Support rpm and deb packaging #122

Conversation

syuu1228 commented Apr 16, 2024 • edited by yaronkaikov Loading

piodul commented Apr 16, 2024

wprzytula commented Apr 16, 2024

roydahan commented May 1, 2024

syuu1228 commented May 2, 2024

syuu1228 commented May 2, 2024

roydahan commented May 9, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Lorak-mmk left a comment

Choose a reason for hiding this comment

wprzytula commented May 10, 2024

Lorak-mmk commented May 10, 2024

wprzytula commented May 10, 2024

syuu1228 commented May 28, 2024

Choose a reason for hiding this comment

Lorak-mmk commented May 28, 2024

syuu1228 commented May 31, 2024

syuu1228 commented May 31, 2024

syuu1228 commented May 31, 2024

Lorak-mmk commented May 31, 2024

syuu1228 commented Jun 1, 2024

syuu1228 commented Jun 1, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

syuu1228 commented Jun 3, 2024

roydahan commented Jun 3, 2024

syuu1228 commented Jun 3, 2024

roydahan commented Jun 3, 2024

wprzytula commented Jun 3, 2024

syuu1228 commented Apr 16, 2024 •

edited by yaronkaikov

Loading