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

sys-libs/db: port to EAPI 6 #8315

Closed
wants to merge 10,000 commits into from
Closed

Conversation

hanetzer
Copy link
Contributor

@hanetzer hanetzer commented May 8, 2018

There are some problems with this one I can't quite solve myself. The upstream
patches seem to be formatted to be -p0, which means they can't be just slapped
into the PATCHES array. One suggestion would be to mirror them in a devspace,
modified to be -p1 applicable (and preferably converted to a unified diff format,
as the context diff used by upstream is 1. very large relatively speaking to a unified
diff and 2. difficult to easily assess visually, as compared to unified diffs).

See also #8314

mattst88 added 30 commits May 6, 2018 13:44
Whissi and others added 7 commits May 8, 2018 01:53
Package-Manager: Portage-2.3.34, Repoman-2.3.9
Package-Manager: Portage-2.3.34, Repoman-2.3.9
Bug: https://bugs.gentoo.org/655110
Package-Manager: Portage-2.3.34, Repoman-2.3.9
Closes: https://bugs.gentoo.org/655110
Package-Manager: Portage-2.3.34, Repoman-2.3.9
RepoMan-Options: --force
Package-Manager: Portage-2.3.24, Repoman-2.3.6
Package-Manager: Portage-2.3.24, Repoman-2.3.6
Package-Manager: Portage-2.3.24, Repoman-2.3.6
@gentoo-repo-qa-bot gentoo-repo-qa-bot added assigned PR successfully assigned to the package maintainer(s). no bug found No Bug/Closes found in the commits. labels May 8, 2018
@Whissi
Copy link
Contributor

Whissi commented May 8, 2018

Are you willing to contribute the reformatted patch? If it will be to large, I/we will host it following Gentoo rules.

@hanetzer
Copy link
Contributor Author

hanetzer commented May 8, 2018

@Whissi yeah, I can do it. the sizes will be cut pretty drastically by changing from a context
diff to a unified one, but the ebuilds here touched don't currently have one.

@robbat2
Copy link
Contributor

robbat2 commented May 8, 2018

Please create a bug for this, and include a diff between ebuild versions for easier review. There's probably a way to avoid reformatting the patch, but I'll have to look a little closer (I would prefer it remain untouched esp. that the newer versions of db upstream went to AGPL).

@hanetzer
Copy link
Contributor Author

hanetzer commented May 8, 2018

@robbat2 Will do, and there probably is a way to do it without reformatting (hypothetical)
patches, it will likely be ugly af and as I mentioned, a unified diff is smaller than a context
diff by about 1/3 to 1/2, depending.

Closes: https://bugs.gentoo.org/655302

Package-Manager: Portage-2.3.36, Repoman-2.3.9
@hanetzer hanetzer changed the title sys-libs/db: port to EAPI 6 sys-libs/db: port to EAPI 6 [please reassign] May 9, 2018
@gentoo-repo-qa-bot gentoo-repo-qa-bot changed the title sys-libs/db: port to EAPI 6 [please reassign] sys-libs/db: port to EAPI 6 May 9, 2018
@gentoo-repo-qa-bot
Copy link
Collaborator

Pull Request assignment

Areas affected: ebuilds
Packages affected: sys-libs/db

sys-libs/db: @gentoo/base-system

Bugs linked: 655302

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


Gentoo Mirror & CI services are provided by Michał Górny. The hardware was kindly provided by Todd Goodman. This unofficial service is not associated with Gentoo Infrastructure or Gentoo Foundation.

This service is provided by the service provider "as is" and any express or implied warranties, including, but not limited to, the implied warranties of merchantability and fitness for a particular purpose are disclaimed. In no event shall the service provider be liable for any direct, indirect, incidental, special, exemplary, or consequential damages (including, but not limited to, procurement of substitute goods or services; loss of use, data, or profits; or business interruption) however caused and on any theory of liability, whether in contract, strict liability, or tort (including negligence or otherwise) arising in any way out of the use of this service, even if advised of the possibility of such damage.

@gentoo-repo-qa-bot gentoo-repo-qa-bot added assigned PR successfully assigned to the package maintainer(s). bug linked Bug/Closes found in footer, and cross-linked with the PR. and removed assigned PR successfully assigned to the package maintainer(s). no bug found No Bug/Closes found in the commits. labels May 9, 2018
@gentoo-repo-qa-bot
Copy link
Collaborator

Pull request CI report

Report generated at: 2018-05-09 02:00 UTC
Newest commit scanned: 98dcbfd
Status: ✅ good

No issues found


Gentoo Mirror & CI services are provided by Michał Górny. The hardware was kindly provided by Todd Goodman. This unofficial service is not associated with Gentoo Infrastructure or Gentoo Foundation.

This service is provided by the service provider "as is" and any express or implied warranties, including, but not limited to, the implied warranties of merchantability and fitness for a particular purpose are disclaimed. In no event shall the service provider be liable for any direct, indirect, incidental, special, exemplary, or consequential damages (including, but not limited to, procurement of substitute goods or services; loss of use, data, or profits; or business interruption) however caused and on any theory of liability, whether in contract, strict liability, or tort (including negligence or otherwise) arising in any way out of the use of this service, even if advised of the possibility of such damage.

@hanetzer
Copy link
Contributor Author

hanetzer commented May 10, 2018

@Whissi @robbat2 been doing a bit of research. One can use quilt to easily convert between
a context and unified diff. Given sys-libs/db-4.3.29_p1-r1 as an example, one would convert the
patch like so:

$ ebuild db-4.3.29_p1-r1.ebuild clean unpack
$ cd $PORTAGE_TMPDIR/sys-libs/db-4.3.29_p1-r1/work/db-4.3.29
# this being EAPI=0 patching happens in src_unpack, which is unfortunate. However, we can
# use patch's -R option to reverse the patches applied. Its probably best to reverse patches
# in the opposite order they are applied. Newer EAPI should not do this, so the following
# step is unneeded for them, but I put it here for documentations sake.
$ patch -p0 -R < ../../distdir/patch.4.3.29.1
$ quilt new patch.4.3.29.1-r1.patch # new name to not conflict with original
# We pass --quiltrc - to ignore both the users and the system's quiltrc files to avoid
# contaminating ''official'' patches with user preferences, if any, and the system
# quiltrc can cause issues as well. -p0 because the original patch is -p0 format
$ quilt --quiltrc - fold -p0 < ../../distdir/patch.4.3.29.1
$ quilt refresh
# your new -p1 unified diff can be found at
# $PORTAGE_TMPDIR/sys-libs/db-4.3.29_p1-r1/work/db-4.3.29/patches/patch.4.3.29.1-r1.patch
$ wc -c ../../distdir/patch.4.3.29.1
835 ../../distdir/patch.4.3.29.1
$ wc -c patches/patch.4.3.29.1-r1.patch
557 patches/patch.4.3.29.1-r1.patch
# An approximate 1/3 size reduction

Before:

*** mp/mp_fget.c.orig	2006-05-30 20:44:11.000000000 -0700
--- mp/mp_fget.c	2006-05-30 20:44:22.000000000 -0700
***************
*** 577,584 ****
  	 */
  	if (state != SECOND_MISS && bhp->ref == 1) {
  		bhp->priority = UINT32_MAX;
! 		SH_TAILQ_REMOVE(&hp->hash_bucket, bhp, hq, __bh);
! 		SH_TAILQ_INSERT_TAIL(&hp->hash_bucket, bhp, hq);
  		hp->hash_priority =
  		    SH_TAILQ_FIRST(&hp->hash_bucket, __bh)->priority;
  	}
--- 577,587 ----
  	 */
  	if (state != SECOND_MISS && bhp->ref == 1) {
  		bhp->priority = UINT32_MAX;
! 		if (SH_TAILQ_FIRST(&hp->hash_bucket, __bh) !=
! 		     SH_TAILQ_LAST(&hp->hash_bucket, hq, __bh)) {
! 			SH_TAILQ_REMOVE(&hp->hash_bucket, bhp, hq, __bh);
! 			SH_TAILQ_INSERT_TAIL(&hp->hash_bucket, bhp, hq);
! 		}
  		hp->hash_priority =
  		    SH_TAILQ_FIRST(&hp->hash_bucket, __bh)->priority;
  	}

After:

--- a/mp/mp_fget.c
+++ b/mp/mp_fget.c
@@ -577,8 +577,11 @@ alloc:		/*
 	 */
 	if (state != SECOND_MISS && bhp->ref == 1) {
 		bhp->priority = UINT32_MAX;
-		SH_TAILQ_REMOVE(&hp->hash_bucket, bhp, hq, __bh);
-		SH_TAILQ_INSERT_TAIL(&hp->hash_bucket, bhp, hq);
+		if (SH_TAILQ_FIRST(&hp->hash_bucket, __bh) !=
+		     SH_TAILQ_LAST(&hp->hash_bucket, hq, __bh)) {
+			SH_TAILQ_REMOVE(&hp->hash_bucket, bhp, hq, __bh);
+			SH_TAILQ_INSERT_TAIL(&hp->hash_bucket, bhp, hq);
+		}
 		hp->hash_priority =
 		    SH_TAILQ_FIRST(&hp->hash_bucket, __bh)->priority;
 	}

@hanetzer
Copy link
Contributor Author

Oh! I just found an even better way. Using dev-util/patchutils-0.3.4::gentoo, you will need
to remove lines like diff -ur SDL2-2.0.8/src/SDL.c SDL2-2.0.8.new/src/SDL.c first, but after
that, its as simple as filterdiff --format=unified < patch.4.3.29.1 > patch.4.3.29.1-r1.patch
You can even do this within vim with :%!filterdiff --format=unified

@robbat2
Copy link
Contributor

robbat2 commented May 10, 2018

Yep, patchutils is one of the conversion tools. I was planning on using it during src_prepare and keeping upstream's patches pristine until that (so it would convert every time, and we would not mirror patches with AGPL issues)

@hanetzer
Copy link
Contributor Author

What problem does the AGPL bring here? And while converting from context to unified
is useful, the real importance here is -p0 to -p1, and we can't always be certain that every
patch coming from that upstream is going to be -p0, so scripting a solution may be more
work than needed.

@robbat2
Copy link
Contributor

robbat2 commented May 10, 2018

There was at least one patch I saw that was a backport from DB-6 to older DB versions, which would have a nasty side-effect of bringing AGPL to the older versions. filterdiff can also convert -pM -> -pN where N > M.

@hanetzer
Copy link
Contributor Author

@robbat2 I'm purely talking about the patches we currently use. If we're already using them, that
'side effect' has already long happened.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
assigned PR successfully assigned to the package maintainer(s). bug linked Bug/Closes found in footer, and cross-linked with the PR.
Projects
None yet