Add analogue to _concat for writable directories

The regular _concat function, when processing a list of strings, adds a
prefix+suffix to each, possibly after expanding the list through a call to
a function. The function is often RDirs, which can add additional paths
(repositories, usually). This is used for things like path-to-include
dir so we get "-Iinclude" etc.

When used for a directory which we tell the compiler to write to, as
opposed to just search, this can give the wrong result. For example,
for $FORTRANMODIR, which describes where to put generated module files,
we only want to write to the target directory (and in fact, for gfortran,
it's an error to specify the module-write-dir more than once), but we
might want to still search the backing directories - any repository,
and in case of non-duplicating variantdir, the directory of the original
source - in case there were already module files there that can be used.

This new function wraps the first string from the expanded list in
the prefix/suffix, and any remaining strings in the extra_prefix and
extra_suffix arguments, for example to produce

"-Jtargetmoddir -Isourcemoddir -Irepository"

instead of

"-Jtargetmoddir -Jsourcemoddir -Jrepository".

The latter would abort the Fortran compiler with an error, and the
equivalent for the D compilers would ignore the first (correct)
directory and select the last one for use. In any case, we're not
allowed to write to the repository, so the previous behavior using
_concat was definitely wrong.

Signed-off-by: Mats Wichmann <[email protected]>

CHANGES.txt

@@ -124,6 +124,18 @@ RELEASE  VERSION/DATE TO BE FILLED IN LATER
     - Cleaned up dblite module (checker warnings, etc.).
     - Some cleanup in the FortranCommon tool.
     - Changed the message about scons -H to clarify it shows built-in options.
+    - Introduce a new _concat_dir function which can be used for
+      building cli strings when you need two sets of prefix/suffix
+      strings. _concat_dir transforms a list so the first item is wrapped
+      with the first pair of prefix/suffix args and the remaining
+      items with the second pair.  The use case is when passing
+      a directory-to-write-to - those typically need to be unique,
+      but RDirs may expand to multiples, and we want to accomodate
+      also searching those directories in case you have repositories
+      setup - those could have already built files in them which could
+      be searched. Example: Fortran module directory, D interface file
+      directory.  Introduces a new construction variable of the same name,
+      just to keep consistent with how existing _concat works.
 
   From Jonathon Reinhart:
     - Fix another instance of `int main()` in CheckLib() causing failures

RELEASE.txt

@@ -23,6 +23,10 @@ NEW FUNCTIONALITY
 - MSVC: If necessary, automatically define VSCMD_SKIP_SENDTELEMETRY for VS2019 and later
   on arm64 hosts when using an arm (32-bit) build of python to prevent a powershell
   error pop-up window (powershell dll not found).
+- Construction variable _concat_dir added which refers to a function to
+  perform translation on a directory to write to that needs to be passed
+  to a compiler. Similar to
+  https://scons.org/doc/production/HTML/scons-man.html#cv-_concat
 
 DEPRECATED FUNCTIONALITY
 ------------------------
@@ -87,6 +91,13 @@ FIXES
   with the python logging module was refactored to prevent test failure.
 - MSVS: Add arm64 to the MSVS supported architectures list for VS2017 and later to be
   consistent with the current documentation of MSVS_ARCH.
+- When expanding $FORTRANMODDIR and $DI_FILE_DIR (the latter new in this
+  release), incorrect results could be obtained when Repository() has been
+  called, and/or when a variant directory is used with duplicate=False.
+  In some cases, this could cause the compiler to abort with an error.
+  The expansion is now handled so the path relative to the build directory
+  is passed to the compiler as the directory to write into, and the
+  additional directories are enabled as search directories.
 
 IMPROVEMENTS
 ------------

SCons/Defaults.py

@@ -36,7 +36,7 @@
 import stat
 import sys
 import time
-from typing import List
+from typing import List, Callable
 
 import SCons.Action
 import SCons.Builder
@@ -420,6 +420,67 @@ def _concat(prefix, items_iter, suffix, env, f=lambda x: x, target=None, source=
 # pylint: enable-msg=too-many-arguments
 
 
+# pylint: disable-msg=too-many-arguments
+def _concat_dir(
+    prefix: str,
+    items,
+    suffix: str,
+    extra_prefix: str,
+    extra_suffix: str,
+    env,
+    f: Callable[[list], list] = lambda x: x,
+    target=None,
+    source=None,
+    affect_signature: bool = True,
+) -> list:
+    """Transform *items* into a list for command-line usage.
+
+    Like :func:`_concat`, but wraps an expanded list differently.
+    If *f* returns multiple items, the first item is wrapped by *prefix*
+    and *suffix*, while the rest are wrapped with *extra_prefix*
+    and *extra_suffix*. Used for special cases like Fortran module
+    directories, where only a single directory can be supplied as the
+    output destination, but repository locations and the source directory
+    in a variantdir setup should be added as search locations - which
+    takes a different syntax from different construction variables.
+
+    Args:
+        prefix: text to prepend to first element
+        items: string or iterable to transform
+        suffix: text to append to first element
+        extra_prefix: text to prepend to additional elements
+        extra_suffix: text to append to additional elements
+        env: construction environment for interpolation
+        f: optional function to perform a transformation on the list.
+          The default is a stub which performs no transformation.
+        target: optional target for interpolation into *items* elements
+        source: optional source for interpolation into *items* elements
+        affect_signature: optional flag: if false, surround contents with
+           markers indicating not to use the contents when calculating the
+           build signature. The default is ``True``.
+    """
+    if not items:
+        return items
+
+    l = f(SCons.PathList.PathList(items).subst_path(env, target, source))
+    if l is not None:
+        items = l
+
+    if not affect_signature:
+        value = ['$(']
+    else:
+        value = []
+    value += _concat_ixes(prefix, items[:1], suffix, env)
+    if len(items) > 1:
+        value += _concat_ixes(extra_prefix, items[1:], extra_suffix, env)
+
+    if not affect_signature:
+        value += ["$)"]
+
+    return value
+# pylint: enable-msg=too-many-arguments
+
+
 def _concat_ixes(prefix, items_iter, suffix, env):
     """
     Creates a new list from 'items_iter' by concatenating the 'prefix' and
@@ -690,6 +751,7 @@ def __lib_either_version_flag(env, version_var1, version_var2, flags_var):
     'ENV': {},
     'IDLSUFFIXES': SCons.Tool.IDLSuffixes,
     '_concat': _concat,
+    '_concat_dir': _concat_dir,
     '_defines': _defines,
     '_stripixes': _stripixes,
     '_LIBFLAGS': '${_concat(LIBLINKPREFIX, LIBS, LIBLINKSUFFIX, __env__)}',

SCons/Defaults.xml

@@ -45,6 +45,116 @@ See its __doc__ string for a discussion of the format.
 </summary>
 </cvar>
 
+<cvar name ="_concat_dir">
+<summary>
+<para>
+A reference to a function which transforms
+a value by concatenating passed prefix and suffix strings
+(after variable substitution and optional additional processing),
+to prepare option strings either to be passed to the external build program
+or to help calculate the &buildsig; - the function will be called
+for both purposes in the course of a build.
+This allows values such as a module directory to be
+to be stored in a portable manner,
+rather than including non-portable compiler syntax.
+Similar to &cv-link-_concat;, but for the special case where the
+build program is being instructed to write to a directory.
+For example, &cv-link-FORTRANMODDIR;
+holds the name of the directory where the compiler will write
+generated Fortran module files. Under some circumstances,
+the processing by the specified function
+can produce several names from the original variable
+(if &f-link-Repository; and/or &f-link-VariantDir; are in use),
+but the directory to write to must be unambiguous.
+The &cv-_concat_dir; function wraps any additional directory
+name strings in a different prefix/suffix pair,
+which are passed in additional arguments to the function.
+</para>
+
+<para>
+The default value for &cv-_concat_dir; is an internal &SCons; function.
+It is possible to supply a custom &cv-_concat_dir; function,
+but care must be taken not not disrupt the behavior
+of existing &consvars; written to use the default function,
+so replacing the default is discouraged.
+</para>
+
+<para>
+A function used for &cv-_concat_dir; must accept six required arguments:
+<parameter>prefix</parameter>,
+<parameter>items</parameter>,
+<parameter>suffix</parameter>,
+<parameter>extra_prefix</parameter>,
+<parameter>extra_suffix</parameter> and
+<parameter>env</parameter>,
+as well as four optional arguments:
+<parameter>f</parameter>,
+<parameter>target</parameter>,
+<parameter>source</parameter> and
+<parameter>affect_signature</parameter>.
+<parameter>items</parameter>
+is the element to proces -
+often it will be the name of a &consvar;.
+<parameter>prefix</parameter> and
+<parameter>suffix</parameter>
+are the strings to add to the front and back of the
+first item of the list generated by calling the function
+<parameter>f</parameter>.
+<parameter>extra_prefix</parameter> and
+<parameter>extra_suffix</parameter>
+are the strings to add to the front and back of the remaining items.
+<parameter>env</parameter>
+is the &consenv; to interpolate &consvars; from;
+the &consvars; will appear as local variables at evaluation time
+and so are referenced without prefixing with a <literal>$</literal>.
+The &cv-_concat_dir; function must return the substituted elements
+as a string or list, either of which can be empty - it must not fail if
+<parameter>items</parameter> has no value or has an empty value.
+The optional arguments
+<parameter>target</parameter> and
+<parameter>source</parameter>
+can be used if the target and/or source files
+need to be interpolated into
+<parameter>items</parameter>;
+the special variables <literal>TARGET</literal>
+and <literal>SOURCE</literal> are often used.
+After source/target interpolation,
+the function <parameter>f</parameter> is called.
+It accepts a single list argument, and returns
+a list with the necessary modifications applied.
+<parameter>f</parameter> must be called before
+the prefix/suffix additions, as it may change the
+contents of the list.
+&cv-link-RDirs; is often used as the function -
+it may add additional directory paths if a repository
+and/or variant directory is in use.
+If the default internal implementation is used for &cv-_concat_dir;,
+the default for <parameter>f</parameter> is a
+pass-through which does not modify the item.
+The optional <parameter>affect_signature</parameter>
+indicates whether the contents should be used for
+&buildsig; calculation. If it evaluates false,
+any non-empty return should be wrapped with
+<literal>$(</literal> and <literal>$)</literal>
+to tell &SCons; to exclude it from the calculation.
+If the default internal implementation is used for &cv-_concat;,
+<parameter>affect_signature</parameter> defaults to
+<constant>True</constant>.
+</para>
+
+<para>
+Example use:
+</para>
+
+<example_commands>
+env['_FORTRANMODFLAG'] = (
+    "$( ${_concat_dir(FORTRANMODDIRPREFIX, FORTRANMODDIR, FORTRANMODDIRSUFFIX, "
+    "INCFORTRANPREFIX, INCFORTRANSUFFIX, __env__, RDirs, TARGET, SOURCE)} $)"
+)
+</example_commands>
+</summary>
+</cvar>
+
 <cvar name="CONFIGUREDIR">
 <summary>
 <para>
@@ -243,7 +353,7 @@ to each directory in &cv-link-CPPPATH;.
 <para>
 The list of directories that the C preprocessor will search for include
 directories. The C/C++ implicit dependency scanner will search these
-directories for include files. 
+directories for include files.
 In general it's not advised to put include directory directives
 directly into &cv-link-CCFLAGS; or &cv-link-CXXFLAGS;
 as the result will be non-portable
@@ -257,9 +367,9 @@ Python's <systemitem>os.sep</systemitem>.
 Note:
 directory names in &cv-CPPPATH;
 will be looked-up relative to the directory of the SConscript file
-when they are used in a command. 
+when they are used in a command.
 To force &scons;
-to look-up a directory relative to the root of the source tree use 
+to look-up a directory relative to the root of the source tree use
 the <literal>#</literal> prefix:
 </para>
 
@@ -529,9 +639,9 @@ as the result will be non-portable.
 Note:
 directory names in &cv-LIBPATH; will be looked-up relative to the
 directory of the SConscript file
-when they are used in a command. 
+when they are used in a command.
 To force &scons;
-to look-up a directory relative to the root of the source tree use 
+to look-up a directory relative to the root of the source tree use
 the <literal>#</literal> prefix:
 </para>
 
@@ -575,7 +685,7 @@ env = Environment(LINKCOM="my_linker $_LIBDIRFLAGS $_LIBFLAGS -o $TARGET $SOURCE
 <para>
 A list of one or more libraries
 that will be added to the link line
-for linking with any executable program, shared library, or loadable module 
+for linking with any executable program, shared library, or loadable module
 created by the &consenv; or override.
 </para>
 

SCons/EnvironmentTests.py

@@ -1522,6 +1522,45 @@ def test_concat_nested(self) -> None:
         x = e.subst('$( ${_concat(PRE, L1, SUF, __env__)} $)')
         assert x == 'preasuf prebsuf precsuf predsuf precsuf predsuf', x
 
+
+    def test_concat_dir(self) -> None:
+        """Test _concat_dir()"""
+        e1 = self.TestEnvironment(
+            PRE='PP',
+            SUF='SS',
+            XPRE='XP',
+            XSUF='XS',
+            STR='a b',
+            LIST=['a', 'b'],
+            LONGLIST=['a', 'b', 'c', 'd'],
+        )
+        s = e1.subst
+        with self.subTest():
+            x = s("${_concat_dir('', '', '', '', '', __env__)}")
+            self.assertEqual(x, '')
+        with self.subTest():
+            x = s("${_concat_dir('', [], '', '', '', __env__)}")
+            self.assertEqual(x, '')
+        with self.subTest():
+            x = s("${_concat_dir(PRE, '', SUF, XPRE, XSUF, __env__)}")
+            self.assertEqual(x, '')
+        with self.subTest():
+            x = s("${_concat_dir(PRE, STR, SUF, XPRE, XSUF, __env__)}")
+            self.assertEqual(x, 'PPa bSS')
+        with self.subTest():
+            x = s("${_concat_dir(PRE, LIST, SUF, XPRE, XSUF, __env__)}")
+            self.assertEqual(x, 'PPaSS XPbXS')
+        with self.subTest():
+            x = s(
+                "${_concat_dir(PRE, LIST, SUF, XPRE, XSUF, __env__, affect_signature=False)}",
+                raw=True,
+            )
+            self.assertEqual(x, '$( PPaSS XPbXS $)')
+        with self.subTest():
+            x = s("${_concat_dir(PRE, LONGLIST, SUF, XPRE, XSUF, __env__)}")
+            self.assertEqual(x, 'PPaSS XPbXS XPcXS XPdXS')
+
+
     def test_gvars(self) -> None:
         """Test the Environment gvars() method"""
         env = self.TestEnvironment(XXX = 'x', YYY = 'y', ZZZ = 'z')

SCons/Tool/FortranCommon.py

@@ -204,7 +204,7 @@ def add_fortran_to_env(env) -> None:
     env['FORTRANMODDIR'] = ''          # where the compiler should place .mod files
     env['FORTRANMODDIRPREFIX'] = ''    # some prefix to $FORTRANMODDIR - similar to $INCPREFIX
     env['FORTRANMODDIRSUFFIX'] = ''    # some suffix to $FORTRANMODDIR - similar to $INCSUFFIX
-    env['_FORTRANMODFLAG'] = '$( ${_concat(FORTRANMODDIRPREFIX, FORTRANMODDIR, FORTRANMODDIRSUFFIX, __env__, RDirs, TARGET, SOURCE)} $)'
+    env['_FORTRANMODFLAG'] = '$( ${_concat_dir(FORTRANMODDIRPREFIX, FORTRANMODDIR, FORTRANMODDIRSUFFIX, INCFORTRANPREFIX, INCFORTRANSUFFIX, __env__, RDirs, TARGET, SOURCE)} $)'
 
 def add_f77_to_env(env) -> None:
     """Add Builders and construction variables for f77 dialect."""

SCons/Tool/dmd.py

@@ -95,9 +95,7 @@ def generate(env) -> None:
     env['_DINCFLAGS'] = '${_concat(DINCPREFIX, DPATH, DINCSUFFIX, __env__, RDirs, TARGET, SOURCE)}'
     env['_DVERFLAGS'] = '${_concat(DVERPREFIX, DVERSIONS, DVERSUFFIX, __env__)}'
     env['_DDEBUGFLAGS'] = '${_concat(DDEBUGPREFIX, DDEBUG, DDEBUGSUFFIX, __env__)}'
-
-    env['_DI_FLAGS'] = "${DI_FILE_DIR and DI_FILE_DIR_PREFIX+DI_FILE_DIR+DI_FILE_DIR_SUFFFIX}"
-
+    env['_DI_FLAGS'] = '$( ${_concat_dir(DI_FILE_DIR_PREFIX, DI_FILE_DIR, DI_FILE_DIR_SUFFFIX, DINCPREFIX, DINCSUFFIX, __env__, RDirs, TARGET, SOURCE)} $)'
     env['_DFLAGS'] = '${_concat(DFLAGPREFIX, DFLAGS, DFLAGSUFFIX, __env__)}'
 
     env['SHDC'] = '$DC'

SCons/Tool/gdc.py

@@ -67,7 +67,7 @@ def generate(env) -> None:
     env['_DINCFLAGS'] = '${_concat(DINCPREFIX, DPATH, DINCSUFFIX, __env__, RDirs, TARGET, SOURCE)}'
     env['_DVERFLAGS'] = '${_concat(DVERPREFIX, DVERSIONS, DVERSUFFIX, __env__)}'
     env['_DDEBUGFLAGS'] = '${_concat(DDEBUGPREFIX, DDEBUG, DDEBUGSUFFIX, __env__)}'
-    env['_DI_FLAGS'] = "${DI_FILE_DIR and DI_FILE_DIR_PREFIX+DI_FILE_DIR+DI_FILE_DIR_SUFFFIX}"
+    env['_DI_FLAGS'] = '$( ${_concat_dir(DI_FILE_DIR_PREFIX, DI_FILE_DIR, DI_FILE_DIR_SUFFFIX, DINCPREFIX, DINCSUFFIX, __env__, RDirs, TARGET, SOURCE)} $)'
     env['_DFLAGS'] = '${_concat(DFLAGPREFIX, DFLAGS, DFLAGSUFFIX, __env__)}'
 
     env['SHDC'] = '$DC'

SCons/Tool/ldc.py

@@ -71,9 +71,7 @@ def generate(env) -> None:
     env['_DINCFLAGS'] = '${_concat(DINCPREFIX, DPATH, DINCSUFFIX, __env__, RDirs, TARGET, SOURCE)}'
     env['_DVERFLAGS'] = '${_concat(DVERPREFIX, DVERSIONS, DVERSUFFIX, __env__)}'
     env['_DDEBUGFLAGS'] = '${_concat(DDEBUGPREFIX, DDEBUG, DDEBUGSUFFIX, __env__)}'
-
-    env['_DI_FLAGS'] = "${DI_FILE_DIR and DI_FILE_DIR_PREFIX+DI_FILE_DIR+DI_FILE_DIR_SUFFFIX}"
-
+    env['_DI_FLAGS'] = '$( ${_concat_dir(DI_FILE_DIR_PREFIX, DI_FILE_DIR, DI_FILE_DIR_SUFFFIX, DINCPREFIX, DINCSUFFIX, __env__, RDirs, TARGET, SOURCE)} $)'
     env['_DFLAGS'] = '${_concat(DFLAGPREFIX, DFLAGS, DFLAGSUFFIX, __env__)}'
 
     env['SHDC'] = '$DC'
@@ -96,10 +94,10 @@ def generate(env) -> None:
     env['DFLAGPREFIX'] = '-'
     env['DFLAGSUFFIX'] = ''
     env['DFILESUFFIX'] = '.d'
-    
+
     env['DI_FILE_DIR'] = ''
     env['DI_FILE_SUFFIX'] = '.di'
-    
+
     env['DI_FILE_DIR_PREFIX'] = '-Hd='
     env['DI_FILE_DIR_SUFFFIX'] = ''