[PATCH] Revert "Don't override a Keep selection"

classic Classic list List threaded Threaded
8 messages Options
Reply | Threaded
Open this post in threaded view
|

[PATCH] Revert "Don't override a Keep selection"

Ken Brown-6
This reverts (the rest of) commit b43b697.  Part of that commit was
already reverted in commit ff0bb3d.  The rest is not needed either
since we no longer send the upgrade flag to the solver after the user
has made their selections.
---
 libsolv.cc     | 14 +++-----------
 libsolv.h      |  1 -
 package_meta.h |  2 --
 3 files changed, 3 insertions(+), 14 deletions(-)

diff --git a/libsolv.cc b/libsolv.cc
index 78e73a8..9aad102 100644
--- a/libsolv.cc
+++ b/libsolv.cc
@@ -512,7 +512,6 @@ SolverTasks::setTasks()
 
       // decode UI state to action
       // skip and keep don't change dependency solution
-      // except when we want to keep an old version
       if (pkg->installed != pkg->desired)
         {
           if (pkg->desired)
@@ -520,13 +519,9 @@ SolverTasks::setTasks()
           else
             add(pkg->installed, taskUninstall); // uninstall
         }
-      else if (pkg->installed)
- {
-  if (pkg->picked())
-    add(pkg->installed, taskReinstall); // reinstall
-  else if (pkg->installed < pkg->default_version)
-    add(pkg->installed, taskKeep); // keep
- }
+      else if (pkg->picked())
+ add(pkg->installed, taskReinstall); // reinstall
+
       // only install action makes sense for source packages
       if (pkg->srcpicked())
         {
@@ -696,9 +691,6 @@ SolverSolution::update(SolverTasks &tasks, updateMode update, bool use_test_pack
           // we don't know how to ask solver for this, so we just add the erase
           // and install later
           break;
- case SolverTasks::taskKeep:
-  queue_push2(&job, SOLVER_LOCK | SOLVER_SOLVABLE, sv.id);
-  break;
         default:
           Log (LOG_PLAIN) << "unknown task " << (*i).second << endLog;
         }
diff --git a/libsolv.h b/libsolv.h
index e448841..04233fc 100644
--- a/libsolv.h
+++ b/libsolv.h
@@ -182,7 +182,6 @@ class SolverTasks
     taskInstall,
     taskUninstall,
     taskReinstall,
-    taskKeep,
   };
   void add(const SolvableVersion &v, task t)
   {
diff --git a/package_meta.h b/package_meta.h
index d91f7c9..b6faab8 100644
--- a/package_meta.h
+++ b/package_meta.h
@@ -131,8 +131,6 @@ public:
   packageversion curr;
   /* ditto for "test" (experimental) */
   packageversion exp;
-  /* which one is the default according to the chooser global state */
-  packageversion default_version;
   /* Now for the user stuff :] */
   /* What version does the user want ? */
   packageversion desired;
--
2.14.2

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Revert "Don't override a Keep selection"

Jon TURNEY
On 16/10/2017 20:13, Ken Brown wrote:
> This reverts (the rest of) commit b43b697.  Part of that commit was
> already reverted in commit ff0bb3d.  The rest is not needed either
> since we no longer send the upgrade flag to the solver after the user
> has made their selections.
> ---
>   libsolv.cc     | 14 +++-----------
>   libsolv.h      |  1 -
>   package_meta.h |  2 --
>   3 files changed, 3 insertions(+), 14 deletions(-)

Hmm... not sure about this.

Say the initial upgrade solution had something like: package A 1.0 ->
1.1, and A 1.1 has a dependency on package B 2.0, where currently B 1.0
is installed (so B 1.0 -> 2.0)

If the user then changes B to 'keep' (at 1.0), we should report a conflict?

Does keeping this cause a problem?

I guess we are generating a huge number of these tasks into the solver
because "Keep" is kind of overloaded between "Don't care (but it just
happens that I know that there's nothing to do)" and "Lock"?
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Revert "Don't override a Keep selection"

Ken Brown-6
On 10/17/2017 2:43 PM, Jon Turney wrote:

> On 16/10/2017 20:13, Ken Brown wrote:
>> This reverts (the rest of) commit b43b697.  Part of that commit was
>> already reverted in commit ff0bb3d.  The rest is not needed either
>> since we no longer send the upgrade flag to the solver after the user
>> has made their selections.
>> ---
>>   libsolv.cc     | 14 +++-----------
>>   libsolv.h      |  1 -
>>   package_meta.h |  2 --
>>   3 files changed, 3 insertions(+), 14 deletions(-)
>
> Hmm... not sure about this.
>
> Say the initial upgrade solution had something like: package A 1.0 ->
> 1.1, and A 1.1 has a dependency on package B 2.0, where currently B 1.0
> is installed (so B 1.0 -> 2.0)
>
> If the user then changes B to 'keep' (at 1.0), we should report a conflict?

Good point.  I wasn't thinking about dependencies with version relations.

> Does keeping this cause a problem?

No, but it's not actually effective at the moment, because after commit
ff0bb3d, package_meta::default_version isn't being set.  Maybe it should
be set to reflect the initial upgrade solution.  I'll play with this
some more.

> I guess we are generating a huge number of these tasks into the solver
> because "Keep" is kind of overloaded between "Don't care (but it just
> happens that I know that there's nothing to do)" and "Lock"?

Yeah, this needs further thought.

Ken
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Revert "Don't override a Keep selection"

Ken Brown-6
On 10/17/2017 3:31 PM, Ken Brown wrote:

> On 10/17/2017 2:43 PM, Jon Turney wrote:
>> On 16/10/2017 20:13, Ken Brown wrote:
>>> This reverts (the rest of) commit b43b697.  Part of that commit was
>>> already reverted in commit ff0bb3d.  The rest is not needed either
>>> since we no longer send the upgrade flag to the solver after the user
>>> has made their selections.
>>> ---
>>>   libsolv.cc     | 14 +++-----------
>>>   libsolv.h      |  1 -
>>>   package_meta.h |  2 --
>>>   3 files changed, 3 insertions(+), 14 deletions(-)
>>
>> Hmm... not sure about this.
>>
>> Say the initial upgrade solution had something like: package A 1.0 ->
>> 1.1, and A 1.1 has a dependency on package B 2.0, where currently B
>> 1.0 is installed (so B 1.0 -> 2.0)
>>
>> If the user then changes B to 'keep' (at 1.0), we should report a
>> conflict?
>
> Good point.  I wasn't thinking about dependencies with version relations.

I just tested this scenario, and it turns out that putting a lock on B
1.0 overrides the dependency of A 1.1 on B 2.0.  There's no problem
reported.  On the other hand, if there's no lock, then the dependency
overrides the Keep choice, again with no problem reported.

Back to the drawing board.

Ken
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Revert "Don't override a Keep selection"

Ken Brown-6
On 10/18/2017 11:01 PM, Ken Brown wrote:

> On 10/17/2017 3:31 PM, Ken Brown wrote:
>> On 10/17/2017 2:43 PM, Jon Turney wrote:
>>> On 16/10/2017 20:13, Ken Brown wrote:
>>>> This reverts (the rest of) commit b43b697.  Part of that commit was
>>>> already reverted in commit ff0bb3d.  The rest is not needed either
>>>> since we no longer send the upgrade flag to the solver after the user
>>>> has made their selections.
>>>> ---
>>>>   libsolv.cc     | 14 +++-----------
>>>>   libsolv.h      |  1 -
>>>>   package_meta.h |  2 --
>>>>   3 files changed, 3 insertions(+), 14 deletions(-)
>>>
>>> Hmm... not sure about this.
>>>
>>> Say the initial upgrade solution had something like: package A 1.0 ->
>>> 1.1, and A 1.1 has a dependency on package B 2.0, where currently B
>>> 1.0 is installed (so B 1.0 -> 2.0)
>>>
>>> If the user then changes B to 'keep' (at 1.0), we should report a
>>> conflict?
>>
>> Good point.  I wasn't thinking about dependencies with version relations.
>
> I just tested this scenario, and it turns out that putting a lock on B
> 1.0 overrides the dependency of A 1.1 on B 2.0.  There's no problem
> reported.  On the other hand, if there's no lock, then the dependency
> overrides the Keep choice, again with no problem reported.
>
> Back to the drawing board.

Thinking about this further, using SOLVER_LOCK is probably the right
thing to do in this situation, even if no problem is reported.  We can't
insist that users install the recommended version.  For example, if I
choose to downgrade binutils while keeping the current gcc, then I'm
responsible for any breakage this might cause, with or without a warning
from setup.

I'll prepare a new patch that restores the SOLVER_LOCK functionality
that was inadvertently removed in commit ff0bb3d.

Ken
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Revert "Don't override a Keep selection"

Ken Brown-6
On 10/19/2017 11:05 AM, Ken Brown wrote:

> On 10/18/2017 11:01 PM, Ken Brown wrote:
>> On 10/17/2017 3:31 PM, Ken Brown wrote:
>>> On 10/17/2017 2:43 PM, Jon Turney wrote:
>>>> On 16/10/2017 20:13, Ken Brown wrote:
>>>>> This reverts (the rest of) commit b43b697.  Part of that commit was
>>>>> already reverted in commit ff0bb3d.  The rest is not needed either
>>>>> since we no longer send the upgrade flag to the solver after the user
>>>>> has made their selections.
>>>>> ---
>>>>>   libsolv.cc     | 14 +++-----------
>>>>>   libsolv.h      |  1 -
>>>>>   package_meta.h |  2 --
>>>>>   3 files changed, 3 insertions(+), 14 deletions(-)
>>>>
>>>> Hmm... not sure about this.
>>>>
>>>> Say the initial upgrade solution had something like: package A 1.0
>>>> -> 1.1, and A 1.1 has a dependency on package B 2.0, where currently
>>>> B 1.0 is installed (so B 1.0 -> 2.0)
>>>>
>>>> If the user then changes B to 'keep' (at 1.0), we should report a
>>>> conflict?
>>>
>>> Good point.  I wasn't thinking about dependencies with version
>>> relations.
>>
>> I just tested this scenario, and it turns out that putting a lock on B
>> 1.0 overrides the dependency of A 1.1 on B 2.0.  There's no problem
>> reported.  On the other hand, if there's no lock, then the dependency
>> overrides the Keep choice, again with no problem reported.
>>
>> Back to the drawing board.
>
> Thinking about this further, using SOLVER_LOCK is probably the right
> thing to do in this situation, even if no problem is reported.  We can't
> insist that users install the recommended version.  For example, if I
> choose to downgrade binutils while keeping the current gcc, then I'm
> responsible for any breakage this might cause, with or without a warning
> from setup.
>
> I'll prepare a new patch that restores the SOLVER_LOCK functionality
> that was inadvertently removed in commit ff0bb3d.
Attached.  I kept making mistakes while doing this, so I hope I got it
right in the end.

Here's a related question.  Currently if libsolv decides I should
install something and I choose Skip instead, it will get installed
anyway (with no problem report).  Maybe we should have a taskSkip that
generates a SOLVER_LOCK in that case, analogous to taskKeep?

Ken

0001-Fix-the-functionality-of-taskKeep.patch (5K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Revert "Don't override a Keep selection"

Ken Brown-6
On 10/19/2017 5:36 PM, Ken Brown wrote:
> Here's a related question.  Currently if libsolv decides I should
> install something and I choose Skip instead, it will get installed
> anyway (with no problem report).  Maybe we should have a taskSkip that
> generates a SOLVER_LOCK in that case, analogous to taskKeep?

A patch to do that is attached.

Ken


0001-Don-t-override-a-Skip-selection.patch (4K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Revert "Don't override a Keep selection"

Jon TURNEY
On 20/10/2017 12:08, Ken Brown wrote:
> On 10/19/2017 5:36 PM, Ken Brown wrote:
>> Here's a related question.  Currently if libsolv decides I should
>> install something and I choose Skip instead, it will get installed
>> anyway (with no problem report).  Maybe we should have a taskSkip that
>> generates a SOLVER_LOCK in that case, analogous to taskKeep?
>
> A patch to do that is attached.

Yeah, this seems the right sort of thing to be doing.

The solver and picker have very different ways of representing things,
so matching them up is a bit tricky, but now we initialize the picker to
the solver's initial solution, anything in the picker that's changed by
the user should be converted to a task.

I was wondering how to get a "lock in uninstalled state", but I guess
you have discovered that it's by locking by name...