Re: [PATCH setup 00/14] Use libsolv, solve all our problems... (WIP)

[Jon Turney]

On 15/01/2018 21:48, Ken Brown wrote:

On 1/15/2018 2:01 PM, Jon Turney wrote:

On 13/01/2018 19:56, Ken Brown wrote:
[...]>>> 1. I think the code you added to PrereqPage::OnNext() also 
needs to

be added to PrereqPage::OnBack().


Hmm... not sure.

I don't think we want to applyDefaultProblemSolutions(), because the 
user should solve the problems their way.


Right.  I was forgetting that the user no longer has to select 'Back' to 
see the default solutions.  So we should probably delete the addendum to 
the problem report that tells them to do that.


Good point.

Re: [PATCH setup 00/14] Use libsolv, solve all our problems... (WIP)

[Ken Brown]

On 1/15/2018 2:01 PM, Jon Turney wrote:

On 13/01/2018 19:56, Ken Brown wrote:

On 1/13/2018 9:14 AM, Jon Turney wrote:

On 09/01/2018 15:49, Ken Brown wrote:

On 1/9/2018 10:37 AM, Ken Brown wrote:


I just did a quick test, trying to uninstall B in the situation above,
and it didn't work as expected.  Even though "Uninstall A" was the 
first

solution, A didn't get uninstalled.


In case you want to replicate this, what I actually tried was 
uninstalling ImageMagick, which is required, directly or indirectly, 
by asymptote, dblatex, and xmlto.  Solution 1 was to uninstall these 
three but it didn't happen.


Doh.

solver_take_solution() modifies the task list (and I went to the 
trouble of refactoring things to make it available to it), but I 
didn't then re-solve with the modified task list.


Also, looking at this again, it looks like the solver places 
solutions which remove a task at the end of of the solution list, so 
this should be the default


(This matches the previous behaviour, where the default is to accept 
dependencies i.e. if you try to remove a package required by other 
packages, the default solution should be to cancel the removal)


Maybe we need more complex criteria to identify the default, but this 
seems to work in my limited testing.


I noticed two things:

1. I think the code you added to PrereqPage::OnNext() also needs to be 
added to PrereqPage::OnBack().


Hmm... not sure.

I don't think we want to applyDefaultProblemSolutions(), because the 
user should solve the problems their way.


Right.  I was forgetting that the user no longer has to select 'Back' to 
see the default solutions.  So we should probably delete the addendum to 
the problem report that tells them to do that.



As to finalize():

Not doing augmentTasks() means that a reinstall would disappear when we 
go back, so I guess we need that.


I don't think we want to addSource() as the consequences of that are 
never shown in the chooser, currently.


2. We should probably remove, or at least reword, the dire warning 
about accepting the default solutions.  I'm not sure we want to 
"strongly recommend" the default solution over the other solution(s).  
I guess what we really want to say is that we strongly recommend 
resolving the problems before continuing.


Yes, good point.  I added your patch.

Re: [PATCH setup 00/14] Use libsolv, solve all our problems... (WIP)

[Jon Turney]

On 13/01/2018 19:56, Ken Brown wrote:

On 1/13/2018 9:14 AM, Jon Turney wrote:

On 09/01/2018 15:49, Ken Brown wrote:

On 1/9/2018 10:37 AM, Ken Brown wrote:


I just did a quick test, trying to uninstall B in the situation above,
and it didn't work as expected.  Even though "Uninstall A" was the 
first

solution, A didn't get uninstalled.


In case you want to replicate this, what I actually tried was 
uninstalling ImageMagick, which is required, directly or indirectly, 
by asymptote, dblatex, and xmlto.  Solution 1 was to uninstall these 
three but it didn't happen.


Doh.

solver_take_solution() modifies the task list (and I went to the 
trouble of refactoring things to make it available to it), but I 
didn't then re-solve with the modified task list.


Also, looking at this again, it looks like the solver places solutions 
which remove a task at the end of of the solution list, so this should 
be the default


(This matches the previous behaviour, where the default is to accept 
dependencies i.e. if you try to remove a package required by other 
packages, the default solution should be to cancel the removal)


Maybe we need more complex criteria to identify the default, but this 
seems to work in my limited testing.


I noticed two things:

1. I think the code you added to PrereqPage::OnNext() also needs to be 
added to PrereqPage::OnBack().


Hmm... not sure.

I don't think we want to applyDefaultProblemSolutions(), because the 
user should solve the problems their way.


As to finalize():

Not doing augmentTasks() means that a reinstall would disappear when we 
go back, so I guess we need that.


I don't think we want to addSource() as the consequences of that are 
never shown in the chooser, currently.


2. We should probably remove, or at least reword, the dire warning about 
accepting the default solutions.  I'm not sure we want to "strongly 
recommend" the default solution over the other solution(s).  I guess 
what we really want to say is that we strongly recommend resolving the 
problems before continuing.


Yes, good point.  I added your patch.

Re: [PATCH setup 00/14] Use libsolv, solve all our problems... (WIP)

[Ken Brown]

On 1/13/2018 8:52 PM, Brian Inglis wrote:

On 2018-01-13 17:00, Ken Brown wrote:

On 1/13/2018 5:55 PM, Ken Brown wrote:

On 1/13/2018 4:29 PM, Brian Inglis wrote:

On 2018-01-13 12:56, Ken Brown wrote:

2. We should probably remove, or at least reword, the dire warning about
accepting the default solutions.  I'm not sure we want to "strongly recommend"
the default solution over the other solution(s).  I guess what we really
want to
say is that we strongly recommend resolving the problems before continuing.


For users who only run setup and use programs, a dire warning and strong
recommendations are appropriate.

Alternatives are to also remove all packages dependent on the package to be
removed, or lastly, to remove only the requested package, leaving the
installation inconsistent. Those alternatives would have to be presented to the
user for selection, then executed.

Anything else requiring the user to resolve would require a FAQ entry explaining
what that meant, what diagnosis and actions would be required, and that would
probably generate emails from users asking what they should do.

Better to allow the solver to resolve issues and just let users choose
straightforward alternatives, with the view of trying to keep the installation
consistent, unless explicitly overridden, such as to test an alternative
implementation of a dependency installed outside of setup.


The current situation on the topic/libsolv branch is the following. Suppose A
requires B and the user asks to uninstall B.  They will get a problem report
showing two possible solutions:

1. Uninstall A.
2. (default) Don't uninstall B.

If they uncheck 'Accept default solutions' and select 'Next', they'll get a
warning that says "We strongly recommend that you accept the default
solutions.  Some packages may not work properly if you don't. Are you sure you
want to proceed?"

This is misleading insofar as it implies that something bad will happen if the
user prefers to solve the problem by uninstalling A.  What is true is that
some packages may not work properly if the user answers 'Yes'.

I think we should be able to find wording that is accurate while still making
it clear that we recommend going back and fixing the problem.  I don't yet
have a good candidate for that wording.


Something like the attached might do the job.


Just saying "Unsolved problems" does not tell the user what they did and what
impact it will have, and invites a FAQ entry for Setup - Unsolved problems.


"WARNING - Unsolved Problems" is simply the caption on the message box.


Could we please be more explicit in the UI and the logs about the action being
taken and the impact: instead of "Unsolved problems" and "Some packages" maybe
something more like "uninstalling package U will break packages P1...", and


We were already explicit when we displayed the prerequisites page with a 
problem report.  There's no need to repeat it.  What we're discussing 
right now is what warning we should give if the user, *after seeing the 
problem report*, unchecks the 'Accept default solutions' box and selects 
'Next'.



instead of "default solutions" maybe something more like "recommended actions"?


I've already explained why I think it would be misleading to say 
"recommended" rather than "default".  In the scenario above with 
packages A and B, the user may be trying to pare down their installation 
by uninstalling some packages.  They try to uninstall B, we warn them 
that this will break A, and we offer them two possible solutions.


It's perfectly reasonable for us to present "Keep B" as the default 
solution, especially since, as Jon pointed out, that's what we currently 
do.  But I don't think it's reasonable for us to *recommend* keeping B 
without knowing what the user wanted to accomplish.  Maybe uninstalling 
A better fits their needs.



That shows and records the action and impact for diagnosis and remediation when
users do something unintended and need to undo it, changing the messaging from
the implementation model to the user's model, and giving users and maintainers
enough information to diagnose what will or did happen, and how to undo that.


The current code in the topic/libsolv branch already does this.  Maybe 
it could do it better.  It would be great if you would test it and 
suggest improvements.


Ken

Re: [PATCH setup 00/14] Use libsolv, solve all our problems... (WIP)

[Brian Inglis]

On 2018-01-13 17:00, Ken Brown wrote:
> On 1/13/2018 5:55 PM, Ken Brown wrote:
>> On 1/13/2018 4:29 PM, Brian Inglis wrote:
>>> On 2018-01-13 12:56, Ken Brown wrote:
 2. We should probably remove, or at least reword, the dire warning about
 accepting the default solutions.  I'm not sure we want to "strongly 
 recommend"
 the default solution over the other solution(s).  I guess what we really
 want to
 say is that we strongly recommend resolving the problems before continuing.
>>>
>>> For users who only run setup and use programs, a dire warning and strong
>>> recommendations are appropriate.
>>>
>>> Alternatives are to also remove all packages dependent on the package to be
>>> removed, or lastly, to remove only the requested package, leaving the
>>> installation inconsistent. Those alternatives would have to be presented to 
>>> the
>>> user for selection, then executed.
>>>
>>> Anything else requiring the user to resolve would require a FAQ entry 
>>> explaining
>>> what that meant, what diagnosis and actions would be required, and that 
>>> would
>>> probably generate emails from users asking what they should do.
>>>
>>> Better to allow the solver to resolve issues and just let users choose
>>> straightforward alternatives, with the view of trying to keep the 
>>> installation
>>> consistent, unless explicitly overridden, such as to test an alternative
>>> implementation of a dependency installed outside of setup.
>>
>> The current situation on the topic/libsolv branch is the following. Suppose A
>> requires B and the user asks to uninstall B.  They will get a problem report
>> showing two possible solutions:
>>
>> 1. Uninstall A.
>> 2. (default) Don't uninstall B.
>>
>> If they uncheck 'Accept default solutions' and select 'Next', they'll get a
>> warning that says "We strongly recommend that you accept the default
>> solutions.  Some packages may not work properly if you don't. Are you sure 
>> you
>> want to proceed?"
>>
>> This is misleading insofar as it implies that something bad will happen if 
>> the
>> user prefers to solve the problem by uninstalling A.  What is true is that
>> some packages may not work properly if the user answers 'Yes'.
>>
>> I think we should be able to find wording that is accurate while still making
>> it clear that we recommend going back and fixing the problem.  I don't yet
>> have a good candidate for that wording.
> 
> Something like the attached might do the job.

Just saying "Unsolved problems" does not tell the user what they did and what
impact it will have, and invites a FAQ entry for Setup - Unsolved problems.

Could we please be more explicit in the UI and the logs about the action being
taken and the impact: instead of "Unsolved problems" and "Some packages" maybe
something more like "uninstalling package U will break packages P1...", and
instead of "default solutions" maybe something more like "recommended actions"?

That shows and records the action and impact for diagnosis and remediation when
users do something unintended and need to undo it, changing the messaging from
the implementation model to the user's model, and giving users and maintainers
enough information to diagnose what will or did happen, and how to undo that.

-- 
Take care. Thanks, Brian Inglis, Calgary, Alberta, Canada

Re: [PATCH setup 00/14] Use libsolv, solve all our problems... (WIP)

[Ken Brown]

On 1/13/2018 5:55 PM, Ken Brown wrote:

On 1/13/2018 4:29 PM, Brian Inglis wrote:

On 2018-01-13 12:56, Ken Brown wrote:

2. We should probably remove, or at least reword, the dire warning about
accepting the default solutions.  I'm not sure we want to "strongly 
recommend"
the default solution over the other solution(s).  I guess what we 
really want to
say is that we strongly recommend resolving the problems before 
continuing.


For users who only run setup and use programs, a dire warning and strong
recommendations are appropriate.

Alternatives are to also remove all packages dependent on the package 
to be

removed, or lastly, to remove only the requested package, leaving the
installation inconsistent. Those alternatives would have to be 
presented to the

user for selection, then executed.

Anything else requiring the user to resolve would require a FAQ entry 
explaining
what that meant, what diagnosis and actions would be required, and 
that would

probably generate emails from users asking what they should do.

Better to allow the solver to resolve issues and just let users choose
straightforward alternatives, with the view of trying to keep the 
installation

consistent, unless explicitly overridden, such as to test an alternative
implementation of a dependency installed outside of setup.


The current situation on the topic/libsolv branch is the following. 
Suppose A requires B and the user asks to uninstall B.  They will get a 
problem report showing two possible solutions:


1. Uninstall A.
2. (default) Don't uninstall B.

If they uncheck 'Accept default solutions' and select 'Next', they'll 
get a warning that says "We strongly recommend that you accept the 
default solutions.  Some packages may not work properly if you don't. 
Are you sure you want to proceed?"


This is misleading insofar as it implies that something bad will happen 
if the user prefers to solve the problem by uninstalling A.  What is 
true is that some packages may not work properly if the user answers 'Yes'.


I think we should be able to find wording that is accurate while still 
making it clear that we recommend going back and fixing the problem.  I 
don't yet have a good candidate for that wording.


Something like the attached might do the job.

Ken

>From d15d18dfa4db91416155385034bccf31be88ece3 Mon Sep 17 00:00:00 2001
From: Ken Brown 
Date: Sat, 13 Jan 2018 18:50:14 -0500
Subject: [PATCH] Clarify the unsolved-problems warning

If the user unchecks the 'Accept default solutions' box and selects
'Next', don't imply that choosing a non-default solution would break
their system.
---
 prereq.cc | 7 +++
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/prereq.cc b/prereq.cc
index a03e79b..4926c65 100644
--- a/prereq.cc
+++ b/prereq.cc
@@ -96,10 +96,9 @@ PrereqPage::OnNext ()
 {
   // breakage imminent!  danger, danger
   int res = MessageBox (h,
-  "We strongly recommend that you accept the default solutions. "
-  "Some packages may not work properly if you don't."
+  "Some packages may not work properly if you continue."
   "\r\n\r\n"
-  "Are you sure you want to proceed?",
+  "Are you sure you want to proceed (NOT RECOMMENDED)?",
   "WARNING - Unsolved Problems",
   MB_YESNO | MB_ICONEXCLAMATION | MB_DEFBUTTON2);
   if (res == IDNO)
@@ -107,7 +106,7 @@ PrereqPage::OnNext ()
   else
 {
   Log (LOG_PLAIN) <<
-"NOTE!  User refused the default solutions!  "
+"NOTE!  User continued with unsolved problems!  "
 "Expect some packages to give errors or not function at all." << 
endLog;
   // Change the solver's transaction list to reflect the user's 
choices.
   db.solution.db2trans();
-- 
2.15.1

Re: [PATCH setup 00/14] Use libsolv, solve all our problems... (WIP)

[Ken Brown]

On 1/13/2018 4:29 PM, Brian Inglis wrote:

On 2018-01-13 12:56, Ken Brown wrote:

2. We should probably remove, or at least reword, the dire warning about
accepting the default solutions.  I'm not sure we want to "strongly recommend"
the default solution over the other solution(s).  I guess what we really want to
say is that we strongly recommend resolving the problems before continuing.


For users who only run setup and use programs, a dire warning and strong
recommendations are appropriate.

Alternatives are to also remove all packages dependent on the package to be
removed, or lastly, to remove only the requested package, leaving the
installation inconsistent. Those alternatives would have to be presented to the
user for selection, then executed.

Anything else requiring the user to resolve would require a FAQ entry explaining
what that meant, what diagnosis and actions would be required, and that would
probably generate emails from users asking what they should do.

Better to allow the solver to resolve issues and just let users choose
straightforward alternatives, with the view of trying to keep the installation
consistent, unless explicitly overridden, such as to test an alternative
implementation of a dependency installed outside of setup.


The current situation on the topic/libsolv branch is the following. 
Suppose A requires B and the user asks to uninstall B.  They will get a 
problem report showing two possible solutions:


1. Uninstall A.
2. (default) Don't uninstall B.

If they uncheck 'Accept default solutions' and select 'Next', they'll 
get a warning that says "We strongly recommend that you accept the 
default solutions.  Some packages may not work properly if you don't. 
Are you sure you want to proceed?"


This is misleading insofar as it implies that something bad will happen 
if the user prefers to solve the problem by uninstalling A.  What is 
true is that some packages may not work properly if the user answers 'Yes'.


I think we should be able to find wording that is accurate while still 
making it clear that we recommend going back and fixing the problem.  I 
don't yet have a good candidate for that wording.


Ken

Re: [PATCH setup 00/14] Use libsolv, solve all our problems... (WIP)

[Brian Inglis]

On 2018-01-13 12:56, Ken Brown wrote:
> On 1/13/2018 9:14 AM, Jon Turney wrote:
>> On 09/01/2018 15:49, Ken Brown wrote:
>>> On 1/9/2018 10:37 AM, Ken Brown wrote:

 I just did a quick test, trying to uninstall B in the situation above,
 and it didn't work as expected.  Even though "Uninstall A" was the first
 solution, A didn't get uninstalled.
>>>
>>> In case you want to replicate this, what I actually tried was uninstalling
>>> ImageMagick, which is required, directly or indirectly, by asymptote,
>>> dblatex, and xmlto.  Solution 1 was to uninstall these three but it didn't
>>> happen.
>>
>> Doh.
>>
>> solver_take_solution() modifies the task list (and I went to the trouble of
>> refactoring things to make it available to it), but I didn't then re-solve
>> with the modified task list.
>>
>> Also, looking at this again, it looks like the solver places solutions which
>> remove a task at the end of of the solution list, so this should be the 
>> default
>>
>> (This matches the previous behaviour, where the default is to accept
>> dependencies i.e. if you try to remove a package required by other packages,
>> the default solution should be to cancel the removal)
>>
>> Maybe we need more complex criteria to identify the default, but this seems 
>> to
>> work in my limited testing.
> 
> I noticed two things:
> 
> 1. I think the code you added to PrereqPage::OnNext() also needs to be added 
> to
> PrereqPage::OnBack().
> 
> 2. We should probably remove, or at least reword, the dire warning about
> accepting the default solutions.  I'm not sure we want to "strongly recommend"
> the default solution over the other solution(s).  I guess what we really want 
> to
> say is that we strongly recommend resolving the problems before continuing.

For users who only run setup and use programs, a dire warning and strong
recommendations are appropriate.

Alternatives are to also remove all packages dependent on the package to be
removed, or lastly, to remove only the requested package, leaving the
installation inconsistent. Those alternatives would have to be presented to the
user for selection, then executed.

Anything else requiring the user to resolve would require a FAQ entry explaining
what that meant, what diagnosis and actions would be required, and that would
probably generate emails from users asking what they should do.

Better to allow the solver to resolve issues and just let users choose
straightforward alternatives, with the view of trying to keep the installation
consistent, unless explicitly overridden, such as to test an alternative
implementation of a dependency installed outside of setup.

-- 
Take care. Thanks, Brian Inglis, Calgary, Alberta, Canada

Re: [PATCH setup 00/14] Use libsolv, solve all our problems... (WIP)

[Ken Brown]

On 1/13/2018 9:14 AM, Jon Turney wrote:

On 09/01/2018 15:49, Ken Brown wrote:

On 1/9/2018 10:37 AM, Ken Brown wrote:


I just did a quick test, trying to uninstall B in the situation above,
and it didn't work as expected.  Even though "Uninstall A" was the first
solution, A didn't get uninstalled.


In case you want to replicate this, what I actually tried was 
uninstalling ImageMagick, which is required, directly or indirectly, 
by asymptote, dblatex, and xmlto.  Solution 1 was to uninstall these 
three but it didn't happen.


Doh.

solver_take_solution() modifies the task list (and I went to the trouble 
of refactoring things to make it available to it), but I didn't then 
re-solve with the modified task list.


Also, looking at this again, it looks like the solver places solutions 
which remove a task at the end of of the solution list, so this should 
be the default


(This matches the previous behaviour, where the default is to accept 
dependencies i.e. if you try to remove a package required by other 
packages, the default solution should be to cancel the removal)


Maybe we need more complex criteria to identify the default, but this 
seems to work in my limited testing.


I noticed two things:

1. I think the code you added to PrereqPage::OnNext() also needs to be 
added to PrereqPage::OnBack().


2. We should probably remove, or at least reword, the dire warning about 
accepting the default solutions.  I'm not sure we want to "strongly 
recommend" the default solution over the other solution(s).  I guess 
what we really want to say is that we strongly recommend resolving the 
problems before continuing.


Ken

Re: [PATCH setup 00/14] Use libsolv, solve all our problems... (WIP)

[Jon Turney]

On 09/01/2018 15:49, Ken Brown wrote:

On 1/9/2018 10:37 AM, Ken Brown wrote:


I just did a quick test, trying to uninstall B in the situation above,
and it didn't work as expected.  Even though "Uninstall A" was the first
solution, A didn't get uninstalled.


In case you want to replicate this, what I actually tried was 
uninstalling ImageMagick, which is required, directly or indirectly, by 
asymptote, dblatex, and xmlto.  Solution 1 was to uninstall these three 
but it didn't happen.


Doh.

solver_take_solution() modifies the task list (and I went to the trouble 
of refactoring things to make it available to it), but I didn't then 
re-solve with the modified task list.


Also, looking at this again, it looks like the solver places solutions 
which remove a task at the end of of the solution list, so this should 
be the default


(This matches the previous behaviour, where the default is to accept 
dependencies i.e. if you try to remove a package required by other 
packages, the default solution should be to cancel the removal)


Maybe we need more complex criteria to identify the default, but this 
seems to work in my limited testing.


I've rebased and updated the topic/libsolv branch.

Re: [PATCH setup 00/14] Use libsolv, solve all our problems... (WIP)

[Ken Brown]

On 1/9/2018 10:37 AM, Ken Brown wrote:

On 1/9/2018 8:25 AM, Jon Turney wrote:

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.


That's certainly easier than what I was trying to do.  If we do that, we
should probably change the UI to remove the implication that the default
solution is recommended.  For example, if A requires B and the user
tries to uninstall B, the first solution seems to usually (always?) be
"uninstall A".  This may or may not be what the user wants.

Maybe we should say something like, "By default, the first proposed
solution will be selected for each problem.  If this is not what you
want, press Back"


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.


I just did a quick test, trying to uninstall B in the situation above,
and it didn't work as expected.  Even though "Uninstall A" was the first
solution, A didn't get uninstalled.


In case you want to replicate this, what I actually tried was 
uninstalling ImageMagick, which is required, directly or indirectly, by 
asymptote, dblatex, and xmlto.  Solution 1 was to uninstall these three 
but it didn't happen.


Ken

Re: [PATCH setup 00/14] Use libsolv, solve all our problems... (WIP)

[Ken Brown]

On 1/9/2018 8:25 AM, Jon Turney wrote:

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.


That's certainly easier than what I was trying to do.  If we do that, we 
should probably change the UI to remove the implication that the default 
solution is recommended.  For example, if A requires B and the user 
tries to uninstall B, the first solution seems to usually (always?) be 
"uninstall A".  This may or may not be what the user wants.


Maybe we should say something like, "By default, the first proposed 
solution will be selected for each problem.  If this is not what you 
want, press Back"


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.


I just did a quick test, trying to uninstall B in the situation above, 
and it didn't work as expected.  Even though "Uninstall A" was the first 
solution, A didn't get uninstalled.


Ken

Re: [PATCH setup 00/14] Use libsolv, solve all our problems... (WIP)

[Jon Turney]

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 
Date: Wed, 6 Dec 2017 17:52:45 +
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();
+}
+
 SolverSolution::~SolverSolution()
 {
   clear();
@@ -615,6 +621,7 @@ SolverSolution::clear()
   solver_free(solv);
   solv = NULL;
 }
+  queue_free();
 }
 
 void
@@ -713,18 +720,9 @@ std::ostream <<(std::ostream ,
   return stream;
 }
 
-bool
-SolverSolution::update(SolverTasks , updateMode update, bool 
use_test_packages, bool include_source)
+void
+SolverSolution::tasksToJobs(SolverTasks , updateMode update, Queue )
 {
-  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();
   // 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 , 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(, SOLVER_LOCK | SOLVER_SOLVABLE, sv.id);
- break;
-   case SolverTasks::taskSkip:
- queue_push2(, SOLVER_LOCK | SOLVER_SOLVABLE_NAME, sv.name_id());
+case SolverTasks::taskKeep:
+  queue_push2(, SOLVER_LOCK | SOLVER_SOLVABLE, sv.id);
+  break;
+case SolverTasks::taskSkip:
+  queue_push2(, SOLVER_LOCK | SOLVER_SOLVABLE_NAME, sv.name_id());
   break;
 default:
   Log 

Re: [PATCH setup 00/14] Use libsolv, solve all our problems... (WIP)

[Ken Brown]

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'.


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.

Ken


0001-Implement-a-default-solution-for-SOLVER_RULE_PKG_REQ.patch

+  if (type == SOLVER_RULE_PKG_REQUIRES)
+   {
+ packagemeta *pkg = 
db.findBinary(PackageSpecification(pool_dep2str(pool.pool, dep)));
+ if (!pkg->desired && pkg->installed < pkg->default_version)
+   // User chose to uninstall or skip a required package.
+   trans.push_back(SolverTransaction(pkg->default_version,
+ SolverTransaction::transInstall));


This isn't quite right.  We also need a transErase if the package is 
installed.  Revised patch attached.


Ken
From 1930460c9f5c8a4c1aea0837778d76d8322642fa Mon Sep 17 00:00:00 2001
From: Ken Brown 
Date: Wed, 13 Dec 2017 17:20:26 -0500
Subject: [PATCH setup libsolv v2] Implement a default solution for
 SOLVER_RULE_PKG_REQUIRES

If libsolv reports a SOLVER_RULE_PKG_REQUIRES problem, it means that
the user chose to uninstall or skip a required package.  Add
appropriate transactions, if necessary, to override this choice.  The
user will have to uncheck the "Accept default problem solutions" box
to insist on the original choice.
---
 libsolv.cc | 26 +++---
 libsolv.h  |  2 +-
 2 files changed, 24 insertions(+), 4 deletions(-)

diff --git a/libsolv.cc b/libsolv.cc
index 0fb39c7..48c24c4 100644
--- a/libsolv.cc
+++ b/libsolv.cc
@@ -868,7 +868,7 @@ SolverSolution::transactions() const
 
 // Construct a string reporting the problems and solutions
 std::string
-SolverSolution::report() const
+SolverSolution::report()
 {
   packagedb db;
   std::string r = "";
@@ -881,8 +881,28 @@ SolverSolution::report() const
   Id probr = solver_findproblemrule(solv, problem);
   Id dep, source, target;
   SolverRuleinfo type = solver_ruleinfo(solv, probr, , , 
);
-  if (source == db.basepkg.id)
-   r += "package " + std::string(pool_dep2str(pool.pool, dep)) + " is a 
Base package and is therefore required";
+  if (type == SOLVER_RULE_PKG_REQUIRES)
+   {
+ // The user chose to uninstall or skip a required package.
+ // libsolv will not create an erase transaction, but we
+ // might need to create our own transactions to implement a
+ // default solution to the problem.
+ packagemeta *pkg = 
db.findBinary(PackageSpecification(pool_dep2str(pool.pool, dep)));
+ if (!pkg->desired && pkg->installed < pkg->default_version)
+   {
+ trans.push_back(SolverTransaction(pkg->default_version,
+   
SolverTransaction::transInstall));
+ if (pkg->installed)
+   trans.push_back(SolverTransaction(pkg->installed,
+ 
SolverTransaction::transErase));
+   }
+ if (source == db.basepkg.id)
+   r += "package " + std::string(pool_dep2str(pool.pool, dep))
+ + " is a Base package and is therefore required";
+ else
+   r += "package " + std::string(pool_dep2str(pool.pool, dep)) +
+ " is  required by " + std::string(pool_solvid2str(pool.pool, 
source));
+   }
   else
r += solver_problemruleinfo2str(solv, type, source, target, dep);
   r += "\n";
diff --git a/libsolv.h b/libsolv.h
index cddf76f..391a96d 100644
--- a/libsolv.h
+++ b/libsolv.h
@@ -253,7 +253,7 @@ class SolverSolution
 updateForce, // distupdate: downgrade if necessary to best version in repo
   };
   bool update(SolverTasks , updateMode update, bool use_test_packages, 
bool include_source);
-  std::string report() const;
+  std::string report();
 
   const SolverTransactionList () const;
 
-- 
2.15.1

Re: [PATCH setup 00/14] Use libsolv, solve all our problems... (WIP)

[Ken Brown]

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'.


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.

Ken

From 65cb5413e81794829cbddeebc6826d04115f2a49 Mon Sep 17 00:00:00 2001
From: Ken Brown 
Date: Wed, 13 Dec 2017 17:20:26 -0500
Subject: [PATCH] Implement a default solution for SOLVER_RULE_PKG_REQUIRES

If libsolv reports a SOLVER_RULE_PKG_REQUIRES problem, it means that
the user chose to uninstall or skip a required package.  Add an
appropriate transInstall transaction if necessary, to override this
choice.  The user will have to uncheck the "Accept default problem
solutions" box to insist on the original choice.
---
 libsolv.cc | 18 +++---
 libsolv.h  |  2 +-
 2 files changed, 16 insertions(+), 4 deletions(-)

diff --git a/libsolv.cc b/libsolv.cc
index 0fb39c7..9977926 100644
--- a/libsolv.cc
+++ b/libsolv.cc
@@ -868,7 +868,7 @@ SolverSolution::transactions() const
 
 // Construct a string reporting the problems and solutions
 std::string
-SolverSolution::report() const
+SolverSolution::report()
 {
   packagedb db;
   std::string r = "";
@@ -881,8 +881,20 @@ SolverSolution::report() const
   Id probr = solver_findproblemrule(solv, problem);
   Id dep, source, target;
   SolverRuleinfo type = solver_ruleinfo(solv, probr, , , 
);
-  if (source == db.basepkg.id)
-   r += "package " + std::string(pool_dep2str(pool.pool, dep)) + " is a 
Base package and is therefore required";
+  if (type == SOLVER_RULE_PKG_REQUIRES)
+   {
+ packagemeta *pkg = 
db.findBinary(PackageSpecification(pool_dep2str(pool.pool, dep)));
+ if (!pkg->desired && pkg->installed < pkg->default_version)
+   // User chose to uninstall or skip a required package.
+   trans.push_back(SolverTransaction(pkg->default_version,
+ SolverTransaction::transInstall));
+ if (source == db.basepkg.id)
+   r += "package " + std::string(pool_dep2str(pool.pool, dep))
+ + " is a Base package and is therefore required";
+ else
+   r += "package " + std::string(pool_dep2str(pool.pool, dep)) +
+ " is  required by " + std::string(pool_solvid2str(pool.pool, 
source));
+   }
   else
r += solver_problemruleinfo2str(solv, type, source, target, dep);
   r += "\n";
diff --git a/libsolv.h b/libsolv.h
index cddf76f..391a96d 100644
--- a/libsolv.h
+++ b/libsolv.h
@@ -253,7 +253,7 @@ class SolverSolution
 updateForce, // distupdate: downgrade if necessary to best version in repo
   };
   bool update(SolverTasks , updateMode update, bool use_test_packages, 
bool include_source);
-  std::string report() const;
+  std::string report();
 
   const SolverTransactionList () const;
 
-- 
2.15.1

Re: [PATCH setup 00/14] Use libsolv, solve all our problems... (WIP)

[Achim Gratz]

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'.

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.


Regards,
Achim.
-- 
+<[Q+ Matrix-12 WAVE#46+305 Neuron microQkb Andromeda XTk Blofeld]>+

Factory and User Sound Singles for Waldorf Q+, Q and microQ:
http://Synth.Stromeko.net/Downloads.html#WaldorfSounds

Re: [PATCH setup 00/14] Use libsolv, solve all our problems... (WIP)

[Ken Brown]

On 12/5/2017 9:32 AM, Jon Turney wrote:
It seems we're missing something to actually apply the default solution, 
so "accept default solutions" makes no changes, at the moment. (looks 
like we have to do this ourselves with solver_take_solution() ?)


I've just looked at this again, and I'm not sure what's happening.

Here's what I tried:  I had two installed packages A and B, where A 
requires B.  I tried to uninstall B and got the expected problem report 
telling me that A requires B.  The solutions presented were


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'.


On the other hand, maybe libsolv's default was to do nothing, and it's 
just a coincidence that this coincided with solution 2.  This could have 
tricked me into thinking that libsolv chose a default solution.


Also, in the dependency problem report, we should identify which of the 
possible solutions is the default one, so it's clearer what "accept 
default solutions" is going to do.


Is there in fact a default solution?  I skimmed through problems.c in 
the libsolv sources, and I didn't see any mention of a default solution.


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.


Ken

Re: [PATCH setup 00/14] Use libsolv, solve all our problems... (WIP)

[Ken Brown]

On 12/5/2017 9:32 AM, Jon Turney wrote:

On 14/09/2017 21:46, Ken Brown wrote:

On 9/14/2017 1:26 PM, Achim Gratz wrote:

Ken Brown writes:

What I've been struggling with, however, is the UI.  But now that I
think about it, maybe it isn't that hard.  It's just a matter of doing
something reasonable if the user unchecks "Accept default problem
solutions".  I'll see what I can come up with.


Well, zypper pretty much just gives you a bunch of possible solutions
and asks you to select one if there is either more than one or the
otherwise preferred solution is blocked by a lock.  There is always one
"break  package by doing " down that list.  You could
maybe offer something along those lines in the inevitable dialog box?


In the long run I think that's the way to go.  But implementing that 
is more work than I feel like doing at the moment.  For now I've gone 
with an approach that was easier to program, more like the current 
setup.exe.   If the solver finds problems (including missing 
dependencies), the user has four choices on the Prerequisite page:


1. Click Back to go back to the Chooser page, with the Pending view 
showing the solver's default solutions.


2. Click Next to accept the default solutions.


Doing some testing of per-version requires, I've been looking at this 
page quite a bit.


It seems we're missing something to actually apply the default solution, 
so "accept default solutions" makes no changes, at the moment. (looks 
like we have to do this ourselves with solver_take_solution() ?)


I'm not sure.  I thought at some point I saw "accept default solutions" 
do something, but there have been a lot of changes since then.


Also, in the dependency problem report, we should identify which of the 
possible solutions is the default one, so it's clearer what "accept 
default solutions" is going to do.


Agreed.

Ken

Re: [PATCH setup 00/14] Use libsolv, solve all our problems... (WIP)

[Jon Turney]

On 14/09/2017 21:46, Ken Brown wrote:

On 9/14/2017 1:26 PM, Achim Gratz wrote:

Ken Brown writes:

What I've been struggling with, however, is the UI.  But now that I
think about it, maybe it isn't that hard.  It's just a matter of doing
something reasonable if the user unchecks "Accept default problem
solutions".  I'll see what I can come up with.


Well, zypper pretty much just gives you a bunch of possible solutions
and asks you to select one if there is either more than one or the
otherwise preferred solution is blocked by a lock.  There is always one
"break  package by doing " down that list.  You could
maybe offer something along those lines in the inevitable dialog box?


In the long run I think that's the way to go.  But implementing that is 
more work than I feel like doing at the moment.  For now I've gone with 
an approach that was easier to program, more like the current setup.exe. 
  If the solver finds problems (including missing dependencies), the 
user has four choices on the Prerequisite page:


1. Click Back to go back to the Chooser page, with the Pending view 
showing the solver's default solutions.


2. Click Next to accept the default solutions.


Doing some testing of per-version requires, I've been looking at this 
page quite a bit.


It seems we're missing something to actually apply the default solution, 
so "accept default solutions" makes no changes, at the moment. (looks 
like we have to do this ourselves with solver_take_solution() ?)


Also, in the dependency problem report, we should identify which of the 
possible solutions is the default one, so it's clearer what "accept 
default solutions" is going to do.


3. Uncheck the "Accept default solutions" box and click Next.  If the 
user dismisses the resulting warning, setup will go ahead and do what 
the user requested.


4. Cancel.

Re: [PATCH setup 00/14] Use libsolv, solve all our problems... (WIP)

[Achim Gratz]

Jon Turney writes:
>>> You can give each repo a priority and it will pick the package from the
>>> one with the highest priority, even if another repo has a higher
>>> version.
>>
>> Yes, that works.
>
> It seems that, when test versions are enabled, a test version is
> preferred over a non-test version with a higher version
>
> I guess this is a consequence of:
>
>> void
>> SolverPool::use_test_packages(bool use_test_packages)
>> {
>>   // Give repos containing test packages higher priority than normal
>>   // if wanted, otherwise lower priority.
>>   SolvRepo::priority_t p = use_test_packages ? SolvRepo::priorityHigh : 
>> SolvRepo::priorityLow;
>
> I wonder if we just want to give test repos priorityNormal when they
> are wanted?

It really depends on what you are trying to achieve (and assuming you always
work on all enabled repositories instead of specifically selecting one).

1) To chose the highest version available among a set of repositories
(these need to be compatible in their versioning), then the priorities
of all those repos need to be the same.

2) To prefer one repo over the other regardless of versioning
(versioning need not be compatible), then raise the priority of the
preferred repo.

We currently can't use the difference between an update and
distribution upgrade in setup since we would have to record which repo
each package was originally installed from.  If we had that, an update
would keep the repository selection (even if another repo has a higher
version) and a distribution upgrade would select the highest version,
switching repositories as necessary.


Regards,
Achim.
-- 
+<[Q+ Matrix-12 WAVE#46+305 Neuron microQkb Andromeda XTk Blofeld]>+

Factory and User Sound Singles for Waldorf rackAttack:
http://Synth.Stromeko.net/Downloads.html#WaldorfSounds

Re: [PATCH setup 00/14] Use libsolv, solve all our problems... (WIP)

[Ken Brown]

On 11/23/2017 1:10 PM, Jon Turney wrote:

On 06/09/2017 03:52, Ken Brown wrote:

On 9/5/2017 2:40 PM, Achim Gratz wrote:

Jon Turney writes:

Yeah, I'm not sure if putting the test packages into a separate repo
which is disabled unless explicitly enabled is the right approach.

(Instead, perhaps it is possible to tell the solver that certain
repositories are disfavoured)


At least for zypper, which is based on the same library:

You can give each repo a priority and it will pick the package from the
one with the highest priority, even if another repo has a higher
version.


Yes, that works.


It seems that, when test versions are enabled, a test version is 
preferred over a non-test version with a higher version


I guess this is a consequence of:


void
SolverPool::use_test_packages(bool use_test_packages)
{
  // Give repos containing test packages higher priority than normal
  // if wanted, otherwise lower priority.
  SolvRepo::priority_t p = use_test_packages ? SolvRepo::priorityHigh 
: SolvRepo::priorityLow;


I wonder if we just want to give test repos priorityNormal when they are 
wanted?


Yes, that sounds right to me.

Ken

Re: [PATCH setup 00/14] Use libsolv, solve all our problems... (WIP)

[Jon Turney]

On 06/09/2017 03:52, Ken Brown wrote:

On 9/5/2017 2:40 PM, Achim Gratz wrote:

Jon Turney writes:

Yeah, I'm not sure if putting the test packages into a separate repo
which is disabled unless explicitly enabled is the right approach.

(Instead, perhaps it is possible to tell the solver that certain
repositories are disfavoured)


At least for zypper, which is based on the same library:

You can give each repo a priority and it will pick the package from the
one with the highest priority, even if another repo has a higher
version.


Yes, that works.


It seems that, when test versions are enabled, a test version is 
preferred over a non-test version with a higher version


I guess this is a consequence of:


void
SolverPool::use_test_packages(bool use_test_packages)
{
  // Give repos containing test packages higher priority than normal
  // if wanted, otherwise lower priority.
  SolvRepo::priority_t p = use_test_packages ? SolvRepo::priorityHigh : 
SolvRepo::priorityLow;


I wonder if we just want to give test repos priorityNormal when they are 
wanted?

Re: [PATCH setup 00/14] Use libsolv, solve all our problems... (WIP)

[Ken Brown]

On 10/18/2017 11:28 AM, Ken Brown wrote:
Similar considerations apply to the other public member functions of 
SolvableVersion.  So my inclination is to go with something like my 
patch...


...with perhaps one tweak.  Maybe we should test 'id' rather than 
'pool', since id being 0 is what's used elsewhere to characterize the 
empty package.  And if id != 0 but pool == 0, there's a bug that we want 
to catch.


Revised patch attached.

Ken

From 948db09180765d89639b63e37a98d3806bf199d5 Mon Sep 17 00:00:00 2001
From: Ken Brown 
Date: Tue, 17 Oct 2017 08:12:48 -0400
Subject: [PATCH] Extend the SolvableVersion member functions to the empty
 package

Currently some of these functions cause crashes when the package is
empty because the libsolv function pool_id2solvable unconditionally
dereferences its first argument ('pool').
---
 libsolv.cc | 16 +++-
 1 file changed, 15 insertions(+), 1 deletion(-)

diff --git a/libsolv.cc b/libsolv.cc
index 78e73a8..289f19c 100644
--- a/libsolv.cc
+++ b/libsolv.cc
@@ -75,6 +75,8 @@ RelId2Operator(Id id)
 const std::string
 SolvableVersion::Name () const
 {
+  if (!id)
+return "";
   Solvable *solvable = pool_id2solvable(pool, id);
   return std::string(pool_id2str(pool, solvable->name));
 }
@@ -82,6 +84,8 @@ SolvableVersion::Name () const
 const std::string
 SolvableVersion::Canonical_version() const
 {
+  if (!id)
+return "";
   Solvable *solvable = pool_id2solvable(pool, id);
   return std::string(pool_id2str(pool, solvable->evr));
 }
@@ -89,6 +93,8 @@ SolvableVersion::Canonical_version() const
 package_type_t
 SolvableVersion::Type () const
 {
+  if (!id)
+return package_binary;
   Solvable *solvable = pool_id2solvable(pool, id);
   if (solvable->arch == ARCH_SRC)
 return package_source;
@@ -112,6 +118,9 @@ SolvableVersion::obsoletes() const
 const PackageDepends
 SolvableVersion::deplist(Id keyname) const
 {
+  static PackageDepends empty_package;
+  if (!id)
+return empty_package;
   Solvable *solvable = pool_id2solvable(pool, id);
 
   Queue q;
@@ -147,13 +156,14 @@ SolvableVersion::deplist(Id keyname) const
 }
 
   // otherwise, return an empty depends list
-  static PackageDepends empty_package;
   return empty_package;
 }
 
 const std::string
 SolvableVersion::SDesc () const
 {
+  if (!id)
+return "";
   Solvable *solvable = pool_id2solvable(pool, id);
   const char *sdesc = repo_lookup_str(solvable->repo, id, SOLVABLE_SUMMARY);
   return sdesc;
@@ -197,6 +207,8 @@ SolvableVersion::sourcePackage () const
 void
 SolvableVersion::fixup_spkg_id (SolvableVersion spkg_id) const
 {
+  if (!id)
+return;
   Solvable *solvable = pool_id2solvable(pool, id);
   Repodata *data = repo_last_repodata(solvable->repo);
   Id handle = id;
@@ -237,6 +249,8 @@ SolvableVersion::accessible () const
 package_stability_t
 SolvableVersion::Stability () const
 {
+  if (!id)
+return TRUST_UNKNOWN;
   Solvable *solvable = pool_id2solvable(pool, id);
   Id stability_attr = pool_str2id(pool, "solvable:stability", 1);
   return (package_stability_t)repo_lookup_num(solvable->repo, id, 
stability_attr, TRUST_UNKNOWN);
-- 
2.14.2

Re: [PATCH setup 00/14] Use libsolv, solve all our problems... (WIP)

[Ken Brown]

On 10/17/2017 2:46 PM, Jon Turney wrote:

On 17/10/2017 13:44, Ken Brown wrote:

On 10/10/2017 7:18 AM, Ken Brown wrote:

On 9/29/2017 4:33 PM, Ken Brown wrote:

I'll resume my testing after I return.


I've just started testing (based on the current HEAD of 
topic/libsolv), and so far everything looks good.


I came across a situation where a SolvableVersion method was being 
called on a trivial object (with pool and id both 0).  This caused a 
crash when pool_id2solvable(pool, id) was called and pool was 
dereferenced.  There's probably a bug that led to this situation.  [It 
involved a local install in which a package was listed in two 
different setup.ini files, but the tarballs existed only in one.]  I 
plan to investigate this further.  But in any case, we shouldn't 
crash.  Patch attached.


I thought about putting this in, but decided against it as it would 
probably catch some mistakes I had made...


But yeah, for production use, I think not crashing is probably a good 
idea :).  Although I guess we might want asserts or something, if we 
think these cases shouldn't be happening.


Here's the situation where I got the crash: Package A is installed and 
requires B.  setup tries to install B, but the install fails for some 
reason.  During the postinstall phase, we're scanning the dependencies 
of A and we call checkForInstalled to see if B is installed.  This ends 
up calling PackageSpecification::satisfies(aPackage), with aPackage 
being the empty package (pool == NULL, id == 0) because B is not 
installed.  A call to aPackage.Name() then causes the crash.


I think this sequence of calls is reasonable.  Name() is part of the 
public interface to SolvableVersion, so we should be able to call Name() 
on any package without a crash or assertion violation.  In the case of 
the empty package, the empty string is a reasonable return value.


Similar considerations apply to the other public member functions of 
SolvableVersion.  So my inclination is to go with something like my 
patch rather than changing all callers.


Ken

Re: [PATCH setup 00/14] Use libsolv, solve all our problems... (WIP)

[Jon Turney]

On 17/10/2017 13:44, Ken Brown wrote:

On 10/10/2017 7:18 AM, Ken Brown wrote:

On 9/29/2017 4:33 PM, Ken Brown wrote:

I'll resume my testing after I return.


I've just started testing (based on the current HEAD of 
topic/libsolv), and so far everything looks good.


I came across a situation where a SolvableVersion method was being 
called on a trivial object (with pool and id both 0).  This caused a 
crash when pool_id2solvable(pool, id) was called and pool was 
dereferenced.  There's probably a bug that led to this situation.  [It 
involved a local install in which a package was listed in two different 
setup.ini files, but the tarballs existed only in one.]  I plan to 
investigate this further.  But in any case, we shouldn't crash.  Patch 
attached.


I thought about putting this in, but decided against it as it would 
probably catch some mistakes I had made...


But yeah, for production use, I think not crashing is probably a good 
idea :).  Although I guess we might want asserts or something, if we 
think these cases shouldn't be happening.

Re: [PATCH setup 00/14] Use libsolv, solve all our problems... (WIP)

[Ken Brown]

On 10/10/2017 7:18 AM, Ken Brown wrote:

On 9/29/2017 4:33 PM, Ken Brown wrote:

I'll resume my testing after I return.


I've just started testing (based on the current HEAD of topic/libsolv), 
and so far everything looks good.


I came across a situation where a SolvableVersion method was being 
called on a trivial object (with pool and id both 0).  This caused a 
crash when pool_id2solvable(pool, id) was called and pool was 
dereferenced.  There's probably a bug that led to this situation.  [It 
involved a local install in which a package was listed in two different 
setup.ini files, but the tarballs existed only in one.]  I plan to 
investigate this further.  But in any case, we shouldn't crash.  Patch 
attached.


Ken
From f3b3c60ed473a1ef4e5b1ae5fcd1bfc46a6210fb Mon Sep 17 00:00:00 2001
From: Ken Brown 
Date: Tue, 17 Oct 2017 08:12:48 -0400
Subject: [PATCH] Avoid dereferencing NULL pointers

The libsolv function pool_id2solvable unconditionally dereferences its
first argument ('pool').  Callers must check that this argument is
non-NULL to avoid crashes.
---
 libsolv.cc | 16 +++-
 1 file changed, 15 insertions(+), 1 deletion(-)

diff --git a/libsolv.cc b/libsolv.cc
index 78e73a8..3a244d4 100644
--- a/libsolv.cc
+++ b/libsolv.cc
@@ -75,6 +75,8 @@ RelId2Operator(Id id)
 const std::string
 SolvableVersion::Name () const
 {
+  if (!pool)
+return "";
   Solvable *solvable = pool_id2solvable(pool, id);
   return std::string(pool_id2str(pool, solvable->name));
 }
@@ -82,6 +84,8 @@ SolvableVersion::Name () const
 const std::string
 SolvableVersion::Canonical_version() const
 {
+  if (!pool)
+return "";
   Solvable *solvable = pool_id2solvable(pool, id);
   return std::string(pool_id2str(pool, solvable->evr));
 }
@@ -89,6 +93,8 @@ SolvableVersion::Canonical_version() const
 package_type_t
 SolvableVersion::Type () const
 {
+  if (!pool)
+return package_binary;
   Solvable *solvable = pool_id2solvable(pool, id);
   if (solvable->arch == ARCH_SRC)
 return package_source;
@@ -112,6 +118,9 @@ SolvableVersion::obsoletes() const
 const PackageDepends
 SolvableVersion::deplist(Id keyname) const
 {
+  static PackageDepends empty_package;
+  if (!pool)
+return empty_package;
   Solvable *solvable = pool_id2solvable(pool, id);
 
   Queue q;
@@ -147,13 +156,14 @@ SolvableVersion::deplist(Id keyname) const
 }
 
   // otherwise, return an empty depends list
-  static PackageDepends empty_package;
   return empty_package;
 }
 
 const std::string
 SolvableVersion::SDesc () const
 {
+  if (!pool)
+return "";
   Solvable *solvable = pool_id2solvable(pool, id);
   const char *sdesc = repo_lookup_str(solvable->repo, id, SOLVABLE_SUMMARY);
   return sdesc;
@@ -197,6 +207,8 @@ SolvableVersion::sourcePackage () const
 void
 SolvableVersion::fixup_spkg_id (SolvableVersion spkg_id) const
 {
+  if (!pool)
+return;
   Solvable *solvable = pool_id2solvable(pool, id);
   Repodata *data = repo_last_repodata(solvable->repo);
   Id handle = id;
@@ -237,6 +249,8 @@ SolvableVersion::accessible () const
 package_stability_t
 SolvableVersion::Stability () const
 {
+  if (!pool)
+return TRUST_UNKNOWN;
   Solvable *solvable = pool_id2solvable(pool, id);
   Id stability_attr = pool_str2id(pool, "solvable:stability", 1);
   return (package_stability_t)repo_lookup_num(solvable->repo, id, 
stability_attr, TRUST_UNKNOWN);
-- 
2.14.2

Re: [PATCH setup 00/14] Use libsolv, solve all our problems... (WIP)

[Jon Turney]

On 10/10/2017 12:18, Ken Brown wrote:
I've just started testing (based on the current HEAD of topic/libsolv), 
and so far everything looks good.  My only suggestion is a cosmetic one. 
  Now that you've made "Test" a checkbox, independent of the radio 
buttons "Keep/Current/Sync", users might be confused by the fact that 
"Current" remains selected when "Test" is checked.


Initially, I didn't want to make any UI change here, but it made it a 
lot easier to think about how map the command line options to solver flags.


But yeah, I wasn't sure if I should change this or not.  I kept it the 
same just to keep the default the same, but that doesn't seem a very 
good reason.


"Test" should probably be something a little less terse, as well.

> In fact, the
"Current" button has a different meaning now than it used to have. Maybe 
the "Current" label should be changed to "Best", in keeping with its new 
meaning (and tooltip).


People seem to sometimes think "Current" means (something like) "Sync", 
when it fact it is (something like) "Best", so perhaps that's another 
reason why a change of name is a good idea.

Re: [PATCH setup 00/14] Use libsolv, solve all our problems... (WIP)

[Jon Turney]

On 02/10/2017 16:17, Marco Atzeri wrote:

On 02/10/2017 16:07, Jon Turney wrote:


https://cygwin.com/setup/setup-2.881-38-g6a01c5.x86_64.exe


Thanks for testing.


interesting question about precedence as it tries to install
cscope-15.8.0.1-2 and speex 1.2.0-2


So, this is change in behaviour. When asked to upgrade, setup will now 
always install the version which libsolv thinks has the highest version.


These versions are higher, because numeric sequences are considered 
higher than alphabetic sequences (i.e. '0' > 'b', '0' > 'rc').


Trying to control which version which gets upgraded to by setting the 
curr: value in setup.ini isn't actually always working, currently 
(perhaps this will be the subject of a whole other email...)



that is right for speex but wrong for cscope following the setup.ini
and the age of the files

version: 15.8b-1
[prev]
version: 15.8.0.1-2


So, this is actually a bug in the version sorting which calm does

(it was comparing the alphabetic sequence 'b' against the separator '.', 
rather than against the numeric sequence '0')



speex
version: 1.2.0-2
[prev]
version: 1.2rc1-1


And this is the right way round currently, only because it was noticed 
it wasn't sorted correctly when uploaded, and an override.hint added.



It seems previous setup was doing the opposite precedence
that is wrong for speex and right for cscope


I'll just point out that these two packages were made by people with 
incompatible ideas about how versions order.


I'm not going down the path of trying to support that, as that way 
madness lies...

Re: [PATCH setup 00/14] Use libsolv, solve all our problems... (WIP)

[Marco Atzeri]

On 02/10/2017 16:07, Jon Turney wrote:


https://cygwin.com/setup/setup-2.881-38-g6a01c5.x86_64.exe



interesting question about precedence as it tries to install
cscope-15.8.0.1-2 and speex 1.2.0-2
that is right for speex but wrong for cscope following the setup.ini
and the age of the files

version: 15.8b-1
[prev]
version: 15.8.0.1-2

speex
version: 1.2.0-2
[prev]
version: 1.2rc1-1

It seems previous setup was doing the opposite precedence
that is wrong for speex and right for cscope

Regards
Marco

Re: [PATCH setup 00/14] Use libsolv, solve all our problems... (WIP)

[Jon Turney]

On 29/09/2017 21:33, Ken Brown wrote:

On 9/29/2017 1:38 PM, Jon Turney wrote:


Thanks, but that was more me thinking out loud.  Since it seems pretty 
straightforward, I've written this change.


Even better!  I'll resume my testing after I return.


If anyone else would like to give this some testing, I built:

https://cygwin.com/setup/setup-2.881-38-g6a01c5.x86.exe
https://cygwin.com/setup/setup-2.881-38-g6a01c5.x86_64.exe

These are cross-built on Fedora, and require mingw{32,64}-libsolv-static 
in addition to the usual cross-packages, which can be built using the 
RPM spec at [1], or installed from this COPR [2]


[1] https://github.com/jon-turney/mingw-libsolv-rpm
[2] https://copr.fedorainfracloud.org/coprs/jturney/mingw-libsolv/

Re: [PATCH setup 00/14] Use libsolv, solve all our problems... (WIP)

[Ken Brown]

On 9/29/2017 1:38 PM, Jon Turney wrote:

On 27/09/2017 21:33, Ken Brown wrote:

On 9/27/2017 3:14 PM, Jon Turney wrote:

On 26/09/2017 17:06, Ken Brown wrote:

On 9/26/2017 10:50 AM, Jon Turney wrote:

On 15/09/2017 17:53, Ken Brown wrote:

On 9/15/2017 11:15 AM, Jon Turney wrote:

[...]
If we select 'curr', then the latest version of all installed 
packages is selected by the picker and will be shown in the pending 
view, and gets fed into the solver.


I guess this is technically wrong: really we should ask the solver 
to do SOLVER_UPDATE | SOLVER_SOLVABLE_ALL, which will come up with 
a solution which updates all installed packages to the latest 
possible version, subject to any other constraints which exist.


I suspect there's no difference between these two at the moment, 
though.


Not sure how to do this properly, though.


One possibility is that we feed all our information to the solver 
before going to the chooser page.  We could then present the 
solver's initial solution in the pending view as the first thing the 
user sees.


Yes, this sounds about right.

Thanks to your work we kind of have a bidirectional conversion 
between packagedb pick/desired state and a SolverTransactionList now, 
so this should be possible.


(Although it should be done when the state of the 'Keep' or 'Current' 
control changes - note that changing this setting clears any manual 
picks currently. [and there's a mechanism to apply the initial state 
of that control initially])


OK, I'll work on this.  I'm traveling at the moment, but I should be 
able to do it in a couple weeks.


Thanks, but that was more me thinking out loud.  Since it seems pretty 
straightforward, I've written this change.


Even better!  I'll resume my testing after I return.

Ken

Re: [PATCH setup 00/14] Use libsolv, solve all our problems... (WIP)

[Jon Turney]

On 27/09/2017 21:33, Ken Brown wrote:

On 9/27/2017 3:14 PM, Jon Turney wrote:

On 26/09/2017 17:06, Ken Brown wrote:

On 9/26/2017 10:50 AM, Jon Turney wrote:

On 15/09/2017 17:53, Ken Brown wrote:

On 9/15/2017 11:15 AM, Jon Turney wrote:

[...]
If we select 'curr', then the latest version of all installed 
packages is selected by the picker and will be shown in the pending 
view, and gets fed into the solver.


I guess this is technically wrong: really we should ask the solver 
to do SOLVER_UPDATE | SOLVER_SOLVABLE_ALL, which will come up with a 
solution which updates all installed packages to the latest possible 
version, subject to any other constraints which exist.


I suspect there's no difference between these two at the moment, 
though.


Not sure how to do this properly, though.


One possibility is that we feed all our information to the solver 
before going to the chooser page.  We could then present the solver's 
initial solution in the pending view as the first thing the user sees.


Yes, this sounds about right.

Thanks to your work we kind of have a bidirectional conversion between 
packagedb pick/desired state and a SolverTransactionList now, so this 
should be possible.


(Although it should be done when the state of the 'Keep' or 'Current' 
control changes - note that changing this setting clears any manual 
picks currently. [and there's a mechanism to apply the initial state 
of that control initially])


OK, I'll work on this.  I'm traveling at the moment, but I should be 
able to do it in a couple weeks.


Thanks, but that was more me thinking out loud.  Since it seems pretty 
straightforward, I've written this change.


I've rebased the topic/libsolv branch, with the following additional 
changes:


* Applied your unrelated change to .gitignore to master
* Dropped a couple patches of mine which weren't supposed to escape yet.
* Squashed your "fix typo" patches into the patch they fix (I'm not sure 
how I managed to make that typo in bootstrap.sh! :))
* Rather than add a parameter to SolvableVersion::depends() to make it 
return obsoletes, I've factored out the key-to-deplist code as a helper 
function, and added SolvableVersion::obsoletes(). (There may be other 
deplist attributes in future, so this seems the sane way to do it.)

* Added handling of Source: lines in setup.ini

Re: [PATCH setup 00/14] Use libsolv, solve all our problems... (WIP)

[Ken Brown]

On 9/27/2017 3:14 PM, Jon Turney wrote:

On 26/09/2017 17:06, Ken Brown wrote:

On 9/26/2017 10:50 AM, Jon Turney wrote:

On 15/09/2017 17:53, Ken Brown wrote:

On 9/15/2017 11:15 AM, Jon Turney wrote:

[...]
If we select 'curr', then the latest version of all installed 
packages is selected by the picker and will be shown in the pending 
view, and gets fed into the solver.


I guess this is technically wrong: really we should ask the solver to 
do SOLVER_UPDATE | SOLVER_SOLVABLE_ALL, which will come up with a 
solution which updates all installed packages to the latest possible 
version, subject to any other constraints which exist.


I suspect there's no difference between these two at the moment, though.

Not sure how to do this properly, though.


One possibility is that we feed all our information to the solver 
before going to the chooser page.  We could then present the solver's 
initial solution in the pending view as the first thing the user sees.


Yes, this sounds about right.

Thanks to your work we kind of have a bidirectional conversion between 
packagedb pick/desired state and a SolverTransactionList now, so this 
should be possible.


(Although it should be done when the state of the 'Keep' or 'Current' 
control changes - note that changing this setting clears any manual 
picks currently. [and there's a mechanism to apply the initial state of 
that control initially])


OK, I'll work on this.  I'm traveling at the moment, but I should be 
able to do it in a couple weeks.


Ken

Re: [PATCH setup 00/14] Use libsolv, solve all our problems... (WIP)

[Jon Turney]

On 26/09/2017 17:06, Ken Brown wrote:

On 9/26/2017 10:50 AM, Jon Turney wrote:

On 15/09/2017 17:53, Ken Brown wrote:

On 9/15/2017 11:15 AM, Jon Turney wrote:

[...]
If we select 'curr', then the latest version of all installed packages 
is selected by the picker and will be shown in the pending view, and 
gets fed into the solver.


I guess this is technically wrong: really we should ask the solver to 
do SOLVER_UPDATE | SOLVER_SOLVABLE_ALL, which will come up with a 
solution which updates all installed packages to the latest possible 
version, subject to any other constraints which exist.


I suspect there's no difference between these two at the moment, though.

Not sure how to do this properly, though.


One possibility is that we feed all our information to the solver before 
going to the chooser page.  We could then present the solver's initial 
solution in the pending view as the first thing the user sees.


Yes, this sounds about right.

Thanks to your work we kind of have a bidirectional conversion between 
packagedb pick/desired state and a SolverTransactionList now, so this 
should be possible.


(Although it should be done when the state of the 'Keep' or 'Current' 
control changes - note that changing this setting clears any manual 
picks currently. [and there's a mechanism to apply the initial state of 
that control initially])

Re: [PATCH setup 00/14] Use libsolv, solve all our problems... (WIP)

[Ken Brown]

On 9/26/2017 10:50 AM, Jon Turney wrote:

On 15/09/2017 17:53, Ken Brown wrote:

On 9/15/2017 11:15 AM, Jon Turney wrote:

On 08/09/2017 19:54, Ken Brown wrote:
Finally, I have a question for you, Jon: You introduced 
PrereqChecker::upgrade, which is true if and only if the user 
selects Current or Test.  I don't see why this is needed.  I've 
disabled the use of it and haven't noticed any ill effects.  Am I 
missing something?


This is supposed to be passed into SolverSolution::update() and used 
to determine if a SOLVER_UPDATE | SOLVER_SOLVABLE_ALL task is given 
to the solver, causing all packages to be updated (if possible) (i.e. 
so 'Keep' doesn't update anything)


I've already arranged (by using SOLVER_LOCK) that 'Keep' doesn't 
update anything.  So I don't think we need to worry about that case.  
On the other hand, if 'Current' or 'Test' is selected, then we already 
upgrade as appropriate in the task list sent to the solver, so I don't 
think it's necessary to send SOLVER_UPDATE | SOLVER_SOLVABLE_ALL to 
the solver in that case either.


Yeah, I see. Sigh.

If we select 'curr', then the latest version of all installed packages 
is selected by the picker and will be shown in the pending view, and 
gets fed into the solver.


I guess this is technically wrong: really we should ask the solver to do 
SOLVER_UPDATE | SOLVER_SOLVABLE_ALL, which will come up with a solution 
which updates all installed packages to the latest possible version, 
subject to any other constraints which exist.


I suspect there's no difference between these two at the moment, though.

Not sure how to do this properly, though.


One possibility is that we feed all our information to the solver before 
going to the chooser page.  We could then present the solver's initial 
solution in the pending view as the first thing the user sees.


Ken

Re: [PATCH setup 00/14] Use libsolv, solve all our problems... (WIP)

[Jon Turney]

On 15/09/2017 17:53, Ken Brown wrote:

On 9/15/2017 11:15 AM, Jon Turney wrote:

On 08/09/2017 19:54, Ken Brown wrote:
Finally, I have a question for you, Jon: You introduced 
PrereqChecker::upgrade, which is true if and only if the user selects 
Current or Test.  I don't see why this is needed.  I've disabled the 
use of it and haven't noticed any ill effects.  Am I missing something?


This is supposed to be passed into SolverSolution::update() and used 
to determine if a SOLVER_UPDATE | SOLVER_SOLVABLE_ALL task is given to 
the solver, causing all packages to be updated (if possible) (i.e. so 
'Keep' doesn't update anything)


I've already arranged (by using SOLVER_LOCK) that 'Keep' doesn't update 
anything.  So I don't think we need to worry about that case.  On the 
other hand, if 'Current' or 'Test' is selected, then we already upgrade 
as appropriate in the task list sent to the solver, so I don't think 
it's necessary to send SOLVER_UPDATE | SOLVER_SOLVABLE_ALL to the solver 
in that case either.


Yeah, I see. Sigh.

If we select 'curr', then the latest version of all installed packages 
is selected by the picker and will be shown in the pending view, and 
gets fed into the solver.


I guess this is technically wrong: really we should ask the solver to do 
SOLVER_UPDATE | SOLVER_SOLVABLE_ALL, which will come up with a solution 
which updates all installed packages to the latest possible version, 
subject to any other constraints which exist.


I suspect there's no difference between these two at the moment, though.

Not sure how to do this properly, though.

Re: [PATCH setup 00/14] Use libsolv, solve all our problems... (WIP)

[Ken Brown]

On 9/19/2017 12:46 PM, Jon Turney wrote:

On 19/09/2017 13:24, Ken Brown wrote:

On 9/16/2017 12:21 PM, Ken Brown wrote:

On 9/15/2017 3:24 PM, Jon Turney wrote:
Can you rebase your and my patches onto master, and push to 
sourceware in a topic/libsolv branch?


I've done the first part, but I don't seem to have the right 
permissions for the second:


$ git push -u origin topic/libsolv
Counting objects: 221, done.
Delta compression using up to 4 threads.
Compressing objects: 100% (221/221), done.
Writing objects: 100% (221/221), 49.10 KiB | 1.82 MiB/s, done.
Total 221 (delta 180), reused 0 (delta 0)
fatal: Unable to create temporary file: Permission denied
error: remote unpack failed: index-pack abnormal exit
To cygwin.com:/git/cygwin-setup.git
  ! [remote rejected] topic/libsolv -> topic/libsolv (n/a (unpacker 
error))

error: failed to push some refs to 'cygwin.com:/git/cygwin-setup.git'

I think I used to have commit access on sourceware, but it's possible 
that my ssh key has changed since I last tried.  I can't really tell 
from the error message whether that's the issue.


Can you or Yaakov help with this?  My recollection is that when 
Corinna first gave me access, she sent me an email with a link to 
click on to submit my ssh key.


I just checked that I can still ssh into sourceware, so I don't think 
it's an issue with my ssh key.  Maybe someone just needs to give me 
permission to write to the cygwin-setup repo.


Yes, additional permissions are required since it's under cygwin-apps.

You should be good now.


Thanks.  I've pushed the branch.

Ken

Re: [PATCH setup 00/14] Use libsolv, solve all our problems... (WIP)

[Jon Turney]

On 19/09/2017 13:24, Ken Brown wrote:

On 9/16/2017 12:21 PM, Ken Brown wrote:

On 9/15/2017 3:24 PM, Jon Turney wrote:
Can you rebase your and my patches onto master, and push to 
sourceware in a topic/libsolv branch?


I've done the first part, but I don't seem to have the right 
permissions for the second:


$ git push -u origin topic/libsolv
Counting objects: 221, done.
Delta compression using up to 4 threads.
Compressing objects: 100% (221/221), done.
Writing objects: 100% (221/221), 49.10 KiB | 1.82 MiB/s, done.
Total 221 (delta 180), reused 0 (delta 0)
fatal: Unable to create temporary file: Permission denied
error: remote unpack failed: index-pack abnormal exit
To cygwin.com:/git/cygwin-setup.git
  ! [remote rejected] topic/libsolv -> topic/libsolv (n/a (unpacker 
error))

error: failed to push some refs to 'cygwin.com:/git/cygwin-setup.git'

I think I used to have commit access on sourceware, but it's possible 
that my ssh key has changed since I last tried.  I can't really tell 
from the error message whether that's the issue.


Can you or Yaakov help with this?  My recollection is that when 
Corinna first gave me access, she sent me an email with a link to 
click on to submit my ssh key.


I just checked that I can still ssh into sourceware, so I don't think 
it's an issue with my ssh key.  Maybe someone just needs to give me 
permission to write to the cygwin-setup repo.


Yes, additional permissions are required since it's under cygwin-apps.

You should be good now.

Re: [PATCH setup 00/14] Use libsolv, solve all our problems... (WIP)

[Ken Brown]

On 9/16/2017 12:21 PM, Ken Brown wrote:

On 9/15/2017 3:24 PM, Jon Turney wrote:
Can you rebase your and my patches onto master, and push to sourceware 
in a topic/libsolv branch?


I've done the first part, but I don't seem to have the right permissions 
for the second:


$ git push -u origin topic/libsolv
Counting objects: 221, done.
Delta compression using up to 4 threads.
Compressing objects: 100% (221/221), done.
Writing objects: 100% (221/221), 49.10 KiB | 1.82 MiB/s, done.
Total 221 (delta 180), reused 0 (delta 0)
fatal: Unable to create temporary file: Permission denied
error: remote unpack failed: index-pack abnormal exit
To cygwin.com:/git/cygwin-setup.git
  ! [remote rejected] topic/libsolv -> topic/libsolv (n/a (unpacker error))
error: failed to push some refs to 'cygwin.com:/git/cygwin-setup.git'

I think I used to have commit access on sourceware, but it's possible 
that my ssh key has changed since I last tried.  I can't really tell 
from the error message whether that's the issue.


Can you or Yaakov help with this?  My recollection is that when Corinna 
first gave me access, she sent me an email with a link to click on to 
submit my ssh key.


I just checked that I can still ssh into sourceware, so I don't think 
it's an issue with my ssh key.  Maybe someone just needs to give me 
permission to write to the cygwin-setup repo.


Ken

Re: [PATCH setup 00/14] Use libsolv, solve all our problems... (WIP)

[Ken Brown]

On 9/15/2017 4:56 PM, cyg Simple wrote:

On 9/15/2017 12:53 PM, Ken Brown wrote:

On 9/15/2017 11:15 AM, Jon Turney wrote:

On 08/09/2017 19:54, Ken Brown wrote:

Finally, I have a question for you, Jon: You introduced
PrereqChecker::upgrade, which is true if and only if the user selects
Current or Test.  I don't see why this is needed.  I've disabled the
use of it and haven't noticed any ill effects.  Am I missing something?


This is supposed to be passed into SolverSolution::update() and used
to determine if a SOLVER_UPDATE | SOLVER_SOLVABLE_ALL task is given to
the solver, causing all packages to be updated (if possible) (i.e. so
'Keep' doesn't update anything)


I've already arranged (by using SOLVER_LOCK) that 'Keep' doesn't update
anything.  So I don't think we need to worry about that case.  On the
other hand, if 'Current' or 'Test' is selected, then we already upgrade
as appropriate in the task list sent to the solver, so I don't think
it's necessary to send SOLVER_UPDATE | SOLVER_SOLVABLE_ALL to the solver
in that case either.

Anyway, as I said, I've already disabled the use of
PrereqChecker::upgrade.  More precisely, I've changed
SolverSolution::update so that it never sends SOLVER_UPDATE |
SOLVER_SOLVABLE_ALL.  As a result, PrereqChecker::upgrade is simply
ignored, and everything seems to be working OK.



Since this discussion appears to resolve around "test" versus
"production" releases


No, it's simply a technical discussion about a new implementation of 
setup's dependency checker.



would it not be better served if there were two
differing base paths, one for production and one for test?  If that
occurred then there may be more testers as what is used for production
isn't borked by a test version of some package or even Cygwin itself.

So for example C:\cygwin64 serves the production path while
C:\cygwin64test serves the test path.  This would help segregate test
from a production environment without worrying the user with needing to
revert back if something fails.


I'm not sure what you're suggesting.  Anyone is free to create two 
different Cygwin installations and use one of them for testing.  In 
fact, I do that all the time.  But I don't see that this is something 
for setup to manage.


Ken

Re: [PATCH setup 00/14] Use libsolv, solve all our problems... (WIP)

[Ken Brown]

On 9/15/2017 3:24 PM, Jon Turney wrote:
Can you rebase your and my patches onto master, and push to sourceware 
in a topic/libsolv branch?


I've done the first part, but I don't seem to have the right permissions 
for the second:


$ git push -u origin topic/libsolv
Counting objects: 221, done.
Delta compression using up to 4 threads.
Compressing objects: 100% (221/221), done.
Writing objects: 100% (221/221), 49.10 KiB | 1.82 MiB/s, done.
Total 221 (delta 180), reused 0 (delta 0)
fatal: Unable to create temporary file: Permission denied
error: remote unpack failed: index-pack abnormal exit
To cygwin.com:/git/cygwin-setup.git
 ! [remote rejected] topic/libsolv -> topic/libsolv (n/a (unpacker error))
error: failed to push some refs to 'cygwin.com:/git/cygwin-setup.git'

I think I used to have commit access on sourceware, but it's possible 
that my ssh key has changed since I last tried.  I can't really tell 
from the error message whether that's the issue.


Can you or Yaakov help with this?  My recollection is that when Corinna 
first gave me access, she sent me an email with a link to click on to 
submit my ssh key.


Ken

Re: [PATCH setup 00/14] Use libsolv, solve all our problems... (WIP)

[cyg Simple]

On 9/15/2017 12:53 PM, Ken Brown wrote:
> On 9/15/2017 11:15 AM, Jon Turney wrote:
>> On 08/09/2017 19:54, Ken Brown wrote:
>>> Finally, I have a question for you, Jon: You introduced
>>> PrereqChecker::upgrade, which is true if and only if the user selects
>>> Current or Test.  I don't see why this is needed.  I've disabled the
>>> use of it and haven't noticed any ill effects.  Am I missing something?
>>
>> This is supposed to be passed into SolverSolution::update() and used
>> to determine if a SOLVER_UPDATE | SOLVER_SOLVABLE_ALL task is given to
>> the solver, causing all packages to be updated (if possible) (i.e. so
>> 'Keep' doesn't update anything)
> 
> I've already arranged (by using SOLVER_LOCK) that 'Keep' doesn't update
> anything.  So I don't think we need to worry about that case.  On the
> other hand, if 'Current' or 'Test' is selected, then we already upgrade
> as appropriate in the task list sent to the solver, so I don't think
> it's necessary to send SOLVER_UPDATE | SOLVER_SOLVABLE_ALL to the solver
> in that case either.
> 
> Anyway, as I said, I've already disabled the use of
> PrereqChecker::upgrade.  More precisely, I've changed
> SolverSolution::update so that it never sends SOLVER_UPDATE |
> SOLVER_SOLVABLE_ALL.  As a result, PrereqChecker::upgrade is simply
> ignored, and everything seems to be working OK.
> 

Since this discussion appears to resolve around "test" versus
"production" releases would it not be better served if there were two
differing base paths, one for production and one for test?  If that
occurred then there may be more testers as what is used for production
isn't borked by a test version of some package or even Cygwin itself.

So for example C:\cygwin64 serves the production path while
C:\cygwin64test serves the test path.  This would help segregate test
from a production environment without worrying the user with needing to
revert back if something fails.

-- 
cyg Simple

Re: [PATCH setup 00/14] Use libsolv, solve all our problems... (WIP)

[Jon Turney]

On 14/09/2017 21:46, Ken Brown wrote:

On 9/14/2017 1:26 PM, Achim Gratz wrote:

Ken Brown writes:

What I've been struggling with, however, is the UI.  But now that I
think about it, maybe it isn't that hard.  It's just a matter of doing
something reasonable if the user unchecks "Accept default problem
solutions".  I'll see what I can come up with.


Well, zypper pretty much just gives you a bunch of possible solutions
and asks you to select one if there is either more than one or the
otherwise preferred solution is blocked by a lock.  There is always one
"break  package by doing " down that list.  You could
maybe offer something along those lines in the inevitable dialog box?


In the long run I think that's the way to go.  But implementing that is 
more work than I feel like doing at the moment.  For now I've gone with 


Yeah, a better interface to the solution list would be nice, but 
:effort: and it's unclear how much use it would get with the loose 
requirements our current package database contains.


I'm not sure there is huge value in the current "don't install 
dependencies" option.  I don't know why anyone would want to do that, 
and it's just going to give you a broken install sometimes...


an approach that was easier to program, more like the current setup.exe. 
  If the solver finds problems (including missing dependencies), the 
user has four choices on the Prerequisite page:


1. Click Back to go back to the Chooser page, with the Pending view 
showing the solver's default solutions.


2. Click Next to accept the default solutions.

3. Uncheck the "Accept default solutions" box and click Next.  If the 
user dismisses the resulting warning, setup will go ahead and do what 
the user requested.


4. Cancel.

Once the inevitable remaining bugs are fixed, I think we'll have a 
setup.exe that's better than the current one, with possibilities for 
further UI improvements along the lines you suggested.


Thanks again for your work on this.

Can you rebase your and my patches onto master, and push to sourceware 
in a topic/libsolv branch?


After that, I think it might be useful to make a binary available for 
wider testing.


Two things I have left to look at:

- Since the distributed setup is cross-built on Linux, I need to look 
into making a RPM for the cross-built libsolv.


- Wire up the "Source:" (note capital 'S') lines in setup.ini.  These 
work in current setup versions (although we don't use it, but I'd like 
to change that).  Since the the source package might appear after the 
package in setup.ini, this seems to conflict with the current approach 
of storing the sourcepackage solvable id, which I did because searching 
the solver repo for a package by name was slow. With hindsight, this 
seems wrong.

Re: [PATCH setup 00/14] Use libsolv, solve all our problems... (WIP)

[Ken Brown]

On 9/15/2017 11:15 AM, Jon Turney wrote:

On 08/09/2017 19:54, Ken Brown wrote:
Finally, I have a question for you, Jon: You introduced 
PrereqChecker::upgrade, which is true if and only if the user selects 
Current or Test.  I don't see why this is needed.  I've disabled the 
use of it and haven't noticed any ill effects.  Am I missing something?


This is supposed to be passed into SolverSolution::update() and used to 
determine if a SOLVER_UPDATE | SOLVER_SOLVABLE_ALL task is given to the 
solver, causing all packages to be updated (if possible) (i.e. so 'Keep' 
doesn't update anything)


I've already arranged (by using SOLVER_LOCK) that 'Keep' doesn't update 
anything.  So I don't think we need to worry about that case.  On the 
other hand, if 'Current' or 'Test' is selected, then we already upgrade 
as appropriate in the task list sent to the solver, so I don't think 
it's necessary to send SOLVER_UPDATE | SOLVER_SOLVABLE_ALL to the solver 
in that case either.


Anyway, as I said, I've already disabled the use of 
PrereqChecker::upgrade.  More precisely, I've changed 
SolverSolution::update so that it never sends SOLVER_UPDATE | 
SOLVER_SOLVABLE_ALL.  As a result, PrereqChecker::upgrade is simply 
ignored, and everything seems to be working OK.


I guess the UI could be improved to make the choice of 'Keep' or 
'Current' ('Update') orthogonal to 'Use test packages?'


I'm not seeing a problem here, but maybe I'm misunderstanding you.

Ken

Re: [PATCH setup 00/14] Use libsolv, solve all our problems... (WIP)

[Jon Turney]

On 08/09/2017 19:54, Ken Brown wrote:
Finally, I have a question for you, Jon: You introduced 
PrereqChecker::upgrade, which is true if and only if the user selects 
Current or Test.  I don't see why this is needed.  I've disabled the use 
of it and haven't noticed any ill effects.  Am I missing something?


This is supposed to be passed into SolverSolution::update() and used to 
determine if a SOLVER_UPDATE | SOLVER_SOLVABLE_ALL task is given to the 
solver, causing all packages to be updated (if possible) (i.e. so 'Keep' 
doesn't update anything)


I guess the UI could be improved to make the choice of 'Keep' or 
'Current' ('Update') orthogonal to 'Use test packages?'

Re: [PATCH setup 00/14] Use libsolv, solve all our problems... (WIP)

[Ken Brown]

On 9/14/2017 1:26 PM, Achim Gratz wrote:

Ken Brown writes:

What I've been struggling with, however, is the UI.  But now that I
think about it, maybe it isn't that hard.  It's just a matter of doing
something reasonable if the user unchecks "Accept default problem
solutions".  I'll see what I can come up with.


Well, zypper pretty much just gives you a bunch of possible solutions
and asks you to select one if there is either more than one or the
otherwise preferred solution is blocked by a lock.  There is always one
"break  package by doing " down that list.  You could
maybe offer something along those lines in the inevitable dialog box?


In the long run I think that's the way to go.  But implementing that is 
more work than I feel like doing at the moment.  For now I've gone with 
an approach that was easier to program, more like the current setup.exe. 
 If the solver finds problems (including missing dependencies), the 
user has four choices on the Prerequisite page:


1. Click Back to go back to the Chooser page, with the Pending view 
showing the solver's default solutions.


2. Click Next to accept the default solutions.

3. Uncheck the "Accept default solutions" box and click Next.  If the 
user dismisses the resulting warning, setup will go ahead and do what 
the user requested.


4. Cancel.

Once the inevitable remaining bugs are fixed, I think we'll have a 
setup.exe that's better than the current one, with possibilities for 
further UI improvements along the lines you suggested.


Ken

Re: [PATCH setup 00/14] Use libsolv, solve all our problems... (WIP)

[Achim Gratz]

Ken Brown writes:
> What I've been struggling with, however, is the UI.  But now that I
> think about it, maybe it isn't that hard.  It's just a matter of doing
> something reasonable if the user unchecks "Accept default problem
> solutions".  I'll see what I can come up with.

Well, zypper pretty much just gives you a bunch of possible solutions
and asks you to select one if there is either more than one or the
otherwise preferred solution is blocked by a lock.  There is always one
"break  package by doing " down that list.  You could
maybe offer something along those lines in the inevitable dialog box?


Regards,
Achim.
-- 
+<[Q+ Matrix-12 WAVE#46+305 Neuron microQkb Andromeda XTk Blofeld]>+

SD adaptations for Waldorf Q V3.00R3 and Q+ V3.54R2:
http://Synth.Stromeko.net/Downloads.html#WaldorfSDada

Re: [PATCH setup 00/14] Use libsolv, solve all our problems... (WIP)

[Ken Brown]

On 9/13/2017 3:17 PM, Achim Gratz wrote:

Ken Brown writes:

I've changed this so that the user can now review the packages that
will be installed.  But I still haven't found a good way to enable the
user to refuse dependencies.


With zypper you'd create a package lock and there'd be no changes to
the state of the package (including from uninstalled to installed).


Yes, I think that would work.  I already use that to prevent upgrade 
when the user has selected "Keep" on a package that would otherwise be 
upgraded.


What I've been struggling with, however, is the UI.  But now that I 
think about it, maybe it isn't that hard.  It's just a matter of doing 
something reasonable if the user unchecks "Accept default problem 
solutions".  I'll see what I can come up with.


Ken

Re: [PATCH setup 00/14] Use libsolv, solve all our problems... (WIP)

[Achim Gratz]

Ken Brown writes:
> I've changed this so that the user can now review the packages that
> will be installed.  But I still haven't found a good way to enable the
> user to refuse dependencies.

With zypper you'd create a package lock and there'd be no changes to
the state of the package (including from uninstalled to installed).


Regards,
Achim.
-- 
+<[Q+ Matrix-12 WAVE#46+305 Neuron microQkb Andromeda XTk Blofeld]>+

SD adaptation for Waldorf rackAttack V1.04R1:
http://Synth.Stromeko.net/Downloads.html#WaldorfSDada

Re: [PATCH setup 00/14] Use libsolv, solve all our problems... (WIP)

[Ken Brown]

On 9/8/2017 2:54 PM, Ken Brown wrote:

On 5/31/2017 6:50 AM, Jon Turney wrote:
... solve some problems, perhaps add some new ones, I guess.  I'm not 
100%

sure this is the right approach to take, but I wrote it, so here it is.


I've now fixed all the bugs I've noticed, and I think I've gotten the 
libsolv branch pretty close to the point where it is usable (barring 
further bugs).


- I've dodged a lot of the UI issues: If the solver reports problems, 
all that

can be done is accept the default solution or cancel.


I've changed this so that the Back button takes the user back to the 
chooser page, with all packages reset to show the solver's default 
solution.  The user can then see what will be done and, if desired, try 
to solve the problems in other ways.


- We had a very poor UI for showing what will actually be done 
(combine in

your head the "Pending" view with packages listed in the text on the
PrereChecker page), and this removes part of that


This is fixed by the above.

- As implemented, selecting "Current" overrides "Keep".  This is 
wrong, and a

change from current behaviour,


Fixed.

Here are some other things I've done, aside from fixing bugs:

- As discussed earlier in the thread, I've made it possible for the user 
to install test packages without clicking the Test button.


- I've artificially created a conflict if the user tries to uninstall a 
Base package.  There's probably a better way to deal with this.


- I've made the solver check dependencies of installed packages.  (It 
doesn't do this by default, which seems strange to me.)


There's still one UI issue that I haven't dealt with: If the solver 
finds missing dependencies, setup simply installs them silently without 
informing the user (except in the log).  In particular, the user can no 
longer refuse to install them.  I'm not sure how to best deal with this.


I've changed this so that the user can now review the packages that will 
be installed.  But I still haven't found a good way to enable the user 
to refuse dependencies.


Ken

Re: [PATCH setup 00/14] Use libsolv, solve all our problems... (WIP)

[Ken Brown]

On 5/31/2017 6:50 AM, Jon Turney wrote:

... solve some problems, perhaps add some new ones, I guess.  I'm not 100%
sure this is the right approach to take, but I wrote it, so here it is.


I've now fixed all the bugs I've noticed, and I think I've gotten the 
libsolv branch pretty close to the point where it is usable (barring 
further bugs).



- I've dodged a lot of the UI issues: If the solver reports problems, all that
can be done is accept the default solution or cancel.


I've changed this so that the Back button takes the user back to the 
chooser page, with all packages reset to show the solver's default 
solution.  The user can then see what will be done and, if desired, try 
to solve the problems in other ways.



- We had a very poor UI for showing what will actually be done (combine in
your head the "Pending" view with packages listed in the text on the
PrereChecker page), and this removes part of that


This is fixed by the above.


- As implemented, selecting "Current" overrides "Keep".  This is wrong, and a
change from current behaviour,


Fixed.

Here are some other things I've done, aside from fixing bugs:

- As discussed earlier in the thread, I've made it possible for the user 
to install test packages without clicking the Test button.


- I've artificially created a conflict if the user tries to uninstall a 
Base package.  There's probably a better way to deal with this.


- I've made the solver check dependencies of installed packages.  (It 
doesn't do this by default, which seems strange to me.)


There's still one UI issue that I haven't dealt with: If the solver 
finds missing dependencies, setup simply installs them silently without 
informing the user (except in the log).  In particular, the user can no 
longer refuse to install them.  I'm not sure how to best deal with this.


Finally, I have a question for you, Jon: You introduced 
PrereqChecker::upgrade, which is true if and only if the user selects 
Current or Test.  I don't see why this is needed.  I've disabled the use 
of it and haven't noticed any ill effects.  Am I missing something?


Ken

P.S. Anyone who wants to look at the code can find it at

  https://github.com/kbrow1i/cygwin-setup ;

checkout the libsolv branch.

Re: [PATCH setup 00/14] Use libsolv, solve all our problems... (WIP)

[Ken Brown]

On 9/5/2017 2:40 PM, Achim Gratz wrote:

Jon Turney writes:

Yeah, I'm not sure if putting the test packages into a separate repo
which is disabled unless explicitly enabled is the right approach.

(Instead, perhaps it is possible to tell the solver that certain
repositories are disfavoured)


At least for zypper, which is based on the same library:

You can give each repo a priority and it will pick the package from the
one with the highest priority, even if another repo has a higher
version.


Yes, that works.

Ken

Re: [PATCH setup 00/14] Use libsolv, solve all our problems... (WIP)

[Achim Gratz]

Jon Turney writes:
> Yeah, I'm not sure if putting the test packages into a separate repo
> which is disabled unless explicitly enabled is the right approach.
>
> (Instead, perhaps it is possible to tell the solver that certain
> repositories are disfavoured)

At least for zypper, which is based on the same library:

You can give each repo a priority and it will pick the package from the
one with the highest priority, even if another repo has a higher
version.  You can also tell the solver not to switch packages to another
repo automatically, so that once you've switched something to testing it
will stay there.


Regards,
Achim.
-- 
+<[Q+ Matrix-12 WAVE#46+305 Neuron microQkb Andromeda XTk Blofeld]>+

SD adaptation for Waldorf rackAttack V1.04R1:
http://Synth.Stromeko.net/Downloads.html#WaldorfSDada

Re: [PATCH setup 00/14] Use libsolv, solve all our problems... (WIP)

[Jon Turney]

On 02/09/2017 17:57, Ken Brown wrote:

On 9/1/2017 11:00 AM, Ken Brown wrote:

On 5/31/2017 6:50 AM, Jon Turney wrote:

What remains to be done:
- I've dodged a lot of the UI issues: If the solver reports problems, 
all that
can be done is accept the default solution or cancel.  This possibly 
isn't a

big problem until we have a package set which can contain problems...
- We had a very poor UI for showing what will actually be done 
(combine in

your head the "Pending" view with packages listed in the text on the
PrereChecker page), and this removes part of that
- As implemented, selecting "Current" overrides "Keep".  This is 
wrong, and a

change from current behaviour, but is probably a symptom of some deeper
confusion in the picker UI I'm not sure how to address


Thanks very much for taking the time to look at these changes and test them.

It was my intention to come back and take another look at this, but that 
hasn't happened yet :S



There are also some issues involving the treatment of test releases:

- It's not possible to install a test release of a package without 
clicking the global Test button.  But then you have to manually choose 
Keep for all the package where you don't want the test release.


Yeah, I'm not sure if putting the test packages into a separate repo 
which is disabled unless explicitly enabled is the right approach.


(Instead, perhaps it is possible to tell the solver that certain 
repositories are disfavoured)


- Once a test release is installed, selecting Reinstall will downgrade 
the package if the global Test button is not pressed.  Presumably this 
is because the test release has become invisible, since its repo has 
been disabled.  But I haven't checked carefully to see exactly what's 
going on.


- I found some glitches involving SHA512 sums of test releases.  It's 
probably not worth pursuing this until the handling of test releases 
is redone.


I fixed the last two problems, which turned out to have nothing to do 
with the way test releases are handle.  So all that's left of my three 
issues is the first one, which is just a UI annoyance.


I don't think it makes sense for me to send further patches here. 
Instead, I'll fork Jon's GitHub repo and continue to try to make 
improvements.


Awesome.

Re: [PATCH setup 00/14] Use libsolv, solve all our problems... (WIP)

[Ken Brown]

On 9/1/2017 11:00 AM, Ken Brown wrote:

On 5/31/2017 6:50 AM, Jon Turney wrote:

What remains to be done:
- I've dodged a lot of the UI issues: If the solver reports problems, 
all that
can be done is accept the default solution or cancel.  This possibly 
isn't a

big problem until we have a package set which can contain problems...
- We had a very poor UI for showing what will actually be done 
(combine in

your head the "Pending" view with packages listed in the text on the
PrereChecker page), and this removes part of that
- As implemented, selecting "Current" overrides "Keep".  This is 
wrong, and a

change from current behaviour, but is probably a symptom of some deeper
confusion in the picker UI I'm not sure how to address


There are also some issues involving the treatment of test releases:

- It's not possible to install a test release of a package without 
clicking the global Test button.  But then you have to manually choose 
Keep for all the package where you don't want the test release.


- Once a test release is installed, selecting Reinstall will downgrade 
the package if the global Test button is not pressed.  Presumably this 
is because the test release has become invisible, since its repo has 
been disabled.  But I haven't checked carefully to see exactly what's 
going on.


- I found some glitches involving SHA512 sums of test releases.  It's 
probably not worth pursuing this until the handling of test releases is 
redone.


I fixed the last two problems, which turned out to have nothing to do 
with the way test releases are handle.  So all that's left of my three 
issues is the first one, which is just a UI annoyance.


I don't think it makes sense for me to send further patches here. 
Instead, I'll fork Jon's GitHub repo and continue to try to make 
improvements.


Ken

Re: [PATCH setup 00/14] Use libsolv, solve all our problems... (WIP)

[Ken Brown]

On 5/31/2017 6:50 AM, Jon Turney wrote:

What remains to be done:
- I've dodged a lot of the UI issues: If the solver reports problems, all that
can be done is accept the default solution or cancel.  This possibly isn't a
big problem until we have a package set which can contain problems...
- We had a very poor UI for showing what will actually be done (combine in
your head the "Pending" view with packages listed in the text on the
PrereChecker page), and this removes part of that
- As implemented, selecting "Current" overrides "Keep".  This is wrong, and a
change from current behaviour, but is probably a symptom of some deeper
confusion in the picker UI I'm not sure how to address


There are also some issues involving the treatment of test releases:

- It's not possible to install a test release of a package without 
clicking the global Test button.  But then you have to manually choose 
Keep for all the package where you don't want the test release.


- Once a test release is installed, selecting Reinstall will downgrade 
the package if the global Test button is not pressed.  Presumably this 
is because the test release has become invisible, since its repo has 
been disabled.  But I haven't checked carefully to see exactly what's 
going on.


- I found some glitches involving SHA512 sums of test releases.  It's 
probably not worth pursuing this until the handling of test releases is 
redone.


Ken

Re: [PATCH setup 00/14] Use libsolv, solve all our problems... (WIP)

[Ken Brown]

On 8/29/2017 9:37 AM, Ken Brown wrote:

On 5/31/2017 6:50 AM, Jon Turney wrote:
... solve some problems, perhaps add some new ones, I guess.  I'm not 
100%

sure this is the right approach to take, but I wrote it, so here it is.

[...]
- As implemented, selecting "Current" overrides "Keep".  This is 
wrong, and a

change from current behaviour, but is probably a symptom of some deeper
confusion in the picker UI I'm not sure how to address


I think the problem might be the following lines in the definition of 
SolverSolution::update:


   if (update)
     queue_push2(, SOLVER_UPDATE | SOLVER_SOLVABLE_ALL, 0);

When the prerequisite checker calls SolverSolution::update, doesn't this 
cause the upgrading of old versions that we want to keep (assuming 
"Current" has been selected)?  As a quick test, I commented out those 
lines and found that setup.exe let me keep an old version of a package.


Maybe you need to add a DISABLE_UPDATE command to the solver task list 
to implement "Keep" for packages that would otherwise be updated.


DISABLE_UPDATE is not a command.  But SOLVER_LOCK seems to do the job. 
Jon, I'm attaching a patch that should apply to the libsolv branch of 
your github cygwin-setup repo.  So far I've only tested it very lightly, 
enough to verify that it lets me keep an old version of a package.


Ken

From 26231d3c83c392e6fa267bcea9135f2b5e5af1ca Mon Sep 17 00:00:00 2001
From: Ken Brown 
Date: Wed, 30 Aug 2017 17:36:13 -0400
Subject: [PATCH] Don't override a Keep selection

---
 libsolv.cc |  3 +++
 libsolv.h  |  3 ++-
 package_db.cc  |  2 +-
 package_meta.h |  2 ++
 prereq.cc  | 12 
 5 files changed, 16 insertions(+), 6 deletions(-)

diff --git a/libsolv.cc b/libsolv.cc
index f509617..67b99b2 100644
--- a/libsolv.cc
+++ b/libsolv.cc
@@ -526,6 +526,9 @@ SolverSolution::update(SolverTasks , bool update, 
bool use_test_packages,
   // 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(, 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 be518e9..7768128 100644
--- a/libsolv.h
+++ b/libsolv.h
@@ -165,7 +165,8 @@ class SolverTasks
   {
 taskInstall,
 taskUninstall,
-taskReinstall
+taskReinstall,
+taskKeep,
   };
   void add(const SolvableVersion , task t)
   {
diff --git a/package_db.cc b/package_db.cc
index 9f9e0a6..d7ec043 100644
--- a/package_db.cc
+++ b/package_db.cc
@@ -522,7 +522,7 @@ packagedb::defaultTrust (trusts trust)
 || pkg.categories.find ("Base") != pkg.categories.end ()
 || pkg.categories.find ("Orphaned") != pkg.categories.end ())
 {
-  pkg.desired = pkg.trustp (true, trust);
+  pkg.desired = pkg.default_version = pkg.trustp (true, trust);
   if (pkg.desired)
 pkg.pick (pkg.desired.accessible() && pkg.desired != 
pkg.installed);
 }
diff --git a/package_meta.h b/package_meta.h
index b6faab8..d91f7c9 100644
--- a/package_meta.h
+++ b/package_meta.h
@@ -131,6 +131,8 @@ 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;
diff --git a/prereq.cc b/prereq.cc
index 49fbd77..8922cc6 100644
--- a/prereq.cc
+++ b/prereq.cc
@@ -170,6 +170,7 @@ PrereqChecker::isMet ()
 
   // 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)
@@ -177,10 +178,13 @@ PrereqChecker::isMet ()
   else
 q.add(pkg->installed, SolverTasks::taskUninstall); // uninstall
 }
-  else
-if (pkg->picked())
-  q.add(pkg->installed, SolverTasks::taskReinstall); // reinstall
-
+  else if (pkg->installed)
+   {
+ if (pkg->picked())
+   q.add(pkg->installed, SolverTasks::taskReinstall); // reinstall
+ else if (upgrade && pkg->installed < pkg->default_version)
+   q.add(pkg->installed, SolverTasks::taskKeep); // keep
+   }
   // only install action makes sense for source packages
   if (pkg->srcpicked())
 {
-- 
2.14.1

Re: [PATCH setup 00/14] Use libsolv, solve all our problems... (WIP)

[Ken Brown]

On 5/31/2017 6:50 AM, Jon Turney wrote:

... solve some problems, perhaps add some new ones, I guess.  I'm not 100%
sure this is the right approach to take, but I wrote it, so here it is.

[...]

- As implemented, selecting "Current" overrides "Keep".  This is wrong, and a
change from current behaviour, but is probably a symptom of some deeper
confusion in the picker UI I'm not sure how to address


I think the problem might be the following lines in the definition of 
SolverSolution::update:


  if (update)
queue_push2(, SOLVER_UPDATE | SOLVER_SOLVABLE_ALL, 0);

When the prerequisite checker calls SolverSolution::update, doesn't this 
cause the upgrading of old versions that we want to keep (assuming 
"Current" has been selected)?  As a quick test, I commented out those 
lines and found that setup.exe let me keep an old version of a package.


Maybe you need to add a DISABLE_UPDATE command to the solver task list 
to implement "Keep" for packages that would otherwise be updated.


(Disclaimer: I have only looked briefly at the libsolv sources, so what 
I'm saying might be complete nonsense.)


Ken

[PATCH setup 00/14] Use libsolv, solve all our problems... (WIP)

[Jon Turney]

... solve some problems, perhaps add some new ones, I guess.  I'm not 100% 
sure this is the right approach to take, but I wrote it, so here it is.

This replaces the current PackageVersion class with a similar one which stores 
the information in a libsolv pool, and the current depsolver with the libsolv 
solver (as used by zypper, dnf and others).

Immediately, this enables:
- use of a version relation in package dependencies
- an obsoletes: relation between packages

This also makes it much easier to support:
- version numbers with an epoch component (I think just how we handle ':' in 
filenames needs auditing to make this work)
- other commonly-implemented package relations such as conflicts:, provides: 
etc.

What remains to be done:
- I've dodged a lot of the UI issues: If the solver reports problems, all that 
can be done is accept the default solution or cancel.  This possibly isn't a 
big problem until we have a package set which can contain problems...
- We had a very poor UI for showing what will actually be done (combine in 
your head the "Pending" view with packages listed in the text on the 
PrereChecker page), and this removes part of that
- As implemented, selecting "Current" overrides "Keep".  This is wrong, and a 
change from current behaviour, but is probably a symptom of some deeper 
confusion in the picker UI I'm not sure how to address

libsolv needs to be lightly patched to build for Win32, see [1].  I will ITP 
that, but it probably also needs an RPM to support cross-building.

[1] https://github.com/jon-turney/libsolv

Jon Turney (14):
  Opaque how PackageDepends is stored
  Factor out reading installed.db
  Hoist addScript() etc. up from packageversion to packagemeta
  Hoist pick() up to packagemeta
  Hoist uninstall up to Installer::uninstallOne()
  Hoist scan() up from packageversion to packagemeta
  Store package stability in class packageversion
  Change to using a libsolv pool for storing package information
  Remove cygpackage class
  Remove packageversion class
  Drop in SolvableVersion as a replacement for packageversion
  Use solver to check for problems and produce a list of package
transactions
  Download/checksum/install/uninstall what transaction wants
  Add obsoletes: support

 IniDBBuilderPackage.cc  | 331 ---
 IniDBBuilderPackage.h   |  32 ++-
 Makefile.am |   7 +-
 PackageSpecification.cc |  12 +
 PackageSpecification.h  |   7 +-
 PickPackageLine.cc  |  14 +-
 PickView.cc |   9 +-
 bootstrap.sh|   2 +-
 choose.cc   |   8 +-
 configure.ac|   1 +
 cygpackage.cc   | 187 -
 cygpackage.h|  88 ---
 desktop.cc  |   1 -
 download.cc |  72 ++---
 ini.cc  |   1 +
 inilex.ll   |   1 +
 iniparse.yy |   3 +
 install.cc  | 176 -
 libsolv.cc  | 689 
 libsolv.h   | 231 
 package_db.cc   | 115 ++--
 package_db.h|   8 +
 package_depends.cc  |  19 +-
 package_depends.h   |   2 +-
 package_meta.cc | 294 -
 package_meta.h  |  30 ++-
 package_version.cc  | 414 -
 package_version.h   | 190 +
 postinstall.cc  |   6 +-
 prereq.cc   | 206 ---
 prereq.h|  22 +-
 res.rc  |   4 +-
 32 files changed, 1606 insertions(+), 1576 deletions(-)
 delete mode 100644 cygpackage.cc
 delete mode 100644 cygpackage.h
 create mode 100644 libsolv.cc
 create mode 100644 libsolv.h
 delete mode 100644 package_version.cc

-- 
2.12.3