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

Ken Brown kbrown@cornell.edu
Mon Nov 20 23:02:00 GMT 2017 

On 11/20/2017 5:59 PM, 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:
>>
>> Thanks for your comments, I understand all your points, and agree with 
>> them.
>> I merged the posts and responses as they addressed the same issues.
>> I also applied Ken's setup patch and rejigged mine where required.
>>
>> KB> Ken Brown
>> JT> Jon Turney
>>
>> KB> I think this is a good idea in principle, but it still needs some 
>> work.
>>
>> JT> use a grid control to present this information.
>>
>> Don't think I want to learn that much about Windows controls!
>> The width of the longest fields in each column would need to adjust 
>> the grid
>> and dialogue box widths, and Add button position, dynamically, which I 
>> did
>> manually, and the internal blank space may be a bit harder to scan for 
>> users
>> who are interested only in their geographic region, country, or territory
>> - these are pretty well aligned as strings.
>>
>> KB> Adding the protocol to the key instead of the full url isn't 
>> enough to
>> KB> guarantee uniqueness of the key.
>> KB> Users can add their own sites for which that fails.
>> KB> Also, you shouldn't be relying on the servername field being 
>> filled in.
>> KB> For example, the function SiteSetting::registerSavedSite() explicitly
>> KB> constructs a site 'tempSite' that has an empty servername.
>>
>> JT> This code needs to handle manually added URLs with non-FQDNs.
>> JT> - - url...'s going to need fixing.
>>
>> If host name provided, use as servername, or default to localhost.
>>
>> If at least two domain components:
>> if CC TLD, use as area in uppercase, or
>> if gTLD, use as area with init cap;
>> if 2-3 char TLD and 2-3 char 2nd level domain, may be subTLD, so if 
>> 3rd level
>> domain exists and does not start with (or contain?) "cygwin", "ftp", 
>> "mirror",
>> "www", add space plus 2-3 char 2nd level domain with init cap to area.
>> Use next level domain as location with init cap and uppercase after 
>> any "-".
>> Otherwise default area to Local, and use servername as location with 
>> init cap.
>> This adhoc approach seems to work well enough and avoids having to 
>> drag in and
>> process the rules in the Public Suffix List (libpsl5 and
>> publicsuffix-list-dafsa).
>>
>> Examples:
>>
>> Input    ->
>>      List
>>      Displayed
>>      Key
>>
>> http://mirror.cpsc.ucalgary.ca/mirror/cygwin.com        ->
>> http://mirror.cpsc.ucalgary.ca/mirror/cygwin.com/;mirror.cpsc.ucalgary.ca;CA;Ucalgary 
>>
>> CA - Ucalgary - http://mirror.cpsc.ucalgary.ca/mirror/cygwin.com
>> CA Ucalgary ca ucalgary cpsc mirror
>> http://mirror.cpsc.ucalgary.ca/mirror/cygwin.com/
>>
>> [cyg]file://server/c$/usr/local/cygwin/var/cache/setup/packages    ->
>> [cyg]file://server/c$/usr/local/cygwin/var/cache/setup/packages/;server;Local;Server 
>>
>> Local - Server - 
>> file://server/c$/usr/local/cygwin/var/cache/setup/packages
>> Local Server server 
>> file://server/c$/usr/local/cygwin/var/cache/setup/packages/
>>
>> cygfile:///var/cache/setup/packages        ->
>> cygfile:///var/cache/setup/packages/;localhost;Local;Localhost
>> Local - Localhost - cygfile://var/cache/setup/packages
>> Local Localhost localhost cygfile://var/cache/setup/packages/
>>
>> file:///c|/usr/local/cygwin/var/cache/setup/packages        ->
>> file:///c|/usr/local/cygwin/var/cache/setup/packages/;localhost;Local;Localhost 
>>
>> Local - Localhost - file:///c|/usr/local/cygwin/var/cache/setup/packages
>> Local Localhost localhost 
>> file:///c|/usr/local/cygwin/var/cache/setup/packages/
>>
>> Cygwin file:// does not appear to use /// or support anything but 
>> local files,
>> so should always be treated as localhost; or does it support servers, 
>> but they
>> can be treated as if directories under Windows, so we need to do a 
>> server or
>> directory check, or that check is done at a lower level?
>>
>> JT> you can also just add a path.
>>
>> If UNC path with server, use as above, but include share name in display,
>> otherwise default as above.
>>
>> Examples:
>>
>> \\server\c$\usr\local\cygwin\var\cache\setup\packages        ->
>> \\server\c$\usr\local\cygwin\var\cache\setup\packages/;server;Local;Server 
>>
>> Local - Server - \\server\c$
>> Local Server server 
>> \\server\c$\usr\local\cygwin\var\cache\setup\packages/
>>
>> c:\usr\local\cygwin\var\cache\setup\packages            ->
>> c:\usr\local\cygwin\var\cache\setup\packages/;localhost;Local;Localhost
>> Local - Localhost - c:\usr\local\cygwin\var\cache\setup\packages
>> Local Localhost localhost c:\usr\local\cygwin\var\cache\setup\packages/
>>
>> Where are the cached sites stashed - setup keeps showing my old test 
>> entries?
>>
>> JT> if you add two URLs with the same protocol/host, they aren't 
>> distinguishable
>>
>> Check for and eliminate duplicates after sort if same site_list fields.
>> Handled by KB patch.
>> Note on flag added by that patch - would is_current, is_good, or 
>> is_known be a
>> better reflection of what that flag means?
>> Should we add ==(string) to compare only the url member?
>> Should we add a constructor(string,is_official) to refactor string to 
>> site
>> conversions, and the url additions?
>>
>> KB> If the saved url corresponds to a site in the list, I think setup 
>> should
>> KB> recognize this rather than listing the same site again as " - - url".
>>
>> JT> If the URL matches one in the list, that item should be selected.
>>
>> Check for URL match with site list and use match as site:
>> added for tempSite in RegisterSavedSites and newsite on Add;
>> required as key now includes area and location, not just URL.
>> Could we add a check to a string contructor to return only a reference 
>> to an
>> existing entry, if the url matches, to refactor the above changes?
>>
>> JT> Store area and location so we can populate those fields if it 
>> doesn't.
>>
>> See above.
>>
>> JT> Please, please use git-format-patch in future.
>>
>> I can't tell from looking, does it differ from diff output, and is 
>> there a
>> reason to use git-format-patch for review?
>> I use diffs for feedback until it DTRT.
>> I can manage one commit and one format-patch on a good day.
>> I should just stick to using what Tortoise Git supports.
>> My record with git includes a lot of rm -rf and re-clone, as the 
>> files, trees,
>> and indices get out of whack when I forget some incantation and try to 
>> fix it!
>> Very brittle and unfriendly - if it's going to bork itself, just don't 
>> do that!
>> I never had a single problem with CVS or Hg in many years ;^>
>>
>> JT> The patch also has some spurious changes to the mode of the 
>> manifest files.
>>
>> Realized I shouldn't tidy up unnecessary x modes under git control - 
>> habit
>> - gets me into trouble and I learn new things - checked out masters.
> 
> Hi Brian,
> 
> 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
> 
> Also, I don't like the fake 'area' and 'location' fields for user-added 
> URLs.  For example, "Org" and "Kernel" are startling in the display 
> above and add no information to the plain URL.  Another example is my 
> own personal repository, http://sanibeltranquility.com/cygwin, which 
> displays as
> 
>    Com - Sanibeltranquility - http://sanibeltranquility.com
> 
> I would rather see the area and location left blank, or perhaps replaced 
> by "Unknown".  A third alternative would be to replace "Area - Location" 
> by "User-added" or something similar.

One other thing I just noticed: If I cancel the installation while the 
mirror list is displayed, the wrong URL(s) is/are written into 
/etc/setup/setup.rc under "last-mirror".  I don't recall this ever 
happening before, but maybe I just never noticed.

Ken

More information about the Cygwin-apps mailing list