Support ldc2.conf being a directory

If ldc2.conf is a directory go through each file (directly) inside it,
ordered by name and apply the settings in it. It would be similar to
appending all the files and treating it like a normal config file.
The specific ordering is the one from
llvm::StringRef::compare_numeric.

The previous behavior was to always error out if ldc2.conf was a
directory, being it passed through `-conf` or being automatically
picked up.

To account for this change the following restrictions that affect
config files have been lifted:
- at least one section must match the current triple
  This is undesirable for cross-compile setups in which one config
  file is dedicated to one triple
- both switches and post-switches must be defined
  This is undesirable for config files that simply append a CLI switch

The above have been replaced by a check that all settings represent
valid keys (so switches, post-switches etc.) and that they are of the
correct type.

Signed-off-by: Andrei Horodniceanu <a.horodniceanu@proton.me>

Author: the-horo

driver/config.d

@@ -26,7 +26,7 @@ class Setting
         group,
     }
 
-    this(string name, Type type)
+    this(string name, Type type) pure
     {
         _name = name;
         _type = type;

driver/configfile.cpp

@@ -16,9 +16,11 @@
 #include "llvm/Support/Path.h"
 #include "llvm/Support/Regex.h"
 #include <cassert>
+#include <cstdio>
 #include <cstring>
 #include <iostream>
 #include <string>
+#include <system_error>
 
 #if _WIN32
 #define WIN32_LEAN_AND_MEAN
@@ -193,9 +195,42 @@ bool ConfigFile::read(const char *explicitConfFile, const char *triple) {
   }
 
   pathcstr = strdup(pathstr.c_str());
-  auto binpath = exe_path::getBinDir();
+  return iterateConfigFiles(pathstr, triple);
+}
+
+bool ConfigFile::iterateConfigFiles(const std::string &path, const char *triple) {
+  auto bindirString = exe_path::getBinDir();
+  auto bindir = bindirString.c_str();
+
+  if (!sys::fs::is_directory(path))
+    return readConfig(pathcstr, triple, bindir);
+
+  llvm::SmallVector<std::string, 8> configFiles;
+
+  std::error_code ec;
+  auto it = sys::fs::directory_iterator(path, ec);
+  do {
+    if (ec) {
+      fprintf(stderr, "Could not iterate config directory at %s: %s\n",
+              path.c_str(), ec.message().c_str());
+      return false;
+    }
+    if (it->type() == sys::fs::file_type::directory_file)
+      continue;
+
+    configFiles.emplace_back(it->path());
+  } while (it.increment(ec) != sys::fs::directory_iterator{});
+
+  llvm::sort(configFiles, [](const std::string &a, const std::string &b) {
+    auto sa = llvm::StringRef(a), sb = llvm::StringRef(b);
+    return sa.compare_numeric(sb) < 0;
+  });
+
+  for (const auto &configPath: configFiles)
+    if (!readConfig(configPath.c_str(), triple, bindir))
+      return false;
 
-  return readConfig(pathcstr, triple, binpath.c_str());
+  return true;
 }
 
 void ConfigFile::extendCommandLine(llvm::SmallVectorImpl<const char *> &args) {

driver/configfile.d

@@ -29,24 +29,19 @@ string normalizeSlashes(const(char)* binDir)
     return cast(string)res; // assumeUnique
 }
 
-const(string)[] findArraySetting(GroupSetting[] sections, string name)
+void findArraySetting(GroupSetting[] sections, string name, scope void delegate(const ArraySetting as) callback)
 {
-    const(string)[] result = null;
     foreach (section; sections)
     {
         foreach (c; section.children)
         {
             if (c.type == Setting.Type.array && c.name == name)
             {
                 auto as = cast(ArraySetting) c;
-                if (as.isAppending)
-                    result ~= as.vals;
-                else
-                    result = as.vals;
+                callback(as);
             }
         }
     }
-    return result;
 }
 
 string findScalarSetting(GroupSetting[] sections, string name)
@@ -128,6 +123,49 @@ string replacePlaceholders(string str, CfgPaths cfgPaths)
         .replace("%%ldcversion%%", cast(string) global.ldc_version);
 }
 
+/++ Check that a section only contains known config keys
+
+ ldc recognizes:
+ - switches
+ - post-switches
+ - lib-dirs
+ - rpath
++/
+void validateSettingNames(const GroupSetting group, const char* filePath) {
+    static void fail(const Setting setting, const char* filePath) {
+        string fmt(Setting.Type type) {
+            final switch(type) {
+                static foreach (mem; __traits(allMembers, Setting.Type))
+                case __traits(getMember, Setting.Type, mem):
+                    return mem;
+            }
+        }
+
+        import dmd.root.string : toDString;
+        string msg;
+        if (setting.type == Setting.Type.group)
+            msg = "Nested group " ~ setting.name ~ " is unsupported";
+        else
+            msg = "Unknown " ~ fmt(setting.type) ~ " setting named " ~ setting.name;
+
+        throw new Exception(msg);
+    }
+
+    alias ST = Setting.Type;
+    static immutable knownSettings = [
+        new Setting("switches", ST.array),
+        new Setting("post-switches", ST.array),
+        new Setting("lib-dirs", ST.array),
+        new Setting("rpath", ST.scalar),
+    ];
+    outer: foreach (setting; group.children) {
+        foreach (known; knownSettings)
+            if (setting.name == known.name && setting.type == known.type)
+                continue outer;
+        fail(setting, filePath);
+    }
+}
+
 extern(C++) struct ConfigFile
 {
     __gshared ConfigFile instance;
@@ -160,37 +198,34 @@ private:
                 }
             }
 
-            if (sections.length == 0)
-            {
-                throw new Exception("No matching section for triple '" ~ cast(string) triple.toDString
-                                    ~ "'");
-            }
-
-            const switches = findArraySetting(sections, "switches");
-            const postSwitches = findArraySetting(sections, "post-switches");
-            if (switches.length + postSwitches.length == 0)
-                throw new Exception("Could not look up switches");
+            foreach (group; sections)
+                validateSettingNames(group, cfPath);
 
-            void applyArray(ref Array!(const(char)*) output, const(string)[] input)
+            void readArraySetting(GroupSetting[] sections, string name, ref Array!(const(char)*) output)
             {
-                output.setDim(0);
-
-                output.reserve(input.length);
-                foreach (sw; input)
+                void applyArray(const ArraySetting input)
                 {
-                    const finalSwitch = sw.replacePlaceholders(cfgPaths).toCString;
-                    output.push(finalSwitch.ptr);
+                    if (!input.isAppending)
+                        output.setDim(0);
+
+                    output.reserve(input.vals.length);
+                    foreach (sw; input.vals)
+                    {
+                        const finalSwitch = sw.replacePlaceholders(cfgPaths).toCString;
+                        output.push(finalSwitch.ptr);
+                    }
                 }
+                findArraySetting(sections, name, &applyArray);
             }
 
-            applyArray(this.switches, switches);
-            applyArray(this.postSwitches, postSwitches);
-
-            const libDirs = findArraySetting(sections, "lib-dirs");
-            applyArray(_libDirs, libDirs);
-
+            readArraySetting(sections, "switches", switches);
+            readArraySetting(sections, "post-switches", postSwitches);
+            readArraySetting(sections, "lib-dirs", _libDirs);
             const rpath = findScalarSetting(sections, "rpath");
-            this.rpathcstr = rpath.length == 0 ? null : rpath.replacePlaceholders(cfgPaths).toCString.ptr;
+            // A missing rpath => do nothing
+            // An empty rpath => clear the setting
+            if (rpath.ptr !is null)
+                this.rpathcstr = rpath.length == 0 ? null : rpath.replacePlaceholders(cfgPaths).toCString.ptr;
 
             return true;
         }

driver/configfile.h

@@ -43,6 +43,8 @@ struct ConfigFile {
   // implemented in D
   bool readConfig(const char *cfPath, const char *triple, const char *binDir);
 
+  bool iterateConfigFiles(const std::string &path, const char *triple);
+
   const char *pathcstr = nullptr;
   Array<const char *> switches;
   Array<const char *> postSwitches;

tests/driver/config_diag.d

@@ -1,13 +1,27 @@
-// RUN: %ldc -o- -conf=%S/inputs/noswitches.conf %s 2>&1 | FileCheck %s --check-prefix=NOSWITCHES
-// NOSWITCHES: Error while reading config file: {{.*}}noswitches.conf
-// NOSWITCHES-NEXT: Could not look up switches
+// RUN: split-file %s %t
 
-// RUN: %ldc -o- -conf=%S/inputs/section_aaa.conf %s 2>&1 | FileCheck %s --check-prefix=NO_SEC
-// NO_SEC: Error while reading config file: {{.*}}section_aaa.conf
-// NO_SEC-NEXT: No matching section for triple '{{.*}}'
+// RUN: %ldc -o- -conf=%t/invalid_setting.conf %s 2>&1 | FileCheck %s --check-prefix=INV_SET
+// INV_SET: Error while reading config file: {{.*}}invalid_setting.conf
+// INV_SET-NEXT: Unknown scalar setting named blah-blah
 
-// RUN: %ldc -o- -conf=%S/inputs/invalid_append.conf %s 2>&1 | FileCheck %s --check-prefix=APP
+// RUN: %ldc -o- -conf=%t/invalid_append.conf %s 2>&1 | FileCheck %s --check-prefix=APP
 // APP: Error while reading config file: {{.*}}invalid_append.conf
 // APP-NEXT: line 3: '~=' is not supported with scalar values
 
 module object;
+
+/+ config dirs used by the tests
+
+//--- invalid_setting.conf
+default:
+{
+	blah-blah = "12";
+};
+
+//--- invalid_append.conf
+default:
+{
+	rpath ~= "/path";
+}
+//--- end_of_files
++/

tests/driver/config_dir.d

@@ -0,0 +1,101 @@
+// RUN: split-file %s %t
+module object;
+
+alias string = immutable(char)[];
+
+template Version(string ident) {
+    mixin(`version(` ~ ident ~ `)
+        enum Version = true;
+    else
+        enum Version = false;
+`);
+}
+
+// RUN: %ldc -d-version=TEST_ORDER -o- -conf=%t/order.conf %s 2>&1
+version (TEST_ORDER) {
+    static assert(!Version!"File1");
+    static assert(Version!"File2");
+    static assert(!Version!"File3");
+}
+
+// RUN: %ldc -d-version=TEST_APPENDING -o- -conf=%t/appending.conf %s 2>&1
+version (TEST_APPENDING) {
+    static assert(!Version!"File1_Sec1");
+    static assert(!Version!"File1_Sec2");
+
+    static assert(!Version!"File2_Sec1");
+    static assert(Version!"File2_Sec2");
+
+    static assert(Version!"File3_Sec1");
+    static assert(Version!"File3_Sec2");
+}
+
+version (TEST_RPATH) {}
+// RUN: not %ldc -d-version=TEST_RPATH -conf=%t/multiple_rpaths.conf -link-defaultlib-shared=true -v %s 2>&1 | FileCheck %s --check-prefix=RPATHS
+// RPATHS-NOT: /first_rpath
+
+// RUN: not %ldc -d-version=TEST_RPATH -conf=%t/rpath_clear.conf -link-defaultlib-shared=true -v %s 2>&1 | FileCheck %s --check-prefix=RPATHSCLEAR
+// RPATHSCLEAR-NOT: /first_rpath
+
+
+/+ config dirs used by the tests
+
+//--- order.conf/conf-11
+default: {
+	switches = [ "-d-version=File1" ]
+}
+//--- order.conf/conf-102
+default: {
+	switches = [ "-d-version=File2" ]
+}
+//--- order.conf/conf-103/conf
+default: {
+	switches = [ "-d-version=File3" ];
+}
+
+//--- appending.conf/01.conf
+default: {
+	switches ~= [ "-d-version=File1_Sec1" ]
+}
+".?": {
+	switches = [ "-d-version=File1_Sec2" ]
+}
+//--- appending.conf/02.conf
+default: {
+	switches ~= [ "-d-version=File2_Sec1" ]
+}
+".?": {
+	switches = [ "-d-version=File2_Sec2" ]
+}
+//--- appending.conf/03.conf
+default: {
+	switches ~= [ "-d-version=File3_Sec1" ]
+}
+".?": {
+	switches ~= [ "-d-version=File3_Sec2" ]
+}
+
+//--- multiple_rpaths.conf/01.conf
+default: {
+	switches = [ "-link-defaultlib-shared" ]
+	rpath = "/first_rpath";
+}
+//--- multiple_rpaths.conf/02.conf
+default: {
+	rpath = "/second_rpath";
+}
+//--- multiple_rpaths.conf/03.conf
+default: {
+	// no rpath
+}
+
+//--- rpath_clear.conf
+default: {
+     rpath = "/first_rpath"
+}
+default: {
+     rpath = "" // this should clear the previous value
+}
+
+//--- end_of_files
++/