jackoalan updated this revision to Diff 396670. jackoalan added a comment. Add ReadConfigFile test case for multiple `<CFGDIR>` in one arg.
Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D115604/new/ https://reviews.llvm.org/D115604 Files: clang/docs/ReleaseNotes.rst clang/docs/UsersManual.rst clang/lib/Tooling/ExpandResponseFilesCompilationDatabase.cpp llvm/include/llvm/Support/CommandLine.h llvm/lib/Support/CommandLine.cpp llvm/unittests/Support/CommandLineTest.cpp
Index: llvm/unittests/Support/CommandLineTest.cpp =================================================================== --- llvm/unittests/Support/CommandLineTest.cpp +++ llvm/unittests/Support/CommandLineTest.cpp @@ -827,7 +827,7 @@ llvm::BumpPtrAllocator A; llvm::StringSaver Saver(A); ASSERT_TRUE(llvm::cl::ExpandResponseFiles( - Saver, llvm::cl::TokenizeGNUCommandLine, Argv, false, true, + Saver, llvm::cl::TokenizeGNUCommandLine, Argv, false, true, false, /*CurrentDir=*/StringRef(TestRoot), FS)); EXPECT_THAT(Argv, testing::Pointwise( StringEquality(), @@ -889,9 +889,9 @@ #else cl::TokenizerCallback Tokenizer = cl::TokenizeGNUCommandLine; #endif - ASSERT_FALSE(cl::ExpandResponseFiles(Saver, Tokenizer, Argv, false, false, - /*CurrentDir=*/llvm::StringRef(TestRoot), - FS)); + ASSERT_FALSE( + cl::ExpandResponseFiles(Saver, Tokenizer, Argv, false, false, false, + /*CurrentDir=*/llvm::StringRef(TestRoot), FS)); EXPECT_THAT(Argv, testing::Pointwise(StringEquality(), @@ -929,7 +929,7 @@ BumpPtrAllocator A; StringSaver Saver(A); ASSERT_FALSE(cl::ExpandResponseFiles(Saver, cl::TokenizeGNUCommandLine, Argv, - false, false, + false, false, false, /*CurrentDir=*/StringRef(TestRoot), FS)); // ASSERT instead of EXPECT to prevent potential out-of-bounds access. @@ -964,7 +964,7 @@ BumpPtrAllocator A; StringSaver Saver(A); ASSERT_TRUE(cl::ExpandResponseFiles(Saver, cl::TokenizeGNUCommandLine, Argv, - false, true, + false, true, false, /*CurrentDir=*/StringRef(TestRoot), FS)); EXPECT_THAT(Argv, testing::Pointwise(StringEquality(), {"test/test", "-flag"})); @@ -984,7 +984,7 @@ BumpPtrAllocator A; StringSaver Saver(A); ASSERT_TRUE(cl::ExpandResponseFiles(Saver, cl::TokenizeWindowsCommandLine, - Argv, true, true, + Argv, true, true, false, /*CurrentDir=*/StringRef(TestRoot), FS)); const char *Expected[] = {"clang", "-Xclang", "-Wno-whatever", nullptr, "input.cpp"}; @@ -1038,25 +1038,39 @@ llvm::SmallVector<const char *, 1> Argv; TempDir TestDir("unittest", /*Unique*/ true); + TempDir TestSubDir(TestDir.path("subdir"), /*Unique*/ false); - llvm::SmallString<128> TestCfg; - llvm::sys::path::append(TestCfg, TestDir.path(), "foo"); - + llvm::SmallString<128> TestCfg = TestDir.path("foo"); TempFile ConfigFile(TestCfg, "", "# Comment\n" "-option_1\n" + "-option_2=<CFGDIR>/dir1\n" + "-option_3=<CFGDIR>\n" + "-option_4 <CFGDIR>\n" + "-option_5=<CFG\\\n" + "DIR>\n" + "-option_6=<CFGDIR>/dir1,<CFGDIR>/dir2\n" "@subconfig\n" - "-option_3=abcd\n" - "-option_4=\\\n" + "-option_11=abcd\n" + "-option_12=\\\n" "cdef\n"); - llvm::SmallString<128> TestCfg2; - llvm::sys::path::append(TestCfg2, TestDir.path(), "subconfig"); + llvm::SmallString<128> TestCfg2 = TestDir.path("subconfig"); TempFile ConfigFile2(TestCfg2, "", - "-option_2\n" + "-option_7\n" + "-option_8=<CFGDIR>/dir2\n" + "@subdir/subfoo\n" "\n" " # comment\n"); + llvm::SmallString<128> TestCfg3 = TestSubDir.path("subfoo"); + TempFile ConfigFile3(TestCfg3, "", + "-option_9=<CFGDIR>/dir3\n" + "@<CFGDIR>/subfoo2\n"); + + llvm::SmallString<128> TestCfg4 = TestSubDir.path("subfoo2"); + TempFile ConfigFile4(TestCfg4, "", "-option_10\n"); + // Make sure the current directory is not the directory where config files // resides. In this case the code that expands response files will not find // 'subconfig' unless it resolves nested inclusions relative to the including @@ -1071,11 +1085,26 @@ bool Result = llvm::cl::readConfigFile(ConfigFile.path(), Saver, Argv); EXPECT_TRUE(Result); - EXPECT_EQ(Argv.size(), 4U); + EXPECT_EQ(Argv.size(), 13U); EXPECT_STREQ(Argv[0], "-option_1"); - EXPECT_STREQ(Argv[1], "-option_2"); - EXPECT_STREQ(Argv[2], "-option_3=abcd"); - EXPECT_STREQ(Argv[3], "-option_4=cdef"); + EXPECT_STREQ(Argv[1], + ("-option_2=" + TestDir.path() + "/dir1").str().c_str()); + EXPECT_STREQ(Argv[2], ("-option_3=" + TestDir.path()).str().c_str()); + EXPECT_STREQ(Argv[3], "-option_4"); + EXPECT_STREQ(Argv[4], TestDir.path().str().c_str()); + EXPECT_STREQ(Argv[5], ("-option_5=" + TestDir.path()).str().c_str()); + EXPECT_STREQ(Argv[6], ("-option_6=" + TestDir.path() + "/dir1," + + TestDir.path() + "/dir2") + .str() + .c_str()); + EXPECT_STREQ(Argv[7], "-option_7"); + EXPECT_STREQ(Argv[8], + ("-option_8=" + TestDir.path() + "/dir2").str().c_str()); + EXPECT_STREQ(Argv[9], + ("-option_9=" + TestSubDir.path() + "/dir3").str().c_str()); + EXPECT_STREQ(Argv[10], "-option_10"); + EXPECT_STREQ(Argv[11], "-option_11=abcd"); + EXPECT_STREQ(Argv[12], "-option_12=cdef"); } TEST(CommandLineTest, PositionalEatArgsError) { Index: llvm/lib/Support/CommandLine.cpp =================================================================== --- llvm/lib/Support/CommandLine.cpp +++ llvm/lib/Support/CommandLine.cpp @@ -1078,11 +1078,45 @@ return (S.size() >= 3 && S[0] == '\xef' && S[1] == '\xbb' && S[2] == '\xbf'); } +// Substitute <CFGDIR> with the file's base path. +static void ExpandBasePaths(StringRef BasePath, StringSaver &Saver, + const char *&Arg) { + assert(sys::path::is_absolute(BasePath)); + constexpr StringLiteral Token("<CFGDIR>"); + const StringRef ArgString(Arg); + + SmallString<128> ResponseFile; + StringRef::size_type StartPos = 0; + for (StringRef::size_type TokenPos = ArgString.find(Token); + TokenPos != StringRef::npos; + TokenPos = ArgString.find(Token, StartPos)) { + // Token may appear more than once per arg (e.g. comma-separated linker + // args). Support by using path-append on any subsequent appearances. + const StringRef LHS = ArgString.substr(StartPos, TokenPos - StartPos); + if (ResponseFile.empty()) + ResponseFile = LHS; + else + llvm::sys::path::append(ResponseFile, LHS); + ResponseFile.append(BasePath); + StartPos = TokenPos + Token.size(); + } + + if (!ResponseFile.empty()) { + // Path-append the remaining arg substring if at least one token appeared. + const StringRef Remaining = ArgString.substr(StartPos); + if (!Remaining.empty()) + llvm::sys::path::append(ResponseFile, Remaining); + Arg = Saver.save(ResponseFile.str()).data(); + } +} + // FName must be an absolute path. -static llvm::Error ExpandResponseFile( - StringRef FName, StringSaver &Saver, TokenizerCallback Tokenizer, - SmallVectorImpl<const char *> &NewArgv, bool MarkEOLs, bool RelativeNames, - llvm::vfs::FileSystem &FS) { +static llvm::Error ExpandResponseFile(StringRef FName, StringSaver &Saver, + TokenizerCallback Tokenizer, + SmallVectorImpl<const char *> &NewArgv, + bool MarkEOLs, bool RelativeNames, + bool ExpandBasePath, + llvm::vfs::FileSystem &FS) { assert(sys::path::is_absolute(FName)); llvm::ErrorOr<std::unique_ptr<MemoryBuffer>> MemBufOrErr = FS.getBufferForFile(FName); @@ -1116,8 +1150,15 @@ // file, replace the included response file names with their full paths // obtained by required resolution. for (auto &Arg : NewArgv) { + if (!Arg) + continue; + + // Substitute <CFGDIR> with the file's base path. + if (ExpandBasePath) + ExpandBasePaths(BasePath, Saver, Arg); + // Skip non-rsp file arguments. - if (!Arg || Arg[0] != '@') + if (Arg[0] != '@') continue; StringRef FileName(Arg + 1); @@ -1129,7 +1170,7 @@ ResponseFile.push_back('@'); ResponseFile.append(BasePath); llvm::sys::path::append(ResponseFile, FileName); - Arg = Saver.save(ResponseFile.c_str()).data(); + Arg = Saver.save(ResponseFile.str()).data(); } return Error::success(); } @@ -1138,7 +1179,7 @@ /// StringSaver and tokenization strategy. bool cl::ExpandResponseFiles(StringSaver &Saver, TokenizerCallback Tokenizer, SmallVectorImpl<const char *> &Argv, bool MarkEOLs, - bool RelativeNames, + bool RelativeNames, bool ExpandBasePath, llvm::Optional<llvm::StringRef> CurrentDir, llvm::vfs::FileSystem &FS) { bool AllExpanded = true; @@ -1218,7 +1259,7 @@ SmallVector<const char *, 0> ExpandedArgv; if (llvm::Error Err = ExpandResponseFile(FName, Saver, Tokenizer, ExpandedArgv, MarkEOLs, - RelativeNames, FS)) { + RelativeNames, ExpandBasePath, FS)) { // We couldn't read this file, so we leave it in the argument stream and // move on. // TODO: The error should be propagated up the stack. @@ -1250,11 +1291,11 @@ bool cl::ExpandResponseFiles(StringSaver &Saver, TokenizerCallback Tokenizer, SmallVectorImpl<const char *> &Argv, bool MarkEOLs, - bool RelativeNames, + bool RelativeNames, bool ExpandBasePath, llvm::Optional<StringRef> CurrentDir) { return ExpandResponseFiles(Saver, std::move(Tokenizer), Argv, MarkEOLs, - RelativeNames, std::move(CurrentDir), - *vfs::getRealFileSystem()); + RelativeNames, ExpandBasePath, + std::move(CurrentDir), *vfs::getRealFileSystem()); } bool cl::expandResponseFiles(int Argc, const char *const *Argv, @@ -1281,16 +1322,17 @@ llvm::sys::path::append(AbsPath, CfgFile); CfgFile = AbsPath.str(); } - if (llvm::Error Err = - ExpandResponseFile(CfgFile, Saver, cl::tokenizeConfigFile, Argv, - /*MarkEOLs=*/false, /*RelativeNames=*/true, - *llvm::vfs::getRealFileSystem())) { + if (llvm::Error Err = ExpandResponseFile( + CfgFile, Saver, cl::tokenizeConfigFile, Argv, + /*MarkEOLs=*/false, /*RelativeNames=*/true, /*ExpandBasePath=*/true, + *llvm::vfs::getRealFileSystem())) { // TODO: The error should be propagated up the stack. llvm::consumeError(std::move(Err)); return false; } return ExpandResponseFiles(Saver, cl::tokenizeConfigFile, Argv, - /*MarkEOLs=*/false, /*RelativeNames=*/true); + /*MarkEOLs=*/false, /*RelativeNames=*/true, + /*ExpandBasePath=*/true, llvm::None); } static void initCommonOptions(); Index: llvm/include/llvm/Support/CommandLine.h =================================================================== --- llvm/include/llvm/Support/CommandLine.h +++ llvm/include/llvm/Support/CommandLine.h @@ -2082,7 +2082,8 @@ /// /// It reads content of the specified file, tokenizes it and expands "@file" /// commands resolving file names in them relative to the directory where -/// CfgFilename resides. +/// CfgFilename resides. It also expands "<CFGDIR>" to the base path of the +/// current config file. /// bool readConfigFile(StringRef CfgFileName, StringSaver &Saver, SmallVectorImpl<const char *> &Argv); @@ -2102,13 +2103,15 @@ /// with nullptrs in the Argv vector. /// \param [in] RelativeNames true if names of nested response files must be /// resolved relative to including file. +/// \param [in] ExpandBasePath If true, "<CFGDIR>" expands to the base path of +/// the current response file. /// \param [in] FS File system used for all file access when running the tool. /// \param [in] CurrentDir Path used to resolve relative rsp files. If set to /// None, process' cwd is used instead. /// \return true if all @files were expanded successfully or there were none. bool ExpandResponseFiles(StringSaver &Saver, TokenizerCallback Tokenizer, SmallVectorImpl<const char *> &Argv, bool MarkEOLs, - bool RelativeNames, + bool RelativeNames, bool ExpandBasePath, llvm::Optional<llvm::StringRef> CurrentDir, llvm::vfs::FileSystem &FS); @@ -2117,7 +2120,7 @@ bool ExpandResponseFiles( StringSaver &Saver, TokenizerCallback Tokenizer, SmallVectorImpl<const char *> &Argv, bool MarkEOLs = false, - bool RelativeNames = false, + bool RelativeNames = false, bool ExpandBasePath = false, llvm::Optional<llvm::StringRef> CurrentDir = llvm::None); /// A convenience helper which concatenates the options specified by the Index: clang/lib/Tooling/ExpandResponseFilesCompilationDatabase.cpp =================================================================== --- clang/lib/Tooling/ExpandResponseFilesCompilationDatabase.cpp +++ clang/lib/Tooling/ExpandResponseFilesCompilationDatabase.cpp @@ -61,7 +61,7 @@ continue; llvm::BumpPtrAllocator Alloc; llvm::StringSaver Saver(Alloc); - llvm::cl::ExpandResponseFiles(Saver, Tokenizer, Argv, false, false, + llvm::cl::ExpandResponseFiles(Saver, Tokenizer, Argv, false, false, false, llvm::StringRef(Cmd.Directory), *FS); // Don't assign directly, Argv aliases CommandLine. std::vector<std::string> ExpandedArgv(Argv.begin(), Argv.end()); Index: clang/docs/UsersManual.rst =================================================================== --- clang/docs/UsersManual.rst +++ clang/docs/UsersManual.rst @@ -843,6 +843,8 @@ option tells Clang to put double-quotes around the entire filename, which is the convention used by NMake and Jom. +.. _configuration-files: + Configuration files ------------------- @@ -917,6 +919,22 @@ `~/.llvm/target.cfg` contains the directive `@os/linux.opts`, the file `linux.opts` is searched for in the directory `~/.llvm/os`. +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 +`--config` to establish every aspect of the SDK with the compiler: + +:: + + --target=foo + -isystem <CFGDIR>/include + -L <CFGDIR>/lib + -T <CFGDIR>/ldscripts/link.ld + Language and Target-Independent Features ======================================== Index: clang/docs/ReleaseNotes.rst =================================================================== --- clang/docs/ReleaseNotes.rst +++ clang/docs/ReleaseNotes.rst @@ -63,6 +63,9 @@ - Maximum _ExtInt size was decreased from 16,777,215 bits to 8,388,608 bits. Motivation for this was discussed in PR51829. +- Configuration file syntax extended with ``<CFGDIR>`` token. This expands to + the base path of the current config file. See :ref:`configuration-files` for + details. New Compiler Flags ------------------
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits