Re: [PATCH setup] site.cc, site.h: code cleanup

[Jon Turney <jon.turney@dronecode.org.uk>]

On 20/11/2017 01:26, Ken Brown wrote:
> Remove site_list_type::init(), which was introduced to work around a
> problem with gcc-2.95.

Please tell me we're not actually using a placement new for these things :S

> Add a bool member 'is_official' to the site_list_type class.  Use it
> to distinguish official mirrors (listed in mirrors.lst) from
> user-added sites.  This replaces the (undocumented) use of
> site_list_type::servername.size() for this purpose.
> 
> When registerSavedSite is called on a URL that's already in
> 'all_site_list', add the version from 'all_site_list' to 'site_list'
> rather than adding a temporary version that contains no information
> other than the URL.
> 
> Similarly, if the user adds a site that was already in
> 'all_site_list', don't replace the existent version with the new one
> (which contains only the URL).

This is a nice bit of cleanup.

My only concern is with the terminology "is_offical".

In general, this code suffers from confusion between (i) package 
repository [a set of packages], and (ii) mirrors [a set of sites 
offering the same package repository]

(See also the musings in [1]. I have a vague recollection that I 
actually started writing some code to do some of that, but that would be 
in an old CVS checkout somewhere)

We have the concept of a label for the package set ('release:' in 
setup.ini [2])

To me "is_official" gives the idea that this flag means "the package set 
at this URL is the package set with the label 'cygwin'", but actually it 
just means "this URL was read from mirrors.lst", so maybe it could be 
named something like that?

Oh, you actually have a comment to that effect...

[1] https://cygwin.com/ml/cygwin-apps/2011-04/msg00014.html
[2] https://sourceware.org/cygwin-apps/setup.ini.html