autotools: extend patching of the libtool script (#30768)

* filter_file: introduce argument 'start_at'

* autotools: extend patching of the libtool script

* autotools: refactor _patch_usr_bin_file

* autotools: improve readability of the filtering

* autotools: keep the modification time of the configure scripts

* autotools: do not try to patch directories

* autotools: explain libtool patching for posterity

Co-authored-by: Massimiliano Culpo <massimiliano.culpo@gmail.com>
Co-authored-by: Harmen Stoppels <harmenstoppels@gmail.com>

4 files changed, 208 insertions, 34 deletions

diff --git a/lib/spack/llnl/util/filesystem.py b/lib/spack/llnl/util/filesystem.py
index ad91e7c876..511f7c4f59 100644
--- a/lib/spack/llnl/util/filesystem.py
+++ b/lib/spack/llnl/util/filesystem.py
@@ -233,9 +233,14 @@ def filter_file(regex, repl, *filenames, **kwargs):
 
     Keyword Arguments:
         string (bool): Treat regex as a plain string. Default it False
-        backup (bool): Make backup file(s) suffixed with ``~``. Default is True
+        backup (bool): Make backup file(s) suffixed with ``~``. Default is False
         ignore_absent (bool): Ignore any files that don't exist.
             Default is False
+        start_at (str): Marker used to start applying the replacements. If a
+            text line matches this marker filtering is started at the next line.
+            All contents before the marker and the marker itself are copied
+            verbatim. Default is to start filtering from the first line of the
+            file.
         stop_at (str): Marker used to stop scanning the file further. If a text
             line matches this marker filtering is stopped and the rest of the
             file is copied verbatim. Default is to filter until the end of the
@@ -244,6 +249,7 @@ def filter_file(regex, repl, *filenames, **kwargs):
     string = kwargs.get("string", False)
     backup = kwargs.get("backup", False)
     ignore_absent = kwargs.get("ignore_absent", False)
+    start_at = kwargs.get("start_at", None)
     stop_at = kwargs.get("stop_at", None)
 
     # Allow strings to use \1, \2, etc. for replacement, like sed
@@ -292,6 +298,7 @@ def filter_file(regex, repl, *filenames, **kwargs):
             # reached or we found a marker in the line if it was specified
             with open(tmp_filename, mode="r", **extra_kwargs) as input_file:
                 with open(filename, mode="w", **extra_kwargs) as output_file:
+                    do_filtering = start_at is None
                     # Using iter and readline is a workaround needed not to
                     # disable input_file.tell(), which will happen if we call
                     # input_file.next() implicitly via the for loop
@@ -301,8 +308,12 @@ def filter_file(regex, repl, *filenames, **kwargs):
                             if stop_at == line.strip():
                                 output_file.write(line)
                                 break
-                        filtered_line = re.sub(regex, repl, line)
-                        output_file.write(filtered_line)
+                        if do_filtering:
+                            filtered_line = re.sub(regex, repl, line)
+                            output_file.write(filtered_line)
+                        else:
+                            do_filtering = start_at == line.strip()
+                            output_file.write(line)
                     else:
                         current_position = None
 
diff --git a/lib/spack/spack/build_systems/autotools.py b/lib/spack/spack/build_systems/autotools.py
index d6707ac27e..81a572de52 100644
--- a/lib/spack/spack/build_systems/autotools.py
+++ b/lib/spack/spack/build_systems/autotools.py
@@ -244,8 +244,11 @@ To resolve this problem, please try the following:
         scripts to use file from path."""
 
         if self.spec.os.startswith("nixos"):
-            for configure_file in fs.find(".", files=["configure"], recursive=True):
-                fs.filter_file("/usr/bin/file", "file", configure_file, string=True)
+            x = fs.FileFilter(
+                *filter(fs.is_exe, fs.find(self.build_directory, "configure", recursive=True))
+            )
+            with fs.keep_modification_time(*x.filenames):
+                x.filter(regex="/usr/bin/file", repl="file", string=True)
 
     @run_before("configure")
     def _set_autotools_environment_variables(self):
@@ -262,35 +265,97 @@ To resolve this problem, please try the following:
         """
         os.environ["FORCE_UNSAFE_CONFIGURE"] = "1"
 
+    @run_before("configure")
+    def _do_patch_libtool_configure(self):
+        """Patch bugs that propagate from libtool macros into "configure" and
+        further into "libtool". Note that patches that can be fixed by patching
+        "libtool" directly should be implemented in the _do_patch_libtool method
+        below."""
+
+        # Exit early if we are required not to patch libtool-related problems:
+        if not self.patch_libtool:
+            return
+
+        x = fs.FileFilter(
+            *filter(fs.is_exe, fs.find(self.build_directory, "configure", recursive=True))
+        )
+
+        # There are distributed automatically generated files that depend on the configure script
+        # and require additional tools for rebuilding.
+        # See https://github.com/spack/spack/pull/30768#issuecomment-1219329860
+        with fs.keep_modification_time(*x.filenames):
+            # Fix parsing of compiler output when collecting predeps and postdeps
+            # https://lists.gnu.org/archive/html/bug-libtool/2016-03/msg00003.html
+            x.filter(regex=r'^(\s*if test x-L = )("\$p" \|\|\s*)$', repl=r"\1x\2")
+            x.filter(
+                regex=r'^(\s*test x-R = )("\$p")(; then\s*)$', repl=r'\1x\2 || test x-l = x"$p"\3'
+            )
+            # Support Libtool 2.4.2 and older:
+            x.filter(regex=r'^(\s*test \$p = "-R")(; then\s*)$', repl=r'\1 || test x-l = x"$p"\2')
+
     @run_after("configure")
     def _do_patch_libtool(self):
         """If configure generates a "libtool" script that does not correctly
         detect the compiler (and patch_libtool is set), patch in the correct
-        flags for the Arm, Clang/Flang, Fujitsu and NVHPC compilers. Also
-        filter out spurious predep_objects for Intel dpcpp builds."""
-
-        # Exit early if we are required not to patch libtool
+        values for libtool variables.
+
+        The generated libtool script supports mixed compilers through tags:
+        ``libtool --tag=CC/CXX/FC/...```. For each tag there is a block with variables,
+        which defines what flags to pass to the compiler. The default variables (which
+        are used by the default tag CC) are set in a block enclosed by
+        ``# ### {BEGIN,END} LIBTOOL CONFIG``. For non-default tags, there are
+        corresponding blocks ``# ### {BEGIN,END} LIBTOOL TAG CONFIG: {CXX,FC,F77}`` at
+        the end of the file (after the exit command). libtool evals these blocks.
+        Whenever we need to update variables that the configure script got wrong
+        (for example cause it did not recognize the compiler), we should properly scope
+        those changes to these tags/blocks so they only apply to the compiler we care
+        about. Below, ``start_at`` and ``stop_at`` are used for that."""
+
+        # Exit early if we are required not to patch libtool:
         if not self.patch_libtool:
             return
 
-        for libtool_path in fs.find(self.build_directory, "libtool", recursive=True):
-            self._patch_libtool(libtool_path)
-
-    def _patch_libtool(self, libtool_path):
-        if (
-            self.spec.satisfies("%arm")
-            or self.spec.satisfies("%clang")
-            or self.spec.satisfies("%fj")
-            or self.spec.satisfies("%nvhpc")
-        ):
-            fs.filter_file('wl=""\n', 'wl="-Wl,"\n', libtool_path)
-            fs.filter_file(
-                'pic_flag=""\n', 'pic_flag="{0}"\n'.format(self.compiler.cc_pic_flag), libtool_path
+        x = fs.FileFilter(
+            *filter(fs.is_exe, fs.find(self.build_directory, "libtool", recursive=True))
+        )
+
+        # Exit early if there is nothing to patch:
+        if not x.filenames:
+            return
+
+        markers = {"cc": "LIBTOOL CONFIG"}
+        for tag in ["cxx", "fc", "f77"]:
+            markers[tag] = "LIBTOOL TAG CONFIG: {0}".format(tag.upper())
+
+        # Replace empty linker flag prefixes:
+        if self.compiler.name == "nag":
+            # Nag is mixed with gcc and g++, which are recognized correctly.
+            # Therefore, we change only Fortran values:
+            for tag in ["fc", "f77"]:
+                marker = markers[tag]
+                x.filter(
+                    regex='^wl=""$',
+                    repl='wl="{0}"'.format(self.compiler.linker_arg),
+                    start_at="# ### BEGIN {0}".format(marker),
+                    stop_at="# ### END {0}".format(marker),
+                )
+        else:
+            x.filter(regex='^wl=""$', repl='wl="{0}"'.format(self.compiler.linker_arg))
+
+        # Replace empty PIC flag values:
+        for cc, marker in markers.items():
+            x.filter(
+                regex='^pic_flag=""$',
+                repl='pic_flag="{0}"'.format(getattr(self.compiler, "{0}_pic_flag".format(cc))),
+                start_at="# ### BEGIN {0}".format(marker),
+                stop_at="# ### END {0}".format(marker),
             )
-        if self.spec.satisfies("%fj"):
-            fs.filter_file("-nostdlib", "", libtool_path)
+
+        # Other compiler-specific patches:
+        if self.compiler.name == "fj":
+            x.filter(regex="-nostdlib", repl="", string=True)
             rehead = r"/\S*/"
-            objfile = [
+            for o in [
                 "fjhpctag.o",
                 "fjcrt0.o",
                 "fjlang08.o",
@@ -298,16 +363,86 @@ To resolve this problem, please try the following:
                 "crti.o",
                 "crtbeginS.o",
                 "crtendS.o",
-            ]
-            for o in objfile:
-                fs.filter_file(rehead + o, "", libtool_path)
-        # Hack to filter out spurious predp_objects when building with
-        # Intel dpcpp; see issue #32863
-        if self.spec.satisfies("%dpcpp"):
-            fs.filter_file(
-                r"^(predep_objects=.*)/tmp/conftest-[0-9A-Fa-f]+\.o", r"\1", libtool_path
+            ]:
+                x.filter(regex=(rehead + o), repl="", string=True)
+        elif self.compiler.name == "dpcpp":
+            # Hack to filter out spurious predep_objects when building with Intel dpcpp
+            # (see https://github.com/spack/spack/issues/32863):
+            x.filter(regex=r"^(predep_objects=.*)/tmp/conftest-[0-9A-Fa-f]+\.o", repl=r"\1")
+            x.filter(regex=r"^(predep_objects=.*)/tmp/a-[0-9A-Fa-f]+\.o", repl=r"\1")
+        elif self.compiler.name == "nag":
+            for tag in ["fc", "f77"]:
+                marker = markers[tag]
+                start_at = "# ### BEGIN {0}".format(marker)
+                stop_at = "# ### END {0}".format(marker)
+                # Libtool 2.4.2 does not know the shared flag:
+                x.filter(
+                    regex=r"\$CC -shared",
+                    repl=r"\$CC -Wl,-shared",
+                    string=True,
+                    start_at=start_at,
+                    stop_at=stop_at,
+                )
+                # Libtool does not know how to inject whole archives
+                # (e.g. https://github.com/pmodels/mpich/issues/4358):
+                x.filter(
+                    regex=r'^whole_archive_flag_spec="\\\$({?wl}?)--whole-archive'
+                    r'\\\$convenience \\\$\1--no-whole-archive"$',
+                    repl=r'whole_archive_flag_spec="\$\1--whole-archive'
+                    r"\`for conv in \$convenience\\\\\"\\\\\"; do test -n \\\\\"\$conv\\\\\" && "
+                    r"new_convenience=\\\\\"\$new_convenience,\$conv\\\\\"; done; "
+                    r'func_echo_all \\\\\"\$new_convenience\\\\\"\` \$\1--no-whole-archive"',
+                    start_at=start_at,
+                    stop_at=stop_at,
+                )
+                # The compiler requires special treatment in certain cases:
+                x.filter(
+                    regex=r"^(with_gcc=.*)$",
+                    repl="\\1\n\n# Is the compiler the NAG compiler?\nwith_nag=yes",
+                    start_at=start_at,
+                    stop_at=stop_at,
+                )
+
+            # Disable the special treatment for gcc and g++:
+            for tag in ["cc", "cxx"]:
+                marker = markers[tag]
+                x.filter(
+                    regex=r"^(with_gcc=.*)$",
+                    repl="\\1\n\n# Is the compiler the NAG compiler?\nwith_nag=no",
+                    start_at="# ### BEGIN {0}".format(marker),
+                    stop_at="# ### END {0}".format(marker),
+                )
+
+            # The compiler does not support -pthread flag, which might come
+            # from the inherited linker flags. We prepend the flag with -Wl,
+            # before using it:
+            x.filter(
+                regex=r"^(\s*)(for tmp_inherited_linker_flag in \$tmp_inherited_linker_flags; "
+                r"do\s*)$",
+                repl='\\1if test "x$with_nag" = xyes; then\n'
+                "\\1  revert_nag_pthread=$tmp_inherited_linker_flags\n"
+                "\\1  tmp_inherited_linker_flags="
+                "`$ECHO \"$tmp_inherited_linker_flags\" | $SED 's% -pthread% -Wl,-pthread%g'`\n"
+                '\\1  test x"$revert_nag_pthread" = x"$tmp_inherited_linker_flags" && '
+                "revert_nag_pthread=no || revert_nag_pthread=yes\n"
+                "\\1fi\n\\1\\2",
+                start_at='if test -n "$inherited_linker_flags"; then',
+                stop_at='case " $new_inherited_linker_flags " in',
+            )
+            # And revert the modification to produce '*.la' files that can be
+            # used with gcc (normally, we do not install the files but they can
+            # still be used during the building):
+            start_at = '# Time to change all our "foo.ltframework" stuff back to "-framework foo"'
+            stop_at = "# installed libraries to the beginning of the library search list"
+            x.filter(
+                regex=r"(\s*)(# move library search paths that coincide with paths to not "
+                r"yet\s*)$",
+                repl='\\1test x"$with_nag$revert_nag_pthread" = xyesyes &&\n'
+                '\\1  new_inherited_linker_flags=`$ECHO " $new_inherited_linker_flags" | '
+                "$SED 's% -Wl,-pthread% -pthread%g'`\n\\1\\2",
+                start_at=start_at,
+                stop_at=stop_at,
             )
-            fs.filter_file(r"^(predep_objects=.*)/tmp/a-[0-9A-Fa-f]+\.o", r"\1", libtool_path)
 
     @property
     def configure_directory(self):
diff --git a/lib/spack/spack/test/data/filter_file/start_stop.txt b/lib/spack/spack/test/data/filter_file/start_stop.txt
new file mode 100644
index 0000000000..1f30c63d74
--- /dev/null
+++ b/lib/spack/spack/test/data/filter_file/start_stop.txt
@@ -0,0 +1,4 @@
+A
+B
+C
+D
\ No newline at end of file
diff --git a/lib/spack/spack/test/llnl/util/filesystem.py b/lib/spack/spack/test/llnl/util/filesystem.py
index 595142f878..4950558db6 100644
--- a/lib/spack/spack/test/llnl/util/filesystem.py
+++ b/lib/spack/spack/test/llnl/util/filesystem.py
@@ -4,6 +4,7 @@
 # SPDX-License-Identifier: (Apache-2.0 OR MIT)
 
 """Tests for ``llnl/util/filesystem.py``"""
+import filecmp
 import os
 import shutil
 import stat
@@ -527,6 +528,29 @@ def test_filter_files_multiple(tmpdir):
         assert "<stdio.h>" not in f.read()
 
 
+def test_filter_files_start_stop(tmpdir):
+    original_file = os.path.join(spack.paths.test_path, "data", "filter_file", "start_stop.txt")
+    target_file = os.path.join(str(tmpdir), "start_stop.txt")
+    shutil.copy(original_file, target_file)
+    # None of the following should happen:
+    #   - filtering starts after A is found in the file:
+    fs.filter_file("A", "X", target_file, string=True, start_at="B")
+    #   - filtering starts exactly when B is found:
+    fs.filter_file("B", "X", target_file, string=True, start_at="B")
+    #   - filtering stops before D is found:
+    fs.filter_file("D", "X", target_file, string=True, stop_at="C")
+
+    assert filecmp.cmp(original_file, target_file)
+
+    # All of the following should happen:
+    fs.filter_file("A", "X", target_file, string=True)
+    fs.filter_file("B", "X", target_file, string=True, start_at="X", stop_at="C")
+    fs.filter_file(r"C|D", "X", target_file, start_at="X", stop_at="E")
+
+    with open(target_file, mode="r") as f:
+        assert all("X" == line.strip() for line in f.readlines())
+
+
 # Each test input is a tuple of entries which prescribe
 # - the 'subdirs' to be created from tmpdir
 # - the 'files' in that directory