From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from lists.gentoo.org (pigeon.gentoo.org [208.92.234.80]) by finch.gentoo.org (Postfix) with ESMTP id 03522138A1A for ; Thu, 13 Nov 2014 17:58:08 +0000 (UTC) Received: from pigeon.gentoo.org (localhost [127.0.0.1]) by pigeon.gentoo.org (Postfix) with SMTP id 86B2AE0CB0; Thu, 13 Nov 2014 17:58:06 +0000 (UTC) Received: from smtp.gentoo.org (smtp.gentoo.org [140.211.166.183]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by pigeon.gentoo.org (Postfix) with ESMTPS id 15621E0CAE for ; Thu, 13 Nov 2014 17:58:06 +0000 (UTC) Received: from [192.168.1.7] (ip70-181-96-121.oc.oc.cox.net [70.181.96.121]) (using TLSv1 with cipher ECDHE-RSA-AES128-SHA (128/128 bits)) (No client certificate requested) (Authenticated sender: zmedico) by smtp.gentoo.org (Postfix) with ESMTPSA id F31853403F4 for ; Thu, 13 Nov 2014 17:58:04 +0000 (UTC) Message-ID: <5464F129.6080404@gentoo.org> Date: Thu, 13 Nov 2014 09:58:01 -0800 From: Zac Medico User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:24.0) Gecko/20100101 Thunderbird/24.8.1 Precedence: bulk List-Post: List-Help: List-Unsubscribe: List-Subscribe: List-Id: Gentoo Linux mail X-BeenThere: gentoo-portage-dev@lists.gentoo.org Reply-to: gentoo-portage-dev@lists.gentoo.org MIME-Version: 1.0 To: gentoo-portage-dev@lists.gentoo.org Subject: Re: [gentoo-portage-dev] [PATCH] FEATURES=case-insensitive-fs for bug #524236 References: <1415841756-8430-1-git-send-email-zmedico@gentoo.org> <5464881D.6030307@gentoo.org> In-Reply-To: <5464881D.6030307@gentoo.org> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-Archives-Salt: 9f960310-330b-4c8a-9d94-98d643bc96a8 X-Archives-Hash: e23d88bdf49ce6e53e9c121eeeb38149 On 11/13/2014 02:29 AM, Alexander Berntsen wrote: > On 13/11/14 02:22, Zac Medico wrote: >> +if "case-insensitive-fs" in portage.settings.features: >> + FIND_EXTANT_CONFIGS = \ >> + FIND_EXTANT_CONFIGS.replace("-name '._cfg", "-iname '._cfg") >> + > Splitting inside the replace will look nicer following PEP indentation > (as you won't need the '\'). Okay, I'll re-format it as you've suggested. >> +Use case\-insensitive file name comparisions when merging and unmerging >> +files. >> +.TP > Maybe mention a) that most people can ignore this option, and b) who > it's actually for. Kind of in the kernel option help style. Okay, I'll add something about it only being needed for case-insensitive file systems, which are usually not used. > > In general I don't like this patch. It handles a bunch of cases separately > by doing lower(), when I think instead it should be handled implicitly. > The data should be in a structure such that it knows whether it is supposed > to be upper or lowercase, and whatever's dealing with it should deal with > it accordingly, rather than checking "is this case insensitive? OK > lowercase it before sending it wherever". Aside from the ConfigProtect constructor, which has a new case_insensitive keyword parameter, all affected methods handle case transformations "implicitly", as far as API consumers are concerned. However, we could improve efficiency for some usage patterns by providing an alternative to dblink.getcontents that is oriented toward case-insensitive handling. For example, every single dblink._match_contents call currently has to transform all names to lowercase, and generate a reverse mapping from lowercase back to preserved case. The dblink._match_contents method would be more efficient if we created an alternative to dblink.getcontents that handled the transformations and cached the results. I intentionally did not implement this optimization yet, since it's probably better to do it in a separate patch, rather than complicate the current patch. > But if you think this is the best way, I'm not going to stand in the way of this patch. As discussed above, think the current approach is pretty reasonable, though I would like to optimize it with a separate patch. -- Thanks, Zac