[setup topic/libsolv] Does "obsoletes:" work?

[setup topic/libsolv] Does "obsoletes:" work?

[Ken Brown]

Jon,

Have you ever tested the "obsoletes:" feature of setup/libsolv?  I tried 
adding an "obsoletes:" line to setup.ini, and it didn't seem to have any 
effect.

Ken

Re: [setup topic/libsolv] Does "obsoletes:" work?

[Ken Brown]

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

On 10/20/2017 6:24 PM, Ken Brown wrote:
> Jon,
> 
> Have you ever tested the "obsoletes:" feature of setup/libsolv?  I tried 
> adding an "obsoletes:" line to setup.ini, and it didn't seem to have any 
> effect.

It turns out that it *is* working (after a minor fix, attached), but not 
always as I expect.  Suppose A requires B and C obsoletes B.  Then the 
"obsoletes" statement appears to have no effect.  If I remove the 
dependence of A on B, then setup does propose uninstalling B and 
installing C.

I guess the issue is that libsolv interprets "C obsoletes B" as 
"uninstall B and install C", and it won't uninstall B while something 
requires it.

Ken



[-- Attachment #2: 0001-Fix-parsing-of-setup.ini.patch --]
[-- Type: text/plain, Size: 1369 bytes --]

From af9829d2ee35686cf0f6e82c8fb04265b1f50f82 Mon Sep 17 00:00:00 2001
From: Ken Brown <kbrown@cornell.edu>
Date: Sat, 21 Oct 2017 12:49:34 -0400
Subject: [PATCH] Fix parsing of setup.ini

Reset "obsoletes" between packages.  Also add a debugging statement.
---
 IniDBBuilderPackage.cc | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/IniDBBuilderPackage.cc b/IniDBBuilderPackage.cc
index 3bc7b86..4e41659 100644
--- a/IniDBBuilderPackage.cc
+++ b/IniDBBuilderPackage.cc
@@ -85,6 +85,7 @@ IniDBBuilderPackage::buildPackage (const std::string& _name)
   cbpv.spkg = PackageSpecification();
   cbpv.spkg_id = packageversion();
   cbpv.requires = NULL;
+  cbpv.obsoletes = NULL;
   cbpv.archive = packagesource();
 
   currentSpec = NULL;
@@ -157,6 +158,7 @@ IniDBBuilderPackage::buildPackageSource (const std::string& path,
   SolverPool::addPackageData cspv = cbpv;
   cspv.type = package_source;
   cspv.requires = NULL;
+  cspv.obsoletes = NULL;
 
   /* set archive path, size, mirror, hash */
   cspv.archive = packagesource();
@@ -229,6 +231,9 @@ IniDBBuilderPackage::buildBeginBuildDepends ()
 void
 IniDBBuilderPackage::buildBeginObsoletes ()
 {
+#if DEBUG
+  Log (LOG_BABBLE) << "Beginning of an obsoletes statement" << endLog;
+#endif
   currentSpec = NULL;
   obsoletesNodeList =   PackageDepends();
   currentNodeList = &obsoletesNodeList;
-- 
2.14.2

Re: [setup topic/libsolv] Does "obsoletes:" work?

[Jon Turney]

On 21/10/2017 21:18, Ken Brown wrote:
> On 10/20/2017 6:24 PM, Ken Brown wrote:
>> Have you ever tested the "obsoletes:" feature of setup/libsolv?  I 
>> tried adding an "obsoletes:" line to setup.ini, and it didn't seem to 
>> have any effect.

It seems I tested it back in May, so it might well have broken since :)

Here's a very small test repo I've been using for some tests:
http://www.dronecode.org.uk/cygwin/test/x86_64/

But yes, your patch looks like it's needed for it to work correctly...

> It turns out that it *is* working (after a minor fix, attached), but not 
> always as I expect.  Suppose A requires B and C obsoletes B.  Then the 
> "obsoletes" statement appears to have no effect.  If I remove the 
> dependence of A on B, then setup does propose uninstalling B and 
> installing C.
> 
> I guess the issue is that libsolv interprets "C obsoletes B" as 
> "uninstall B and install C", and it won't uninstall B while something 
> requires it.

The 'targeted' vs. 'untargeted' distinction is relevant here? Perhaps we 
are doing the wrong one?

Re: [setup topic/libsolv] Does "obsoletes:" work?

[Ken Brown]

On 10/23/2017 7:38 AM, Jon Turney wrote:
> On 21/10/2017 21:18, Ken Brown wrote:
>> On 10/20/2017 6:24 PM, Ken Brown wrote:
>>> Have you ever tested the "obsoletes:" feature of setup/libsolv?  I 
>>> tried adding an "obsoletes:" line to setup.ini, and it didn't seem to 
>>> have any effect.
> 
> It seems I tested it back in May, so it might well have broken since :)
> 
> Here's a very small test repo I've been using for some tests:
> http://www.dronecode.org.uk/cygwin/test/x86_64/
> 
> But yes, your patch looks like it's needed for it to work correctly...
> 
>> It turns out that it *is* working (after a minor fix, attached), but 
>> not always as I expect.  Suppose A requires B and C obsoletes B.  Then 
>> the "obsoletes" statement appears to have no effect.  If I remove the 
>> dependence of A on B, then setup does propose uninstalling B and 
>> installing C.
>>
>> I guess the issue is that libsolv interprets "C obsoletes B" as 
>> "uninstall B and install C", and it won't uninstall B while something 
>> requires it.
> 
> The 'targeted' vs. 'untargeted' distinction is relevant here? Perhaps we 
> are doing the wrong one?

Maybe.  I've read and re-read the discussion of this in 
libsolv-bindings.txt, and I'm still not sure I understand it.

But here's a simpler case where "obsoletes" isn't working as I expect. 
Using your test repo, in which A requires C and obsoletes B, I start 
with none of the packages installed.  I choose B for installation 
(either interactively or on the command line), and B gets installed.  If 
I now run setup a second time, A and C get installed and B gets uninstalled.

I expected A and C to be installed on the first run.  I don't think this 
has anything to do with targeted vs. untargeted, because that 
distinction is only relevant for updating installed packages.

Ken

Re: [setup topic/libsolv] Does "obsoletes:" work?

[Jon Turney]

On 23/10/2017 18:43, Ken Brown wrote:
> On 10/23/2017 7:38 AM, Jon Turney wrote:
>> On 21/10/2017 21:18, Ken Brown wrote:
>>> On 10/20/2017 6:24 PM, Ken Brown wrote:
>>>> Have you ever tested the "obsoletes:" feature of setup/libsolv?  I 
>>>> tried adding an "obsoletes:" line to setup.ini, and it didn't seem 
>>>> to have any effect.
>>
>> It seems I tested it back in May, so it might well have broken since :)
>>
>> Here's a very small test repo I've been using for some tests:
>> http://www.dronecode.org.uk/cygwin/test/x86_64/
>>
>> But yes, your patch looks like it's needed for it to work correctly...
>>
>>> It turns out that it *is* working (after a minor fix, attached), but 
>>> not always as I expect.  Suppose A requires B and C obsoletes B.  
>>> Then the "obsoletes" statement appears to have no effect.  If I 
>>> remove the dependence of A on B, then setup does propose uninstalling 
>>> B and installing C.
>>>
>>> I guess the issue is that libsolv interprets "C obsoletes B" as 
>>> "uninstall B and install C", and it won't uninstall B while something 
>>> requires it.
>>
>> The 'targeted' vs. 'untargeted' distinction is relevant here? Perhaps 
>> we are doing the wrong one?
> 
> Maybe.  I've read and re-read the discussion of this in 
> libsolv-bindings.txt, and I'm still not sure I understand it.

Yeah, the documentation is a bit impenetrable.

> But here's a simpler case where "obsoletes" isn't working as I expect. 
> Using your test repo, in which A requires C and obsoletes B, I start 
> with none of the packages installed.  I choose B for installation 
> (either interactively or on the command line), and B gets installed.  If 
> I now run setup a second time, A and C get installed and B gets 
> uninstalled.
> 
> I expected A and C to be installed on the first run.  I don't think this 
> has anything to do with targeted vs. untargeted, because that 
> distinction is only relevant for updating installed packages.

I guess I had the opposite expectation (if I ask for A to be installed, 
that's what should happen, because if it insists on upgrading it behind 
my back there's no way to do that...)

The actual behaviour you mention fits what's described there pretty well.

Re: [setup topic/libsolv] Does "obsoletes:" work?

[Ken Brown]

On 10/24/2017 4:09 PM, Jon Turney wrote:
> On 23/10/2017 18:43, Ken Brown wrote:
>> On 10/23/2017 7:38 AM, Jon Turney wrote:
>>> On 21/10/2017 21:18, Ken Brown wrote:
>>>> On 10/20/2017 6:24 PM, Ken Brown wrote:
>>>>> Have you ever tested the "obsoletes:" feature of setup/libsolv?  I 
>>>>> tried adding an "obsoletes:" line to setup.ini, and it didn't seem 
>>>>> to have any effect.
>>>
>>> It seems I tested it back in May, so it might well have broken since :)
>>>
>>> Here's a very small test repo I've been using for some tests:
>>> http://www.dronecode.org.uk/cygwin/test/x86_64/
>>>
>>> But yes, your patch looks like it's needed for it to work correctly...
>>>
>>>> It turns out that it *is* working (after a minor fix, attached), but 
>>>> not always as I expect.  Suppose A requires B and C obsoletes B. 
>>>> Then the "obsoletes" statement appears to have no effect.  If I 
>>>> remove the dependence of A on B, then setup does propose 
>>>> uninstalling B and installing C.
>>>>
>>>> I guess the issue is that libsolv interprets "C obsoletes B" as 
>>>> "uninstall B and install C", and it won't uninstall B while 
>>>> something requires it.
>>>
>>> The 'targeted' vs. 'untargeted' distinction is relevant here? Perhaps 
>>> we are doing the wrong one?
>>
>> Maybe.  I've read and re-read the discussion of this in 
>> libsolv-bindings.txt, and I'm still not sure I understand it.
> 
> Yeah, the documentation is a bit impenetrable.
> 
>> But here's a simpler case where "obsoletes" isn't working as I expect. 
>> Using your test repo, in which A requires C and obsoletes B, I start 
>> with none of the packages installed.  I choose B for installation 
>> (either interactively or on the command line), and B gets installed.  
>> If I now run setup a second time, A and C get installed and B gets 
>> uninstalled.
>>
>> I expected A and C to be installed on the first run.  I don't think 
>> this has anything to do with targeted vs. untargeted, because that 
>> distinction is only relevant for updating installed packages.
> 
> I guess I had the opposite expectation (if I ask for A to be installed, 
> that's what should happen, because if it insists on upgrading it behind 
> my back there's no way to do that...)
> 
> The actual behaviour you mention fits what's described there pretty well.

OK, so maybe there's no real problem here.  In any case, the situation 
is unlikely to happen often -- the user has to intentionally choose to 
install an obsolete package.

I think we might have reached the point where more widespread testing 
would be useful.  If it would help, I could put together a patch series 
containing the various (sometimes revised) patches we've discussed recently.

Ken

Re: [setup topic/libsolv] Does "obsoletes:" work?

[Jon Turney]

On 24/10/2017 21:24, Ken Brown wrote:
> On 10/24/2017 4:09 PM, Jon Turney wrote:
>> On 23/10/2017 18:43, Ken Brown wrote:
>>> On 10/23/2017 7:38 AM, Jon Turney wrote:
>>>> On 21/10/2017 21:18, Ken Brown wrote:
>>>>> On 10/20/2017 6:24 PM, Ken Brown wrote:
>>>>>> Have you ever tested the "obsoletes:" feature of setup/libsolv?  I 
>>>>>> tried adding an "obsoletes:" line to setup.ini, and it didn't seem 
>>>>>> to have any effect.
>>>>
>>>> It seems I tested it back in May, so it might well have broken since :)
>>>>
>>>> Here's a very small test repo I've been using for some tests:
>>>> http://www.dronecode.org.uk/cygwin/test/x86_64/
>>>>
>>>> But yes, your patch looks like it's needed for it to work correctly...
>>>>
>>>>> It turns out that it *is* working (after a minor fix, attached), 
>>>>> but not always as I expect.  Suppose A requires B and C obsoletes 
>>>>> B. Then the "obsoletes" statement appears to have no effect.  If I 
>>>>> remove the dependence of A on B, then setup does propose 
>>>>> uninstalling B and installing C.
>>>>>
>>>>> I guess the issue is that libsolv interprets "C obsoletes B" as 
>>>>> "uninstall B and install C", and it won't uninstall B while 
>>>>> something requires it.
>>>>
>>>> The 'targeted' vs. 'untargeted' distinction is relevant here? 
>>>> Perhaps we are doing the wrong one?
>>>
>>> Maybe.  I've read and re-read the discussion of this in 
>>> libsolv-bindings.txt, and I'm still not sure I understand it.
>>
>> Yeah, the documentation is a bit impenetrable.
>>
>>> But here's a simpler case where "obsoletes" isn't working as I 
>>> expect. Using your test repo, in which A requires C and obsoletes B, 
>>> I start with none of the packages installed.  I choose B for 
>>> installation (either interactively or on the command line), and B 
>>> gets installed. If I now run setup a second time, A and C get 
>>> installed and B gets uninstalled.
>>>
>>> I expected A and C to be installed on the first run.  I don't think 
>>> this has anything to do with targeted vs. untargeted, because that 
>>> distinction is only relevant for updating installed packages.
>>
>> I guess I had the opposite expectation (if I ask for A to be 
>> installed, that's what should happen, because if it insists on 
>> upgrading it behind my back there's no way to do that...)
>>
>> The actual behaviour you mention fits what's described there pretty well.
> 
> OK, so maybe there's no real problem here.  In any case, the situation 
> is unlikely to happen often -- the user has to intentionally choose to 
> install an obsolete package.

I was wondering if there might be some scenario where A is in the base 
category, and obsoleted by B, where we'd really want to install B the 
first time on fresh installs, but, yeah, something we'd want to avoid in 
general...

> I think we might have reached the point where more widespread testing 
> would be useful.  If it would help, I could put together a patch series 
> containing the various (sometimes revised) patches we've discussed 
> recently.
Cool, I was going to ask you how far along you were in your test plan :)

I think I've been keeping track of your patches, so I've updated 
topic/libsolv with your patches and rebased onto master.  If that looks 
good to you, I'll do test release.

(I squashed "Fix parsing setup.ini" (for obsoletes) into "Add obsoletes: 
support", and added a missing break; in "Don't override a Skip selection")

Re: [setup topic/libsolv] Does "obsoletes:" work?

[Ken Brown]

On 10/24/2017 4:37 PM, Jon Turney wrote:
> On 24/10/2017 21:24, Ken Brown wrote:
>> On 10/24/2017 4:09 PM, Jon Turney wrote:
>>> On 23/10/2017 18:43, Ken Brown wrote:
>>>> On 10/23/2017 7:38 AM, Jon Turney wrote:
>>>>> On 21/10/2017 21:18, Ken Brown wrote:
>>>>>> On 10/20/2017 6:24 PM, Ken Brown wrote:
>>>>>>> Have you ever tested the "obsoletes:" feature of setup/libsolv?  
>>>>>>> I tried adding an "obsoletes:" line to setup.ini, and it didn't 
>>>>>>> seem to have any effect.
>>>>>
>>>>> It seems I tested it back in May, so it might well have broken 
>>>>> since :)
>>>>>
>>>>> Here's a very small test repo I've been using for some tests:
>>>>> http://www.dronecode.org.uk/cygwin/test/x86_64/
>>>>>
>>>>> But yes, your patch looks like it's needed for it to work correctly...
>>>>>
>>>>>> It turns out that it *is* working (after a minor fix, attached), 
>>>>>> but not always as I expect.  Suppose A requires B and C obsoletes 
>>>>>> B. Then the "obsoletes" statement appears to have no effect.  If I 
>>>>>> remove the dependence of A on B, then setup does propose 
>>>>>> uninstalling B and installing C.
>>>>>>
>>>>>> I guess the issue is that libsolv interprets "C obsoletes B" as 
>>>>>> "uninstall B and install C", and it won't uninstall B while 
>>>>>> something requires it.
>>>>>
>>>>> The 'targeted' vs. 'untargeted' distinction is relevant here? 
>>>>> Perhaps we are doing the wrong one?
>>>>
>>>> Maybe.  I've read and re-read the discussion of this in 
>>>> libsolv-bindings.txt, and I'm still not sure I understand it.
>>>
>>> Yeah, the documentation is a bit impenetrable.
>>>
>>>> But here's a simpler case where "obsoletes" isn't working as I 
>>>> expect. Using your test repo, in which A requires C and obsoletes B, 
>>>> I start with none of the packages installed.  I choose B for 
>>>> installation (either interactively or on the command line), and B 
>>>> gets installed. If I now run setup a second time, A and C get 
>>>> installed and B gets uninstalled.
>>>>
>>>> I expected A and C to be installed on the first run.  I don't think 
>>>> this has anything to do with targeted vs. untargeted, because that 
>>>> distinction is only relevant for updating installed packages.
>>>
>>> I guess I had the opposite expectation (if I ask for A to be 
>>> installed, that's what should happen, because if it insists on 
>>> upgrading it behind my back there's no way to do that...)
>>>
>>> The actual behaviour you mention fits what's described there pretty 
>>> well.
>>
>> OK, so maybe there's no real problem here.  In any case, the situation 
>> is unlikely to happen often -- the user has to intentionally choose to 
>> install an obsolete package.
> 
> I was wondering if there might be some scenario where A is in the base 
> category, and obsoleted by B, where we'd really want to install B the 
> first time on fresh installs, but, yeah, something we'd want to avoid in 
> general...
> 
>> I think we might have reached the point where more widespread testing 
>> would be useful.  If it would help, I could put together a patch 
>> series containing the various (sometimes revised) patches we've 
>> discussed recently.
> Cool, I was going to ask you how far along you were in your test plan :)
> 
> I think I've been keeping track of your patches, so I've updated 
> topic/libsolv with your patches and rebased onto master.  If that looks 
> good to you, I'll do test release.
> 
> (I squashed "Fix parsing setup.ini" (for obsoletes) into "Add obsoletes: 
> support", and added a missing break; in "Don't override a Skip selection")

Looks good to me.  The only issue I can think of that hasn't yet been 
fully addressed is the one I mentioned here:

   https://sourceware.org/ml/cygwin-apps/2017-10/msg00094.html .

But I admit that this is an obscure corner case, and the patch I 
proposed at the beginning of that thread is probably not the best way to 
deal with it.  So we might want to just leave it for now.

Ken