[PATCH setup] Finish providing support for provides: and conflicts:

[PATCH setup] Finish providing support for provides: and conflicts:

[Ken Brown]

Introduce member functions SolvableVersion::provides() and
SolvableVersion::conflicts().  This enables packagedb::read() to
access provides and conflicts lists from setup.ini.
---
 libsolv.cc    | 12 ++++++++++++
 libsolv.h     |  4 ++++
 package_db.cc | 18 ++++++++++++------
 3 files changed, 28 insertions(+), 6 deletions(-)

diff --git a/libsolv.cc b/libsolv.cc
index 2eb04e2..d85fec4 100644
--- a/libsolv.cc
+++ b/libsolv.cc
@@ -121,6 +121,18 @@ SolvableVersion::obsoletes() const
   return deplist(SOLVABLE_OBSOLETES);
 }
 
+const PackageDepends
+SolvableVersion::provides() const
+{
+  return deplist(SOLVABLE_PROVIDES);
+}
+
+const PackageDepends
+SolvableVersion::conflicts() const
+{
+  return deplist(SOLVABLE_CONFLICTS);
+}
+
 // helper function which returns the deplist for a given key, as a PackageDepends
 const PackageDepends
 SolvableVersion::deplist(Id keyname) const
diff --git a/libsolv.h b/libsolv.h
index 7bb0be2..c218ab8 100644
--- a/libsolv.h
+++ b/libsolv.h
@@ -60,6 +60,10 @@ class SolvableVersion
   const PackageDepends depends() const;
   // Return the obsoletes list
   const PackageDepends obsoletes() const;
+  // Return the provides list
+  const PackageDepends provides() const;
+  // Return the conflicts list
+  const PackageDepends conflicts() const;
   bool accessible () const;
   package_type_t Type () const;
   package_stability_t Stability () const;
diff --git a/package_db.cc b/package_db.cc
index d12e841..799ee39 100644
--- a/package_db.cc
+++ b/package_db.cc
@@ -142,8 +142,10 @@ packagedb::read ()
                   // supplement this with sdesc, source, and stability
                   // information from setup.ini, if possible...
                   packageversion pv = findBinaryVersion(PackageSpecification(pkgname, f.ver));
-                  PackageDepends dep;
-                  PackageDepends obs;
+                  PackageDepends depends;
+                  PackageDepends obsoletes;
+                  PackageDepends provides;
+                  PackageDepends conflicts;
                   if (pv)
                     {
                       data.sdesc = pv.SDesc();
@@ -151,10 +153,14 @@ packagedb::read ()
                       data.stability = pv.Stability();
                       data.spkg_id = pv.sourcePackage();
                       data.spkg = pv.sourcePackageName();
-                      dep = pv.depends();
-                      data.requires = &dep;
-                      obs = pv.obsoletes();
-                      data.obsoletes = &obs;
+                      depends = pv.depends();
+                      data.requires = &depends;
+                      obsoletes = pv.obsoletes();
+                      data.obsoletes = &obsoletes;
+                      provides = pv.provides();
+                      data.provides = &provides;
+                      conflicts = pv.conflicts();
+                      data.conflicts = &conflicts;
                     }
                   else
                     // This version is no longer available.  It could
-- 
2.16.2

Re: [PATCH setup] Finish providing support for provides: and conflicts:

[Ken Brown]

On 3/21/2018 3:38 PM, Ken Brown wrote:
> Introduce member functions SolvableVersion::provides() and
> SolvableVersion::conflicts().  This enables packagedb::read() to
> access provides and conflicts lists from setup.ini.

Ignore this patch.  It breaks libsolv's dependency processing.

Ken

Re: [PATCH setup] Finish providing support for provides: and conflicts:

[Jon Turney]

On 04/04/2018 14:21, Ken Brown wrote:
> On 3/21/2018 3:38 PM, Ken Brown wrote:
>> Introduce member functions SolvableVersion::provides() and
>> SolvableVersion::conflicts().  This enables packagedb::read() to
>> access provides and conflicts lists from setup.ini.
> 
> Ignore this patch.  It breaks libsolv's dependency processing.
> 

Thanks, even if it's not right, I'd completely overlooked the need for 
something like this.

Re: [PATCH setup] Finish providing support for provides: and conflicts:

[Ken Brown]

[Redirecting to cygwin-apps.]

On 6/18/2018 1:32 PM, Jon Turney wrote:
> On 06/06/2018 21:44, Jon Turney wrote:
>> On 04/04/2018 14:21, Ken Brown wrote:
>>> On 3/21/2018 3:38 PM, Ken Brown wrote:
>>>> Introduce member functions SolvableVersion::provides() and
>>>> SolvableVersion::conflicts().  This enables packagedb::read() to
>>>> access provides and conflicts lists from setup.ini.
>>>
>>> Ignore this patch.  It breaks libsolv's dependency processing.
>>>
>>
>> Thanks, even if it's not right, I'd completely overlooked the need for 
>> something like this.
> 
> Do you have any details on what broke with this?  I've been trying it 
> and I don't notice any problems.

No, I'm sorry.  I don't remember, and I didn't keep any notes about it. 
But I'm pretty sure I had applied the following patchset while testing:

   https://sourceware.org/ml/cygwin-apps/2018-03/msg00033.html

You weren't sure about the second and third patches in that set.  It's 
possible that the breakage only occurs in conjunction with those patches.

Ken

Re: [PATCH setup] Finish providing support for provides: and conflicts:

[Ken Brown]

On 6/18/2018 1:49 PM, Ken Brown wrote:
> [Redirecting to cygwin-apps.]
> 
> On 6/18/2018 1:32 PM, Jon Turney wrote:
>> On 06/06/2018 21:44, Jon Turney wrote:
>>> On 04/04/2018 14:21, Ken Brown wrote:
>>>> On 3/21/2018 3:38 PM, Ken Brown wrote:
>>>>> Introduce member functions SolvableVersion::provides() and
>>>>> SolvableVersion::conflicts().  This enables packagedb::read() to
>>>>> access provides and conflicts lists from setup.ini.
>>>>
>>>> Ignore this patch.  It breaks libsolv's dependency processing.
>>>>
>>>
>>> Thanks, even if it's not right, I'd completely overlooked the need 
>>> for something like this.
>>
>> Do you have any details on what broke with this?  I've been trying it 
>> and I don't notice any problems.
> 
> No, I'm sorry.  I don't remember, and I didn't keep any notes about it.

It's coming back to me now.  I think I was testing the following 
situation: I created an installed package A that was being updated 
(possibly because of a command-line option).  I made A require a version 
of a package B higher than the installed version, so that the update of 
A should have forced an update of B.  This wasn't working reliably.  And 
if I manually chose to keep the installed version of B, libsolv wasn't 
reliably reporting a problem.

I'll try again to reproduce this, but it might be a few days until I can 
get to it.

Ken

Re: [PATCH setup] Finish providing support for provides: and conflicts:

[Ken Brown]

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

On 6/19/2018 8:43 AM, Ken Brown wrote:
> On 6/18/2018 1:49 PM, Ken Brown wrote:
>> [Redirecting to cygwin-apps.]
>>
>> On 6/18/2018 1:32 PM, Jon Turney wrote:
>>> On 06/06/2018 21:44, Jon Turney wrote:
>>>> On 04/04/2018 14:21, Ken Brown wrote:
>>>>> On 3/21/2018 3:38 PM, Ken Brown wrote:
>>>>>> Introduce member functions SolvableVersion::provides() and
>>>>>> SolvableVersion::conflicts().  This enables packagedb::read() to
>>>>>> access provides and conflicts lists from setup.ini.
>>>>>
>>>>> Ignore this patch.  It breaks libsolv's dependency processing.
>>>>>
>>>>
>>>> Thanks, even if it's not right, I'd completely overlooked the need 
>>>> for something like this.
>>>
>>> Do you have any details on what broke with this?  I've been trying it 
>>> and I don't notice any problems.
>>
>> No, I'm sorry.  I don't remember, and I didn't keep any notes about it.
> 
> It's coming back to me now.  I think I was testing the following 
> situation: I created an installed package A that was being updated 
> (possibly because of a command-line option).  I made A require a version 
> of a package B higher than the installed version, so that the update of 
> A should have forced an update of B.  This wasn't working reliably.  And 
> if I manually chose to keep the installed version of B, libsolv wasn't 
> reliably reporting a problem.
> 
> I'll try again to reproduce this, but it might be a few days until I can 
> get to it.

Here's a recipe for reproducing the problem:

1. Create a repository with two packages, A and B, and two versions 1-1 
and 2-1 of each.  Make A-1-1 require B>=1, and make A-2-1 require B>=2. 
The attached script does all this.

2. Run setup on this repo and install A-1-1 and B-1-1.

3. Run setup again.  It will offer to update both A and B.  Choose to 
keep B and press 'Next'.

setup built from the current HEAD correctly reports the dependency 
problem.  But if I apply my patch, then setup doesn't report the problem 
and lets me proceed to update A without updating B.

Ken

[-- Attachment #2: make_test.sh --]
[-- Type: text/plain, Size: 1117 bytes --]

#!/bin/sh
arch=x86_64
mkdir -p testrepo/${arch}/release/{A,B}
pushd testrepo/${arch}/release/A
tar -Jcf A-1-1.tar.xz --files-from /dev/null
tar -Jcf A-1-1-src.tar.xz --files-from /dev/null
cat > A-1-1.hint << EOF
sdesc: "Test package A"
ldesc: "Test package A"
category: Libs
depends: B (>=1)
EOF
tar -Jcf A-2-1.tar.xz --files-from /dev/null
tar -Jcf A-2-1-src.tar.xz --files-from /dev/null
cat > A-2-1.hint << EOF
sdesc: "Test package A"
ldesc: "Test package A"
category: Libs
depends: B (>=2)
EOF
popd
pushd testrepo/${arch}/release/B
tar -Jcf B-1-1.tar.xz --files-from /dev/null
tar -Jcf B-1-1-src.tar.xz --files-from /dev/null
cat > B-1-1.hint << EOF
sdesc: "Test package B"
ldesc: "Test package B"
category: Libs
depends: cygwin
EOF
tar -Jcf B-2-1.tar.xz --files-from /dev/null
tar -Jcf B-2-1-src.tar.xz --files-from /dev/null
cat > B-2-1.hint << EOF
sdesc: "Test package B"
ldesc: "Test package B"
category: Libs
depends: cygwin
EOF
popd
cd testrepo
mksetupini --arch ${arch} --inifile=${arch}/setup.ini --releasearea=. \
	   --disable-check=missing-depended-package 
xz -c ${arch}/setup.ini > ${arch}/setup.xz

Re: [PATCH setup] Finish providing support for provides: and conflicts:

[Jon Turney]

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

On 23/06/2018 16:09, Ken Brown wrote:
>> I'll try again to reproduce this, but it might be a few days until I 
>> can get to it.
> 
> Here's a recipe for reproducing the problem:
> 
> 1. Create a repository with two packages, A and B, and two versions 1-1 
> and 2-1 of each.  Make A-1-1 require B>=1, and make A-2-1 require B>=2. 
> The attached script does all this.
> 
> 2. Run setup on this repo and install A-1-1 and B-1-1.
> 
> 3. Run setup again.  It will offer to update both A and B.  Choose to 
> keep B and press 'Next'.
> 
> setup built from the current HEAD correctly reports the dependency 
> problem.  But if I apply my patch, then setup doesn't report the problem 
> and lets me proceed to update A without updating B.
Thanks very much for this testcase, it helped a lot.

This looks like completely my fault:  SolvableVersion::deplist()'s 
handling of dependencies with a version relation was completely bogus.

So provides() on B was always returning unversioned B, rather than 
B-1-1, so the installed B ended up with that as provides, which appears 
to be enough to satisfy a depends of B-2-1.

(previously we'd just have the correct, implicit provide made at 
libsolv.cpp:472)

Attached patch seems to make it work.


[-- Attachment #2: 0001-Fix-SolvableVersion-deplist-to-handle-version-relati.patch --]
[-- Type: text/plain, Size: 1086 bytes --]

From 5947bc929bc772bf067a6b457fc825bbbfc4b0e6 Mon Sep 17 00:00:00 2001
From: Jon Turney <jon.turney@dronecode.org.uk>
Date: Fri, 6 Jul 2018 17:16:16 +0100
Subject: [PATCH setup] Fix SolvableVersion::deplist() to handle
 version-relations correctly

This was using completely the wrong Id for the version-relation, so it was
always returning unversioned dependencies
---
 libsolv.cc | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/libsolv.cc b/libsolv.cc
index fc61651..63942b2 100644
--- a/libsolv.cc
+++ b/libsolv.cc
@@ -159,9 +159,9 @@ SolvableVersion::deplist(Id keyname) const
           const char *name = pool_id2str(pool, q.elements[i]);
           PackageSpecification *spec = new PackageSpecification (name);
 
-          if (ISRELDEP(id))
+          if (ISRELDEP(q.elements[i]))
             {
-              Reldep *rd = GETRELDEP(pool, id);
+              Reldep *rd = GETRELDEP(pool, q.elements[i]);
               spec->setOperator(RelId2Operator(rd->flags));
               spec->setVersion(pool_id2str(pool, rd->evr));
             }
-- 
2.17.0

Re: [PATCH setup] Finish providing support for provides: and conflicts:

[Ken Brown]

On 7/6/2018 12:28 PM, Jon Turney wrote:
> On 23/06/2018 16:09, Ken Brown wrote:
>>> I'll try again to reproduce this, but it might be a few days until I 
>>> can get to it.
>>
>> Here's a recipe for reproducing the problem:
>>
>> 1. Create a repository with two packages, A and B, and two versions 
>> 1-1 and 2-1 of each.  Make A-1-1 require B>=1, and make A-2-1 require 
>> B>=2. The attached script does all this.
>>
>> 2. Run setup on this repo and install A-1-1 and B-1-1.
>>
>> 3. Run setup again.  It will offer to update both A and B.  Choose to 
>> keep B and press 'Next'.
>>
>> setup built from the current HEAD correctly reports the dependency 
>> problem.  But if I apply my patch, then setup doesn't report the 
>> problem and lets me proceed to update A without updating B.
> Thanks very much for this testcase, it helped a lot.
> 
> This looks like completely my fault:  SolvableVersion::deplist()'s 
> handling of dependencies with a version relation was completely bogus.

Great.  I'm glad the problem turned out to have an easy fix.

> So provides() on B was always returning unversioned B, rather than 
> B-1-1, so the installed B ended up with that as provides, which appears 
> to be enough to satisfy a depends of B-2-1.
> 
> (previously we'd just have the correct, implicit provide made at 
> libsolv.cpp:472)
> 
> Attached patch seems to make it work.

I can confirm that it fixes my testcase.

Ken