Re: [PATCH] make setup mirror list more like web page not just urls

[Brian Inglis <Brian.Inglis@SystematicSw.ab.ca>]

On 2017-11-24 09:21, Ken Brown wrote:
> On 11/24/2017 1:23 AM, Brian Inglis wrote:
>> On 2017-11-23 14:07, Ken Brown wrote:
>>> On 11/22/2017 11:42 AM, Brian Inglis wrote:
>>>> On 2017-11-20 15:59, Ken Brown wrote:
>>>>> On 11/20/2017 4:30 PM, Brian Inglis wrote:
>>>>>> On 2017-11-17 06:53, Jon Turney wrote:
>>>>>> On 11/17/2017 8:48 AM, Ken Brown wrote:
>>>>>>> On 15/11/2017 21:35, Brian Inglis wrote:
>>>>> The issue of recognizing a URL that's already in the list is not fixed. Here's
>>>>> what I tried:
>>>>>
>>>>> I ran setup with the last mirror (as stored in /etc/setup/setup.rc) being
>>>>> http://mirrors.kernel.org/sourceware/cygwin/.  The resulting mirror list
>>>>> display
>>>>> contained the following, highlighted:
>>>>>
>>>>>     Org - Kernel - http://mirrors.kernel.org
>>>>>
>>>>> So setup didn't recognize that http://mirrors.kernel.org/sourceware/cygwin/
>>>>> was
>>>>> already in the list, displayed as
>>>>>
>>>>>     United States - California - http://mirrors.kernel.org
>>>>
>>>> Should have been working so I added more LOG_BABBLE and it looks like setup.rc
>>>> is processed at the start as you would expect, but it is merged into empty site
>>>> lists, as mirrors.lst is downloaded only after you say you can, and proxy
>>>> settings, in a separate download thread: see attached.
>>>> And do you know where the cached list comes from - that seems to keep old
>>>> mirrors around from testing, but I can't find any cached list with the test
>>>> mirrors appearing in the list - I would expect that to come from setup.rc - but
>>>> appears to be loaded when mirrors.lst is.
>>>> So we need to defer adding the last mirror with some kind of lazy evaluation
>>>> hooked after cached mirrors and/or mirrors.lst is available.
>>>> If we have a setup.rc, I would expect the cached mirrors site list loaded from
>>>> there at startup, before the last mirror is merged (and found!).
>>>> Can make those changes but would like ny assumptions questioned, corrected, or
>>>> validated!
>>>
>>> I'm not aware of any place previously used sites could come from other than
>>> setup.rc.  Are you sure your test mirrors aren't listed there?
>>
>> No - that's why I've been scratching or beating my head about cache location!
>>
>>> The fact that mirrors.lst is merged into all_site_list after the last-mirror
>>> list shouldn't prevent a site in mirrors.lst from replacing one with the same
>>> URL.  load_site_list() tries to do exactly that.  I think the problem is that
>>> 'find (theSites.begin(), theSites.end(), newsite)' is using an operator== based
>>> on the key instead of the URL. Changing that as you suggested in a different
>>> message might fix the problem, but I haven't thought about whether it could
>>> cause problems elsewhere.

Comparison of only the url is the same as done currently with they which is only
the url, so should have no adverse effect: the other members are used only for
sorting and display, not I/O.

>> That's why I suggested (string) constructor and/or operator== (string)
>> overloads, to allow very selective behaviour change, without changing other
>> uses. I have not used non-C-like OO C++ much, with too many casts required, but
>> find that approach very natural using JS object prototypes to augment behaviour.
>>
>> Currently both the cached and downloaded mirror lists are set up after the
>> download. It may be more obvious to set up the cached list (if there is one,
>> which will be almost always, except brand new setups) at the start, before
>> "merging" the last mirror, which currently just adds that entry to an empty
>> cached list. If there is no cached list, there will be no last mirror to merge.
> 
> Sorry, but I'm having trouble following what you're saying.  By "cached list",
> are you talking about the variable 'cached_site_list'?  This is populated with
> the mirrors listed under 'mirrors-lst' in setup.rc.  The URLs listed under
> 'last-mirror' don't go into that list.  They go into 'site_list' and
> 'all_site_list'.  See site.cc starting around line 85, where these variables are
> defined with explanatory comments.

Sorry last line first word cached should be removed - I was unclear about what
is and what could be done.
I am suggesting we split get_site_list to create the setup.rc cached list at the
start, flag entries false, merge the last-mirror entry into that, checking the
url for a match to use the list entry instead of the last mirror entry, then
after the mirrors.lst download, compare the cached entries with the mirrors list
entries, checking the url for a match to use the current mirrors list entry
instead of the cached entry, which would be flagged true in the cached list, and
proceeding with the rest of get_site_list and then check_dropped_mirrors.

(Also noticed, the site list search and merge operations in registerSavedSite
and load_site_list could be refactored into a separate site_list::merge() member
function, by allowing for an optional exclusion site list.)

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