public inbox for gentoo-portage-dev@lists.gentoo.org
 help / color / mirror / Atom feed
* [gentoo-portage-dev] [PATCH] portage.versions: Drop support for non-EAPI "cvs." version prefix
@ 2017-11-27 19:09 gmt
  2017-11-27 20:01 ` [gentoo-portage-dev] [PATCH v2: fix commitmsg thinko] " gmt
  2017-11-27 23:01 ` [gentoo-portage-dev] [PATCH v3]] versions: Drop non-PMS "cvs." prefix in ${PV} gmt
  0 siblings, 2 replies; 4+ messages in thread
From: gmt @ 2017-11-27 19:09 UTC (permalink / raw
  To: gentoo-portage-dev

From: "Gregory M. Turner" <gmt@be-evil.net>

This feature was introduced 12 years ago in (the cvs commit
corresponding to the git commit) 9f3a46665c.  There are a lot of
reasons not to continue to support it:

  o EAPI permits no such prefix
  o Nobody uses it (perhaps nobody /ever/ used it)
  o It treats cvs as special, which doesn't make a ton of
    sense in 2017
  o If this prefix /were/ added to PMS, it seems* to create
    ambiguity between PN and V in obscure EAPIs which support
    dots in package names

Therefore, remove it from from the version regular expression and
renumber the constants referring to the affected re groups.

*PMS would barely avoid true abiguity as §3.1.2 has a rule that
logically necessitates that any such ambiguity must be resolved in
favor of V.  Although this appears to be by design, expecting
people to figure this out seems a tad optimistic.

Signed-off-by: Gregory M. Turner <gmt@be-evil.net>
---
 pym/portage/tests/versions/test_vercmp.py |  3 ---
 pym/portage/versions.py                   | 38 +++++++++++++------------------
 2 files changed, 16 insertions(+), 25 deletions(-)

diff --git a/pym/portage/tests/versions/test_vercmp.py b/pym/portage/tests/versions/test_vercmp.py
index 78fe7ede8..b55518f02 100644
--- a/pym/portage/tests/versions/test_vercmp.py
+++ b/pym/portage/tests/versions/test_vercmp.py
@@ -15,7 +15,6 @@ class VerCmpTestCase(TestCase):
 			("6.0", "5.0"), ("5.0", "5"),
 			("1.0-r1", "1.0-r0"),
 			("1.0-r1", "1.0"),
-			("cvs.9999", "9999"),
 			("999999999999999999999999999999", "999999999999999999999999999998"),
 			("1.0.0", "1.0"),
 			("1.0.0", "1.0b"),
@@ -36,7 +35,6 @@ class VerCmpTestCase(TestCase):
 			("1.0_alpha2", "1.0_p2"), ("1.0_alpha1", "1.0_beta1"), ("1.0_beta3", "1.0_rc3"),
 			("1.001000000000000000001", "1.001000000000000000002"),
 			("1.00100000000", "1.0010000000000000001"),
-			("9999", "cvs.9999"),
 			("999999999999999999999999999998", "999999999999999999999999999999"),
 			("1.01", "1.1"),
 			("1.0-r0", "1.0-r1"),
@@ -69,7 +67,6 @@ class VerCmpTestCase(TestCase):
 		tests = [
 			("1", "2"), ("1.0_alpha", "1.0_pre"), ("1.0_beta", "1.0_alpha"),
 			("0", "0.0"),
-			("cvs.9999", "9999"),
 			("1.0-r0", "1.0-r1"),
 			("1.0-r1", "1.0-r0"),
 			("1.0", "1.0-r1"),
diff --git a/pym/portage/versions.py b/pym/portage/versions.py
index adfb1c3e2..7b6a57673 100644
--- a/pym/portage/versions.py
+++ b/pym/portage/versions.py
@@ -50,7 +50,7 @@ _pkg = {
 	"dots_allowed_in_PN":    r'[\w+][\w+.-]*?',
 }
 
-_v = r'(cvs\.)?(\d+)((\.\d+)*)([a-z]?)((_(pre|p|beta|alpha|rc)\d*)*)'
+_v = r'(\d+)((\.\d+)*)([a-z]?)((_(pre|p|beta|alpha|rc)\d*)*)'
 _rev = r'\d+'
 _vr = _v + '(-r(' + _rev + '))?'
 
@@ -156,21 +156,15 @@ def vercmp(ver1, ver2, silent=1):
 			print(_("!!! syntax error in version: %s") % ver2)
 		return None
 
-	# shortcut for cvs ebuilds (new style)
-	if match1.group(1) and not match2.group(1):
-		return 1
-	elif match2.group(1) and not match1.group(1):
-		return -1
-
 	# building lists of the version parts before the suffix
 	# first part is simple
-	list1 = [int(match1.group(2))]
-	list2 = [int(match2.group(2))]
+	list1 = [int(match1.group(1))]
+	list2 = [int(match2.group(1))]
 
 	# this part would greatly benefit from a fixed-length version pattern
-	if match1.group(3) or match2.group(3):
-		vlist1 = match1.group(3)[1:].split(".")
-		vlist2 = match2.group(3)[1:].split(".")
+	if match1.group(2) or match2.group(2):
+		vlist1 = match1.group(2)[1:].split(".")
+		vlist2 = match2.group(2)[1:].split(".")
 
 		for i in range(0, max(len(vlist1), len(vlist2))):
 			# Implcit .0 is given a value of -1, so that 1.0.0 > 1.0, since it
@@ -206,10 +200,10 @@ def vercmp(ver1, ver2, silent=1):
 	# may seem counter-intuitive. However, if you really think about it, it
 	# seems like it's probably safe to assume that this is the behavior that
 	# is intended by anyone who would use versions such as these.
-	if len(match1.group(5)):
-		list1.append(ord(match1.group(5)))
-	if len(match2.group(5)):
-		list2.append(ord(match2.group(5)))
+	if len(match1.group(4)):
+		list1.append(ord(match1.group(4)))
+	if len(match2.group(4)):
+		list2.append(ord(match2.group(4)))
 
 	for i in range(0, max(len(list1), len(list2))):
 		if len(list1) <= i:
@@ -223,8 +217,8 @@ def vercmp(ver1, ver2, silent=1):
 			return rval
 
 	# main version is equal, so now compare the _suffix part
-	list1 = match1.group(6).split("_")[1:]
-	list2 = match2.group(6).split("_")[1:]
+	list1 = match1.group(5).split("_")[1:]
+	list2 = match2.group(5).split("_")[1:]
 
 	for i in range(0, max(len(list1), len(list2))):
 		# Implicit _p0 is given a value of -1, so that 1 < 1_p0
@@ -257,12 +251,12 @@ def vercmp(ver1, ver2, silent=1):
 				return rval
 
 	# the suffix part is equal to, so finally check the revision
-	if match1.group(10):
-		r1 = int(match1.group(10))
+	if match1.group(9):
+		r1 = int(match1.group(9))
 	else:
 		r1 = 0
-	if match2.group(10):
-		r2 = int(match2.group(10))
+	if match2.group(9):
+		r2 = int(match2.group(9))
 	else:
 		r2 = 0
 	rval = (r1 > r2) - (r1 < r2)
-- 
2.15.0



^ permalink raw reply related	[flat|nested] 4+ messages in thread

* [gentoo-portage-dev] [PATCH v2: fix commitmsg thinko] portage.versions: Drop support for non-EAPI "cvs." version prefix
  2017-11-27 19:09 [gentoo-portage-dev] [PATCH] portage.versions: Drop support for non-EAPI "cvs." version prefix gmt
@ 2017-11-27 20:01 ` gmt
  2017-11-27 23:01 ` [gentoo-portage-dev] [PATCH v3]] versions: Drop non-PMS "cvs." prefix in ${PV} gmt
  1 sibling, 0 replies; 4+ messages in thread
From: gmt @ 2017-11-27 20:01 UTC (permalink / raw
  To: gentoo-portage-dev

From: "Gregory M. Turner" <gmt@be-evil.net>

This feature was introduced 12 years ago in (the cvs commit
corresponding to the git commit) 9f3a46665c.  There are a lot of
reasons not to continue to support it:

  o PMS permits no such prefix
  o Nobody uses it (perhaps nobody /ever/ used it)
  o It treats cvs as special, which doesn't make a ton of
    sense in 2017
  o If this prefix /were/ added to PMS, it seems* to create
    ambiguity between PN and V in obscure EAPIs which support
    dots in package names

Therefore, remove it from from the version regular expression and
renumber the constants referring to the affected re groups.

*PMS would barely avoid true abiguity as §3.1.2 has a rule that
logically necessitates that any such ambiguity must be resolved in
favor of V.  Although this appears to be by design, expecting
people to figure this out seems a tad optimistic.

Signed-off-by: Gregory M. Turner <gmt@be-evil.net>
---
 pym/portage/tests/versions/test_vercmp.py |  3 ---
 pym/portage/versions.py                   | 38 +++++++++++++------------------
 2 files changed, 16 insertions(+), 25 deletions(-)

diff --git a/pym/portage/tests/versions/test_vercmp.py b/pym/portage/tests/versions/test_vercmp.py
index 78fe7ede8..b55518f02 100644
--- a/pym/portage/tests/versions/test_vercmp.py
+++ b/pym/portage/tests/versions/test_vercmp.py
@@ -15,7 +15,6 @@ class VerCmpTestCase(TestCase):
 			("6.0", "5.0"), ("5.0", "5"),
 			("1.0-r1", "1.0-r0"),
 			("1.0-r1", "1.0"),
-			("cvs.9999", "9999"),
 			("999999999999999999999999999999", "999999999999999999999999999998"),
 			("1.0.0", "1.0"),
 			("1.0.0", "1.0b"),
@@ -36,7 +35,6 @@ class VerCmpTestCase(TestCase):
 			("1.0_alpha2", "1.0_p2"), ("1.0_alpha1", "1.0_beta1"), ("1.0_beta3", "1.0_rc3"),
 			("1.001000000000000000001", "1.001000000000000000002"),
 			("1.00100000000", "1.0010000000000000001"),
-			("9999", "cvs.9999"),
 			("999999999999999999999999999998", "999999999999999999999999999999"),
 			("1.01", "1.1"),
 			("1.0-r0", "1.0-r1"),
@@ -69,7 +67,6 @@ class VerCmpTestCase(TestCase):
 		tests = [
 			("1", "2"), ("1.0_alpha", "1.0_pre"), ("1.0_beta", "1.0_alpha"),
 			("0", "0.0"),
-			("cvs.9999", "9999"),
 			("1.0-r0", "1.0-r1"),
 			("1.0-r1", "1.0-r0"),
 			("1.0", "1.0-r1"),
diff --git a/pym/portage/versions.py b/pym/portage/versions.py
index adfb1c3e2..7b6a57673 100644
--- a/pym/portage/versions.py
+++ b/pym/portage/versions.py
@@ -50,7 +50,7 @@ _pkg = {
 	"dots_allowed_in_PN":    r'[\w+][\w+.-]*?',
 }
 
-_v = r'(cvs\.)?(\d+)((\.\d+)*)([a-z]?)((_(pre|p|beta|alpha|rc)\d*)*)'
+_v = r'(\d+)((\.\d+)*)([a-z]?)((_(pre|p|beta|alpha|rc)\d*)*)'
 _rev = r'\d+'
 _vr = _v + '(-r(' + _rev + '))?'
 
@@ -156,21 +156,15 @@ def vercmp(ver1, ver2, silent=1):
 			print(_("!!! syntax error in version: %s") % ver2)
 		return None
 
-	# shortcut for cvs ebuilds (new style)
-	if match1.group(1) and not match2.group(1):
-		return 1
-	elif match2.group(1) and not match1.group(1):
-		return -1
-
 	# building lists of the version parts before the suffix
 	# first part is simple
-	list1 = [int(match1.group(2))]
-	list2 = [int(match2.group(2))]
+	list1 = [int(match1.group(1))]
+	list2 = [int(match2.group(1))]
 
 	# this part would greatly benefit from a fixed-length version pattern
-	if match1.group(3) or match2.group(3):
-		vlist1 = match1.group(3)[1:].split(".")
-		vlist2 = match2.group(3)[1:].split(".")
+	if match1.group(2) or match2.group(2):
+		vlist1 = match1.group(2)[1:].split(".")
+		vlist2 = match2.group(2)[1:].split(".")
 
 		for i in range(0, max(len(vlist1), len(vlist2))):
 			# Implcit .0 is given a value of -1, so that 1.0.0 > 1.0, since it
@@ -206,10 +200,10 @@ def vercmp(ver1, ver2, silent=1):
 	# may seem counter-intuitive. However, if you really think about it, it
 	# seems like it's probably safe to assume that this is the behavior that
 	# is intended by anyone who would use versions such as these.
-	if len(match1.group(5)):
-		list1.append(ord(match1.group(5)))
-	if len(match2.group(5)):
-		list2.append(ord(match2.group(5)))
+	if len(match1.group(4)):
+		list1.append(ord(match1.group(4)))
+	if len(match2.group(4)):
+		list2.append(ord(match2.group(4)))
 
 	for i in range(0, max(len(list1), len(list2))):
 		if len(list1) <= i:
@@ -223,8 +217,8 @@ def vercmp(ver1, ver2, silent=1):
 			return rval
 
 	# main version is equal, so now compare the _suffix part
-	list1 = match1.group(6).split("_")[1:]
-	list2 = match2.group(6).split("_")[1:]
+	list1 = match1.group(5).split("_")[1:]
+	list2 = match2.group(5).split("_")[1:]
 
 	for i in range(0, max(len(list1), len(list2))):
 		# Implicit _p0 is given a value of -1, so that 1 < 1_p0
@@ -257,12 +251,12 @@ def vercmp(ver1, ver2, silent=1):
 				return rval
 
 	# the suffix part is equal to, so finally check the revision
-	if match1.group(10):
-		r1 = int(match1.group(10))
+	if match1.group(9):
+		r1 = int(match1.group(9))
 	else:
 		r1 = 0
-	if match2.group(10):
-		r2 = int(match2.group(10))
+	if match2.group(9):
+		r2 = int(match2.group(9))
 	else:
 		r2 = 0
 	rval = (r1 > r2) - (r1 < r2)
-- 
2.15.0



^ permalink raw reply related	[flat|nested] 4+ messages in thread

* [gentoo-portage-dev] [PATCH v3]] versions: Drop non-PMS "cvs." prefix in ${PV}
  2017-11-27 19:09 [gentoo-portage-dev] [PATCH] portage.versions: Drop support for non-EAPI "cvs." version prefix gmt
  2017-11-27 20:01 ` [gentoo-portage-dev] [PATCH v2: fix commitmsg thinko] " gmt
@ 2017-11-27 23:01 ` gmt
  2017-11-28 22:30   ` Zac Medico
  1 sibling, 1 reply; 4+ messages in thread
From: gmt @ 2017-11-27 23:01 UTC (permalink / raw
  To: gentoo-portage-dev

From: "Gregory M. Turner" <gmt@be-evil.net>

This feature was introduced 12 years ago in (the cvs commit
corresponding to the git commit) 9f3a46665c.  There are a lot
of reasons not to keep it around:

  o PMS permits no such prefix in ${PV}
  o Apparently nobody uses it (perhaps nobody /ever/ used it)
  o It special-cases cvs, which nobody uses, either, in 2017
  o Almost* causes ambiguity between ${PN} and ${PV} in ${P} if
    some future EAPI tried to support both this and dots in
    package names simultaneously.

Therefore, remove it from from the "_v" regular expression,
renumber hard-coded group indexes and nuke corresponding tests.

*PMS would technically avoid abiguity as §3.1.2 requires ${PV}
to "win" any such conflict over contested bits in the middle of
${P}.  However, clearly it's prefereable for this rule to be as
redundant as possible.

Note: this is my 3rd spin of this commitmsg; it's coming along
nicely I think :P

Signed-off-by: Gregory M. Turner <gmt@be-evil.net>
---
 pym/portage/tests/versions/test_vercmp.py |  3 ---
 pym/portage/versions.py                   | 38 +++++++++++++------------------
 2 files changed, 16 insertions(+), 25 deletions(-)

diff --git a/pym/portage/tests/versions/test_vercmp.py b/pym/portage/tests/versions/test_vercmp.py
index 78fe7ede8..b55518f02 100644
--- a/pym/portage/tests/versions/test_vercmp.py
+++ b/pym/portage/tests/versions/test_vercmp.py
@@ -15,7 +15,6 @@ class VerCmpTestCase(TestCase):
 			("6.0", "5.0"), ("5.0", "5"),
 			("1.0-r1", "1.0-r0"),
 			("1.0-r1", "1.0"),
-			("cvs.9999", "9999"),
 			("999999999999999999999999999999", "999999999999999999999999999998"),
 			("1.0.0", "1.0"),
 			("1.0.0", "1.0b"),
@@ -36,7 +35,6 @@ class VerCmpTestCase(TestCase):
 			("1.0_alpha2", "1.0_p2"), ("1.0_alpha1", "1.0_beta1"), ("1.0_beta3", "1.0_rc3"),
 			("1.001000000000000000001", "1.001000000000000000002"),
 			("1.00100000000", "1.0010000000000000001"),
-			("9999", "cvs.9999"),
 			("999999999999999999999999999998", "999999999999999999999999999999"),
 			("1.01", "1.1"),
 			("1.0-r0", "1.0-r1"),
@@ -69,7 +67,6 @@ class VerCmpTestCase(TestCase):
 		tests = [
 			("1", "2"), ("1.0_alpha", "1.0_pre"), ("1.0_beta", "1.0_alpha"),
 			("0", "0.0"),
-			("cvs.9999", "9999"),
 			("1.0-r0", "1.0-r1"),
 			("1.0-r1", "1.0-r0"),
 			("1.0", "1.0-r1"),
diff --git a/pym/portage/versions.py b/pym/portage/versions.py
index adfb1c3e2..7b6a57673 100644
--- a/pym/portage/versions.py
+++ b/pym/portage/versions.py
@@ -50,7 +50,7 @@ _pkg = {
 	"dots_allowed_in_PN":    r'[\w+][\w+.-]*?',
 }
 
-_v = r'(cvs\.)?(\d+)((\.\d+)*)([a-z]?)((_(pre|p|beta|alpha|rc)\d*)*)'
+_v = r'(\d+)((\.\d+)*)([a-z]?)((_(pre|p|beta|alpha|rc)\d*)*)'
 _rev = r'\d+'
 _vr = _v + '(-r(' + _rev + '))?'
 
@@ -156,21 +156,15 @@ def vercmp(ver1, ver2, silent=1):
 			print(_("!!! syntax error in version: %s") % ver2)
 		return None
 
-	# shortcut for cvs ebuilds (new style)
-	if match1.group(1) and not match2.group(1):
-		return 1
-	elif match2.group(1) and not match1.group(1):
-		return -1
-
 	# building lists of the version parts before the suffix
 	# first part is simple
-	list1 = [int(match1.group(2))]
-	list2 = [int(match2.group(2))]
+	list1 = [int(match1.group(1))]
+	list2 = [int(match2.group(1))]
 
 	# this part would greatly benefit from a fixed-length version pattern
-	if match1.group(3) or match2.group(3):
-		vlist1 = match1.group(3)[1:].split(".")
-		vlist2 = match2.group(3)[1:].split(".")
+	if match1.group(2) or match2.group(2):
+		vlist1 = match1.group(2)[1:].split(".")
+		vlist2 = match2.group(2)[1:].split(".")
 
 		for i in range(0, max(len(vlist1), len(vlist2))):
 			# Implcit .0 is given a value of -1, so that 1.0.0 > 1.0, since it
@@ -206,10 +200,10 @@ def vercmp(ver1, ver2, silent=1):
 	# may seem counter-intuitive. However, if you really think about it, it
 	# seems like it's probably safe to assume that this is the behavior that
 	# is intended by anyone who would use versions such as these.
-	if len(match1.group(5)):
-		list1.append(ord(match1.group(5)))
-	if len(match2.group(5)):
-		list2.append(ord(match2.group(5)))
+	if len(match1.group(4)):
+		list1.append(ord(match1.group(4)))
+	if len(match2.group(4)):
+		list2.append(ord(match2.group(4)))
 
 	for i in range(0, max(len(list1), len(list2))):
 		if len(list1) <= i:
@@ -223,8 +217,8 @@ def vercmp(ver1, ver2, silent=1):
 			return rval
 
 	# main version is equal, so now compare the _suffix part
-	list1 = match1.group(6).split("_")[1:]
-	list2 = match2.group(6).split("_")[1:]
+	list1 = match1.group(5).split("_")[1:]
+	list2 = match2.group(5).split("_")[1:]
 
 	for i in range(0, max(len(list1), len(list2))):
 		# Implicit _p0 is given a value of -1, so that 1 < 1_p0
@@ -257,12 +251,12 @@ def vercmp(ver1, ver2, silent=1):
 				return rval
 
 	# the suffix part is equal to, so finally check the revision
-	if match1.group(10):
-		r1 = int(match1.group(10))
+	if match1.group(9):
+		r1 = int(match1.group(9))
 	else:
 		r1 = 0
-	if match2.group(10):
-		r2 = int(match2.group(10))
+	if match2.group(9):
+		r2 = int(match2.group(9))
 	else:
 		r2 = 0
 	rval = (r1 > r2) - (r1 < r2)
-- 
2.15.0



^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [gentoo-portage-dev] [PATCH v3]] versions: Drop non-PMS "cvs." prefix in ${PV}
  2017-11-27 23:01 ` [gentoo-portage-dev] [PATCH v3]] versions: Drop non-PMS "cvs." prefix in ${PV} gmt
@ 2017-11-28 22:30   ` Zac Medico
  0 siblings, 0 replies; 4+ messages in thread
From: Zac Medico @ 2017-11-28 22:30 UTC (permalink / raw
  To: gentoo-portage-dev, gmt


[-- Attachment #1.1: Type: text/plain, Size: 1258 bytes --]

On 11/27/2017 03:01 PM, gmt@malth.us wrote:
> From: "Gregory M. Turner" <gmt@be-evil.net>
> 
> This feature was introduced 12 years ago in (the cvs commit
> corresponding to the git commit) 9f3a46665c.  There are a lot
> of reasons not to keep it around:
> 
>   o PMS permits no such prefix in ${PV}
>   o Apparently nobody uses it (perhaps nobody /ever/ used it)
>   o It special-cases cvs, which nobody uses, either, in 2017
>   o Almost* causes ambiguity between ${PN} and ${PV} in ${P} if
>     some future EAPI tried to support both this and dots in
>     package names simultaneously.
> 
> Therefore, remove it from from the "_v" regular expression,
> renumber hard-coded group indexes and nuke corresponding tests.
> 
> *PMS would technically avoid abiguity as §3.1.2 requires ${PV}
> to "win" any such conflict over contested bits in the middle of
> ${P}.  However, clearly it's prefereable for this rule to be as
> redundant as possible.
> 
> Note: this is my 3rd spin of this commitmsg; it's coming along
> nicely I think :P
> 
> Signed-off-by: Gregory M. Turner <gmt@be-evil.net>

Thanks, merged:

https://gitweb.gentoo.org/proj/portage.git/commit/?id=77d8abd747a15f90d7a45c334efecaf47261501a

-- 
Thanks,
Zac


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 224 bytes --]

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2017-11-28 22:30 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-11-27 19:09 [gentoo-portage-dev] [PATCH] portage.versions: Drop support for non-EAPI "cvs." version prefix gmt
2017-11-27 20:01 ` [gentoo-portage-dev] [PATCH v2: fix commitmsg thinko] " gmt
2017-11-27 23:01 ` [gentoo-portage-dev] [PATCH v3]] versions: Drop non-PMS "cvs." prefix in ${PV} gmt
2017-11-28 22:30   ` Zac Medico

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox