From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 55732 invoked by alias); 20 Nov 2017 23:02:46 -0000 Mailing-List: contact cygwin-apps-help@cygwin.com; run by ezmlm Precedence: bulk Sender: cygwin-apps-owner@cygwin.com List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Mail-Followup-To: cygwin-apps@cygwin.com Received: (qmail 55668 invoked by uid 89); 20 Nov 2017 23:02:41 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.5 required=5.0 tests=AWL,BAYES_00,KB_WAM_FROM_NAME_SINGLEWORD,SPF_HELO_PASS,SPF_PASS,T_RP_MATCHES_RCVD autolearn=no version=3.3.2 spammy=california, California, Area, displays X-HELO: limerock01.mail.cornell.edu Received: from limerock01.mail.cornell.edu (HELO limerock01.mail.cornell.edu) (128.84.13.241) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Mon, 20 Nov 2017 23:02:39 +0000 X-CornellRouted: This message has been Routed already. Received: from authusersmtp.mail.cornell.edu (granite4.serverfarm.cornell.edu [10.16.197.9]) by limerock01.mail.cornell.edu (8.14.4/8.14.4_cu) with ESMTP id vAKN2adU018154 for ; Mon, 20 Nov 2017 18:02:37 -0500 Received: from [192.168.0.4] (mta-68-175-129-7.twcny.rr.com [68.175.129.7] (may be forged)) (authenticated bits=0) by authusersmtp.mail.cornell.edu (8.14.4/8.12.10) with ESMTP id vAKN2ZLj018532 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES128-SHA bits=128 verify=NOT) for ; Mon, 20 Nov 2017 18:02:36 -0500 Subject: Re: [PATCH] make setup mirror list more like web page not just urls To: cygwin-apps@cygwin.com References: <09c0dc5b-f975-42a9-a204-b02edef6a3ad@SystematicSw.ab.ca> <53a8001e-10ae-6318-ba3d-31d0cfa18f14@SystematicSw.ab.ca> From: Ken Brown Message-ID: Date: Mon, 20 Nov 2017 23:02:00 -0000 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:52.0) Gecko/20100101 Thunderbird/52.4.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 8bit X-PMX-Cornell-Gauge: Gauge=XXXXX X-PMX-CORNELL-AUTH-RESULTS: dkim-out=none; X-IsSubscribed: yes X-SW-Source: 2017-11/txt/msg00091.txt.bz2 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