Re: [PATCH] Warn user when 2 files conflict on a case insensitive fs

[Patrice Dumas]

On Fri, Jan 06, 2023 at 09:52:34PM +0000, Gavin Smith wrote:
> On Fri, Jan 06, 2023 at 08:47:52PM +0100, Patrice Dumas wrote:
> > On Fri, Jan 06, 2023 at 01:28:04PM +0100, Torbjörn SVENSSON wrote:
> > > Hi,
> > > 
> > > I can't find where to send patches, so pardon me if this is the wrong
> > > channel.
> > 
> > It is the right channel.
> > 
> > > I recently noticed a problem doing a building GDB for Windows on a
> > > GNU/Linux host. The documentation contains 'qMemTags' and 'QMemTags'
> > > and as GDB is using the --split-size argument, the --node-files is
> > > automatically activated and a bunch of redirect files are generated,
> > > including qMemTags.html and QMemTags.html. As these 2 files have the
> > > "same" filename on Windows, the content of one of them will be used,
> > > but it's unpredictable witch one. So, to make the developers aware
> > > of this situation, I added a simple warning to makeinfo about the
> > > potential problem for case insensitive file systems.
> > 
> > I recall some discussion on this list, and I remember that the end
> > result was that it was not a good thing to consider something special
> > on platforms that have case sensitive filesystems. One possibility would

Some previous discussions:
https://lists.gnu.org/archive/html/bug-texinfo/2006-05/msg00018.html
https://lists.gnu.org/archive/html/bug-texinfo/2012-07/msg00032.html

> > be to have the warning emitted conditionally on having the
> > CASE_INSENSITIVE_FILENAMES customization variable set.  With that setup,
> > also, I think that the warning will be emitted only in case of a conflict
> > between a redirection node and another file, conflict among normal file
> > output will be silenced because the nodes will simply be appended
> > without trying to open a new file.
> 
> I agree, it would be fine to have warning if more than one node/"output
> unit" actually gets appended to an output file because of
> CASE_INSENSITIVE_FILENAMES.
> 
> The manual seems to imply that CASE_INSENSITIVE_FILENAMES does not
> need to be set by the user if the file system is case insensitive.  If
> that is true, then the following shouldn't happen:

As far as I can tell, this is not true, CASE_INSENSITIVE_FILENAMES needs
to be set by the user.  My recalling is that it can be used to emulate what
would happen on a case insensitive filesystem even if the filesystem is
case sensitive.  But it could also be necessary to set it on case
insensitive filesystems manually.

> > As these 2 files have the "same" filename on Windows, the content
> > of one of them will be used, but it's unpredictable witch one.
> 
> It shouldn't happen that the output is lost for the contents of
> one of the files - all the output should be present in the file.
> Can we confirm what is actually happening?

I do not think that this can be done.  The file is not a regular file,
but a file used only for redirection.  It could be possible to append
some text to the file instead of rewriting, without changing the
redirection, but it does not make much sense to append for a redirection
file.  It seems to me that on a platform with case insensitive
filenames, there is no other possibility than having only one
redirection file.

> 
> 
> > There are some issues with the patch, I think.
> > * the warning messages comes somewhat too late, there is not much
> >   information on the node or nodes conflicting, it is up to the user
> >   to find out which nodes are in clonflict.  Maybe this check should
> >   be done before, and even quite early, after the determination of the
> >   file names, but before any actual output.  Maybe after the call of
> >   _set_root_commands_targets_node_files and only if NODE_FILES is set.
> 
> Maybe in Texinfo::Convert::Converter::set_tree_unit_file or
> Texinfo::Covert::HTML::_html_set_pages_files.  The following in the
> latter function already keeps track of how often an output file
> is used:
> 
>     $self->set_tree_unit_file($tree_unit, $filename);
>     my $tree_unit_filename = $tree_unit->{'structure'}->{'unit_filename'};
>     $self->{'file_counters'}->{$tree_unit_filename} = 0
>        if (!exists($self->{'file_counters'}->{$tree_unit_filename}));
>     $self->{'file_counters'}->{$tree_unit_filename}++;
> 
> I haven't investigated it but I suspect this counter only goes above
> 1 if there is the case-insensitive filename issue, in which case
> a warning could be issued that multiple "tree units" are being
> concatenated in a single file.

It is as you say if split at node, but otherwise it could be ok
to have concatenanted files, or it could not be ok, but in any case it
should be possible to find that there is something amiss, maybe in the
_normalize_filename_case function. However I do not think that we want
to warn in that case, it seems to me that in an insensitive file system
concatenating multiple tree elements together is correct and does not
require a warning.  For redirection files, however, concatenating is not
right, but I think that there should only be a warning for redirection
files, not for regular tree units.


Something is puzzling me, upon reading the code, it seems to me that on
case insensitive systems , and if CASE_INSENSITIVE_FILENAMES is not set,
and if split at node, the node files should be overwritten, as a file
including two files will be opened and closed twice instead of elements
being concatenated.

Does anybody knows if nodes are correctly concatenated with case
insensitive filesystems if CASE_INSENSITIVE_FILENAMES is not set?

-- 
Pat