llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang-tools-extra Author: Bartosz Wiklak (bwiklak) <details> <summary>Changes</summary> The main use case is to serve windows index on linux server. Without this change server wasn't able to check if project_root path is absolute and was closed with error: "Index root should be an absolute path." This change is meant to solve issue: https://github.com/clangd/clangd/issues/2646 --- Full diff: https://github.com/llvm/llvm-project/pull/207202.diff 4 Files Affected: - (modified) clang-tools-extra/clangd/index/remote/marshalling/Marshalling.cpp (+4-2) - (modified) clang-tools-extra/clangd/index/remote/server/Server.cpp (+2-1) - (added) clang-tools-extra/clangd/test/remote-index/cross-platform-root.test (+9) - (modified) clang-tools-extra/clangd/unittests/remote/MarshallingTests.cpp (+33) ``````````diff diff --git a/clang-tools-extra/clangd/index/remote/marshalling/Marshalling.cpp b/clang-tools-extra/clangd/index/remote/marshalling/Marshalling.cpp index aa607926a6d51..903583414dd43 100644 --- a/clang-tools-extra/clangd/index/remote/marshalling/Marshalling.cpp +++ b/clang-tools-extra/clangd/index/remote/marshalling/Marshalling.cpp @@ -59,14 +59,16 @@ Marshaller::Marshaller(llvm::StringRef RemoteIndexRoot, : Strings(Arena) { llvm::StringRef PosixSeparator = get_separator(Style::posix); if (!RemoteIndexRoot.empty()) { - assert(is_absolute(RemoteIndexRoot)); + assert(is_absolute(RemoteIndexRoot, Style::posix) || + is_absolute(RemoteIndexRoot, Style::windows)); this->RemoteIndexRoot = convert_to_slash(RemoteIndexRoot, Style::windows); llvm::StringRef Path(this->RemoteIndexRoot); if (!is_separator(this->RemoteIndexRoot.back(), Style::posix)) this->RemoteIndexRoot += PosixSeparator; } if (!LocalIndexRoot.empty()) { - assert(is_absolute(LocalIndexRoot)); + assert(is_absolute(LocalIndexRoot, Style::posix) || + is_absolute(LocalIndexRoot, Style::windows)); this->LocalIndexRoot = convert_to_slash(LocalIndexRoot, Style::windows); llvm::StringRef Path(this->LocalIndexRoot); if (!is_separator(this->LocalIndexRoot.back(), Style::posix)) diff --git a/clang-tools-extra/clangd/index/remote/server/Server.cpp b/clang-tools-extra/clangd/index/remote/server/Server.cpp index 87e4c2d938e63..f3602515691ce 100644 --- a/clang-tools-extra/clangd/index/remote/server/Server.cpp +++ b/clang-tools-extra/clangd/index/remote/server/Server.cpp @@ -589,7 +589,8 @@ int main(int argc, char *argv[]) { llvm::sys::PrintStackTraceOnErrorSignal(argv[0]); llvm::sys::SetInterruptFunction(&clang::clangd::requestShutdown); - if (!llvm::sys::path::is_absolute(IndexRoot)) { + if (!llvm::sys::path::is_absolute(IndexRoot, llvm::sys::path::Style::posix) && + !llvm::sys::path::is_absolute(IndexRoot, llvm::sys::path::Style::windows)) { llvm::errs() << "Index root should be an absolute path.\n"; return -1; } diff --git a/clang-tools-extra/clangd/test/remote-index/cross-platform-root.test b/clang-tools-extra/clangd/test/remote-index/cross-platform-root.test new file mode 100644 index 0000000000000..04148283b7c15 --- /dev/null +++ b/clang-tools-extra/clangd/test/remote-index/cross-platform-root.test @@ -0,0 +1,9 @@ +# Verify that clangd-index-server accepts Windows-style absolute paths as +# project root, allowing the server to run on Linux with a Windows-built index. +# "C:/project" is absolute in Windows style but not POSIX style. +# REQUIRES: clangd-remote-index +# RUN: rm -f %t.idx +# RUN: not clangd-index-server --server-address=localhost:0 %t.idx "C:/project" 2>&1 | FileCheck %s +# The server should pass the path validation and fail later on missing index. +# CHECK-NOT: Index root should be an absolute path +# CHECK: does not exist diff --git a/clang-tools-extra/clangd/unittests/remote/MarshallingTests.cpp b/clang-tools-extra/clangd/unittests/remote/MarshallingTests.cpp index 85e79eae331bc..b4f5c7d89341b 100644 --- a/clang-tools-extra/clangd/unittests/remote/MarshallingTests.cpp +++ b/clang-tools-extra/clangd/unittests/remote/MarshallingTests.cpp @@ -449,6 +449,39 @@ TEST(RemoteMarshallingTest, URIToRelativePathTranslation) { llvm::consumeError(RelativePath.takeError()); } +TEST(RemoteMarshallingTest, CrossPlatformPathsRoundTrip) { + llvm::BumpPtrAllocator Arena; + llvm::UniqueStringSaver Strings(Arena); + + // Simulate a Windows-built index + Marshaller ProtobufMarshaller("C:/remote/project/", + testPath("local/project/")); + + clangd::Ref Ref; + Ref.Kind = clangd::RefKind::Declaration; + clangd::SymbolLocation Location; + Location.Start.setLine(1); + Location.Start.setColumn(2); + Location.End.setLine(3); + Location.End.setColumn(4); + // Construct the URI as a Windows machine would have serialized it into the + // index: file:///C:/remote/project/lib/File.cpp. + Location.FileURI = + Strings.save("file:///C:/remote/project/lib/File.cpp").begin(); + Ref.Location = Location; + + auto Serialized = ProtobufMarshaller.toProtobuf(Ref); + ASSERT_TRUE(bool(Serialized)); + // Wire path should be relative with forward slashes. + EXPECT_EQ(Serialized->location().file_path(), "lib/File.cpp"); + + auto Deserialized = ProtobufMarshaller.fromProtobuf(*Serialized); + ASSERT_TRUE(bool(Deserialized)); + // Deserialized URI should be under the local root. + EXPECT_STREQ(Deserialized->Location.FileURI, + testPathURI("local/project/lib/File.cpp", Strings)); +} + } // namespace } // namespace remote } // namespace clangd `````````` </details> https://github.com/llvm/llvm-project/pull/207202 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
