Re: [gentoo-portage-dev] [PATCH] FEATURES=case-insensitive-fs for bug #524236

[Zac Medico]

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