Merge pull request #11523 from github/alexdenisov/preserve-symlinks

Swift: resolve symlinks conditionally

swift/extractor/SwiftExtractor.cpp

@@ -12,15 +12,12 @@
 #include "swift/extractor/translators/SwiftVisitor.h"
 #include "swift/extractor/TargetTrapFile.h"
 #include "swift/extractor/SwiftBuiltinSymbols.h"
+#include "swift/extractor/infra/Path.h"
 
 using namespace codeql;
 using namespace std::string_literals;
 namespace fs = std::filesystem;
 
-static fs::path toPath(llvm::StringRef s) {
-  return {static_cast<std::string_view>(s)};
-}
-
 static void ensureDirectory(const char* label, const fs::path& dir) {
   std::error_code ec;
   fs::create_directories(dir, ec);
@@ -34,7 +31,7 @@ static void archiveFile(const SwiftExtractorConfiguration& config, swift::Source
   ensureDirectory("TRAP", config.trapDir);
   ensureDirectory("source archive", config.sourceArchiveDir);
 
-  fs::path srcFilePath = fs::absolute(toPath(file.getFilename()));
+  fs::path srcFilePath = codeql::getCodeQLPath(file.getFilename());
   auto dstFilePath = config.sourceArchiveDir;
   dstFilePath += srcFilePath;
 
@@ -60,7 +57,7 @@ static fs::path getFilename(swift::ModuleDecl& module, swift::SourceFile* primar
     // Moreover, pcm files may come from caches located in different directories, but are
     // unambiguously identified by the base file name, so we can discard the absolute directory
     fs::path filename = "/pcms";
-    filename /= toPath(module.getModuleFilename()).filename();
+    filename /= getCodeQLPath(module.getModuleFilename()).filename();
     filename += "-";
     filename += module.getName().str();
     return filename;
@@ -69,7 +66,7 @@ static fs::path getFilename(swift::ModuleDecl& module, swift::SourceFile* primar
     // The Builtin module has an empty filename, let's fix that
     return "/__Builtin__";
   }
-  auto filename = toPath(module.getModuleFilename());
+  auto filename = getCodeQLPath(module.getModuleFilename());
   // there is a special case of a module without an actual filename reporting `<imports>`: in this
   // case we want to avoid the `<>` characters, in case a dirty DB is imported on Windows
   if (filename == "<imports>") {

swift/extractor/infra/Path.cpp

@@ -0,0 +1,38 @@
+#include "swift/extractor/infra/Path.h"
+#include <iostream>
+#include <unistd.h>
+
+namespace codeql {
+
+static bool shouldCanonicalize() {
+  auto preserve = getenv("CODEQL_PRESERVE_SYMLINKS");
+  if (preserve && std::string(preserve) == "true") {
+    return false;
+  }
+  preserve = getenv("SEMMLE_PRESERVE_SYMLINKS");
+  if (preserve && std::string(preserve) == "true") {
+    return false;
+  }
+  return true;
+}
+
+std::filesystem::path getCodeQLPath(std::string_view path) {
+  // TODO: this needs more testing
+  // TODO: check canonicalization of names on a case insensitive filesystems
+  std::error_code ec;
+  std::filesystem::path ret = {};
+  static const auto canonicalize = shouldCanonicalize();
+  if (canonicalize) {
+    ret = std::filesystem::canonical(path, ec);
+  } else {
+    ret = std::filesystem::absolute(path, ec);
+  }
+  if (ec) {
+    std::cerr << "Cannot get " << (canonicalize ? "canonical" : "absolute")
+              << " path: " << std::quoted(path) << ": " << ec.message() << "\n";
+    return {};
+  }
+  return ret;
+}
+
+}  // namespace codeql

swift/extractor/infra/Path.h

@@ -0,0 +1,7 @@
+#pragma once
+
+#include <filesystem>
+
+namespace codeql {
+std::filesystem::path getCodeQLPath(std::string_view path);
+}

swift/extractor/infra/SwiftLocationExtractor.cpp

@@ -5,22 +5,10 @@
 #include "swift/extractor/trap/generated/TrapEntries.h"
 #include "swift/extractor/trap/generated/TrapClasses.h"
 #include "swift/extractor/infra/SwiftLocationExtractor.h"
+#include "swift/extractor/infra/Path.h"
 
 using namespace codeql;
 
-static std::filesystem::path getFilePath(std::string_view path) {
-  // TODO: this needs more testing
-  // TODO: check canonicalization of names on a case insensitive filesystems
-  // TODO: make symlink resolution conditional on CODEQL_PRESERVE_SYMLINKS=true
-  std::error_code ec;
-  auto ret = std::filesystem::canonical(path, ec);
-  if (ec) {
-    std::cerr << "Cannot get real path: " << std::quoted(path) << ": " << ec.message() << "\n";
-    return {};
-  }
-  return ret;
-}
-
 void SwiftLocationExtractor::attachLocation(const swift::SourceManager& sourceManager,
                                             swift::SourceLoc start,
                                             swift::SourceLoc end,
@@ -29,7 +17,7 @@ void SwiftLocationExtractor::attachLocation(const swift::SourceManager& sourceMa
     // invalid locations seem to come from entities synthesized by the compiler
     return;
   }
-  auto file = getFilePath(sourceManager.getDisplayNameForLoc(start));
+  auto file = getCodeQLPath(sourceManager.getDisplayNameForLoc(start));
   DbLocation entry{{}};
   entry.file = fetchFileLabel(file);
   std::tie(entry.start_line, entry.start_column) = sourceManager.getLineAndColumnInBuffer(start);
@@ -42,7 +30,7 @@ void SwiftLocationExtractor::attachLocation(const swift::SourceManager& sourceMa
 }
 
 void SwiftLocationExtractor::emitFile(llvm::StringRef path) {
-  fetchFileLabel(getFilePath(path));
+  fetchFileLabel(getCodeQLPath(path));
 }
 
 TrapLabel<FileTag> SwiftLocationExtractor::fetchFileLabel(const std::filesystem::path& file) {

swift/integration-tests/posix-only/symlinks/.gitignore

@@ -0,0 +1 @@
+*/Sources/A.swift

swift/integration-tests/posix-only/symlinks/Files.expected

@@ -0,0 +1,5 @@
+| file://:0:0:0:0 |  |
+| main.swift:0:0:0:0 | main.swift |
+| preserve/Package.swift:0:0:0:0 | preserve/Package.swift |
+| preserve/Sources/A.swift:0:0:0:0 | preserve/Sources/A.swift |
+| resolve/Package.swift:0:0:0:0 | resolve/Package.swift |

swift/integration-tests/posix-only/symlinks/Files.ql

@@ -0,0 +1,4 @@
+import swift
+
+from File f
+select f

swift/integration-tests/posix-only/symlinks/main.swift

@@ -0,0 +1,6 @@
+public struct preserve {
+    public private(set) var text = "Hello, World!"
+
+    public init() {
+    }
+}

swift/integration-tests/posix-only/symlinks/preserve/Package.swift

@@ -0,0 +1,18 @@
+// swift-tools-version: 5.7
+// The swift-tools-version declares the minimum version of Swift required to build this package.
+
+import PackageDescription
+
+let package = Package(
+    name: "preserve",
+    products: [
+        .library(
+            name: "preserve",
+            targets: ["preserve"]),
+    ],
+    targets: [
+        .target(
+            name: "preserve",
+            path: "Sources"),
+    ]
+)

swift/integration-tests/posix-only/symlinks/resolve/Package.swift

@@ -0,0 +1,18 @@
+// swift-tools-version: 5.7
+// The swift-tools-version declares the minimum version of Swift required to build this package.
+
+import PackageDescription
+
+let package = Package(
+    name: "resolve",
+    products: [
+        .library(
+            name: "resolve",
+            targets: ["resolve"]),
+    ],
+    targets: [
+        .target(
+            name: "resolve",
+            path: "Sources"),
+    ]
+)

swift/integration-tests/posix-only/symlinks/test.py

@@ -0,0 +1,20 @@
+from create_database_utils import *
+import os
+
+symlinks = ['preserve/Sources/A.swift', 'resolve/Sources/A.swift']
+
+for s in symlinks:
+    try:
+        os.symlink(os.getcwd() + '/main.swift', s)
+    except:
+        pass
+
+run_codeql_database_create([
+    'swift package clean --package-path resolve',
+    'swift build --package-path resolve',
+    'swift package clean --package-path preserve',
+    'env CODEQL_PRESERVE_SYMLINKS=true swift build --package-path preserve'
+], lang='swift', keep_trap=True)
+
+for s in symlinks:
+    os.unlink(s)