[PATCH setup v3 0/6] Distinguish between user URLs and cygwin mirrors in UI

[Ken Brown]

On 12/7/2017 8:55 AM, Jon Turney wrote:
> On 06/12/2017 22:15, Brian Inglis wrote:
>> On 2017-12-06 14:55, Ken Brown wrote:
>>> On 12/6/2017 4:07 PM, Brian Inglis wrote:
>>>> On 2017-12-06 13:45, Ken Brown wrote:
>>>>> This is a followup to
>>>>> Â Â Â  https://cygwin.com/ml/cygwin-apps/2011-04/msg00014.html,
>>>>> in which Jon suggested splitting site selection into two pages, one
>>>>> for cygwin.com mirrors and one for additional user URLs.  The latter
>>>>> would be visible only if the user had previously checked a suitable
>>>>> checkbox.  This patch series implements that suggestion.
> 
> Thanks very much for looking at this.
> 
> This looks pretty good.
> 
> My main concern is how this deals with private cygwin mirrors.  I guess 
> you need to not select anything at the "choose a mirror page", then tick 
> "allow sites that are not cygwin.com mirrors" to get to the page that 
> lets you enter the URL, which is somewhat confusing.
> I'm going to guess that private cygwin mirrors are used more than repos 
> of additional packages.

I hadn't thought about that.  I'll try to think of different terminology 
to make it clear that the first page is for mirrors listed on 
https://cygwin.com/mirrors.html, and the second page is for private 
mirrors and other repos of packages.  I'll try to send v4 with these 
changes later today and you can see if it looks better.

> (Another facet to consider: At the moment, I think we do something like 
> trying all the keys we know to validate the signature of every setup.ini 
> we download. It would be nice to maintain an idea of which URLs are 
> supposed to be validated using the cygwin signing key.)

After my patches, I think 'all_mirror_list' is precisely the list of 
such URLs.  Maybe I should rename it to 'official_mirror_list' or 
something similar?

> I guess we could achieve this by keeping the 'Add URL' control on the 
> mirrors page, but then the migration becomes a big problem (as we don't 
> know if a URL is a mirror or not until we read it)
> 
> (I'm not sure migrating anything not in mirrors.lst to the usersite list 
> as you currently do is the best idea, as it will migrate the currently 
> selected mirror if it happens to be stale when we are run?)

I tried to handle stale mirrors correctly in this snippet from patch 2:

+      if (i->from_mirrors_lst)
+	mirror = true;
+      else if ((j = find (cached_mirror_list.begin (), 
cached_mirror_list.end (), *i))
+	       != cached_mirror_list.end ())
+	{
+	  // It's a stale mirror.
+	  mirror = true;
[...]
+      if (mirror)
+	{
+	  result.push_back (*i);
[...]
+      else
+	{
+	  merge_site (all_usersite_list, *i);

> A question about scheduling:Â  I think I'd like to make a new setup 
> release in the next week or so, to pick up changes accumulated so far, 
> and perhaps also cherry-picking some of the non-libsolv related changes 
> which have been accumulating in the topic/libsolv branch.
> 
> Too soon? Should I wait for this change to be ready? Any other changes 
> you'd like to get in?

This change is pretty invasive and needs more testing and more feedback 
from others.  I think you should go ahead with the release and save this 
change for the next release.

The only other thing I can think of that's still pending is the attempt 
to make Enter do the right thing in the package chooser textbox:

   https://sourceware.org/ml/cygwin-apps/2017-12/msg00024.html

Ken