From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 54517 invoked by alias); 22 Nov 2017 17:02:18 -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 54506 invoked by uid 89); 22 Nov 2017 17:02:17 -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,RCVD_IN_DNSWL_LOW autolearn=ham version=3.3.2 spammy=Ken, Brown, UD:lst, UD:mirrors.lst X-HELO: out4-smtp.messagingengine.com Received: from out4-smtp.messagingengine.com (HELO out4-smtp.messagingengine.com) (66.111.4.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 22 Nov 2017 17:02:16 +0000 Received: from compute6.internal (compute6.nyi.internal [10.202.2.46]) by mailout.nyi.internal (Postfix) with ESMTP id 8E0B720D06; Wed, 22 Nov 2017 12:02:14 -0500 (EST) Received: from frontend1 ([10.202.2.160]) by compute6.internal (MEProxy); Wed, 22 Nov 2017 12:02:14 -0500 X-ME-Sender: Received: from [192.168.1.102] (host86-158-32-25.range86-158.btcentralplus.com [86.158.32.25]) by mail.messagingengine.com (Postfix) with ESMTPA id F35AF7E8BF; Wed, 22 Nov 2017 12:02:13 -0500 (EST) From: Jon Turney Subject: Re: [PATCH setup] site.cc, site.h: code cleanup To: cygwin-apps@cygwin.com Cc: Ken Brown References: <20171120012615.10428-1-kbrown@cornell.edu> Message-ID: <6bb98204-de00-8658-9415-6727aa59a550@dronecode.org.uk> Date: Wed, 22 Nov 2017 17: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: <20171120012615.10428-1-kbrown@cornell.edu> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit X-SW-Source: 2017-11/txt/msg00098.txt.bz2 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