feat: Add support for cross-repo code navigation (#338)

Author: varungandhi-src

README.md

@@ -1,7 +1,8 @@
 # scip-clang: SCIP indexer for C and C++ ![(Status: Beta)](https://img.shields.io/badge/status-beta-yellow?style=flat)
 
-Status: scip-clang currently supports single-repository precise code navigation
-for C and C++.
+scip-clang is a precise code indexer based on Clang 16,
+which supports cross-repository code navigation for C and C++
+in Sourcegraph.
 
 Here are some code navigation examples in [llvm/llvm-project](https://sourcegraph.com/github.com/llvm/llvm-project):
 - [Find references for #include](https://sourcegraph.com/github.com/llvm/llvm-project@97a03eb2eb5acf269db6253fe540626b52950f97/-/blob/llvm/include/llvm/ADT/SmallSet.h?L1:1-1:81#tab=references)
@@ -24,7 +25,8 @@ You can also test out [code navigation in Chromium](https://sourcegraph.com/gith
   - [Generating a compilation database](#generating-a-compilation-database)
   - [Building code](#building-code)
   - [Initial scip-clang testing](#initial-scip-clang-testing)
-  - [Running scip-clang on the entire codebase](#running-scip-clang-on-the-entire-codebase)
+  - [Running scip-clang on a single repo](#running-scip-clang-on-the-entire-codebase)
+  - [Setting up cross-repo code navigation](#setting-up-cross-repo-code-navigation)
 - [Troubleshooting](#troubleshooting)
 - [Reporting issues](#reporting-issues)
 - [Documentation](#documentation)
@@ -138,7 +140,7 @@ If there are any other errors,
 such as standard library or platform headers not being found,
 please [report an issue](#reporting-issues).
 
-### Running scip-clang on the entire codebase
+### Running scip-clang on a single repo
 
 ```bash
 scip-clang --compdb-path=build/compile_commands.json
@@ -149,6 +151,10 @@ since scip-clang is still able to index code in the presence of
 compiler errors, and any errors in headers will get repeated
 for each translation unit in which the header is included.
 
+### Setting up cross-repo code navigation
+
+See the [cross-repository setup docs](/docs/CrossRepo.md).
+
 ## Troubleshooting
 
 See the [Troubleshooting docs](/docs/Troubleshooting.md).

docs/CrossRepo.md

@@ -0,0 +1,107 @@
+# Cross-repository code navigation
+
+scip-clang supports cross-repository configuration via explicit configuration.
+Before following the steps here, we recommend that you first
+[set up single-repository code navigation](/README.md#usage)
+and check that it works as expected.
+
+Specifically, each project (i.e. different repo in Sourcegraph)
+of interest needs to be indexed with
+an extra JSON file describing package information.
+
+```bash
+# Run from package0's root directory
+scip-clang --package-map-path=package-map.json <other flags>...
+```
+
+The package map JSON file contains a list of objects in the following format:
+
+```json
+[
+  {"path": ".", "package": "package0@v0"},
+  {"path": "path/to/package1_root", "package": "package1@v1"},
+  ...
+]
+```
+
+1. The `path` key may be an absolute path, or a path relative to the current directory
+   (which must be the project root).
+2. The `package` key consists of a `name` followed by an `@` separator and a `version`.
+   - The name and version must only contain characters belonging to `[a-zA-Z0-9_\-.]`.
+   - The version should be chosen based on release information.
+     For example, if you use git tags to mark releases in repos,
+     and repositories only depend on tagged releases (instead of arbitrary commits),
+     then the git tag can be used as the version.
+     The important thing is that the version needs to be consistent when
+     different projects are indexed, and it should not be reused over time.
+
+     The reason for this is that cross-repo code navigation works
+     by treating the concatenation of (1) the package name,
+     (2) the package version and (3) the qualified symbol name
+     (e.g `std::vector`) as the unique symbol ID across a Sourcegraph instance.
+
+Files under the directories `path/to/package1_root`
+will be treated as belonging to `package1`'s `v1` version.
+
+If one package root is a prefix of another, package information
+is assigned based on the longest match.
+
+For cross-repository navigation to work,
+`package1` must also be indexed with the same version information:
+
+```bash
+# This doesn't contain information about package0,
+# as package0 depends on package1, but not vice-versa.
+$ cat other-package-map.json
+[
+  {"path": ".", "package": "package1@v1"},
+  ...
+]
+$ scip-clang --package-map-path=other-package-map.json <other flags>...
+```
+
+Once both these indexing operations are performed and the indexes
+are uploaded to a Sourcegraph instance, cross-repository navigation
+should work across `package0` and `package1`.
+
+## Limitations
+
+At the moment, the amount of indexing work required
+scales quadratically with the depth of the dependency graph.
+Specifically, if package PC depends on PB which depends on PA,
+then PA will need to be indexed thrice (once by itself, once when PB is indexed,
+once when PC is indexed), PB will be indexed twice (once by itself,
+once when PC is indexed), and PC will be indexed once.
+
+The reason for this is that the indexer needs to identify
+package information for which declaration is defined in which package,
+so that it can correctly support code navigation for forward declarations.
+However, strictly speaking, any package can forward-declare any entity.
+
+For example, if PA defines a function f, and a header in PC
+forward-declares `pa::f`, then when C is indexed, the indexer
+needs to somehow know that the definition of `f` lives in some
+file in PA. There are broadly 3 possible ways to do this:
+
+1. Always index all TUs. This is the current strategy.
+   This is the build equivalent of building everything from source.
+2. Provide a way to reuse the indexes from dependencies,
+   and use those to resolve forward declarations.
+   This is the build equivalent of using archives/shared libraries/TBDs.
+3. Only index the TUs for the "current" package. If the
+   definition for a forward declaration is not found,
+   no reference information is emitted (slightly worse UX, but faster).
+   This is not supported directly in scip-clang,
+   but can be achieved by removing entries for
+   out-of-project files from the compilation database.
+4. Rely on some heuristics and/or user-supplied hint.
+   For example, if there was a way to provide a hint that
+   unresolved forward declarations in namespace `pa` must map
+   to declarations in package PA, then indexer could
+   trust that information and correctly emit a reference
+   for `pa::f` at the forward declaration without having
+   to re-index TUs in PA.
+
+We're [looking for feedback](https://github.com/sourcegraph/scip-clang/issues/358)
+on which approach would work best for your use case,
+before implementing a solution for this.

docs/IndexingProjects.md

@@ -1,5 +1,45 @@
 # Indexing test projects
 
+## scip-clang
+
+For the sake of this example, I'll only describe how
+to index scip-clang with cross-repo code navigation support
+for [Abseil](https://github.com/abseil/abseil-cpp/).
+
+First, index Abseil.
+
+```
+git checkout 4ffaea74
+cmake -B build -G Ninja -DCMAKE_EXPORT_COMPILE_COMMANDS=ON -DCMAKE_BUILD_TYPE=Debug -DCMAKE_CXX_FLAGS="-g0" -DABSL_BUILD_TESTING=ON -DABSL_USE_GOOGLETEST_HEAD=ON
+scip-clang --compdb-path=build/compile_commands.json --package-map=abseil-package-map.json
+```
+
+The `abseil-package-map.json` file used here is:
+
+```json
+[ {"path": ".", "package": "abseil-cpp@4ffaea74"} ]
+```
+
+Next, index scip-clang itself. We subset out the compilation
+database first to not index TUs in other packages.
+
+```bash
+jq '[.[] | select(.file | (contains("indexer/") or startswith("test/") or contains("com_google_absl")))]' compile_commands.json > min.json
+./bazel-bin/indexer/scip-clang --compdb-path=min.json --package-map-path=package-map.json
+```
+
+Here, the `package-map.json` is as follows:
+
+```json
+[
+  {"path": ".", "package": "scip-clang@v0.1.3"},
+  {"path": "./bazel-scip-clang/external/com_google_absl", "package": "abseil-cpp@4ffaea74"}
+]
+```
+
+The exact versions need to be determined based on current tag
+or hashes provided to Bazel.
+
 ## LLVM
 
 Tested environments: Ubuntu 18.04, Ubuntu 22.04, macOS 13.

indexer/AstConsumer.cc

@@ -192,7 +192,8 @@ void IndexerAstConsumer::HandleTranslationUnit(clang::ASTContext &astContext) {
   }
 
   FileMetadataMap fileMetadataMap{this->options.projectRootPath,
-                                  this->options.buildRootPath};
+                                  this->options.buildRootPath,
+                                  this->options.packageMap};
   FileIdsToBeIndexedSet toBeIndexed{};
   this->computeFileIdsToBeIndexed(astContext, emitIndexDetails,
                                   clangIdLookupMap, fileMetadataMap,

indexer/AstConsumer.h

@@ -21,6 +21,7 @@ class CompilerInstance;
 
 namespace scip_clang {
 class ClangIdLookupMap;
+class PackageMap;
 class FileMetadataMap;
 class IndexerPreprocessorWrapper;
 class MacroIndexer;
@@ -41,6 +42,7 @@ struct IndexerAstConsumerOptions {
   RootPath buildRootPath;
   WorkerCallback getEmitIndexDetails;
   bool deterministic;
+  PackageMap &packageMap;
 };
 
 struct TuIndexingOutput {

indexer/CliOptions.h

@@ -30,6 +30,7 @@ struct CliOptions {
   std::string temporaryOutputDir;
   std::string indexOutputPath;
   std::string statsFilePath;
+  std::string packageMapPath;
   bool showCompilerDiagnostics;
 
   size_t ipcSizeHintBytes;

indexer/Driver.cc

@@ -206,6 +206,7 @@ struct DriverOptions {
   AbsolutePath compdbPath;
   AbsolutePath indexOutputPath;
   AbsolutePath statsFilePath;
+  AbsolutePath packageMapPath;
   bool showCompilerDiagnostics;
   DriverIpcOptions ipcOptions;
   size_t numWorkers;
@@ -224,7 +225,7 @@ struct DriverOptions {
   explicit DriverOptions(std::string driverId, const CliOptions &cliOpts)
       : workerExecutablePath(),
         projectRootPath(AbsolutePath("/"), RootKind::Project), compdbPath(),
-        indexOutputPath(), statsFilePath(),
+        indexOutputPath(), statsFilePath(), packageMapPath(),
         showCompilerDiagnostics(cliOpts.showCompilerDiagnostics),
         ipcOptions{cliOpts.ipcSizeHintBytes, cliOpts.receiveTimeout},
         numWorkers(cliOpts.numWorkers), deterministic(cliOpts.deterministic),
@@ -279,6 +280,7 @@ struct DriverOptions {
     setAbsolutePath(cliOpts.indexOutputPath, this->indexOutputPath);
     setAbsolutePath(cliOpts.compdbPath, this->compdbPath);
     setAbsolutePath(cliOpts.statsFilePath, this->statsFilePath);
+    setAbsolutePath(cliOpts.packageMapPath, this->packageMapPath);
 
     auto makeDirs = [](const StdPath &path, const char *name) {
       std::error_code error;
@@ -322,6 +324,10 @@ struct DriverOptions {
     if (!this->statsFilePath.asStringRef().empty()) {
       args.push_back("--measure-statistics");
     }
+    if (!this->packageMapPath.asStringRef().empty()) {
+      args.push_back(
+          fmt::format("--package-map-path={}", packageMapPath.asStringRef()));
+    }
     if (this->noStacktrace) {
       args.push_back("--no-stack-trace");
     }

indexer/FileMetadata.h

@@ -58,6 +58,7 @@ struct PackageId {
 struct PackageMetadata {
   PackageId id;
   AbsolutePathRef rootPath;
+  bool isMainPackage;
 };
 
 /// Represents important metadata related to a file.

indexer/FileSystem.cc

@@ -0,0 +1,32 @@
+#include <string>
+
+#include "llvm/Support/raw_ostream.h"
+
+#include "indexer/Enforce.h"
+#include "indexer/FileSystem.h"
+
+namespace scip_clang {
+
+std::string readFileToString(const StdPath &path, std::error_code &ec) {
+  llvm::raw_fd_stream stream(path.c_str(), ec);
+  if (ec) {
+    return "";
+  }
+  std::string out;
+  char buf[16384] = {0};
+  while (true) {
+    auto nread = stream.read(buf, sizeof(buf));
+    if (nread == 0) {
+      break;
+    }
+    if (nread == -1) {
+      ec = stream.error();
+      return "";
+    }
+    ENFORCE(nread >= 1);
+    out.append(std::string_view(buf, nread));
+  }
+  return out;
+}
+
+} // namespace scip_clang

indexer/IdPathMappings.cc

@@ -85,7 +85,8 @@ bool FileMetadataMap::insert(clang::FileID fileId, AbsolutePathRef absPathRef) {
   ENFORCE(!absPathRef.asStringView().empty(),
           "inserting file with empty absolute path");
 
-  std::optional<PackageMetadata> optPackageMetadata = std::nullopt;
+  auto optPackageMetadata = this->packageMap.lookup(absPathRef);
+
   auto insertRelPath = [&](RootRelativePathRef relPathRef,
                            bool isInProject) -> bool {
     ENFORCE(!relPathRef.asStringView().empty(),
@@ -98,12 +99,24 @@ bool FileMetadataMap::insert(clang::FileID fileId, AbsolutePathRef absPathRef) {
     return this->map.insert({{fileId}, std::move(metadata)}).second;
   };
 
-  // In practice, CMake ends up passing paths to project files as well
-  // as files inside the build root. Normally, files inside the build root
-  // are generated ones, but to be safe, check if the corresponding file
-  // exists in the project. Since the build root itself is typically inside
-  // the project root, check the build root first.
-  if (auto buildRootRelPath = this->buildRootPath.tryMakeRelative(absPathRef)) {
+  if (optPackageMetadata.has_value()) {
+    if (auto optStrView =
+            optPackageMetadata->rootPath.makeRelative(absPathRef)) {
+      return insertRelPath(RootRelativePathRef(*optStrView, RootKind::External),
+                           /*isInProject*/ optPackageMetadata->isMainPackage);
+    } else {
+      spdlog::warn("package info map determined '{}' as root for path '{}', "
+                   "but prefix check failed",
+                   optPackageMetadata->rootPath.asStringView(),
+                   absPathRef.asStringView());
+    }
+    // In practice, CMake ends up passing paths to project files as well
+    // as files inside the build root. Normally, files inside the build root
+    // are generated ones, but to be safe, check if the corresponding file
+    // exists in the project. Since the build root itself is typically inside
+    // the project root, check the build root first.
+  } else if (auto buildRootRelPath =
+                 this->buildRootPath.tryMakeRelative(absPathRef)) {
     auto originalFileSourcePath =
         this->projectRootPath.makeAbsoluteAllowKindMismatch(
             buildRootRelPath.value());
@@ -123,18 +136,7 @@ bool FileMetadataMap::insert(clang::FileID fileId, AbsolutePathRef absPathRef) {
                  this->projectRootPath.tryMakeRelative(absPathRef)) {
     return insertRelPath(optProjectRootRelPath.value(), /*isInProject*/ true);
   }
-  if (optPackageMetadata.has_value()) {
-    if (auto optStrView =
-            optPackageMetadata->rootPath.makeRelative(absPathRef)) {
-      return insertRelPath(RootRelativePathRef(*optStrView, RootKind::External),
-                           /*isInProject*/ false);
-    } else {
-      spdlog::warn("package info map determined '{}' as root for path '{}', "
-                   "but prefix check failed",
-                   optPackageMetadata->rootPath.asStringView(),
-                   absPathRef.asStringView());
-    }
-  }
+
   auto optFileName = absPathRef.fileName();
   ENFORCE(optFileName.has_value(),
           "Clang returned file path {} without a file name",