Author: Sam McCall Date: 2019-12-02T22:13:29+01:00 New Revision: 93f77617abba512d2861e2fc50ce385883f587b6
URL: https://github.com/llvm/llvm-project/commit/93f77617abba512d2861e2fc50ce385883f587b6 DIFF: https://github.com/llvm/llvm-project/commit/93f77617abba512d2861e2fc50ce385883f587b6.diff LOG: Revert "[clangd] repair mac tests for 88bccded8fa1" Revert "[clangd] Try harder to find a plausible `clang` as argv0, particularly on Mac." Added: Modified: clang-tools-extra/clangd/GlobalCompilationDatabase.cpp clang-tools-extra/clangd/unittests/BackgroundIndexTests.cpp clang-tools-extra/clangd/unittests/GlobalCompilationDatabaseTests.cpp Removed: ################################################################################ diff --git a/clang-tools-extra/clangd/GlobalCompilationDatabase.cpp b/clang-tools-extra/clangd/GlobalCompilationDatabase.cpp index 8e78fedf44bb..ed3b86f0f55b 100644 --- a/clang-tools-extra/clangd/GlobalCompilationDatabase.cpp +++ b/clang-tools-extra/clangd/GlobalCompilationDatabase.cpp @@ -18,9 +18,7 @@ #include "llvm/ADT/STLExtras.h" #include "llvm/ADT/SmallString.h" #include "llvm/Support/FileSystem.h" -#include "llvm/Support/FileUtilities.h" #include "llvm/Support/Path.h" -#include "llvm/Support/Program.h" #include <string> #include <tuple> #include <vector> @@ -29,113 +27,6 @@ namespace clang { namespace clangd { namespace { -// Query apple's `xcrun` launcher, which is the source of truth for "how should" -// clang be invoked on this system. -llvm::Optional<std::string> queryXcrun(llvm::ArrayRef<llvm::StringRef> Argv) { - auto Xcrun = llvm::sys::findProgramByName("xcrun"); - if (!Xcrun) { - log("Couldn't find xcrun. Hopefully you have a non-apple toolchain..."); - return llvm::None; - } - llvm::SmallString<64> OutFile; - llvm::sys::fs::createTemporaryFile("clangd-xcrun", "", OutFile); - llvm::FileRemover OutRemover(OutFile); - llvm::Optional<llvm::StringRef> Redirects[3] = { - /*stdin=*/{""}, /*stdout=*/{OutFile}, /*stderr=*/{""}}; - vlog("Invoking {0} to find clang installation", *Xcrun); - int Ret = llvm::sys::ExecuteAndWait(*Xcrun, Argv, - /*Env=*/llvm::None, Redirects, - /*SecondsToWait=*/10); - if (Ret != 0) { - log("xcrun exists but failed with code {0}. " - "If you have a non-apple toolchain, this is OK. " - "Otherwise, try xcode-select --install.", - Ret); - return llvm::None; - } - - auto Buf = llvm::MemoryBuffer::getFile(OutFile); - if (!Buf) { - log("Can't read xcrun output: {0}", Buf.getError().message()); - return llvm::None; - } - StringRef Path = Buf->get()->getBuffer().trim(); - if (Path.empty()) { - log("xcrun produced no output"); - return llvm::None; - } - return Path.str(); -} - -// On Mac, `which clang` is /usr/bin/clang. It runs `xcrun clang`, which knows -// where the real clang is kept. We need to do the same thing, -// because cc1 (not the driver!) will find libc++ relative to argv[0]. -llvm::Optional<std::string> queryMacClangPath() { -#ifndef __APPLE__ - return llvm::None; -#endif - - return queryXcrun({"xcrun", "--find", "clang"}); -} - -// Resolve symlinks if possible. -std::string resolve(std::string Path) { - llvm::SmallString<128> Resolved; - if (llvm::sys::fs::real_path(Path, Resolved)) - return Path; // On error; - return Resolved.str(); -} - -// Get a plausible full `clang` path. -// This is used in the fallback compile command, or when the CDB returns a -// generic driver with no path. -llvm::StringRef getFallbackClangPath() { - static const std::string &MemoizedFallbackPath = [&]() -> std::string { - // The driver and/or cc1 sometimes depend on the binary name to compute - // useful things like the standard library location. - // We need to emulate what clang on this system is likely to see. - // cc1 in particular looks at the "real path" of the running process, and - // so if /usr/bin/clang is a symlink, it sees the resolved path. - // clangd doesn't have that luxury, so we resolve symlinks ourselves. - - // /usr/bin/clang on a mac is a program that redirects to the right clang. - // We resolve it as if it were a symlink. - if (auto MacClang = queryMacClangPath()) - return resolve(std::move(*MacClang)); - // On other platforms, just look for compilers on the PATH. - for (const char* Name : {"clang", "gcc", "cc"}) - if (auto PathCC = llvm::sys::findProgramByName(Name)) - return resolve(std::move(*PathCC)); - // Fallback: a nonexistent 'clang' binary next to clangd. - static int Dummy; - std::string ClangdExecutable = - llvm::sys::fs::getMainExecutable("clangd", (void *)&Dummy); - SmallString<128> ClangPath; - ClangPath = llvm::sys::path::parent_path(ClangdExecutable); - llvm::sys::path::append(ClangPath, "clang"); - return ClangPath.str(); - }(); - return MemoizedFallbackPath; -} - -// On mac, /usr/bin/clang sets SDKROOT and then invokes the real clang. -// The effect of this is to set -isysroot correctly. We do the same. -const std::string *getMacSysroot() { -#ifndef __APPLE__ - return nullptr; -#endif - - // SDKROOT overridden in environment, respect it. Driver will set isysroot. - if (::getenv("SDKROOT")) - return nullptr; - static const llvm::Optional<std::string> &Sysroot = - queryXcrun({"xcrun", "--show-sdk-path"}); - return Sysroot ? Sysroot.getPointer() : nullptr; -} - -// Transform a command into the form we want to send to the driver. -// The command was originally either from the CDB or is the fallback command, -// and may have been modified by OverlayCDB. void adjustArguments(tooling::CompileCommand &Cmd, llvm::StringRef ResourceDir) { tooling::ArgumentsAdjuster ArgsAdjuster = tooling::combineAdjusters( @@ -149,35 +40,10 @@ void adjustArguments(tooling::CompileCommand &Cmd, tooling::getClangSyntaxOnlyAdjuster())); Cmd.CommandLine = ArgsAdjuster(Cmd.CommandLine, Cmd.Filename); - // Check whether the flag exists, either as -flag or -flag=* - auto Has = [&](llvm::StringRef Flag) { - for (llvm::StringRef Arg : Cmd.CommandLine) { - if (Arg.consume_front(Flag) && (Arg.empty() || Arg[0] == '=')) - return true; - } - return false; - }; // Inject the resource dir. - if (!ResourceDir.empty() && !Has("-resource-dir")) + // FIXME: Don't overwrite it if it's already there. + if (!ResourceDir.empty()) Cmd.CommandLine.push_back(("-resource-dir=" + ResourceDir).str()); - if (!Has("-isysroot")) - if (const std::string *MacSysroot = getMacSysroot()) { - Cmd.CommandLine.push_back("-isysroot"); - Cmd.CommandLine.push_back(*MacSysroot); - } - - // If the driver is a generic name like "g++" with no path, add a clang path. - // This makes it easier for us to find the standard libraries on mac. - if (!Cmd.CommandLine.empty()) { - std::string &Driver = Cmd.CommandLine.front(); - if (Driver == "clang" || Driver == "clang++" || Driver == "gcc" || - Driver == "g++" || Driver == "cc" || Driver == "c++") { - llvm::SmallString<128> QualifiedDriver = - llvm::sys::path::parent_path(getFallbackClangPath()); - llvm::sys::path::append(QualifiedDriver, Driver); - Driver = QualifiedDriver.str(); - } - } } std::string getStandardResourceDir() { @@ -197,9 +63,19 @@ void actOnAllParentDirectories(PathRef FileName, } // namespace +static std::string getFallbackClangPath() { + static int Dummy; + std::string ClangdExecutable = + llvm::sys::fs::getMainExecutable("clangd", (void *)&Dummy); + SmallString<128> ClangPath; + ClangPath = llvm::sys::path::parent_path(ClangdExecutable); + llvm::sys::path::append(ClangPath, "clang"); + return ClangPath.str(); +} + tooling::CompileCommand GlobalCompilationDatabase::getFallbackCommand(PathRef File) const { - std::vector<std::string> Argv = {"clang"}; + std::vector<std::string> Argv = {getFallbackClangPath()}; // Clang treats .h files as C by default and files without extension as linker // input, resulting in unhelpful diagnostics. // Parsing as Objective C++ is friendly to more cases. diff --git a/clang-tools-extra/clangd/unittests/BackgroundIndexTests.cpp b/clang-tools-extra/clangd/unittests/BackgroundIndexTests.cpp index ee7ba4355c05..c01910e43b40 100644 --- a/clang-tools-extra/clangd/unittests/BackgroundIndexTests.cpp +++ b/clang-tools-extra/clangd/unittests/BackgroundIndexTests.cpp @@ -527,13 +527,6 @@ TEST_F(BackgroundIndexTest, UncompilableFiles) { } } -MATCHER_P(HasPrefix, Prefix, "") { - auto Arg = arg; // Force copy. - if (Arg.size() > Prefix.size()) - Arg.resize(Prefix.size()); - return Arg == Prefix; -} - TEST_F(BackgroundIndexTest, CmdLineHash) { MockFSProvider FS; llvm::StringMap<std::string> Storage; @@ -549,7 +542,7 @@ TEST_F(BackgroundIndexTest, CmdLineHash) { FS.Files[testPath("A.h")] = ""; Cmd.Filename = "../A.cc"; Cmd.Directory = testPath("build"); - Cmd.CommandLine = {"/bin/clang++", "../A.cc", "-fsyntax-only"}; + Cmd.CommandLine = {"clang++", "../A.cc", "-fsyntax-only"}; CDB.setCompileCommand(testPath("build/../A.cc"), Cmd); ASSERT_TRUE(Idx.blockUntilIdleForTest()); @@ -559,14 +552,13 @@ TEST_F(BackgroundIndexTest, CmdLineHash) { { tooling::CompileCommand CmdStored = *MSS.loadShard(testPath("A.cc"))->Cmd; - // Accept prefix because -isysroot gets added on mac. - EXPECT_THAT(CmdStored.CommandLine, HasPrefix(Cmd.CommandLine)); + EXPECT_EQ(CmdStored.CommandLine, Cmd.CommandLine); EXPECT_EQ(CmdStored.Directory, Cmd.Directory); } // FIXME: Changing compile commands should be enough to invalidate the cache. FS.Files[testPath("A.cc")] = " "; - Cmd.CommandLine = {"/bin/clang++", "../A.cc", "-Dfoo", "-fsyntax-only"}; + Cmd.CommandLine = {"clang++", "../A.cc", "-Dfoo", "-fsyntax-only"}; CDB.setCompileCommand(testPath("build/../A.cc"), Cmd); ASSERT_TRUE(Idx.blockUntilIdleForTest()); @@ -574,7 +566,6 @@ TEST_F(BackgroundIndexTest, CmdLineHash) { { tooling::CompileCommand CmdStored = *MSS.loadShard(testPath("A.cc"))->Cmd; - EXPECT_THAT(CmdStored.CommandLine, HasPrefix(Cmd.CommandLine)); EXPECT_EQ(CmdStored.CommandLine, Cmd.CommandLine); EXPECT_EQ(CmdStored.Directory, Cmd.Directory); } diff --git a/clang-tools-extra/clangd/unittests/GlobalCompilationDatabaseTests.cpp b/clang-tools-extra/clangd/unittests/GlobalCompilationDatabaseTests.cpp index 857bf26f00dc..6ac363c5933e 100644 --- a/clang-tools-extra/clangd/unittests/GlobalCompilationDatabaseTests.cpp +++ b/clang-tools-extra/clangd/unittests/GlobalCompilationDatabaseTests.cpp @@ -102,20 +102,11 @@ TEST_F(OverlayCDBTest, GetCompileCommand) { Contains("-DA=3")); } -// Remove -isysroot injected on mac, if present, to simplify tests. -std::vector<std::string> stripSysroot(std::vector<std::string> Cmd) { - // Allow -isysroot injection on Mac. - if (Cmd.size() > 2 && Cmd[Cmd.size() - 2] == "-isysroot") - Cmd.resize(Cmd.size() - 2); - return Cmd; -} - TEST_F(OverlayCDBTest, GetFallbackCommand) { OverlayCDB CDB(Base.get(), {"-DA=4"}); - EXPECT_THAT( - stripSysroot(CDB.getFallbackCommand(testPath("bar.cc")).CommandLine), - ElementsAre(EndsWith("clang"), "-DA=2", testPath("bar.cc"), "-DA=4", - "-fsyntax-only", StartsWith("-resource-dir"))); + EXPECT_THAT(CDB.getFallbackCommand(testPath("bar.cc")).CommandLine, + ElementsAre("clang", "-DA=2", testPath("bar.cc"), "-DA=4", + "-fsyntax-only", StartsWith("-resource-dir"))); } TEST_F(OverlayCDBTest, NoBase) { @@ -126,10 +117,9 @@ TEST_F(OverlayCDBTest, NoBase) { EXPECT_THAT(CDB.getCompileCommand(testPath("bar.cc"))->CommandLine, Contains("-DA=5")); - EXPECT_THAT( - stripSysroot(CDB.getFallbackCommand(testPath("foo.cc")).CommandLine), - ElementsAre(EndsWith("clang"), testPath("foo.cc"), "-DA=6", - "-fsyntax-only")); + EXPECT_THAT(CDB.getFallbackCommand(testPath("foo.cc")).CommandLine, + ElementsAre(EndsWith("clang"), testPath("foo.cc"), "-DA=6", + "-fsyntax-only")); } TEST_F(OverlayCDBTest, Watch) { _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
