setup: problems with local install

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

setup: problems with local install

Ken Brown-6
This is a followup to the thread started here:

   https://cygwin.com/ml/cygwin/2018-03/msg00027.html

There are two problems with installing from a local directory.

1. In the Category view, "No packages found" is displayed where it
should say "All" in the first line of the list.  This is probably just a
cosmetic issue, and I haven't tried to track it down yet.

2. In several of the views, all packages from setup.ini are listed, even
if there is no corresponding archive in the local directory.  What
happens is that packagemeta::scan() calls pkg.source()->sites.clear()
for such packages, but this information is never used to prevent the
package from appearing in the list.

It used to be that such packages would be declared inaccessible, but
SolvableVersion::accessible() no longer does this.

Jon, you wrote the following comment in the definition of
SolvableVersion::accessible():

"The 'accessible' check used to test if an archive was available locally
or from a mirror.  This seems utterly pointless as binary packages which
aren't 'accessible' never get to appear in the package list."

Do we need to reinstate the old function of the accessibility check?

Ken
Reply | Threaded
Open this post in threaded view
|

Re: setup: problems with local install

Ken Brown-6
On 3/5/2018 1:34 PM, Ken Brown wrote:
> This is a followup to the thread started here:
>
>    https://cygwin.com/ml/cygwin/2018-03/msg00027.html
>
> There are two problems with installing from a local directory.
>
> 1. In the Category view, "No packages found" is displayed where it
> should say "All" in the first line of the list.  This is probably just a
> cosmetic issue, and I haven't tried to track it down yet.

This actually happens in all setup runs, not just local installs.  It
looks like it was caused by the rearranging that was done in the
following commit:

commit 0c539f7f7d86fb100f260f21367682fa2c0bb529
Author: Jon Turney <[hidden email]>
Date:   Sat Nov 4 18:01:49 2017 +0000

     Correctly order preparing packagedb for chooser

I think the problem might be that createListview() is now called too
early in ChooserPage::OnInit().  But it's not immediately obvious to me
how to fix this without breaking something else.

Ken
Reply | Threaded
Open this post in threaded view
|

Re: setup: problems with local install

Jon TURNEY
In reply to this post by Ken Brown-6
On 05/03/2018 18:34, Ken Brown wrote:
> This is a followup to the thread started here:
>
>    https://cygwin.com/ml/cygwin/2018-03/msg00027.html
>
> There are two problems with installing from a local directory.

Thanks very much for looking into these.

> 2. In several of the views, all packages from setup.ini are listed, even
> if there is no corresponding archive in the local directory.  What
> happens is that packagemeta::scan() calls pkg.source()->sites.clear()
> for such packages, but this information is never used to prevent the
> package from appearing in the list.
>
> It used to be that such packages would be declared inaccessible, but
> SolvableVersion::accessible() no longer does this.
>
> Jon, you wrote the following comment in the definition of
> SolvableVersion::accessible():
>
> "The 'accessible' check used to test if an archive was available locally
> or from a mirror.  This seems utterly pointless as binary packages which
> aren't 'accessible' never get to appear in the package list."
>
> Do we need to reinstate the old function of the accessibility check?
I guess I looked at packagemeta::ScanDownloadedFiles() and saw that it
was removing versions, and thought everything was good.

I didn't notice accessible() was indirectly how the result of scan() was
returned :S

So yeah, I guess putting some complexity back in accessible() would
work, or perhaps the attached?  (This doesn't do the right thing for a
few packages, for reasons I'm still looking into...)

(I also note we have also have another 'erase an element from a vector
while we are iterating over it' here, so that needs fixing, as well)

I'm kind of uncertain what the side-effects of this code are when source
!= IDC_SOURCE_LOCALDIR, or if they are desired?  Perhaps it's removing
packages which have corrupt local copies or something?  It would be
clearer to omit the whole thing in that case.


0001-Fix-packagemeta-ScanDownloadedFiles.patch (3K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: setup: problems with local install

Jon TURNEY
In reply to this post by Ken Brown-6
On 06/03/2018 02:18, Ken Brown wrote:

> On 3/5/2018 1:34 PM, Ken Brown wrote:
>> This is a followup to the thread started here:
>>
>>    https://cygwin.com/ml/cygwin/2018-03/msg00027.html
>>
>> There are two problems with installing from a local directory.
>>
>> 1. In the Category view, "No packages found" is displayed where it
>> should say "All" in the first line of the list.  This is probably just
>> a cosmetic issue, and I haven't tried to track it down yet.
>
> This actually happens in all setup runs, not just local installs.  It
> looks like it was caused by the rearranging that was done in the
> following commit:
>
> commit 0c539f7f7d86fb100f260f21367682fa2c0bb529
> Author: Jon Turney <[hidden email]>
> Date:   Sat Nov 4 18:01:49 2017 +0000
>
>      Correctly order preparing packagedb for chooser
>
> I think the problem might be that createListview() is now called too
> early in ChooserPage::OnInit().  But it's not immediately obvious to me
> how to fix this without breaking something else.
I'm kind of inclined to fix this with the attached: I think we only ever
got "No packages found" before if there were 0 packages, which
presumably doesn't happen very often :)

We don't update this header if a search reduces the number of packages
shown to 0, which might make some sort of sense.

(Also, weirdly, we don't remove empty categories from view, except if we
are doing a search, but this seems to be deliberate? (PickView.cc:311))

0001-Always-give-the-fake-root-category-the-name-All.patch (1K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: setup: problems with local install

Jon TURNEY
In reply to this post by Jon TURNEY
On 06/03/2018 15:18, Jon Turney wrote:
> So yeah, I guess putting some complexity back in accessible() would
> work, or perhaps the attached?  (This doesn't do the right thing for a
> few packages, for reasons I'm still looking into...)

To be specific it was doing the wrong thing for those few packages with
no source

>   /* scan for local copies of package */
> -void
> +bool
>   packagemeta::scan (const packageversion &pkg, bool mirror_mode)
>   {
> -  /* Already have something */
> +  /* empty version */
>     if (!pkg)
> -    return;
> +    return true;

So, this needs to be 'return false', as the empty version is always
inaccessible, to get the same behaviour as before.

Reply | Threaded
Open this post in threaded view
|

Re: setup: problems with local install

Ken Brown-6
In reply to this post by Jon TURNEY
On 3/6/2018 10:18 AM, Jon Turney wrote:
> (I also note we have also have another 'erase an element from a vector
> while we are iterating over it' here, so that needs fixing, as well)

I think this one might be OK.  If I'm not mistaken,
pkg.versions.erase(i++) passes a copy of i to erase, and then increments
i before erase() has done its work.  But I'm no expert on this.

Ken
Reply | Threaded
Open this post in threaded view
|

Re: setup: problems with local install

Jon TURNEY
On 06/03/2018 19:31, Ken Brown wrote:
> On 3/6/2018 10:18 AM, Jon Turney wrote:
>> (I also note we have also have another 'erase an element from a vector
>> while we are iterating over it' here, so that needs fixing, as well)
>
> I think this one might be OK.  If I'm not mistaken,
> pkg.versions.erase(i++) passes a copy of i to erase, and then increments
> i before erase() has done its work.  But I'm no expert on this.

I think that's still not ok for a vector, but this is actually a set, so
there's actually no problem.

See e.g. [1], or refer to your copy of the C++ standard :)

[1] https://stackoverflow.com/questions/6438086/iterator-invalidation-rules
Reply | Threaded
Open this post in threaded view
|

Re: setup: problems with local install

Achim Gratz
In reply to this post by Ken Brown-6
Ken Brown writes:
> I think this one might be OK.  If I'm not mistaken,
> pkg.versions.erase(i++) passes a copy of i to erase, and then
> increments i before erase() has done its work.  But I'm no expert on
> this.

I can't cite chapter and verse right now, but AFAIK iterators over sets
are explicitly OK'ed for deleting the current element of the set
iterated over (that may be restricted to not changing direction of
iteration depending on which version of the standard the implementation
conforms to).


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

SD adaptation for Waldorf microQ V2.22R2:
http://Synth.Stromeko.net/Downloads.html#WaldorfSDada
Reply | Threaded
Open this post in threaded view
|

Re: setup: problems with local install

Ken Brown-6
In reply to this post by Jon TURNEY
On 3/6/2018 1:47 PM, Jon Turney wrote:

> On 06/03/2018 15:18, Jon Turney wrote:
>> So yeah, I guess putting some complexity back in accessible() would
>> work, or perhaps the attached?  (This doesn't do the right thing for a
>> few packages, for reasons I'm still looking into...)
>
> To be specific it was doing the wrong thing for those few packages with
> no source
>
>>   /* scan for local copies of package */
>> -void
>> +bool
>>   packagemeta::scan (const packageversion &pkg, bool mirror_mode)
>>   {
>> -  /* Already have something */
>> +  /* empty version */
>>     if (!pkg)
>> -    return;
>> +    return true;
>
> So, this needs to be 'return false', as the empty version is always
> inaccessible, to get the same behaviour as before.

I've found another problem with local installs: If a package needs
upgrading, then the chooser will offer the upgraded version for install,
even if there's no archive available.  As a result, the current version
will get uninstalled, and then setup will discover that it doesn't have
the archive to install the new version.

The problem occurs because packagedb::defaultTrust() is called after
ScanDownloadedFiles() has already done its work.  solution.update() and
solution.trans2db() are called, and pkg->desired is set equal to an
inaccessible version pv (which has been previously removed from
pkg->versions).

I guess trans2db() should check that pv is in pkg->versions before
acting on an install transaction for pv.  And then we also have to make
sure to ignore the erase transaction for the current version of pkg.

Alternatively, can we just remove an inaccessible packageversion from
the libsolv pool, or at at least just tell libsolv that we don't want to
install it?

Ken

Reply | Threaded
Open this post in threaded view
|

Re: setup: problems with local install

Ken Brown-6
On 3/7/2018 4:52 PM, Ken Brown wrote:

> On 3/6/2018 1:47 PM, Jon Turney wrote:
>> On 06/03/2018 15:18, Jon Turney wrote:
>>> So yeah, I guess putting some complexity back in accessible() would
>>> work, or perhaps the attached?  (This doesn't do the right thing for
>>> a few packages, for reasons I'm still looking into...)
>>
>> To be specific it was doing the wrong thing for those few packages
>> with no source
>>
>>>   /* scan for local copies of package */
>>> -void
>>> +bool
>>>   packagemeta::scan (const packageversion &pkg, bool mirror_mode)
>>>   {
>>> -  /* Already have something */
>>> +  /* empty version */
>>>     if (!pkg)
>>> -    return;
>>> +    return true;
>>
>> So, this needs to be 'return false', as the empty version is always
>> inaccessible, to get the same behaviour as before.
>
> I've found another problem with local installs: If a package needs
> upgrading, then the chooser will offer the upgraded version for install,
> even if there's no archive available.  As a result, the current version
> will get uninstalled, and then setup will discover that it doesn't have
> the archive to install the new version.
>
> The problem occurs because packagedb::defaultTrust() is called after
> ScanDownloadedFiles() has already done its work.  solution.update() and
> solution.trans2db() are called, and pkg->desired is set equal to an
> inaccessible version pv (which has been previously removed from
> pkg->versions).
>
> I guess trans2db() should check that pv is in pkg->versions before
> acting on an install transaction for pv.  And then we also have to make
> sure to ignore the erase transaction for the current version of pkg.
>
> Alternatively, can we just remove an inaccessible packageversion from
> the libsolv pool, or at at least just tell libsolv that we don't want to
> install it?

Still another alternative, and maybe the simplest, is to make sure that
the chooser never shows a version that is not in pkg->versions.
packagemeta::set_action (trusts const trust) almost does this, except
for one useless desired.accessible() call.  So that should be fixed, as
well as the setting of the initial action.

Ken

Reply | Threaded
Open this post in threaded view
|

Re: setup: problems with local install

Ken Brown-6
On 3/8/2018 10:59 AM, Ken Brown wrote:

> On 3/7/2018 4:52 PM, Ken Brown wrote:
>> On 3/6/2018 1:47 PM, Jon Turney wrote:
>>> On 06/03/2018 15:18, Jon Turney wrote:
>>>> So yeah, I guess putting some complexity back in accessible() would
>>>> work, or perhaps the attached?  (This doesn't do the right thing for
>>>> a few packages, for reasons I'm still looking into...)
>>>
>>> To be specific it was doing the wrong thing for those few packages
>>> with no source
>>>
>>>>   /* scan for local copies of package */
>>>> -void
>>>> +bool
>>>>   packagemeta::scan (const packageversion &pkg, bool mirror_mode)
>>>>   {
>>>> -  /* Already have something */
>>>> +  /* empty version */
>>>>     if (!pkg)
>>>> -    return;
>>>> +    return true;
>>>
>>> So, this needs to be 'return false', as the empty version is always
>>> inaccessible, to get the same behaviour as before.
>>
>> I've found another problem with local installs: If a package needs
>> upgrading, then the chooser will offer the upgraded version for
>> install, even if there's no archive available.  As a result, the
>> current version will get uninstalled, and then setup will discover
>> that it doesn't have the archive to install the new version.
>>
>> The problem occurs because packagedb::defaultTrust() is called after
>> ScanDownloadedFiles() has already done its work.  solution.update()
>> and solution.trans2db() are called, and pkg->desired is set equal to
>> an inaccessible version pv (which has been previously removed from
>> pkg->versions).
>>
>> I guess trans2db() should check that pv is in pkg->versions before
>> acting on an install transaction for pv.  And then we also have to
>> make sure to ignore the erase transaction for the current version of pkg.
>>
>> Alternatively, can we just remove an inaccessible packageversion from
>> the libsolv pool, or at at least just tell libsolv that we don't want
>> to install it?
>
> Still another alternative, and maybe the simplest, is to make sure that
> the chooser never shows a version that is not in pkg->versions.
> packagemeta::set_action (trusts const trust) almost does this, except
> for one useless desired.accessible() call.  So that should be fixed, as
> well as the setting of the initial action.

Sorry for this stream of consciousness series of posts, but I'll stop
after this one.  I think it's possible that restoring the previous
meaning of 'accessible()', at least for local installs, might solve this
problem.

Ken

Reply | Threaded
Open this post in threaded view
|

Re: setup: problems with local install

Ken Brown-6
On 3/8/2018 4:59 PM, Ken Brown wrote:

> On 3/8/2018 10:59 AM, Ken Brown wrote:
>> On 3/7/2018 4:52 PM, Ken Brown wrote:
>>> On 3/6/2018 1:47 PM, Jon Turney wrote:
>>>> On 06/03/2018 15:18, Jon Turney wrote:
>>>>> So yeah, I guess putting some complexity back in accessible() would
>>>>> work, or perhaps the attached?  (This doesn't do the right thing
>>>>> for a few packages, for reasons I'm still looking into...)
>>>>
>>>> To be specific it was doing the wrong thing for those few packages
>>>> with no source
>>>>
>>>>>   /* scan for local copies of package */
>>>>> -void
>>>>> +bool
>>>>>   packagemeta::scan (const packageversion &pkg, bool mirror_mode)
>>>>>   {
>>>>> -  /* Already have something */
>>>>> +  /* empty version */
>>>>>     if (!pkg)
>>>>> -    return;
>>>>> +    return true;
>>>>
>>>> So, this needs to be 'return false', as the empty version is always
>>>> inaccessible, to get the same behaviour as before.
>>>
>>> I've found another problem with local installs: If a package needs
>>> upgrading, then the chooser will offer the upgraded version for
>>> install, even if there's no archive available.  As a result, the
>>> current version will get uninstalled, and then setup will discover
>>> that it doesn't have the archive to install the new version.
>>>
>>> The problem occurs because packagedb::defaultTrust() is called after
>>> ScanDownloadedFiles() has already done its work.  solution.update()
>>> and solution.trans2db() are called, and pkg->desired is set equal to
>>> an inaccessible version pv (which has been previously removed from
>>> pkg->versions).
>>>
>>> I guess trans2db() should check that pv is in pkg->versions before
>>> acting on an install transaction for pv.  And then we also have to
>>> make sure to ignore the erase transaction for the current version of
>>> pkg.
>>>
>>> Alternatively, can we just remove an inaccessible packageversion from
>>> the libsolv pool, or at at least just tell libsolv that we don't want
>>> to install it?
>>
>> Still another alternative, and maybe the simplest, is to make sure
>> that the chooser never shows a version that is not in pkg->versions.
>> packagemeta::set_action (trusts const trust) almost does this, except
>> for one useless desired.accessible() call.  So that should be fixed,
>> as well as the setting of the initial action.
>
> Sorry for this stream of consciousness series of posts, but I'll stop
> after this one.  I think it's possible that restoring the previous
> meaning of 'accessible()', at least for local installs, might solve this
> problem.
Patches attached (for the topic/fix-local-install branch).


0001-Make-SolvableVersion-accessible-useful-for-local-ins.patch (2K) Download Attachment
0002-Don-t-put-inaccessible-packageversions-in-the-packag.patch (2K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: setup: problems with local install

Jon TURNEY
On 12/03/2018 13:22, Ken Brown wrote:

> On 3/8/2018 4:59 PM, Ken Brown wrote:
>> On 3/8/2018 10:59 AM, Ken Brown wrote:
>>> On 3/7/2018 4:52 PM, Ken Brown wrote:
>>>> On 3/6/2018 1:47 PM, Jon Turney wrote:
>>>>> On 06/03/2018 15:18, Jon Turney wrote:
>>>>>> So yeah, I guess putting some complexity back in accessible()
>>>>>> would work, or perhaps the attached?  (This doesn't do the right
>>>>>> thing for a few packages, for reasons I'm still looking into...)
>>>>>
>>>>> To be specific it was doing the wrong thing for those few packages
>>>>> with no source
>>>>>
>>>>>>   /* scan for local copies of package */
>>>>>> -void
>>>>>> +bool
>>>>>>   packagemeta::scan (const packageversion &pkg, bool mirror_mode)
>>>>>>   {
>>>>>> -  /* Already have something */
>>>>>> +  /* empty version */
>>>>>>     if (!pkg)
>>>>>> -    return;
>>>>>> +    return true;
>>>>>
>>>>> So, this needs to be 'return false', as the empty version is always
>>>>> inaccessible, to get the same behaviour as before.
>>>>
>>>> I've found another problem with local installs: If a package needs
>>>> upgrading, then the chooser will offer the upgraded version for
>>>> install, even if there's no archive available.  As a result, the
>>>> current version will get uninstalled, and then setup will discover
>>>> that it doesn't have the archive to install the new version.
Thanks very much for finding this.

>>>> The problem occurs because packagedb::defaultTrust() is called after
>>>> ScanDownloadedFiles() has already done its work.  solution.update()
>>>> and solution.trans2db() are called, and pkg->desired is set equal to
>>>> an inaccessible version pv (which has been previously removed from
>>>> pkg->versions).
>>>>
>>>> I guess trans2db() should check that pv is in pkg->versions before
>>>> acting on an install transaction for pv.  And then we also have to
>>>> make sure to ignore the erase transaction for the current version of
>>>> pkg.
>>>>
>>>> Alternatively, can we just remove an inaccessible packageversion
>>>> from the libsolv pool, or at at least just tell libsolv that we
>>>> don't want to install it?
Attached is an attempt at that.

I think this is preferable, if it works correctly, as I think avoiding
solutions with these unavailable versions is better than trying to
massage the solution afterwards.

>>> Still another alternative, and maybe the simplest, is to make sure
>>> that the chooser never shows a version that is not in pkg->versions.
>>> packagemeta::set_action (trusts const trust) almost does this, except
>>> for one useless desired.accessible() call.  So that should be fixed,
>>> as well as the setting of the initial action.
>>
>> Sorry for this stream of consciousness series of posts, but I'll stop
>> after this one.  I think it's possible that restoring the previous
>> meaning of 'accessible()', at least for local installs, might solve
>> this problem.
Thanks for the patches.



0001-Remove-packages-not-found-by-scan-from-solver.patch (2K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: setup: problems with local install

Ken Brown-6
On 3/14/2018 12:07 PM, Jon Turney wrote:

> On 12/03/2018 13:22, Ken Brown wrote:
>> On 3/8/2018 4:59 PM, Ken Brown wrote:
>>> On 3/8/2018 10:59 AM, Ken Brown wrote:
>>>> On 3/7/2018 4:52 PM, Ken Brown wrote:
>>>>> On 3/6/2018 1:47 PM, Jon Turney wrote:
>>>>>> On 06/03/2018 15:18, Jon Turney wrote:
>>>>>>> So yeah, I guess putting some complexity back in accessible()
>>>>>>> would work, or perhaps the attached?  (This doesn't do the right
>>>>>>> thing for a few packages, for reasons I'm still looking into...)
>>>>>>
>>>>>> To be specific it was doing the wrong thing for those few packages
>>>>>> with no source
>>>>>>
>>>>>>>   /* scan for local copies of package */
>>>>>>> -void
>>>>>>> +bool
>>>>>>>   packagemeta::scan (const packageversion &pkg, bool mirror_mode)
>>>>>>>   {
>>>>>>> -  /* Already have something */
>>>>>>> +  /* empty version */
>>>>>>>     if (!pkg)
>>>>>>> -    return;
>>>>>>> +    return true;
>>>>>>
>>>>>> So, this needs to be 'return false', as the empty version is
>>>>>> always inaccessible, to get the same behaviour as before.
>>>>>
>>>>> I've found another problem with local installs: If a package needs
>>>>> upgrading, then the chooser will offer the upgraded version for
>>>>> install, even if there's no archive available.  As a result, the
>>>>> current version will get uninstalled, and then setup will discover
>>>>> that it doesn't have the archive to install the new version.
>
> Thanks very much for finding this.
>
>>>>> The problem occurs because packagedb::defaultTrust() is called
>>>>> after ScanDownloadedFiles() has already done its work.  
>>>>> solution.update() and solution.trans2db() are called, and
>>>>> pkg->desired is set equal to an inaccessible version pv (which has
>>>>> been previously removed from pkg->versions).
>>>>>
>>>>> I guess trans2db() should check that pv is in pkg->versions before
>>>>> acting on an install transaction for pv.  And then we also have to
>>>>> make sure to ignore the erase transaction for the current version
>>>>> of pkg.
>>>>>
>>>>> Alternatively, can we just remove an inaccessible packageversion
>>>>> from the libsolv pool, or at at least just tell libsolv that we
>>>>> don't want to install it?
>
> Attached is an attempt at that.
>
> I think this is preferable, if it works correctly, as I think avoiding
> solutions with these unavailable versions is better than trying to
> massage the solution afterwards.

I agree, and it does seem to work correctly.  But there's one other case
that it doesn't cover:  Suppose there's no archive for the installed
version.  Even with your patch, the user can still choose 'Reinstall',
which will fail.  The first of the patches from my previous email fixes
this case.

Ken
Reply | Threaded
Open this post in threaded view
|

Re: setup: problems with local install

Jon TURNEY
On 14/03/2018 19:24, Ken Brown wrote:

> On 3/14/2018 12:07 PM, Jon Turney wrote:
>> On 12/03/2018 13:22, Ken Brown wrote:
>>> On 3/8/2018 4:59 PM, Ken Brown wrote:
>>>> On 3/8/2018 10:59 AM, Ken Brown wrote:
>>>>> On 3/7/2018 4:52 PM, Ken Brown wrote:
>>>>>> On 3/6/2018 1:47 PM, Jon Turney wrote:
>>>>>>> On 06/03/2018 15:18, Jon Turney wrote:
>>>>>> I've found another problem with local installs: If a package needs
>>>>>> upgrading, then the chooser will offer the upgraded version for
>>>>>> install, even if there's no archive available.  As a result, the
>>>>>> current version will get uninstalled, and then setup will discover
>>>>>> that it doesn't have the archive to install the new version.
>>
>> Thanks very much for finding this.
>>
>>>>>> The problem occurs because packagedb::defaultTrust() is called
>>>>>> after ScanDownloadedFiles() has already done its work.
>>>>>> solution.update() and solution.trans2db() are called, and
>>>>>> pkg->desired is set equal to an inaccessible version pv (which has
>>>>>> been previously removed from pkg->versions).
>>>>>>
>>>>>> I guess trans2db() should check that pv is in pkg->versions before
>>>>>> acting on an install transaction for pv.  And then we also have to
>>>>>> make sure to ignore the erase transaction for the current version
>>>>>> of pkg.
>>>>>>
>>>>>> Alternatively, can we just remove an inaccessible packageversion
>>>>>> from the libsolv pool, or at at least just tell libsolv that we
>>>>>> don't want to install it?
>>
>> Attached is an attempt at that.
>>
>> I think this is preferable, if it works correctly, as I think avoiding
>> solutions with these unavailable versions is better than trying to
>> massage the solution afterwards.
>
> I agree, and it does seem to work correctly.  But there's one other case
> that it doesn't cover:  Suppose there's no archive for the installed
> version.  Even with your patch, the user can still choose 'Reinstall',
> which will fail.  The first of the patches from my previous email fixes
> this case.

Yes.  I applied that as well.

The subtlety I'd failed to grasp when I wrote that comment is that
ScanDownloadedFiles() prunes versions which aren't available locally,
*except* if they are already installed.

Thanks.