* [PATCH] Revert "Don't override a Keep selection" @ 2017-10-16 19:13 Ken Brown 2017-10-17 18:43 ` Jon Turney 0 siblings, 1 reply; 8+ messages in thread From: Ken Brown @ 2017-10-16 19:13 UTC (permalink / raw) To: cygwin-apps 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 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] Revert "Don't override a Keep selection" 2017-10-16 19:13 [PATCH] Revert "Don't override a Keep selection" Ken Brown @ 2017-10-17 18:43 ` Jon Turney 2017-10-17 19:32 ` Ken Brown 0 siblings, 1 reply; 8+ messages in thread From: Jon Turney @ 2017-10-17 18:43 UTC (permalink / raw) To: cygwin-apps 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"? ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] Revert "Don't override a Keep selection" 2017-10-17 18:43 ` Jon Turney @ 2017-10-17 19:32 ` Ken Brown 2017-10-19 3:01 ` Ken Brown 0 siblings, 1 reply; 8+ messages in thread From: Ken Brown @ 2017-10-17 19:32 UTC (permalink / raw) To: cygwin-apps 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 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] Revert "Don't override a Keep selection" 2017-10-17 19:32 ` Ken Brown @ 2017-10-19 3:01 ` Ken Brown 2017-10-19 15:05 ` Ken Brown 0 siblings, 1 reply; 8+ messages in thread From: Ken Brown @ 2017-10-19 3:01 UTC (permalink / raw) To: cygwin-apps 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 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] Revert "Don't override a Keep selection" 2017-10-19 3:01 ` Ken Brown @ 2017-10-19 15:05 ` Ken Brown 2017-10-19 21:36 ` Ken Brown 0 siblings, 1 reply; 8+ messages in thread From: Ken Brown @ 2017-10-19 15:05 UTC (permalink / raw) To: cygwin-apps 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 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] Revert "Don't override a Keep selection" 2017-10-19 15:05 ` Ken Brown @ 2017-10-19 21:36 ` Ken Brown 2017-10-20 11:08 ` Ken Brown 0 siblings, 1 reply; 8+ messages in thread From: Ken Brown @ 2017-10-19 21:36 UTC (permalink / raw) To: cygwin-apps [-- Attachment #1: Type: text/plain, Size: 2318 bytes --] 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 [-- Attachment #2: 0001-Fix-the-functionality-of-taskKeep.patch --] [-- Type: text/plain, Size: 3883 bytes --] From 610426eabeccb3c04bbd7f49862248beb82b280a Mon Sep 17 00:00:00 2001 From: Ken Brown <kbrown@cornell.edu> Date: Thu, 19 Oct 2017 14:44:45 -0400 Subject: [PATCH] Fix the functionality of taskKeep A taskKeep is generated whenever the user wants to keep an installed version that's different from the default_version, where the latter is whatever the solver has chosen. We need to make sure that default_version is set appropriately wherever it is needed. --- choose.cc | 7 +++++++ libsolv.cc | 11 ++++++----- package_meta.h | 2 +- prereq.cc | 3 +-- 4 files changed, 15 insertions(+), 8 deletions(-) diff --git a/choose.cc b/choose.cc index f0e6802..ad37639 100644 --- a/choose.cc +++ b/choose.cc @@ -420,6 +420,13 @@ ChooserPage::changeTrust(int button, bool test, bool initial) { // but initially we want a task list with any package changes caused by // command line options + // and we don't want to generate spurious Keep tasks + for (packagedb::packagecollection::iterator p = db.packages.begin (); + p != db.packages.end (); ++p) + { + packagemeta *pkg = p->second; + pkg->default_version = pkg->installed; + } q.setTasks(); } db.defaultTrust(q, mode, test); diff --git a/libsolv.cc b/libsolv.cc index 289f19c..e623555 100644 --- a/libsolv.cc +++ b/libsolv.cc @@ -526,7 +526,8 @@ SolverTasks::setTasks() // decode UI state to action // skip and keep don't change dependency solution - // except when we want to keep an old version + // except when we want to keep a version different from the one + // chosen by the solver if (pkg->installed != pkg->desired) { if (pkg->desired) @@ -538,7 +539,7 @@ SolverTasks::setTasks() { if (pkg->picked()) add(pkg->installed, taskReinstall); // reinstall - else if (pkg->installed < pkg->default_version) + else if (pkg->installed != pkg->default_version) add(pkg->installed, taskKeep); // keep } // only install action makes sense for source packages @@ -591,7 +592,7 @@ SolverSolution::trans2db() const i != db.packages.end(); i++) { packagemeta *pkg = i->second; - pkg->desired = pkg->installed; + pkg->desired = pkg->default_version = pkg->installed; pkg->pick(false); } // Now make changes according to trans. transErase requires some @@ -614,7 +615,7 @@ SolverSolution::trans2db() const case SolverTransaction::transInstall: if (pv.Type() == package_binary) { - pkg->desired = pv; + pkg->desired = pkg->default_version = pv; pkg->pick(true); } else // source package @@ -623,7 +624,7 @@ SolverSolution::trans2db() const case SolverTransaction::transErase: // Only relevant if pkg is still in its "no change" state if (pkg->desired == pkg->installed && !pkg->picked()) - pkg->desired = packageversion(); + pkg->desired = pkg->default_version = packageversion(); break; default: break; diff --git a/package_meta.h b/package_meta.h index d91f7c9..98253d0 100644 --- a/package_meta.h +++ b/package_meta.h @@ -131,7 +131,7 @@ public: packageversion curr; /* ditto for "test" (experimental) */ packageversion exp; - /* which one is the default according to the chooser global state */ + /* which one is the default according to the solver */ packageversion default_version; /* Now for the user stuff :] */ /* What version does the user want ? */ diff --git a/prereq.cc b/prereq.cc index c5a1fd5..bf7661a 100644 --- a/prereq.cc +++ b/prereq.cc @@ -136,8 +136,7 @@ PrereqPage::whatNext () long PrereqPage::OnBack () { - // Reset the package database to correspond to the solver's default - // solution + // Reset the package database to correspond to the solver's solution packagedb db; db.solution.trans2db(); -- 2.14.2 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] Revert "Don't override a Keep selection" 2017-10-19 21:36 ` Ken Brown @ 2017-10-20 11:08 ` Ken Brown 2017-10-23 12:17 ` Jon Turney 0 siblings, 1 reply; 8+ messages in thread From: Ken Brown @ 2017-10-20 11:08 UTC (permalink / raw) To: cygwin-apps [-- Attachment #1: Type: text/plain, Size: 361 bytes --] 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 [-- Attachment #2: 0001-Don-t-override-a-Skip-selection.patch --] [-- Type: text/plain, Size: 3419 bytes --] From a15334af176a4452ef6eef8d42a4de3648ed8b54 Mon Sep 17 00:00:00 2001 From: Ken Brown <kbrown@cornell.edu> Date: Fri, 20 Oct 2017 06:59:54 -0400 Subject: [PATCH] Don't override a Skip selection Introduce SolverTasks::taskSkip, and generate it when the user chooses to Skip a package that the solver wants to install. Implement it by sending a SOLVER_LOCK command on the package name. --- choose.cc | 2 +- libsolv.cc | 20 +++++++++++++++----- libsolv.h | 2 ++ 3 files changed, 18 insertions(+), 6 deletions(-) diff --git a/choose.cc b/choose.cc index ad37639..2a7774d 100644 --- a/choose.cc +++ b/choose.cc @@ -420,7 +420,7 @@ ChooserPage::changeTrust(int button, bool test, bool initial) { // but initially we want a task list with any package changes caused by // command line options - // and we don't want to generate spurious Keep tasks + // and we don't want to generate spurious taskKeep or taskSkip tasks for (packagedb::packagecollection::iterator p = db.packages.begin (); p != db.packages.end (); ++p) { diff --git a/libsolv.cc b/libsolv.cc index e623555..2a37a92 100644 --- a/libsolv.cc +++ b/libsolv.cc @@ -72,13 +72,19 @@ RelId2Operator(Id id) // a wrapper around a libsolv Solvable // --------------------------------------------------------------------------- +Id +SolvableVersion::name_id () const +{ + Solvable *solvable = pool_id2solvable(pool, id); + return solvable->name; +} + const std::string SolvableVersion::Name () const { if (!id) return ""; - Solvable *solvable = pool_id2solvable(pool, id); - return std::string(pool_id2str(pool, solvable->name)); + return pool_id2str(pool, name_id()); } const std::string @@ -525,9 +531,8 @@ SolverTasks::setTasks() packagemeta *pkg = p->second; // decode UI state to action - // skip and keep don't change dependency solution - // except when we want to keep a version different from the one - // chosen by the solver + // keep and skip need attention only when they differ from the + // solver's solution if (pkg->installed != pkg->desired) { if (pkg->desired) @@ -542,6 +547,9 @@ SolverTasks::setTasks() else if (pkg->installed != pkg->default_version) add(pkg->installed, taskKeep); // keep } + else if (pkg->default_version) + add(pkg->default_version, taskSkip); // skip + // only install action makes sense for source packages if (pkg->srcpicked()) { @@ -714,6 +722,8 @@ SolverSolution::update(SolverTasks &tasks, updateMode update, bool use_test_pack case SolverTasks::taskKeep: queue_push2(&job, SOLVER_LOCK | SOLVER_SOLVABLE, sv.id); break; + case SolverTasks::taskSkip: + queue_push2(&job, SOLVER_LOCK | SOLVER_SOLVABLE_NAME, sv.name_id()); default: Log (LOG_PLAIN) << "unknown task " << (*i).second << endLog; } diff --git a/libsolv.h b/libsolv.h index e448841..65e1610 100644 --- a/libsolv.h +++ b/libsolv.h @@ -97,6 +97,7 @@ class SolvableVersion friend SolverSolution; const PackageDepends deplist(Id keyname) const; + Id name_id () const; }; // --------------------------------------------------------------------------- @@ -183,6 +184,7 @@ class SolverTasks taskUninstall, taskReinstall, taskKeep, + taskSkip, }; void add(const SolvableVersion &v, task t) { -- 2.14.2 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] Revert "Don't override a Keep selection" 2017-10-20 11:08 ` Ken Brown @ 2017-10-23 12:17 ` Jon Turney 0 siblings, 0 replies; 8+ messages in thread From: Jon Turney @ 2017-10-23 12:17 UTC (permalink / raw) To: cygwin-apps 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... ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2017-10-23 12:17 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2017-10-16 19:13 [PATCH] Revert "Don't override a Keep selection" Ken Brown 2017-10-17 18:43 ` Jon Turney 2017-10-17 19:32 ` Ken Brown 2017-10-19 3:01 ` Ken Brown 2017-10-19 15:05 ` Ken Brown 2017-10-19 21:36 ` Ken Brown 2017-10-20 11:08 ` Ken Brown 2017-10-23 12:17 ` Jon Turney
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).