[gentoo-portage-dev] [PATCH] match_from_list: restrict =* to match only on version part boundaries (bug 560466)

[gentoo-portage-dev] [PATCH] match_from_list: restrict =* to match only on version part boundaries (bug 560466)

[Zac Medico]

Make the =* glob match only on boundaries between version parts, in order
to eliminate ambiguity (so that 1* does not match version 10). Only break
compatibility in cases where dependencies have been specified ambiguously.

X-Gentoo-Bug: 560466
X-Gentoo-Bug-url: https://bugs.gentoo.org/show_bug.cgi?id=560466
---
 pym/portage/dep/__init__.py                   |  7 ++++++-
 pym/portage/tests/dep/test_match_from_list.py | 21 +++++++++++++++------
 2 files changed, 21 insertions(+), 7 deletions(-)

diff --git a/pym/portage/dep/__init__.py b/pym/portage/dep/__init__.py
index 0a13d9f..5dd1638 100644
--- a/pym/portage/dep/__init__.py
+++ b/pym/portage/dep/__init__.py
@@ -2250,7 +2250,12 @@ def match_from_list(mydep, candidate_list):
 					pkg.cp + "-" + xs[2],
 					pkg.cp + "-" + myver, 1)
 			if xcpv.startswith(mycpv_cmp):
-				mylist.append(x)
+				# =* glob matches only on boundaries between version parts,
+				# so 1* does not match 10 (bug 560466).
+				next_char = xcpv[len(mycpv_cmp):len(mycpv_cmp)+1]
+				if (not next_char or next_char in '._-' or
+					mycpv_cmp[-1].isdigit() != next_char.isdigit()):
+					mylist.append(x)
 
 	elif operator == "~": # version, any revision, match
 		for x in candidate_list:
diff --git a/pym/portage/tests/dep/test_match_from_list.py b/pym/portage/tests/dep/test_match_from_list.py
index 75ac8fd..3080479 100644
--- a/pym/portage/tests/dep/test_match_from_list.py
+++ b/pym/portage/tests/dep/test_match_from_list.py
@@ -73,12 +73,21 @@ class Test_match_from_list(TestCase):
 			("sys-apps/portage:0", [Package("=sys-apps/portage-045:0")], ["sys-apps/portage-045"]),
 			("sys-apps/portage:0", [Package("=sys-apps/portage-045:1")], []),
 			("=cat/pkg-1-r1*", ["cat/pkg-1_alpha1"], []),
-			("=cat/pkg-1-r1*", ["cat/pkg-1-r11"], ["cat/pkg-1-r11"]),
-			("=cat/pkg-1-r1*", ["cat/pkg-01-r11"], ["cat/pkg-01-r11"]),
-			("=cat/pkg-01-r1*", ["cat/pkg-1-r11"], ["cat/pkg-1-r11"]),
-			("=cat/pkg-01-r1*", ["cat/pkg-001-r11"], ["cat/pkg-001-r11"]),
-			("=sys-fs/udev-1*", ["sys-fs/udev-123"], ["sys-fs/udev-123"]),
-			("=sys-fs/udev-4*", ["sys-fs/udev-456"], ["sys-fs/udev-456"]),
+			# =* glob matches only on boundaries between version parts,
+			# so 1* does not match 10 (bug 560466).
+			("=cat/pkg-1.1*", ["cat/pkg-1.1-r1", "cat/pkg-1.10-r1"], ["cat/pkg-1.1-r1"]),
+			("=cat/pkg-1-r1*", ["cat/pkg-1-r11"], []),
+			("=cat/pkg-1_pre*", ["cat/pkg-1_pre1"], ["cat/pkg-1_pre1"]),
+			("=cat/pkg-1-r1*", ["cat/pkg-1-r1"], ["cat/pkg-1-r1"]),
+			("=cat/pkg-1-r11*", ["cat/pkg-1-r11"], ["cat/pkg-1-r11"]),
+			("=cat/pkg-1-r11*", ["cat/pkg-01-r11"], ["cat/pkg-01-r11"]),
+			("=cat/pkg-01-r11*", ["cat/pkg-1-r11"], ["cat/pkg-1-r11"]),
+			("=cat/pkg-01-r11*", ["cat/pkg-001-r11"], ["cat/pkg-001-r11"]),
+			("=sys-fs/udev-1*", ["sys-fs/udev-123", "sys-fs/udev-123-r1"], []),
+			("=sys-fs/udev-123*", ["sys-fs/udev-123"], ["sys-fs/udev-123"]),
+			("=sys-fs/udev-123*", ["sys-fs/udev-123-r1"], ["sys-fs/udev-123-r1"]),
+			("=sys-fs/udev-4*", ["sys-fs/udev-456", "sys-fs/udev-456-r1"], []),
+			("=sys-fs/udev-456*", ["sys-fs/udev-456"], ["sys-fs/udev-456"]),
 			("*/*", ["sys-fs/udev-456"], ["sys-fs/udev-456"]),
 			("*/*:0", ["sys-fs/udev-456:0"], ["sys-fs/udev-456:0"]),
 			("*/*:1", ["sys-fs/udev-456:0"], []),
-- 
2.4.6

Re: [gentoo-portage-dev] [PATCH] match_from_list: restrict =* to match only on version part boundaries (bug 560466)

[Ulrich Mueller]

[-- Attachment #1: Type: text/plain, Size: 643 bytes --]

>>>>> On Sun, 20 Sep 2015, Zac Medico wrote:

> Make the =* glob match only on boundaries between version parts, in
> order to eliminate ambiguity (so that 1* does not match version 10).
> Only break compatibility in cases where dependencies have been
> specified ambiguously.

Does this patch consider suffix parts (including the integer if
present) as a single component?

That is, the following should match:

   =cat/foo-1_alpha* cat/foo-1_alpha_beta

But these should *not*:

   =cat/foo-1_p* cat/foo-1_pre
   =cat/foo-1_alpha* cat/foo-1_alpha1

(The boundary of the version components is after _alpha and _alpha1,
respectively.)

Ulrich

[-- Attachment #2: Type: application/pgp-signature, Size: 490 bytes --]

Re: [gentoo-portage-dev] [PATCH] match_from_list: restrict =* to match only on version part boundaries (bug 560466)

[Zac Medico]

On 09/20/2015 10:39 PM, Ulrich Mueller wrote:
>>>>>> On Sun, 20 Sep 2015, Zac Medico wrote:
> 
>> Make the =* glob match only on boundaries between version parts, in
>> order to eliminate ambiguity (so that 1* does not match version 10).
>> Only break compatibility in cases where dependencies have been
>> specified ambiguously.
> 
> Does this patch consider suffix parts (including the integer if
> present) as a single component?

No, it considers the integer parts as a separate components, in order to
maintain as much backward compatibility as possible.

So, it deviates from your test cases only because =cat/foo-1_alpha*
matches cat/foo-1_alpha1. Note that this case is not ambiguous like the
others, so we may not want to break compatibility here.

> 
> That is, the following should match:
> 
>    =cat/foo-1_alpha* cat/foo-1_alpha_beta
> 
> But these should *not*:
> 
>    =cat/foo-1_p* cat/foo-1_pre
>    =cat/foo-1_alpha* cat/foo-1_alpha1
> 
> (The boundary of the version components is after _alpha and _alpha1,
> respectively.)
> 
> Ulrich
> 


-- 
Thanks,
Zac

Re: [gentoo-portage-dev] [PATCH] match_from_list: restrict =* to match only on version part boundaries (bug 560466)

[Brian Dolbec]

On Mon, 21 Sep 2015 09:09:04 -0700
Zac Medico <zmedico@gentoo.org> wrote:

> On 09/20/2015 10:39 PM, Ulrich Mueller wrote:
> >>>>>> On Sun, 20 Sep 2015, Zac Medico wrote:
> > 
> >> Make the =* glob match only on boundaries between version parts, in
> >> order to eliminate ambiguity (so that 1* does not match version
> >> 10). Only break compatibility in cases where dependencies have been
> >> specified ambiguously.
> > 
> > Does this patch consider suffix parts (including the integer if
> > present) as a single component?
> 
> No, it considers the integer parts as a separate components, in order
> to maintain as much backward compatibility as possible.
> 
> So, it deviates from your test cases only because =cat/foo-1_alpha*
> matches cat/foo-1_alpha1. Note that this case is not ambiguous like
> the others, so we may not want to break compatibility here.
> 
> > 
> > That is, the following should match:
> > 
> >    =cat/foo-1_alpha* cat/foo-1_alpha_beta
> > 
> > But these should *not*:
> > 
> >    =cat/foo-1_p* cat/foo-1_pre
> >    =cat/foo-1_alpha* cat/foo-1_alpha1
> > 
> > (The boundary of the version components is after _alpha and _alpha1,
> > respectively.)
> > 
> > Ulrich
> > 
> 
> 

Ok, merged approved, we can tighten it up more later easier than going
too tight now and causing some tree havoc.


-- 
Brian Dolbec <dolsen>

Re: [gentoo-portage-dev] [PATCH] match_from_list: restrict =* to match only on version part boundaries (bug 560466)

[Ulrich Mueller]

[-- Attachment #1: Type: text/plain, Size: 796 bytes --]

>>>>> On Mon, 21 Sep 2015, Zac Medico wrote:

>> Does this patch consider suffix parts (including the integer if
>> present) as a single component?

> No, it considers the integer parts as a separate components, in
> order to maintain as much backward compatibility as possible.

> So, it deviates from your test cases only because =cat/foo-1_alpha*
> matches cat/foo-1_alpha1. Note that this case is not ambiguous like
> the others, so we may not want to break compatibility here.

In fact, PMS sections 3.2 and 3.3 don't define if a suffix is one or
two components. I believe that we could clarify the wording in either
way, both would be consistent.

Function get_version_components() from versionator.eclass treats
suffixes as single components, though. But maybe we can ignore this.

Ulrich

[-- Attachment #2: Type: application/pgp-signature, Size: 490 bytes --]