Skip to content
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

dev-libs/leveldb: bump to 1.20, sub-slot to avoid ABI breakage #8848

Closed
wants to merge 10 commits into from

Conversation

sbraz
Copy link
Member

@sbraz sbraz commented Jun 16, 2018

Hi,
This bumps leveldb to 1.20. Because of an ABI change without a SONAME change, I had to introduce a sub-slot. I made sure that all reverse dependencies have the correct slot operator. I changed the stable ceph and qtwebkit straight-to-stable. Other affected ebuilds were unstable.

Changes to the ebuild:

--- leveldb-1.18-r2.ebuild	2017-11-19 00:05:14.641597450 +0100
+++ leveldb-1.20.ebuild	2018-06-16 02:28:09.808573354 +0200
@@ -1,40 +1,35 @@
-# Copyright 1999-2017 Gentoo Foundation
+# Copyright 1999-2018 Gentoo Foundation
 # Distributed under the terms of the GNU General Public License v2
 
-EAPI=5
+EAPI=7
 
-inherit eutils multilib toolchain-funcs versionator
+inherit multilib toolchain-funcs
 
 DESCRIPTION="a fast key-value storage library written at Google"
 HOMEPAGE="http://leveldb.org/ https://github.com/google/leveldb"
 SRC_URI="https://github.com/google/${PN}/archive/v${PV}.tar.gz -> ${P}.tar.gz"
 
 LICENSE="BSD"
-SLOT="0"
-KEYWORDS="amd64 arm ~arm64 ~mips ppc ppc64 x86 ~amd64-fbsd ~amd64-linux ~x86-linux"
-IUSE="+snappy static-libs +tcmalloc kernel_FreeBSD"
+# https://github.com/google/leveldb/issues/536
+SLOT="0/1"
+KEYWORDS="~amd64 ~arm ~arm64 ~mips ~ppc ~ppc64 ~x86 ~amd64-fbsd ~amd64-linux ~x86-linux"
+IUSE="+snappy static-libs kernel_FreeBSD +tcmalloc test"
 
 DEPEND="tcmalloc? ( dev-util/google-perftools )
 	snappy? (
 		app-arch/snappy:=
-		static-libs? ( app-arch/snappy[static-libs] )
 	)"
 RDEPEND="${DEPEND}"
 
-src_prepare() {
-	epatch "${FILESDIR}"/${PN}-1.18-mips.patch
-	epatch "${FILESDIR}"/${PN}-1.18-configure.patch #541186
-
-	local SHARED_MINOR=$(get_version_component_range 2)
-	sed \
-		-e "s/\(^ SHARED_MINOR =\).*/\1 ${SHARED_MINOR}/" \
-		"${FILESDIR}/${PN}-1.9.0-memenv-so.patch" > memenv-so.patch
-	epatch memenv-so.patch
-}
+# https://bugs.gentoo.org/651604
+REQUIRED_USE="snappy? ( !static-libs )"
+
+# https://github.com/google/leveldb/issues/234
+# https://github.com/google/leveldb/issues/236
+PATCHES=( "${FILESDIR}"/{${PN}-1.18-configure.patch,${P}-memenv-so.patch} )
 
 src_configure() {
 	# These vars all get picked up by build_detect_platform
-	# which the Makefile runs for us automatically.
 	tc-export AR CC CXX
 	export OPT="-DNDEBUG ${CPPFLAGS}"
 	local targetos
@@ -48,11 +43,13 @@
 	USE_SNAPPY=$(usex snappy) \
 	USE_TCMALLOC=no \
 	TMPDIR=${T} \
-	sh -x ./build_detect_platform build_config.mk ./
+	sh -x ./build_detect_platform build_config.mk ./ || die
 }
 
 src_compile() {
-	emake $(usex static-libs 'libmemenv.a' 'LIBRARY=') all libmemenv.SHARED
+	default
+	usex static-libs && emake out-static/lib{leveldb,memenv}.a
+	use test && emake static_programs
 }
 
 src_test() {
@@ -61,12 +58,12 @@
 
 src_install() {
 	insinto /usr/include
-	doins -r include/*
-	# This matches the path Debian picked.  Upstream provides no guidance.
+	doins -r include/.
+	# This matches the path Debian picked. Upstream provides no guidance.
 	insinto /usr/include/leveldb/helpers
 	doins helpers/memenv/memenv.h
 
-	dolib.so libleveldb*$(get_libname)*
-	use static-libs && dolib.a libleveldb.a libmemenv.a
-	dolib.so libmemenv*$(get_libname)*
+	dolib.so out-shared/libleveldb*$(get_libname)*
+	use static-libs && dolib.a out-static/lib{leveldb,memenv}.a
+	dolib.so out-shared/libmemenv*$(get_libname)*
 }

I've also removed a bunch of older unstable versions which aren't required by anything.

@luke-jr Once this is merged and you know leveldb 1.20 works with bitcoin, I think one of the options would be to add slot operators for virtual/bitcoin-leveldb to bitcoind and bitcoin-qt. Then, you would create a new version of the virtual that uses a new sub-slot and only corresponds to leveldb versions that have the same sub-slot, maybe something like :

SLOT="0/1"
RDEPEND="=dev-libs/leveldb-1.20:0/1"

Package-Manager: Portage-2.3.40, Repoman-2.3.9
Package-Manager: Portage-2.3.40, Repoman-2.3.9
Package-Manager: Portage-2.3.40, Repoman-2.3.9
Package-Manager: Portage-2.3.40, Repoman-2.3.9
Package-Manager: Portage-2.3.40, Repoman-2.3.9
@gentoo-bot
Copy link

Pull Request assignment

Areas affected: ebuilds, profiles
Packages affected: dev-libs/hyperleveldb, dev-libs/leveldb, dev-python/plyvel, dev-qt/qtwebkit, games-action/minetest...

@gentoo/github: Too many disjoint maintainers, disabling auto-assignment.

Bugs linked: 609966

In order to force reassignment and/or bug reference scan, please append [please reassign] to the pull request title.

@gentoo-bot gentoo-bot added maintainer-needed There is at least one affected package with no maintainer. Review it if you can. need assignment It was impossible to assign the PR correctly. Please assign it manually. bug linked Bug/Closes found in footer, and cross-linked with the PR. labels Jun 16, 2018
@sbraz
Copy link
Member Author

sbraz commented Jun 16, 2018

Ping @patricklauer

@luke-jr
Copy link
Contributor

luke-jr commented Jun 16, 2018

Might make more sense to just have bitcoin stuff depend on the virtual and dev-libs/leveldb:= ?

targetos="FreeBSD"
else
targetos="Linux"
fi
Copy link
Member

Choose a reason for hiding this comment

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

local targetos="$(usex kernel_FreeBSD FreeBSD Linux)"

}

src_install() {
insinto /usr/include
Copy link
Member

Choose a reason for hiding this comment

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

this is kinda awful, but ok...

@gentoo-repo-qa-bot
Copy link
Collaborator

Pull request CI report

Report generated at: 2018-06-17 00:33 UTC
Newest commit scanned: cfb89a6
Status: ✅ good

No issues found

@luke-jr
Copy link
Contributor

luke-jr commented Jun 17, 2018

Actually, sub-slots don't seem sufficient for this...? Stuff linking to it will break between when the update is installed, and when the rebuild finishes..

@luke-jr
Copy link
Contributor

luke-jr commented Jun 17, 2018

(whereas with a proper soname bump, preserved libs will keep them working in the interim)

@sbraz
Copy link
Member Author

sbraz commented Jun 17, 2018

@luke-jr We don't really have a better solution though. And if the programme is already running, it will keep using the deleted leveldb.so until it is closed so it's not such a big deal IMO.

@SoapGentoo
Copy link
Member

@luke-jr

  1. preserved-libs is a portage feature, not a PMS feature
  2. preserved-libs and subslots are complementary and not mutually exclusive
  3. using portage as package manager, nothing should break

Package-Manager: Portage-2.3.40, Repoman-2.3.9
Package-Manager: Portage-2.3.40, Repoman-2.3.9
Package-Manager: Portage-2.3.40, Repoman-2.3.9
* Upstream broke the ABI without changing the SONAME so we have to
  introduce a sub-slot, see
  google/leveldb#536
  All reverse dependencies should now have the slot operator set, which
  will trigger rebuilds when required. The case of bitcoin is special
  because they only support specific leveldb versions.
* Properly handle USE=static-libs when USE=snappy isn't set, change
  package.use.mask entry. See https://bugs.gentoo.org/651604
* Update the patch to build libmemenv.so which is required by bitcoin,
  see google/leveldb#236
* The patch also disables test building, allowing us to make src_compile
  more elegant.
* Remove MIPS patch which was merged: google/leveldb#272
* Bump to EAPI=7

Closes: https://bugs.gentoo.org/609966
Package-Manager: Portage-2.3.40, Repoman-2.3.9
Package-Manager: Portage-2.3.40, Repoman-2.3.9
@luke-jr
Copy link
Contributor

luke-jr commented Jun 17, 2018

@SoapGentoo The ABI is broken without a soname bump, so this will replace the current library. Replaced libraries cannot be preserved, and will be silently linked against by software built against the old version.

@sbraz sbraz deleted the leveldb branch June 26, 2018 09:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug linked Bug/Closes found in footer, and cross-linked with the PR. maintainer-needed There is at least one affected package with no maintainer. Review it if you can. need assignment It was impossible to assign the PR correctly. Please assign it manually.
Projects
None yet
5 participants