sepavloff created this revision. sepavloff added reviewers: rjmccall, aaron.ballman, kadircet, sammccall, jackoalan. Herald added a subscriber: hiraditya. Herald added a project: All. sepavloff requested review of this revision. Herald added subscribers: llvm-commits, MaskRay. Herald added projects: clang, LLVM.
Normally a file included into configuration file by directive `@file` is searched for relative the including configuration file. In some cases it is not convenient. Configuration parameters may be logically partitioned into caterories (like 'platform variant' and 'processor variant') and it is convenient to have different configuration files for these categories maintained separately. Similar motivation was described in https://discourse.llvm.org/t/rfc-adding-a-default-file-location-to-config-file-support/63606. Simple file inclusion by `@file` does not help, because in this case the path to `file` is specified relative to the directory of the used configuration file, but it actually is not dependent on the latter. Using absolute paths is not convenient and it still do not allow user to easily override the settings. This change implements option `--search-config-dirs`, which modifies the search algorithm, - files included by `@file` are searched for as configuration files, in the same directories, and can be overridden by a user. With this facility a configuration file can include other files like in: @platform.cfg @processor.cfg ...other options... The effect is as if two configuration files are loaded, both are searched for in well-known places and can be overridden if user provides a file with the same name in user configuration directory. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D133325 Files: clang/docs/UsersManual.rst clang/include/clang/Driver/Driver.h clang/include/clang/Driver/Options.td clang/lib/Driver/Driver.cpp clang/test/Driver/Inputs/config/config-3.cfg clang/test/Driver/config-file.c clang/unittests/Driver/ToolChainTest.cpp llvm/include/llvm/Support/CommandLine.h llvm/lib/Support/CommandLine.cpp
Index: llvm/lib/Support/CommandLine.cpp =================================================================== --- llvm/lib/Support/CommandLine.cpp +++ llvm/lib/Support/CommandLine.cpp @@ -1184,6 +1184,15 @@ if (!RelativeNames) return Error::success(); + + if (InConfigFile) { + if (std::find_if(NewArgv.begin(), NewArgv.end(), + [](const char *Arg) -> bool { + return StringRef("--search-config-dirs") == Arg; + }) != NewArgv.end()) + setSearchAsConfig(true); + } + 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 @@ -1207,8 +1216,17 @@ SmallString<128> ResponseFile; ResponseFile.push_back('@'); - ResponseFile.append(BasePath); - llvm::sys::path::append(ResponseFile, FileName); + if (SearchAsConfig) { + std::string FilePath; + if (!findConfigFile(FileName, FilePath)) + return llvm::createStringError( + std::make_error_code(std::errc::no_such_file_or_directory), + "Could not find config file: " + FileName); + ResponseFile.append(FilePath); + } else { + ResponseFile.append(BasePath); + llvm::sys::path::append(ResponseFile, FileName); + } Arg = Saver.save(ResponseFile.str()).data(); } return Error::success(); @@ -1350,15 +1368,48 @@ llvm::vfs::FileSystem *FS) : Saver(S), Tokenizer(T), FS(FS ? FS : vfs::getRealFileSystem().get()) {} -bool ExpansionContext::readConfigFile(StringRef CfgFile, - SmallVectorImpl<const char *> &Argv) { - SmallString<128> AbsPath; - if (sys::path::is_relative(CfgFile)) { - AbsPath.assign(CfgFile); - if (std::error_code EC = FS->makeAbsolute(AbsPath)) +bool ExpansionContext::findConfigFile(StringRef FileName, + std::string &FilePath) { + SmallString<128> CfgFilePath; + const auto FileExists = [this](SmallString<128> Path) -> bool { + auto Status = FS->status(Path); + return Status && + Status->getType() == llvm::sys::fs::file_type::regular_file; + }; + + // If file name contains directory separator, treat it as a path to + // configuration file. + if (llvm::sys::path::has_parent_path(FileName)) { + CfgFilePath = FileName; + if (llvm::sys::path::is_relative(FileName)) { + if (FS->makeAbsolute(CfgFilePath)) + return false; + } + if (!FileExists(CfgFilePath)) return false; - CfgFile = AbsPath.str(); + FilePath = CfgFilePath.str(); + return true; + } + + // Look for the file in search directories. + for (const StringRef &Dir : SearchDirs) { + if (Dir.empty()) + continue; + CfgFilePath.assign(Dir); + llvm::sys::path::append(CfgFilePath, FileName); + llvm::sys::path::native(CfgFilePath); + if (FileExists(CfgFilePath)) { + FilePath = CfgFilePath.str(); + return true; + } } + + return false; +} + +bool ExpansionContext::readConfigFile(StringRef CfgFile, + SmallVectorImpl<const char *> &Argv) { + assert(llvm::sys::path::is_absolute(CfgFile)); InConfigFile = true; RelativeNames = true; if (llvm::Error Err = expandResponseFile(CfgFile, Argv)) { Index: llvm/include/llvm/Support/CommandLine.h =================================================================== --- llvm/include/llvm/Support/CommandLine.h +++ llvm/include/llvm/Support/CommandLine.h @@ -2080,6 +2080,9 @@ /// used instead. StringRef CurrentDir; + /// Directories used for search of config files. + ArrayRef<StringRef> SearchDirs; + /// True if names of nested response files must be resolved relative to /// including file. bool RelativeNames = false; @@ -2091,6 +2094,10 @@ /// If true, body of config file is expanded. bool InConfigFile = false; + /// If true, the file included by '@file' is searched for as a config file, in + /// the config search directories. + bool SearchAsConfig = false; + llvm::Error expandResponseFile(StringRef FName, SmallVectorImpl<const char *> &NewArgv); @@ -2116,6 +2123,29 @@ return *this; } + ArrayRef<StringRef> getSearchDirs() const { return SearchDirs; } + ExpansionContext &setSearchDirs(ArrayRef<StringRef> X) { + SearchDirs = X; + return *this; + } + + bool getSearchAsConfig() const { return SearchAsConfig; } + ExpansionContext &setSearchAsConfig(bool X) { + SearchAsConfig = X; + return *this; + } + + /// Looks for the specified configuration file. + /// + /// \param[in] FileName Name of the file to search for. + /// \param[out] FilePath File absolute path, if it was found. + /// \return True if file was found. + /// + /// If the specified file name contains a directory separator, it is searched + /// for by its absolute path. Otherwise looks for file sequentially in + /// directories specified by SearchDirs field. + bool findConfigFile(StringRef FileName, std::string &FilePath); + /// Reads command line options from the given configuration file. /// /// \param [in] CfgFile Path to configuration file. Index: clang/unittests/Driver/ToolChainTest.cpp =================================================================== --- clang/unittests/Driver/ToolChainTest.cpp +++ clang/unittests/Driver/ToolChainTest.cpp @@ -506,9 +506,12 @@ FS->addFile("/home/test/bin/root.cfg", 0, llvm::MemoryBuffer::getMemBuffer("--sysroot=/opt/sdk/platform2\n")); + SmallString<128> ClangExecutable("/home/test/bin/clang"); + FS->makeAbsolute(ClangExecutable); + { - Driver TheDriver("/home/test/bin/clang", "arm-linux-gnueabi", Diags, - "clang LLVM compiler", FS); + 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", @@ -519,7 +522,7 @@ EXPECT_EQ("/opt/sdk/platform1", TheDriver.SysRoot); } { - Driver TheDriver("/home/test/bin/clang", "arm-linux-gnueabi", Diags, + Driver TheDriver(ClangExecutable, "arm-linux-gnueabi", Diags, "clang LLVM compiler", FS); std::unique_ptr<Compilation> C( TheDriver.BuildCompilation({ "/home/test/bin/clang", @@ -531,7 +534,7 @@ EXPECT_EQ("/opt/sdk/platform0", TheDriver.SysRoot); } { - Driver TheDriver("/home/test/bin/clang", "arm-linux-gnueabi", Diags, + Driver TheDriver(ClangExecutable, "arm-linux-gnueabi", Diags, "clang LLVM compiler", FS); std::unique_ptr<Compilation> C( TheDriver.BuildCompilation({ "/home/test/bin/clang", @@ -544,4 +547,129 @@ } } +TEST(ToolChainTest, ConfigFileSearch2) { + 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("@platform.cfg\n")); + FS->addFile("/opt/sdk/root2.cfg", 0, + llvm::MemoryBuffer::getMemBuffer("@platform.cfg\n" + "--search-config-dirs\n")); + FS->addFile("/opt/sdk/platform.cfg", 0, + llvm::MemoryBuffer::getMemBuffer("--sysroot=/platform-sys\n")); + FS->addFile("/home/test/sdk/platform.cfg", 0, + llvm::MemoryBuffer::getMemBuffer("--sysroot=/platform-user\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); + + { + // Without '--search-config-dirs' included file is searched for in the same + // directory where including file is resided. + 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); + } + + { + // With '--search-config-dirs' included file is searched for in the same + // directories as config files, so the file in user directory is found. + 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", + "--search-config-dirs"})); + ASSERT_TRUE(C); + ASSERT_FALSE(C->containsError()); + EXPECT_EQ("/platform-user", TheDriver.SysRoot); + } + + { + // The option '--search-config-dirs' can be specified inside configuration + // file. + 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); + } +} + +TEST(ToolChainTest, ConfigFileSearch3) { + 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); + + SmallString<128> ClangExecutable("/opt/sdk/bin/clang"); + FS->makeAbsolute(ClangExecutable); + + // Configuration file is loaded from system directory, it includes file + // from binary one. + FS->addFile("/opt/sdk/root.cfg", 0, + llvm::MemoryBuffer::getMemBuffer("--search-config-dirs\n" + "@workspace.cfg\n")); + FS->addFile("/opt/sdk/bin/workspace.cfg", 0, + llvm::MemoryBuffer::getMemBuffer("--sysroot=/settings-bin\n")); + + { + Driver TheDriver(ClangExecutable, "arm-linux-gnueabi", Diags, + "clang LLVM compiler", FS); + std::unique_ptr<Compilation> C(TheDriver.BuildCompilation( + {ClangExecutable.c_str(), "--config", "root.cfg", + "--config-system-dir=/opt/sdk", "--config-user-dir=/home/test/sdk"})); + ASSERT_TRUE(C); + ASSERT_FALSE(C->containsError()); + EXPECT_EQ("/settings-bin", TheDriver.SysRoot); + } + + // User can override included file by providing file with the same name in + // the user directory. + FS->addFile("/home/test/sdk/workspace.cfg", 0, + llvm::MemoryBuffer::getMemBuffer("--sysroot=/settings-user\n")); + + { + Driver TheDriver(ClangExecutable, "arm-linux-gnueabi", Diags, + "clang LLVM compiler", FS); + std::unique_ptr<Compilation> C(TheDriver.BuildCompilation( + {ClangExecutable.c_str(), "--config", "root.cfg", + "--config-system-dir=/opt/sdk", "--config-user-dir=/home/test/sdk"})); + ASSERT_TRUE(C); + ASSERT_FALSE(C->containsError()); + EXPECT_EQ("/settings-user", TheDriver.SysRoot); + } +} + } // end anonymous namespace. Index: clang/test/Driver/config-file.c =================================================================== --- clang/test/Driver/config-file.c +++ clang/test/Driver/config-file.c @@ -39,14 +39,14 @@ //--- 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 +// RUN: %clang --config-system-dir=%S/Inputs --config-user-dir=%S/Inputs/config --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 +// RUN: %clang --config-system-dir=%S/Inputs --config-user-dir=%S/Inputs/config --config config-2.cfg -S --search-config-dirs %s -### 2>&1 | FileCheck %s -check-prefix CHECK-NESTED2 // CHECK-NESTED2: Configuration file: {{.*}}Inputs{{.}}config-2.cfg -// CHECK-NESTED2: -Wundefined-func-template - +// CHECK-NESTED2: -isysroot +// CHECK-NESTED2-SAME: /opt/sdk/user // 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 Index: clang/test/Driver/Inputs/config/config-3.cfg =================================================================== --- /dev/null +++ clang/test/Driver/Inputs/config/config-3.cfg @@ -0,0 +1 @@ +-isysroot /opt/sdk/user Index: clang/lib/Driver/Driver.cpp =================================================================== --- clang/lib/Driver/Driver.cpp +++ clang/lib/Driver/Driver.cpp @@ -900,40 +900,11 @@ // } -/// Looks the given directories for the specified file. -/// -/// \param[out] FilePath File path, if the file was found. -/// \param[in] Dirs Directories used for the search. -/// \param[in] FileName Name of the file to search for. -/// \return True if file was found. -/// -/// Looks for file specified by FileName sequentially in directories specified -/// by Dirs. -/// -static bool searchForFile(SmallVectorImpl<char> &FilePath, - ArrayRef<StringRef> Dirs, StringRef FileName, - llvm::vfs::FileSystem &FS) { - SmallString<128> WPath; - for (const StringRef &Dir : Dirs) { - if (Dir.empty()) - continue; - WPath.clear(); - llvm::sys::path::append(WPath, Dir, FileName); - llvm::sys::path::native(WPath); - auto Status = FS.status(WPath); - if (Status && Status->getType() == llvm::sys::fs::file_type::regular_file) { - FilePath = std::move(WPath); - return true; - } - } - return false; -} - -bool Driver::readConfigFile(StringRef FileName) { +bool Driver::readConfigFile(StringRef FileName, + llvm::cl::ExpansionContext &ECtx) { // Try reading the given file. SmallVector<const char *, 32> NewCfgArgs; - llvm::cl::ExpansionContext ExpCtx(Saver, llvm::cl::tokenizeConfigFile, &getVFS()); - if (!ExpCtx.readConfigFile(FileName, NewCfgArgs)) { + if (!ECtx.readConfigFile(FileName, NewCfgArgs)) { Diag(diag::err_drv_cannot_read_config_file) << FileName; return true; } @@ -966,6 +937,8 @@ bool Driver::loadConfigFile() { std::string CfgFileName; bool FileSpecifiedExplicitly = false; + llvm::cl::ExpansionContext ExpCtx(Saver, llvm::cl::tokenizeConfigFile, + &getVFS()); // Process options that change search path for config files. if (CLOptions) { @@ -987,6 +960,8 @@ else UserConfigDir = static_cast<std::string>(CfgDir); } + if (CLOptions->hasArg(options::OPT_search_config_dirs)) + ExpCtx.setSearchAsConfig(true); } // First try to find config file specified in command line. @@ -1018,7 +993,7 @@ return true; } } - return readConfigFile(CfgFilePath); + return readConfigFile(CfgFilePath, ExpCtx); } FileSpecifiedExplicitly = true; @@ -1071,30 +1046,31 @@ // Prepare list of directories where config file is searched for. StringRef CfgFileSearchDirs[] = {UserConfigDir, SystemConfigDir, Dir}; + ExpCtx.setSearchDirs(CfgFileSearchDirs); // Try to find config file. First try file with corrected architecture. - llvm::SmallString<128> CfgFilePath; + std::string CfgFilePath; if (!FixedConfigFile.empty()) { - if (searchForFile(CfgFilePath, CfgFileSearchDirs, FixedConfigFile, getVFS())) - return readConfigFile(CfgFilePath); + if (ExpCtx.findConfigFile(FixedConfigFile, CfgFilePath)) + return readConfigFile(CfgFilePath, ExpCtx); // If 'x86_64-clang.cfg' was not found, try 'x86_64.cfg'. FixedConfigFile.resize(FixedArchPrefixLen); FixedConfigFile.append(".cfg"); - if (searchForFile(CfgFilePath, CfgFileSearchDirs, FixedConfigFile, getVFS())) - return readConfigFile(CfgFilePath); + if (ExpCtx.findConfigFile(FixedConfigFile, CfgFilePath)) + return readConfigFile(CfgFilePath, ExpCtx); } // Then try original file name. - if (searchForFile(CfgFilePath, CfgFileSearchDirs, CfgFileName, getVFS())) - return readConfigFile(CfgFilePath); + if (ExpCtx.findConfigFile(CfgFileName, CfgFilePath)) + return readConfigFile(CfgFilePath, ExpCtx); // Finally try removing driver mode part: 'x86_64-clang.cfg' -> 'x86_64.cfg'. if (!ClangNameParts.ModeSuffix.empty() && !ClangNameParts.TargetPrefix.empty()) { CfgFileName.assign(ClangNameParts.TargetPrefix); CfgFileName.append(".cfg"); - if (searchForFile(CfgFilePath, CfgFileSearchDirs, CfgFileName, getVFS())) - return readConfigFile(CfgFilePath); + if (ExpCtx.findConfigFile(CfgFileName, CfgFilePath)) + return readConfigFile(CfgFilePath, ExpCtx); } // Report error but only if config file was specified explicitly, by option @@ -1156,7 +1132,8 @@ if (HasConfigFile) for (auto *Opt : *CLOptions) { - if (Opt->getOption().matches(options::OPT_config)) + if (Opt->getOption().matches(options::OPT_config) || + Opt->getOption().matches(options::OPT_search_config_dirs)) continue; const Arg *BaseArg = &Opt->getBaseArg(); if (BaseArg == Opt) Index: clang/include/clang/Driver/Options.td =================================================================== --- clang/include/clang/Driver/Options.td +++ clang/include/clang/Driver/Options.td @@ -905,6 +905,8 @@ HelpText<"System directory for configuration files">; def config_user_dir_EQ : Joined<["--"], "config-user-dir=">, Flags<[NoXarchOption, HelpHidden]>, HelpText<"User directory for configuration files">; +def search_config_dirs : Flag<["--"], "search-config-dirs">, Flags<[NoXarchOption]>, + HelpText<"Search for files included by configuration file is search directories">; def coverage : Flag<["-", "--"], "coverage">, Group<Link_Group>, Flags<[CoreOption]>; def cpp_precomp : Flag<["-"], "cpp-precomp">, Group<clang_ignored_f_Group>; def current__version : JoinedOrSeparate<["-"], "current_version">; Index: clang/include/clang/Driver/Driver.h =================================================================== --- clang/include/clang/Driver/Driver.h +++ clang/include/clang/Driver/Driver.h @@ -34,6 +34,9 @@ namespace vfs { class FileSystem; } +namespace cl { +class ExpansionContext; +} } // namespace llvm namespace clang { @@ -673,7 +676,7 @@ /// /// \param [in] FileName File to read. /// \returns true, if error occurred while reading. - bool readConfigFile(StringRef FileName); + bool readConfigFile(StringRef FileName, llvm::cl::ExpansionContext &ECtx); /// Set the driver mode (cl, gcc, etc) from the value of the `--driver-mode` /// option. Index: clang/docs/UsersManual.rst =================================================================== --- clang/docs/UsersManual.rst +++ clang/docs/UsersManual.rst @@ -940,10 +940,14 @@ @linux.options Files included by `@file` directives in configuration files are resolved -relative to the including file. For example, if a configuration file +relative to the including file by default. 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`. +Option `--search-config-dirs` changes the search algorith if the included +file is specified without a directory separator. In this case the file is looked +for in the same directories as 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.
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits