support 0 to many config files (#371)

Summary:
Previously OID/OIL required exactly one configuration file. This change makes it so you can supply 0 or more configuration files. 0 is useful if you have pre-generated the cache or use some sort of remote generation system. 1 is useful for the common case, where you have a configuration file that describes your entire source and use just that. More are useful if you have supplemental bits of config you wish to apply/override - see the changes to the integration test framework where we do exactly this.


Test Plan:
This isn't super well tested. It works for the test cases which add features via the config or enable `codegen.ignore`.

- CI

Reviewed By: ajor

Differential Revision: D49758032

Pulled By: JakeHillion

Author: JakeHillion

CMakeLists.txt

@@ -278,11 +278,11 @@ include_directories(${CMAKE_CURRENT_BINARY_DIR} ${CMAKE_CURRENT_SOURCE_DIR})
 add_subdirectory(oi)
 add_subdirectory(resources)
 add_library(oicore
+  oi/Config.cpp
   oi/Descs.cpp
   oi/Metrics.cpp
   oi/OICache.cpp
   oi/OICompiler.cpp
-  oi/OIUtils.cpp
   oi/PaddingHunter.cpp
   oi/Serialize.cpp
 )

include/oi/oi-jit.h

@@ -34,7 +34,7 @@ class OILibraryImpl;
 namespace oi {
 
 struct GeneratorOptions {
-  std::filesystem::path configFilePath;
+  std::vector<std::filesystem::path> configFilePaths;
   std::filesystem::path sourceFileDumpPath;
   int debugLevel = 0;
 };

oi/OIUtils.cpp renamed to oi/Config.cpp

@@ -13,7 +13,7 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
-#include "oi/OIUtils.h"
+#include "oi/Config.h"
 
 #include <glog/logging.h>
 
@@ -27,16 +27,48 @@
 
 namespace fs = std::filesystem;
 
-namespace oi::detail::utils {
+namespace oi::detail::config {
 
 using namespace std::literals;
 
+namespace {
+
+std::optional<FeatureSet> processConfigFile(const std::string& configFilePath,
+                                            OICompiler::Config& compilerConfig,
+                                            OICodeGen::Config& generatorConfig);
+
+}
+
+std::optional<FeatureSet> processConfigFiles(
+    std::span<const fs::path> configFilePaths,
+    std::map<Feature, bool> featureMap,
+    OICompiler::Config& compilerConfig,
+    OICodeGen::Config& generatorConfig) {
+  FeatureSet enables;
+  FeatureSet disables;
+
+  for (fs::path p : configFilePaths) {
+    auto fs = processConfigFile(p, compilerConfig, generatorConfig);
+    if (!fs.has_value())
+      return std::nullopt;
+    enables |= *fs;
+  }
+
+  // Override anything from the config files with command line options
+  for (auto [k, v] : featureMap) {
+    enables[k] = v;
+    disables[k] = !v;
+  }
+  return handleFeatureConflicts(enables, disables);
+}
+
+namespace {
+
 std::optional<FeatureSet> processConfigFile(
     const std::string& configFilePath,
-    std::map<Feature, bool> featureMap,
     OICompiler::Config& compilerConfig,
     OICodeGen::Config& generatorConfig) {
-  fs::path configDirectory = fs::path(configFilePath).remove_filename();
+  fs::path configDirectory = fs::path{configFilePath}.remove_filename();
 
   toml::table config;
   try {
@@ -47,6 +79,7 @@ std::optional<FeatureSet> processConfigFile(
     return {};
   }
 
+  FeatureSet enabledFeatures;
   if (toml::array* features = config["features"].as_array()) {
     for (auto&& el : *features) {
       auto* featureStr = el.as_string();
@@ -57,10 +90,7 @@ std::optional<FeatureSet> processConfigFile(
 
       if (auto f = featureFromStr(featureStr->get());
           f != Feature::UnknownFeature) {
-        // Inserts element(s) into the container, if the container doesn't
-        // already contain an element with an equivalent key. Hence prefer
-        // command line enabling/disabling.
-        featureMap.insert({f, true});
+        enabledFeatures[f] = true;
       } else {
         LOG(ERROR) << "unrecognised feature: " << featureStr->get()
                    << " specified in config";
@@ -204,16 +234,8 @@ std::optional<FeatureSet> processConfigFile(
     }
   }
 
-  FeatureSet enabledFeatures;
-  FeatureSet disabledFeatures;
-  for (auto [k, v] : featureMap) {
-    if (v) {
-      enabledFeatures[k] = true;
-    } else {
-      disabledFeatures[k] = true;
-    }
-  }
-  return handleFeatureConflicts(enabledFeatures, disabledFeatures);
+  return enabledFeatures;
 }
 
-}  // namespace oi::detail::utils
+}  // namespace
+}  // namespace oi::detail::config

oi/OIUtils.h renamed to oi/Config.h

@@ -15,18 +15,21 @@
  */
 #pragma once
 
+#include <filesystem>
 #include <optional>
 #include <set>
+#include <span>
 
 #include "oi/Features.h"
 #include "oi/OICodeGen.h"
 #include "oi/OICompiler.h"
 
-namespace oi::detail::utils {
+namespace oi::detail::config {
 
-std::optional<FeatureSet> processConfigFile(const std::string& configFilePath,
-                                            std::map<Feature, bool> featureMap,
-                                            OICompiler::Config& compilerConfig,
-                                            OICodeGen::Config& generatorConfig);
+std::optional<FeatureSet> processConfigFiles(
+    std::span<const std::filesystem::path> configFilePaths,
+    std::map<Feature, bool> featureMap,
+    OICompiler::Config& compilerConfig,
+    OICodeGen::Config& generatorConfig);
 
-}  // namespace oi::detail::utils
+}  // namespace oi::detail::config

oi/EnumBitset.h

@@ -47,6 +47,11 @@ class EnumBitset {
     return bitset.none();
   }
 
+  EnumBitset<T, N>& operator|=(const EnumBitset<T, N>& that) {
+    bitset |= that.bitset;
+    return *this;
+  }
+
  private:
   BitsetType bitset;
 };

oi/OID.cpp

@@ -23,17 +23,19 @@
 #include <filesystem>
 #include <iostream>
 #include <map>
+#include <string>
+#include <vector>
 
 extern "C" {
 #include <getopt.h>
 #include <libgen.h>
 }
 
+#include "oi/Config.h"
 #include "oi/Features.h"
 #include "oi/Metrics.h"
 #include "oi/OIDebugger.h"
 #include "oi/OIOpts.h"
-#include "oi/OIUtils.h"
 #include "oi/PaddingHunter.h"
 #include "oi/TimeUtils.h"
 #include "oi/TreeBuilder.h"
@@ -249,7 +251,7 @@ namespace Oid {
 struct Config {
   pid_t pid;
   std::string debugInfoFile;
-  std::string configFile;
+  std::vector<fs::path> configFiles;
   fs::path cacheBasePath;
   fs::path customCodeFile;
   size_t dataSegSize;
@@ -563,10 +565,11 @@ int main(int argc, char* argv[]) {
         oidConfig.compAndExit = true;
         break;
       case 'c':
-        oidConfig.configFile = std::string(optarg);
+        oidConfig.configFiles.emplace_back(optarg);
 
-        if (!fs::exists(oidConfig.configFile)) {
-          LOG(ERROR) << "Non existent config file: " << oidConfig.configFile;
+        if (!fs::exists(oidConfig.configFiles.back())) {
+          LOG(ERROR) << "Non existent config file: "
+                     << oidConfig.configFiles.back();
           usage();
           return ExitStatus::FileNotFoundError;
         }
@@ -630,27 +633,13 @@ int main(int argc, char* argv[]) {
     }
   }
 
-  if (oidConfig.configFile.empty()) {
-    oidConfig.configFile = "/usr/local/share/oi/base.oid.toml";
-
-    if (!fs::exists(oidConfig.configFile)) {
-      LOG(ERROR) << "Non existent default config file: "
-                 << oidConfig.configFile;
-      usage();
-      return ExitStatus::FileNotFoundError;
-    }
-
-    LOG(INFO) << "Using default config file " << oidConfig.configFile;
-  }
-
   if (oidConfig.pid != 0 && !oidConfig.debugInfoFile.empty()) {
     LOG(INFO) << "'-p' and '-b' are mutually exclusive";
     usage();
     return ExitStatus::UsageError;
   }
 
-  if ((oidConfig.pid == 0 && oidConfig.debugInfoFile.empty()) ||
-      oidConfig.configFile.empty()) {
+  if ((oidConfig.pid == 0 && oidConfig.debugInfoFile.empty())) {
     usage();
     return ExitStatus::UsageError;
   }
@@ -682,8 +671,8 @@ int main(int argc, char* argv[]) {
       .jsonPath = jsonPath,
   };
 
-  auto featureSet = utils::processConfigFile(oidConfig.configFile, features,
-                                             compilerConfig, codeGenConfig);
+  auto featureSet = config::processConfigFiles(oidConfig.configFiles, features,
+                                               compilerConfig, codeGenConfig);
   if (!featureSet) {
     return ExitStatus::UsageError;
   }

oi/OIDebugger.cpp

@@ -45,11 +45,11 @@ extern "C" {
 #include <glog/logging.h>
 
 #include "oi/CodeGen.h"
+#include "oi/Config.h"
 #include "oi/ContainerInfo.h"
 #include "oi/Headers.h"
 #include "oi/Metrics.h"
 #include "oi/OILexer.h"
-#include "oi/OIUtils.h"
 #include "oi/PaddingHunter.h"
 #include "oi/Syscall.h"
 #include "oi/type_graph/DrgnParser.h"

oi/OIGenerator.cpp

@@ -26,9 +26,9 @@
 #include <variant>
 
 #include "oi/CodeGen.h"
+#include "oi/Config.h"
 #include "oi/DrgnUtils.h"
 #include "oi/Headers.h"
-#include "oi/OIUtils.h"
 
 namespace oi::detail {
 
@@ -193,8 +193,9 @@ int OIGenerator::generate(fs::path& primaryObject, SymbolService& symbols) {
   OICompiler::Config compilerConfig{};
   compilerConfig.usePIC = pic;
 
-  auto features = utils::processConfigFile(configFilePath, featuresMap,
-                                           compilerConfig, generatorConfig);
+  auto features =
+      config::processConfigFiles(std::vector<fs::path>{configFilePath},
+                                 featuresMap, compilerConfig, generatorConfig);
   if (!features) {
     LOG(ERROR) << "failed to process config file";
     return -1;

oi/OILibraryImpl.cpp

@@ -25,9 +25,9 @@
 #include <fstream>
 #include <stdexcept>
 
+#include "oi/Config.h"
 #include "oi/DrgnUtils.h"
 #include "oi/Headers.h"
-#include "oi/OIUtils.h"
 
 namespace oi::detail {
 namespace {
@@ -94,8 +94,8 @@ std::pair<void*, const exporters::inst::Inst&> OILibraryImpl::init() {
 
 void OILibraryImpl::processConfigFile() {
   auto features =
-      utils::processConfigFile(opts_.configFilePath, requestedFeatures_,
-                               compilerConfig_, generatorConfig_);
+      config::processConfigFiles(opts_.configFilePaths, requestedFeatures_,
+                                 compilerConfig_, generatorConfig_);
   if (!features)
     throw std::runtime_error("failed to process configuration");
 

test/integration/gen_tests.py

@@ -136,7 +136,7 @@ def define_traceable_func(name, params, body):
 
         oil_func_body = (
             f"    oi::GeneratorOptions opts{{\n"
-            f"      .configFilePath = configFile,\n"
+            f"      .configFilePaths = configFiles,\n"
             f'      .sourceFileDumpPath = "oil_jit_code.cpp",\n'
             f"      .debugLevel = 3,\n"
             f"    }};\n\n"
@@ -164,11 +164,11 @@ def add_common_code(f):
         """
 void usage(const std::string &argv0) {
   std::cerr << "usage: " << argv0 << " oid CASE ITERATIONS" << std::endl;
-  std::cerr << "       " << argv0 << " oil CASE CONFIG_FILE" << std::endl;
+  std::cerr << "       " << argv0 << " oil CASE CONFIG_FILE..." << std::endl;
 }
 
 int main(int argc, char *argv[]) {
-  if (argc != 4) {
+  if (argc < 4) {
     usage(argv[0]);
     return -1;
   }
@@ -179,6 +179,10 @@ def add_common_code(f):
   int iterations = 1;
 
   if (mode == "oid") {
+    if (argc != 4) {
+      usage(argv[0]);
+      return -1;
+    }
     std::istringstream iss(argv[3]);
     iss >> iterations;
     if (iss.fail()) {
@@ -187,7 +191,9 @@ def add_common_code(f):
     }
   }
   else if (mode == "oil") {
-    configFile = argv[3];
+    for (int i = 3; i < argc; i++) {
+      configFiles.emplace_back(argv[i]);
+    }
   }
   else {
     usage(argv[0]);
@@ -241,7 +247,7 @@ def gen_target(output_target_name, test_configs):
                     f"thrift/annotation/gen-cpp2/{config['suite']}_types.h"
                 ]
         add_headers(f, sorted(headers), thrift_headers)
-        f.write("std::string configFile;")
+        f.write("std::vector<std::filesystem::path> configFiles;")
 
         for config in test_configs:
             add_test_setup(f, config)
@@ -415,11 +421,13 @@ def gen_runner(output_runner_name, test_configs):
         f.write(
             "#include <boost/property_tree/json_parser.hpp>\n"
             "#include <boost/property_tree/ptree.hpp>\n"
+            "#include <filesystem>\n"
             "#include <fstream>\n"
             "#include <gmock/gmock.h>\n"
             "#include <gtest/gtest.h>\n"
-            "#include <string>\n"
             "#include <sstream>\n"
+            "#include <string>\n"
+            "#include <vector>\n"
             '#include "runner_common.h"\n'
             "\n"
             "namespace ba = boost::asio;\n"