[Driver] Enable nested configuration files

[sepavloff]

Summary

Users may partition parameters specified by configuration file and put
different groups into separate files. These files are inserted into the
main file using constructs @file. Relative file names in it are
resolved relative to the including configuration file and this is not
convenient in some cases. A configuration file, which resides in system
directory, may need to include a file with user-defined parameters and
still provide default definitions if such file is absent.

To solve such problems, the option --config is allowed inside
configuration files. Like @file it results in insertion of
command-line arguments but the algorithm of file search is different and
allows overriding system definitions with user ones.

[mgorny]

I like this more than the alternative solution. The code still has some complexity but the behavior is fairly logical and consistent.

[sepavloff]

Rebase patch

[sepavloff]

Ping.

[mgorny]

llvm/lib/Support/CommandLine.cpp

1213–1220

Not sure if we want to add extra complexity for the legacy spelling here. @MaskRay, what do you think?

[MaskRay]

llvm/lib/Support/CommandLine.cpp

1213–1220

I agree. Supporting just --config= suffices.

[sepavloff]

Removed support of --config file.

[sepavloff]

Missed changes in tests

[mgorny]

LGTM (without testing, just eyeball review).

[MaskRay]

To solve such problems, the option --config is allowed inside ...

--config=

clang/docs/UsersManual.rst

989

--config=

[sepavloff]

Thanks!

clang/docs/UsersManual.rst

::
     include/c++/5.4.0
 
     # other config files may be included
     @linux.options
 
 Files included by ``@file`` directives in configuration files are resolved
 relative to the including file. For example, if a configuration file
 ``~/.llvm/target.cfg`` contains the directive ``@os/linux.opts``, the file
-``linux.opts`` is searched for in the directory ``~/.llvm/os``.
+``linux.opts`` is searched for in the directory ``~/.llvm/os``. Another way to
+include a file content is using the command line option ``--config``. It works
+similarly but the included file is searched for using the rules for configuration
+files.
 
 To generate paths relative to the configuration file, the ``<CFGDIR>`` token may
 be used. This will expand to the absolute path of the directory containing the
 configuration file.
 
 In cases where a configuration file is deployed alongside SDK contents, the
 SDK directory can remain fully portable by using ``<CFGDIR>`` prefixed paths.
 In this way, the user may only need to specify a root configuration file with

clang/include/clang/Basic/DiagnosticDriverKinds.td

 def err_drv_config_file_not_exist : Error<
   "configuration file '%0' does not exist">;
 def err_drv_config_file_not_found : Error<
   "configuration file '%0' cannot be found">;
 def note_drv_config_file_searched_in : Note<
   "was searched for in the directory: %0">;
 def err_drv_cannot_read_config_file : Error<
   "cannot read configuration file '%0'">;
-def err_drv_nested_config_file: Error<
-  "option '--config' is not allowed inside configuration file">;
 def err_drv_arg_requires_bitcode_input: Error<
   "option '%0' requires input to be LLVM bitcode">;
 
 def err_target_unsupported_arch
   : Error<"the target architecture '%0' is not supported by the target '%1'">;
 def err_cpu_unsupported_isa
   : Error<"CPU '%0' does not support '%1' execution mode">;
 def err_arch_unsupported_isa

clang/lib/Driver/Driver.cpp

bool Driver::readConfigFile(StringRef FileName,
   llvm::SmallString<128> CfgFileName(FileName);
   llvm::sys::path::native(CfgFileName);
   bool ContainErrors;
   std::unique_ptr<InputArgList> NewOptions = std::make_unique<InputArgList>(
       ParseArgStrings(NewCfgArgs, IsCLMode(), ContainErrors));
   if (ContainErrors)
     return true;
 
-  if (NewOptions->hasArg(options::OPT_config)) {
-    Diag(diag::err_drv_nested_config_file);
-    return true;
-  }
-
   // Claim all arguments that come from a configuration file so that the driver
   // does not warn on any that is unused.
   for (Arg *A : *NewOptions)
     A->claim();
 
   if (!CfgOptions)
     CfgOptions = std::move(NewOptions);
   else {

clang/test/Driver/Inputs/config-6.cfg

---config config-5
+--config config-4.cfg

clang/test/Driver/config-file-errs.c

 //--- '--config' must be followed by config file name.
 //
 // RUN: not %clang --config 2>&1 | FileCheck %s -check-prefix CHECK-MISSING-FILE
 // CHECK-MISSING-FILE: argument to '--config' is missing (expected 1 value)
 
 
-//--- '--config' must not be found in config files.
-//
-// RUN: not %clang --config %S/Inputs/config-6.cfg 2>&1 | FileCheck %s -check-prefix CHECK-NESTED
-// CHECK-NESTED: option '--config' is not allowed inside configuration file
-
-
 //--- Argument of '--config' must be existing file, if it is specified by path.
 //
 // RUN: not %clang --config somewhere/nonexistent-config-file 2>&1 | FileCheck %s -check-prefix CHECK-NONEXISTENT
 // CHECK-NONEXISTENT: configuration file '{{.*}}somewhere/nonexistent-config-file' does not exist
 
 
 //--- All '--config' arguments must be existing files.
 //

clang/test/Driver/config-file.c

 
 
 //--- Nested config files
 //
 // RUN: %clang --config-system-dir=%S/Inputs --config-user-dir= --config config-2.cfg -S %s -### 2>&1 | FileCheck %s -check-prefix CHECK-NESTED
 // CHECK-NESTED: Configuration file: {{.*}}Inputs{{.}}config-2.cfg
 // CHECK-NESTED: -Wundefined-func-template
 
-// RUN: %clang --config-system-dir=%S/Inputs --config-user-dir= --config config-2.cfg -S %s -### 2>&1 | FileCheck %s -check-prefix CHECK-NESTED2
-// CHECK-NESTED2: Configuration file: {{.*}}Inputs{{.}}config-2.cfg
-// CHECK-NESTED2: -Wundefined-func-template
+// RUN: %clang --config-system-dir=%S/Inputs --config-user-dir=%S/Inputs/config --config config-6.cfg -S %s -### 2>&1 | FileCheck %s -check-prefix CHECK-NESTED2
+// CHECK-NESTED2: Configuration file: {{.*}}Inputs{{.}}config-6.cfg
+// CHECK-NESTED2: -isysroot
+// CHECK-NESTED2-SAME: /opt/data
 
 
 // RUN: %clang --config %S/Inputs/config-2a.cfg -S %s -### 2>&1 | FileCheck %s -check-prefix CHECK-NESTEDa
 // CHECK-NESTEDa: Configuration file: {{.*}}Inputs{{.}}config-2a.cfg
 // CHECK-NESTEDa: -isysroot
 // CHECK-NESTEDa-SAME: /opt/data
 
 // RUN: %clang --config-system-dir=%S/Inputs --config-user-dir= --config config-2a.cfg -S %s -### 2>&1 | FileCheck %s -check-prefix CHECK-NESTED2a

clang/unittests/Driver/ToolChainTest.cpp

std::unique_ptr<Compilation> C(TheDriver.BuildCompilation(
         {"/home/test/bin/clang", "--config", "root.cfg",
          "--config-system-dir=", "--config-user-dir="}));
     ASSERT_TRUE(C);
     ASSERT_FALSE(C->containsError());
     EXPECT_EQ("/opt/sdk/platform2", TheDriver.SysRoot);
   }
 }
 
+TEST(ToolChainTest, NestedConfigFile) {
+  IntrusiveRefCntPtr<DiagnosticOptions> DiagOpts = new DiagnosticOptions();
+  IntrusiveRefCntPtr<DiagnosticIDs> DiagID(new DiagnosticIDs());
+  struct TestDiagnosticConsumer : public DiagnosticConsumer {};
+  DiagnosticsEngine Diags(DiagID, &*DiagOpts, new TestDiagnosticConsumer);
+  IntrusiveRefCntPtr<llvm::vfs::InMemoryFileSystem> FS(
+      new llvm::vfs::InMemoryFileSystem);
+
+#ifdef _WIN32
+  const char *TestRoot = "C:\\";
+#else
+  const char *TestRoot = "/";
+#endif
+  FS->setCurrentWorkingDirectory(TestRoot);
+
+  FS->addFile("/opt/sdk/root.cfg", 0,
+              llvm::MemoryBuffer::getMemBuffer("--config platform.cfg\n"));
+  FS->addFile("/opt/sdk/root2.cfg", 0,
+              llvm::MemoryBuffer::getMemBuffer("--config=platform.cfg\n"));
+  FS->addFile("/opt/sdk/platform.cfg", 0,
+              llvm::MemoryBuffer::getMemBuffer("--sysroot=/platform-sys\n"));
+  FS->addFile("/home/test/bin/platform.cfg", 0,
+              llvm::MemoryBuffer::getMemBuffer("--sysroot=/platform-bin\n"));
+
+  SmallString<128> ClangExecutable("/home/test/bin/clang");
+  FS->makeAbsolute(ClangExecutable);
+
+  // User file is absent - use system definitions.
+  {
+    Driver TheDriver(ClangExecutable, "arm-linux-gnueabi", Diags,
+                     "clang LLVM compiler", FS);
+    std::unique_ptr<Compilation> C(TheDriver.BuildCompilation(
+        {"/home/test/bin/clang", "--config", "root.cfg",
+         "--config-system-dir=/opt/sdk", "--config-user-dir=/home/test/sdk"}));
+    ASSERT_TRUE(C);
+    ASSERT_FALSE(C->containsError());
+    EXPECT_EQ("/platform-sys", TheDriver.SysRoot);
+  }
+
+  // User file overrides system definitions.
+  FS->addFile("/home/test/sdk/platform.cfg", 0,
+              llvm::MemoryBuffer::getMemBuffer("--sysroot=/platform-user\n"));
+  {
+    Driver TheDriver(ClangExecutable, "arm-linux-gnueabi", Diags,
+                     "clang LLVM compiler", FS);
+    std::unique_ptr<Compilation> C(TheDriver.BuildCompilation(
+        {"/home/test/bin/clang", "--config", "root2.cfg",
+         "--config-system-dir=/opt/sdk", "--config-user-dir=/home/test/sdk"}));
+    ASSERT_TRUE(C);
+    ASSERT_FALSE(C->containsError());
+    EXPECT_EQ("/platform-user", TheDriver.SysRoot);
+  }
+}
+
 } // end anonymous namespace.

llvm/lib/Support/CommandLine.cpp

ExpansionContext::expandResponseFile(StringRef FName,
   // these bytes before parsing.
   // Reference: http://en.wikipedia.org/wiki/UTF-8#Byte_order_mark
   else if (hasUTF8ByteOrderMark(BufRef))
     Str = StringRef(BufRef.data() + 3, BufRef.size() - 3);
 
   // Tokenize the contents into NewArgv.
   Tokenizer(Str, Saver, NewArgv, MarkEOLs);
 
-  if (!RelativeNames)
+  // Expanded file content may require additional transformations, like using
+  // absolute paths instead of relative in '@file' constructs or expanding
+  // macros.
+  if (!RelativeNames && !InConfigFile)
     return Error::success();
-  llvm::StringRef BasePath = llvm::sys::path::parent_path(FName);
-  // If names of nested response files should be resolved relative to including
-  // file, replace the included response file names with their full paths
-  // obtained by required resolution.
-  for (auto &Arg : NewArgv) {
-    if (!Arg)
+
+  StringRef BasePath = llvm::sys::path::parent_path(FName);
+  for (auto I = NewArgv.begin(), E = NewArgv.end(); I != E; ++I) {
+    const char *&Arg = *I;
+    if (Arg == nullptr)
       continue;
 
     // Substitute <CFGDIR> with the file's base path.
     if (InConfigFile)
       ExpandBasePaths(BasePath, Saver, Arg);
 
-    // Skip non-rsp file arguments.
-    if (Arg[0] != '@')
-      continue;
-
-    StringRef FileName(Arg + 1);
-    // Skip if non-relative.
+    // Get expanded file name.
+    StringRef ArgStr(Arg);
+    StringRef FileName;
+    bool ConfigInclusion = false;
+    if (ArgStr[0] == '@') {
+      FileName = ArgStr.drop_front(1);
       if (!llvm::sys::path::is_relative(FileName))
         continue;
+    } else if (ArgStr == "--config") {
+      if (I + 1 == E)
+        return createStringError(std::errc::invalid_argument,
+                                 "Option '--config' requires an argument");
+      I = NewArgv.erase(I);
+      E = NewArgv.end();
+      FileName = *I;
+      ConfigInclusion = true;
+    } else if (ArgStr.startswith("--config=")) {
+      FileName = ArgStr.drop_front(std::char_traits<char>::length("--config="));
+      ConfigInclusion = true;
+    } else {
+      continue;
+    }
 
     SmallString<128> ResponseFile;
     ResponseFile.push_back('@');
+
+    // Update expansion construct.
+    if (ConfigInclusion && !llvm::sys::path::has_parent_path(FileName)) {
+      SmallString<128> FilePath;
+      if (!findConfigFile(FileName, FilePath))
+        return createStringError(
+            std::make_error_code(std::errc::no_such_file_or_directory),
+            "cannot not find configuration file: " + FileName);
+      ResponseFile.append(FilePath);
+    } else {
       ResponseFile.append(BasePath);
       llvm::sys::path::append(ResponseFile, FileName);
+    }
     Arg = Saver.save(ResponseFile.str()).data();
   }
   return Error::success();
 }
 
 /// Expand response files on a command line recursively using the given
 /// StringSaver and tokenization strategy.
 bool ExpansionContext::expandResponseFiles(