On 24/12/2017 15:00, Ken Brown wrote:
On 12/13/2017 5:31 PM, Ken Brown wrote:
On 12/13/2017 1:05 PM, Achim Gratz wrote:
Ken Brown writes:
1. Uninstall A.
2. Don't uninstall B.
On the surface, it would seem that libsolv chose 2 by default, because
it returned an empty transaction list. This was reflected in the log
and was also clear when I selected 'Back'.
Yeah, I think what is actually happening here is that the solver returns
a partial solution, without the problematic transaction.
But yeah, there's no real concept of a default solution, so (lacking a
UI to choose, which I think is a bit out of scope for the moment), it's
up to us to define one.
I don't think there is a default in this case. I also see in zypper
that the order of the proposed solutions (there can be way more than two
if the dependencies are more complicated) is not always the same, so
there is no preference implied by the order as well.
Maybe we have to deal with this situation ourselves. Whenever a
problem involves a missing dependency, we could choose as default
solution the one that installs/keeps the dependent package, as is
currently done.
That solution unfortunately isn't always the one that causes the least
amount of transactions or even the least amount of breakage.
That may be true, but I still think it's a reasonable default. The
user doesn't have to accept it. Also, it's consistent with what setup
currently does, so it won't surprise anyone.
The attached patch attempts to implement my suggestion.
I came up with a slightly different solution of just picking the first
solution as a default.
After solving problems we also need to consider the 'install source for
everything I install' flag, which unfortunately requires quite a bit of
refactoring.
See attached.
From a60bbd19aa5504e0e7da6722dc7f3b81ac3afd6b Mon Sep 17 00:00:00 2001
From: Jon Turney <jon.tur...@dronecode.org.uk>
Date: Wed, 6 Dec 2017 17:52:45 +0000
Subject: [PATCH setup] Apply default solution(s)
Refactoring of SolverSolution::update() so we can apply the default
solution.
Also:
Break out logging of the task list, so we can show it in the "dependency
problems exists, but don't use the default solution, just do what I ask"
case.
Break out 'include-source' process, so it can have effect in the case where
dependency problems exist.
---
libsolv.cc | 83 ++++++++++++++++++++++++++++++++++++++++++++---------------
libsolv.h | 11 ++++++--
package_db.cc | 2 +-
prereq.cc | 28 ++++++++++++++++----
prereq.h | 4 +++
5 files changed, 99 insertions(+), 29 deletions(-)
diff --git a/libsolv.cc b/libsolv.cc
index 0fb39c7..34af50b 100644
--- a/libsolv.cc
+++ b/libsolv.cc
@@ -544,6 +544,7 @@ SolverTasks::setTasks()
{
// go through all packages, adding changed ones to the solver task list
packagedb db;
+ tasks.clear();
for (packagedb::packagecollection::iterator p = db.packages.begin ();
p != db.packages.end (); ++p)
@@ -602,6 +603,11 @@ SolverPool::use_test_packages(bool use_test_packages)
// A wrapper around the libsolv solver
// ---------------------------------------------------------------------------
+SolverSolution::SolverSolution(SolverPool &_pool) : pool(_pool), solv(NULL)
+{
+ queue_init(&job);
+}
+
SolverSolution::~SolverSolution()
{
clear();
@@ -615,6 +621,7 @@ SolverSolution::clear()
solver_free(solv);
solv = NULL;
}
+ queue_free(&job);
}
void
@@ -713,18 +720,9 @@ std::ostream &operator<<(std::ostream &stream,
return stream;
}
-bool
-SolverSolution::update(SolverTasks &tasks, updateMode update, bool
use_test_packages, bool include_source)
+void
+SolverSolution::tasksToJobs(SolverTasks &tasks, updateMode update, Queue &job)
{
- Log (LOG_PLAIN) << "solving: " << tasks.tasks.size() << " tasks," <<
- " update: " << (update ? "yes" : "no") << "," <<
- " use test packages: " << (use_test_packages ? "yes" : "no") << "," <<
- " include_source: " << (include_source ? "yes" : "no") << endLog;
-
- pool.use_test_packages(use_test_packages);
-
- Queue job;
- queue_init(&job);
// solver accepts a queue containing pairs of (cmd, id) tasks
// cmd is job and selection flags ORed together
for (SolverTasks::taskList::const_iterator i = tasks.tasks.begin();
@@ -745,11 +743,11 @@ 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;
- case SolverTasks::taskSkip:
- queue_push2(&job, SOLVER_LOCK | SOLVER_SOLVABLE_NAME, sv.name_id());
+ 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());
break;
default:
Log (LOG_PLAIN) << "unknown task " << (*i).second << endLog;
@@ -776,6 +774,19 @@ SolverSolution::update(SolverTasks &tasks, updateMode
update, bool use_test_pack
// Ask solver to check dependencies of installed packages.
queue_push2(&job, SOLVER_VERIFY | SOLVER_SOLVABLE_ALL, 0);
+}
+
+bool
+SolverSolution::update(SolverTasks &tasks, updateMode update, bool
use_test_packages)
+{
+ Log (LOG_PLAIN) << "solving: " << tasks.tasks.size() << " tasks," <<
+ " update: " << (update ? "yes" : "no") << "," <<
+ " use test packages: " << (use_test_packages ? "yes" : "no") << "," <<
endLog;
+
+ pool.use_test_packages(use_test_packages);
+
+ queue_free(&job);
+ tasksToJobs(tasks, update, job);
if (!solv)
solv = solver_create(pool.pool);
@@ -785,11 +796,18 @@ SolverSolution::update(SolverTasks &tasks, updateMode
update, bool use_test_pack
solver_set_flag(solv, SOLVER_FLAG_DUP_ALLOW_VENDORCHANGE, 1);
solver_set_flag(solv, SOLVER_FLAG_DUP_ALLOW_DOWNGRADE, 1);
solver_solve(solv, &job);
- queue_free(&job);
int pcnt = solver_problem_count(solv);
solver_printdecisions(solv);
+ solutionToTransactionList();
+
+ return (pcnt == 0);
+}
+
+void
+SolverSolution::solutionToTransactionList()
+{
// get transactions for solution
Transaction *t = solver_create_transaction(solv);
transaction_order(t, 0);
@@ -797,7 +815,6 @@ SolverSolution::update(SolverTasks &tasks, updateMode
update, bool use_test_pack
// massage into SolverTransactions form
trans.clear();
-
for (int i = 0; i < t->steps.count; i++)
{
Id id = t->steps.elements[i];
@@ -806,6 +823,14 @@ SolverSolution::update(SolverTasks &tasks, updateMode
update, bool use_test_pack
trans.push_back(SolverTransaction(SolvableVersion(id, pool.pool), tt));
}
+ transaction_free(t);
+
+ dumpTransactionList();
+}
+
+void
+SolverSolution::augmentTasks(SolverTasks &tasks)
+{
// add install and remove tasks for anything marked as reinstall
for (SolverTasks::taskList::const_iterator i = tasks.tasks.begin();
i != tasks.tasks.end();
@@ -821,7 +846,11 @@ SolverSolution::update(SolverTasks &tasks, updateMode
update, bool use_test_pack
SolverTransaction::transInstall));
}
}
+}
+void
+SolverSolution::addSource(bool include_source)
+{
// if include_source mode is on, also install source for everything we are
// installing
if (include_source)
@@ -840,11 +869,15 @@ SolverSolution::update(SolverTasks &tasks, updateMode
update, bool use_test_pack
}
}
}
+}
+void
+SolverSolution::dumpTransactionList() const
+{
if (trans.size())
{
Log (LOG_PLAIN) << "Augmented Transaction List:" << endLog;
- for (SolverTransactionList::iterator i = trans.begin ();
+ for (SolverTransactionList::const_iterator i = trans.begin ();
i != trans.end ();
++i)
{
@@ -854,10 +887,17 @@ SolverSolution::update(SolverTasks &tasks, updateMode
update, bool use_test_pack
<< std::setw(20) << i->version.Canonical_version()
<< endLog;
}
}
+ else
+ Log (LOG_PLAIN) << "Augmented Transaction List: is empty" << endLog;
+}
- transaction_free(t);
+void SolverSolution::applyDefaultProblemSolutions()
+{
+ int pcnt = solver_problem_count(solv);
+ for (Id problem = 1; problem <= pcnt; problem++)
+ solver_take_solution(solv, problem, 1, &job);
- return (pcnt == 0);
+ solutionToTransactionList();
}
const SolverTransactionList &
@@ -891,6 +931,7 @@ SolverSolution::report() const
for (Id solution = 1; solution <= scnt; solution++)
{
r += "Solution " + std::to_string(solution) + "/" +
std::to_string(scnt);
+ if (solution == 1) r += " (default)";
r += "\n";
Id p, rp, element;
diff --git a/libsolv.h b/libsolv.h
index cddf76f..78b6881 100644
--- a/libsolv.h
+++ b/libsolv.h
@@ -236,7 +236,7 @@ typedef std::vector<SolverTransaction>
SolverTransactionList;
class SolverSolution
{
public:
- SolverSolution(SolverPool &_pool) : pool(_pool), solv(NULL) {};
+ SolverSolution(SolverPool &_pool);
~SolverSolution();
void clear();
@@ -252,16 +252,23 @@ class SolverSolution
updateBest, // update to best version
updateForce, // distupdate: downgrade if necessary to best version in repo
};
- bool update(SolverTasks &tasks, updateMode update, bool use_test_packages,
bool include_source);
+ bool update(SolverTasks &tasks, updateMode update, bool use_test_packages);
+ void augmentTasks(SolverTasks &tasks);
+ void addSource(bool include_source);
+ void applyDefaultProblemSolutions();
std::string report() const;
const SolverTransactionList &transactions() const;
+ void dumpTransactionList() const;
private:
static SolverTransaction::transType type(Transaction *trans, int pos);
+ void tasksToJobs(SolverTasks &tasks, updateMode update, Queue &job);
+ void solutionToTransactionList();
SolverPool &pool;
Solver *solv;
+ Queue job;
SolverTransactionList trans;
};
diff --git a/package_db.cc b/package_db.cc
index 92fe4f9..e2443bf 100644
--- a/package_db.cc
+++ b/package_db.cc
@@ -602,7 +602,7 @@ packagedb::fillMissingCategory ()
void
packagedb::defaultTrust (SolverTasks &q, SolverSolution::updateMode mode, bool
test)
{
- solution.update(q, mode, test, FALSE);
+ solution.update(q, mode, test);
// reflect that task list into packagedb
solution.trans2db();
diff --git a/prereq.cc b/prereq.cc
index bf7661a..5b3e785 100644
--- a/prereq.cc
+++ b/prereq.cc
@@ -90,6 +90,7 @@ long
PrereqPage::OnNext ()
{
HWND h = GetHWND ();
+ packagedb db;
if (!IsDlgButtonChecked (h, IDC_PREREQ_CHECK))
{
@@ -109,10 +110,16 @@ PrereqPage::OnNext ()
"NOTE! User refused the default solutions! "
"Expect some packages to give errors or not function at all." <<
endLog;
// Change the solver's transaction list to reflect the user's choices.
- packagedb db;
db.solution.db2trans();
}
}
+ else
+ {
+ db.solution.applyDefaultProblemSolutions();
+ }
+
+ PrereqChecker p;
+ p.finalize();
return whatNext();
}
@@ -157,8 +164,9 @@ PrereqPage::OnUnattended ()
// implements class PrereqChecker
// ---------------------------------------------------------------------------
-// instantiate the static member
+// instantiate the static members
bool PrereqChecker::use_test_packages;
+SolverTasks PrereqChecker::q;
bool
PrereqChecker::isMet ()
@@ -170,11 +178,19 @@ PrereqChecker::isMet ()
Progress.SetText3 ("");
// Create task list corresponding to current state of package database
- SolverTasks q;
q.setTasks();
- // apply solver to those tasks and global state (use test, include source)
- return db.solution.update(q, SolverSolution::keep, use_test_packages,
IncludeSource);
+ // apply solver to those tasks and global state (use test or not)
+ return db.solution.update(q, SolverSolution::keep, use_test_packages);
+}
+
+void
+PrereqChecker::finalize ()
+{
+ packagedb db;
+ db.solution.augmentTasks(q);
+ db.solution.addSource(IncludeSource);
+ db.solution.dumpTransactionList();
}
/* Formats problems and solutions as a string for display to the user. */
@@ -205,6 +221,8 @@ do_prereq_check_thread(HINSTANCE h, HWND owner)
if (p.isMet ())
{
+ p.finalize();
+
if (source == IDC_SOURCE_LOCALDIR)
Progress.SetActivateTask (WM_APP_START_INSTALL); // install
else
diff --git a/prereq.h b/prereq.h
index 5ae9323..f1561fa 100644
--- a/prereq.h
+++ b/prereq.h
@@ -40,10 +40,14 @@ public:
// formats 'unmet' as a string for display
void getUnmetString (std::string &s);
+ // finialize the transaction list
+ void finalize ();
+
static void setTestPackages (bool t) { use_test_packages = t; };
private:
static bool use_test_packages;
+ static SolverTasks q;
};
#endif /* SETUP_PREREQ_H */
--
2.15.1
