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

[Patrice Dumas]

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
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.

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.

* I have not checked precisely, and I do not have a platform with case
  insensitive filesystems, but it seems to me that the test you propose
  will not behave as expected on a case insensitive filesystem,
  could fail in that setup, and also that even unpacking the source
  with the two files in the result differing only by case would be
  problematic on those platforms?

I think that it is actually probably quite difficult or even impossible
to test for file conflicts in a way that works on both case sensitive
and case insensitive file systems.  That being said, it could still be
relevant to have a test ready and have explanation on how to run it
manually.  For that the test you provide could be used, with an
explanation stating that the test need to be run on a case sensitive
filesystem.

Do you or anybody else have other comments?  Do you want to rework the
patch based on my comments?

> 
> Kind regards,
> Torbjörn
> 
> ---
> 
> * tp/Texinfo/Common.pm (output_files_initialize, output_files_open_out):
> Add warning when paths conflicts on a case insensitive file system.
> 
> * tp/tests/formatting/list-of-tests (unique_filenames): New test.
> 
> Signed-off-by: Torbjörn SVENSSON <torbjorn.svensson@foss.st.com>
> ---
>  ChangeLog                                     |  7 +++
>  tp/Texinfo/Common.pm                          |  7 ++-
>  tp/tests/formatting/list-of-tests             |  1 +
>  .../res_parser/unique_filenames/Foo.html      | 23 ++++++++
>  .../res_parser/unique_filenames/foo.html      | 23 ++++++++
>  .../res_parser/unique_filenames/index.html    | 52 +++++++++++++++++++
>  .../unique_filenames/unique_filenames.1       |  0
>  .../unique_filenames/unique_filenames.2       |  1 +
>  tp/tests/formatting/unique_filenames.texi     | 27 ++++++++++
>  .../formatting_unique_filenames.sh            | 19 +++++++
>  10 files changed, 159 insertions(+), 1 deletion(-)
>  create mode 100644 tp/tests/formatting/res_parser/unique_filenames/Foo.html
>  create mode 100644 tp/tests/formatting/res_parser/unique_filenames/foo.html
>  create mode 100644 tp/tests/formatting/res_parser/unique_filenames/index.html
>  create mode 100644 
> tp/tests/formatting/res_parser/unique_filenames/unique_filenames.1
>  create mode 100644 
> tp/tests/formatting/res_parser/unique_filenames/unique_filenames.2
>  create mode 100644 tp/tests/formatting/unique_filenames.texi
>  create mode 100755 tp/tests/test_scripts/formatting_unique_filenames.sh
> 
> diff --git a/ChangeLog b/ChangeLog
> index d416b25a45..5821561f47 100644
> --- a/ChangeLog
> +++ b/ChangeLog
> @@ -1,3 +1,10 @@
> +2023-01-06  Torbjörn Svensson <torbjorn.svensson@foss.st.com>
> +
> +     * tp/Texinfo/Common.pm (output_files_initialize, output_files_open_out):
> +     Add warning when paths conflicts on a case insensitive file system.
> +
> +     * tp/tests/formatting/list-of-tests (unique_filenames): New test.
> +
>  2023-01-05  Patrice Dumas  <pertusus@free.fr>
>  
>       * tp/Texinfo/ParserNonXS.pm (%parser_state_initialization, _next_text)
> diff --git a/tp/Texinfo/Common.pm b/tp/Texinfo/Common.pm
> index 9a8f1d2f9e..060de25394 100644
> --- a/tp/Texinfo/Common.pm
> +++ b/tp/Texinfo/Common.pm
> @@ -754,7 +754,7 @@ sub locate_init_file($$$)
>  # both in POD and in HTML Customization API.
>  sub output_files_initialize
>  {
> -  return {'unclosed_files' => {}, 'opened_files' => []};
> +  return {'unclosed_files' => {}, 'opened_files' => [], 'unique_files' => 
> {}};
>  }
>  #
>  # All the opened files are registered, except for stdout,
> @@ -811,6 +811,11 @@ sub output_files_open_out($$$;$$)
>    if ($self) {
>      push @{$self->{'opened_files'}}, $file_path;
>      $self->{'unclosed_files'}->{$file_path} = $filehandle;
> +    if (exists $self->{'unique_files'}{lc $file_path}) {
> +      print STDERR "warning: $file_path is in conflict with another file on 
> a ".
> +        "case insensitive file system.\n";
> +    }
> +    $self->{'unique_files'}{lc $file_path} = ();
>    }
>    return $filehandle, undef;
>  }
> diff --git a/tp/tests/formatting/list-of-tests 
> b/tp/tests/formatting/list-of-tests
> index 8a6d40cbd4..bd39fcf77c 100644
> --- a/tp/tests/formatting/list-of-tests
> +++ b/tp/tests/formatting/list-of-tests
> @@ -33,3 +33,4 @@ index_entries_relate_to_item 
> index_entries_relate_to_item.texi --html --no-split
>  
>  info_extension_warning foo.info
>  
> +unique_filenames unique_filenames.texi --html --node-files
> diff --git a/tp/tests/formatting/res_parser/unique_filenames/Foo.html 
> b/tp/tests/formatting/res_parser/unique_filenames/Foo.html
> new file mode 100644
> index 0000000000..30cce03d42
> --- /dev/null
> +++ b/tp/tests/formatting/res_parser/unique_filenames/Foo.html
> @@ -0,0 +1,23 @@
> +<!DOCTYPE html>
> +<html>
> +<!-- Created by texinfo, http://www.gnu.org/software/texinfo/ -->
> +<!-- This file redirects to the location of a node or anchor -->
> +<head>
> +<meta http-equiv="Content-Type" content="text/html; charset=utf-8">
> +<title>Foo (top section)</title>
> +
> +<meta name="description" content="Foo (top section)">
> +<meta name="keywords" content="Foo (top section)">
> +<meta name="resource-type" content="document">
> +<meta name="distribution" content="global">
> +<meta name="Generator" content="texi2any">
> +
> +<meta http-equiv="Refresh" content="0; url=index.html#Foo">
> +<meta name="viewport" content="width=device-width,initial-scale=1">
> +
> +</head>
> +
> +<body lang="en">
> +
> +<p>The node you are looking for is at <a href="index.html#Foo">Foo</a>.</p>
> +</body>
> diff --git a/tp/tests/formatting/res_parser/unique_filenames/foo.html 
> b/tp/tests/formatting/res_parser/unique_filenames/foo.html
> new file mode 100644
> index 0000000000..7b0603002d
> --- /dev/null
> +++ b/tp/tests/formatting/res_parser/unique_filenames/foo.html
> @@ -0,0 +1,23 @@
> +<!DOCTYPE html>
> +<html>
> +<!-- Created by texinfo, http://www.gnu.org/software/texinfo/ -->
> +<!-- This file redirects to the location of a node or anchor -->
> +<head>
> +<meta http-equiv="Content-Type" content="text/html; charset=utf-8">
> +<title>foo (top section)</title>
> +
> +<meta name="description" content="foo (top section)">
> +<meta name="keywords" content="foo (top section)">
> +<meta name="resource-type" content="document">
> +<meta name="distribution" content="global">
> +<meta name="Generator" content="texi2any">
> +
> +<meta http-equiv="Refresh" content="0; url=index.html#foo">
> +<meta name="viewport" content="width=device-width,initial-scale=1">
> +
> +</head>
> +
> +<body lang="en">
> +
> +<p>The node you are looking for is at <a href="index.html#foo">foo</a>.</p>
> +</body>
> diff --git a/tp/tests/formatting/res_parser/unique_filenames/index.html 
> b/tp/tests/formatting/res_parser/unique_filenames/index.html
> new file mode 100644
> index 0000000000..29a735be57
> --- /dev/null
> +++ b/tp/tests/formatting/res_parser/unique_filenames/index.html
> @@ -0,0 +1,52 @@
> +<!DOCTYPE html>
> +<html>
> +<!-- Created by texinfo, http://www.gnu.org/software/texinfo/ -->
> +<head>
> +<meta http-equiv="Content-Type" content="text/html; charset=utf-8">
> +<title>Top (top section)</title>
> +
> +<meta name="description" content="Top (top section)">
> +<meta name="keywords" content="Top (top section)">
> +<meta name="resource-type" content="document">
> +<meta name="distribution" content="global">
> +<meta name="Generator" content="texi2any">
> +<meta name="viewport" content="width=device-width,initial-scale=1">
> +
> +<link href="#Top" rel="start" title="Top">
> +
> +
> +</head>
> +
> +<body lang="en">
> +
> +<div class="top-level-extent" id="Top">
> +<h1 class="top" id="top-section">top section</h1>
> +
> +<ul class="mini-toc">
> +<li><a href="#Chapter" accesskey="1">Chapter</a></li>
> +</ul>
> +<div class="chapter-level-extent" id="Chapter">
> +<h2 class="chapter">1 Chapter</h2>
> +
> +<dl class="table">
> +<dt>&lsquo;<samp class="samp">foo</samp>&rsquo;</dt>
> +<dd><a class="anchor" id="foo"></a><p>Some text about &lsquo;<samp 
> class="samp">foo</samp>&rsquo;
> +</p>
> +</dd>
> +<dt>&lsquo;<samp class="samp">Foo</samp>&rsquo;</dt>
> +<dd><a class="anchor" id="Foo"></a><p>Some text about &lsquo;<samp 
> class="samp">Foo</samp>&rsquo;
> +</p>
> +</dd>
> +</dl>
> +
> +<p>See <a class="xref" href="#foo">foo</a>
> +</p>
> +<p>See <a class="xref" href="#Foo">Foo</a>
> +</p>
> +</div>
> +</div>
> +
> +
> +
> +</body>
> +</html>
> diff --git 
> a/tp/tests/formatting/res_parser/unique_filenames/unique_filenames.1 
> b/tp/tests/formatting/res_parser/unique_filenames/unique_filenames.1
> new file mode 100644
> index 0000000000..e69de29bb2
> diff --git 
> a/tp/tests/formatting/res_parser/unique_filenames/unique_filenames.2 
> b/tp/tests/formatting/res_parser/unique_filenames/unique_filenames.2
> new file mode 100644
> index 0000000000..792c88f2f9
> --- /dev/null
> +++ b/tp/tests/formatting/res_parser/unique_filenames/unique_filenames.2
> @@ -0,0 +1 @@
> +warning: formatting/out_parser/unique_filenames/foo.html is in conflict with 
> another file on a case insensitive file system.
> diff --git a/tp/tests/formatting/unique_filenames.texi 
> b/tp/tests/formatting/unique_filenames.texi
> new file mode 100644
> index 0000000000..4fe491b751
> --- /dev/null
> +++ b/tp/tests/formatting/unique_filenames.texi
> @@ -0,0 +1,27 @@
> +\input texinfo @c -*-texinfo-*-
> +
> +@setfilename unique_filenames.info
> +
> +@node Top
> +@top top section
> +
> +@chapter Chapter
> +
> +@table @samp
> +
> +@item foo
> +@anchor{foo}
> +Some text about @samp{foo}
> +
> +@item Foo
> +@anchor{Foo}
> +Some text about @samp{Foo}
> +
> +@end table
> +
> +@xref{foo}
> +
> +@xref{Foo}
> +
> +@bye
> +
> diff --git a/tp/tests/test_scripts/formatting_unique_filenames.sh 
> b/tp/tests/test_scripts/formatting_unique_filenames.sh
> new file mode 100755
> index 0000000000..e722a4a78e
> --- /dev/null
> +++ b/tp/tests/test_scripts/formatting_unique_filenames.sh
> @@ -0,0 +1,19 @@
> +#! /bin/sh
> +# This file generated by maintain/regenerate_cmd_tests.sh
> +
> +if test z"$srcdir" = "z"; then
> +  srcdir=.
> +fi
> +
> +one_test_logs_dir=test_log
> +
> +
> +dir=formatting
> +name='unique_filenames'
> +mkdir -p $dir
> +
> +"$srcdir"/run_parser_all.sh -dir $dir $name
> +exit_status=$?
> +cat $dir/$one_test_logs_dir/$name.log
> +exit $exit_status
> +
> -- 
> 2.25.1
> 
>