[clang-tools-extra] r284020 - [clang-move] Compare with real paths of symlinks

[Haojian Wu via cfe-commits]

Author: hokein
Date: Wed Oct 12 10:50:30 2016
New Revision: 284020

URL: http://llvm.org/viewvc/llvm-project?rev=284020&view=rev
Log:
[clang-move] Compare with real paths of symlinks

Summary: MakeAbsolutePath does wrong things with symlinks previously. When comparing with a symlink, we need to compare with the real path of it. This fixes issues when the build directory is a symlink.

Reviewers: ioeric

Subscribers: beanz, mgorny, cfe-commits, bkramer

Differential Revision: https://reviews.llvm.org/D25508

Modified:
    clang-tools-extra/trunk/clang-move/ClangMove.cpp
    clang-tools-extra/trunk/test/clang-move/Inputs/database_template.json
    clang-tools-extra/trunk/test/clang-move/move-class.cpp

Modified: clang-tools-extra/trunk/clang-move/ClangMove.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-move/ClangMove.cpp?rev=284020&r1=284019&r2=284020&view=diff
==============================================================================
--- clang-tools-extra/trunk/clang-move/ClangMove.cpp (original)
+++ clang-tools-extra/trunk/clang-move/ClangMove.cpp Wed Oct 12 10:50:30 2016
@@ -51,8 +51,18 @@ std::string MakeAbsolutePath(const Sourc
        SM.getFileManager().getVirtualFileSystem()->makeAbsolute(AbsolutePath))
     llvm::errs() << "Warning: could not make absolute file: '" <<  EC.message()
                  << '\n';
-  llvm::sys::path::remove_dots(AbsolutePath, /*remove_dot_dot=*/true);
-  llvm::sys::path::native(AbsolutePath);
+  // Handle symbolic link path cases.
+  // We are trying to get the real file path of the symlink.
+  const DirectoryEntry *Dir = SM.getFileManager().getDirectory(
+       llvm::sys::path::parent_path(AbsolutePath.str()));
+  if (Dir) {
+    StringRef DirName = SM.getFileManager().getCanonicalName(Dir);
+    SmallVector<char, 128> AbsoluteFilename;
+    llvm::sys::path::append(AbsoluteFilename, DirName,
+                            llvm::sys::path::filename(AbsolutePath.str()));
+    return llvm::StringRef(AbsoluteFilename.data(), AbsoluteFilename.size())
+        .str();
+  }
   return AbsolutePath.str();
 }
 
@@ -382,24 +392,27 @@ void ClangMoveTool::addIncludes(llvm::St
                                 llvm::StringRef SearchPath,
                                 llvm::StringRef FileName,
                                 const SourceManager& SM) {
-  auto AbsoluteSearchPath = MakeAbsolutePath(SM, SearchPath);
+  SmallVector<char, 128> HeaderWithSearchPath;
+  llvm::sys::path::append(HeaderWithSearchPath, SearchPath, IncludeHeader);
+  std::string AbsoluteOldHeader =
+      MakeAbsolutePath(OriginalRunningDirectory, Spec.OldHeader);
   // FIXME: Add old.h to the new.cc/h when the new target has dependencies on
   // old.h/c. For instance, when moved class uses another class defined in
   // old.h, the old.h should be added in new.h.
-  if (MakeAbsolutePath(OriginalRunningDirectory, Spec.OldHeader) ==
-      MakeAbsolutePath(AbsoluteSearchPath, IncludeHeader))
+  if (AbsoluteOldHeader ==
+      MakeAbsolutePath(SM, llvm::StringRef(HeaderWithSearchPath.data(),
+                                           HeaderWithSearchPath.size())))
     return;
 
   std::string IncludeLine =
       IsAngled ? ("#include <" + IncludeHeader + ">\n").str()
                : ("#include \"" + IncludeHeader + "\"\n").str();
 
-  std::string AbsolutePath = MakeAbsolutePath(SM, FileName);
-  if (MakeAbsolutePath(OriginalRunningDirectory, Spec.OldHeader) ==
-      AbsolutePath) {
+  std::string AbsoluteCurrentFile = MakeAbsolutePath(SM, FileName);
+  if (AbsoluteOldHeader == AbsoluteCurrentFile) {
     HeaderIncludes.push_back(IncludeLine);
   } else if (MakeAbsolutePath(OriginalRunningDirectory, Spec.OldCC) ==
-             AbsolutePath) {
+             AbsoluteCurrentFile) {
     CCIncludes.push_back(IncludeLine);
   }
 }

Modified: clang-tools-extra/trunk/test/clang-move/Inputs/database_template.json
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-move/Inputs/database_template.json?rev=284020&r1=284019&r2=284020&view=diff
==============================================================================
--- clang-tools-extra/trunk/test/clang-move/Inputs/database_template.json (original)
+++ clang-tools-extra/trunk/test/clang-move/Inputs/database_template.json Wed Oct 12 10:50:30 2016
@@ -1,7 +1,7 @@
 [
 {
   "directory": "$test_dir/build",
-  "command": "clang++ -o test.o $test_dir/test.cpp",
-  "file": "$test_dir/test.cpp"
+  "command": "clang++ -o test.o -I../include $test_dir/src/test.cpp",
+  "file": "$test_dir/src/test.cpp"
 }
 ]

Modified: clang-tools-extra/trunk/test/clang-move/move-class.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-move/move-class.cpp?rev=284020&r1=284019&r2=284020&view=diff
==============================================================================
--- clang-tools-extra/trunk/test/clang-move/move-class.cpp (original)
+++ clang-tools-extra/trunk/test/clang-move/move-class.cpp Wed Oct 12 10:50:30 2016
@@ -1,21 +1,25 @@
 // RUN: mkdir -p %T/clang-move/build
+// RUN: mkdir -p %T/clang-move/include
+// RUN: mkdir -p %T/clang-move/src
 // RUN: sed 's|$test_dir|%/T/clang-move|g' %S/Inputs/database_template.json > %T/clang-move/compile_commands.json
-// RUN: cp %S/Inputs/test*  %T/clang-move/
-// RUN: touch %T/clang-move/test2.h
-// RUN: cd %T/clang-move
-// RUN: clang-move -name="a::Foo" -new_cc=%T/clang-move/new_test.cpp -new_header=%T/clang-move/new_test.h -old_cc=../clang-move/test.cpp -old_header=../clang-move/test.h %T/clang-move/test.cpp
+// RUN: cp %S/Inputs/test.h  %T/clang-move/include
+// RUN: cp %S/Inputs/test.cpp %T/clang-move/src
+// RUN: touch %T/clang-move/include/test2.h
+// RUN: cd %T/clang-move/build
+// RUN: clang-move -name="a::Foo" -new_cc=%T/clang-move/new_test.cpp -new_header=%T/clang-move/new_test.h -old_cc=../src/test.cpp -old_header=../include/test.h %T/clang-move/src/test.cpp
 // RUN: FileCheck -input-file=%T/clang-move/new_test.cpp -check-prefix=CHECK-NEW-TEST-CPP %s
 // RUN: FileCheck -input-file=%T/clang-move/new_test.h -check-prefix=CHECK-NEW-TEST-H %s
-// RUN: FileCheck -input-file=%T/clang-move/test.cpp -check-prefix=CHECK-OLD-TEST-CPP %s
-// RUN: FileCheck -input-file=%T/clang-move/test.h %s -implicit-check-not='{{namespace.*}}'
+// RUN: FileCheck -input-file=%T/clang-move/src/test.cpp -check-prefix=CHECK-OLD-TEST-CPP %s
+// RUN: FileCheck -input-file=%T/clang-move/include/test.h %s -implicit-check-not='{{namespace.*}}'
 //
-// RUN: cp %S/Inputs/test*  %T/clang-move/
-// RUN: cd %T/clang-move
-// RUN: clang-move -name="a::Foo" -new_cc=%T/clang-move/new_test.cpp -new_header=%T/clang-move/new_test.h -old_cc=%T/clang-move/test.cpp -old_header=%T/clang-move/test.h %T/clang-move/test.cpp
+// RUN: cp %S/Inputs/test.h  %T/clang-move/include
+// RUN: cp %S/Inputs/test.cpp %T/clang-move/src
+// RUN: cd %T/clang-move/build
+// RUN: clang-move -name="a::Foo" -new_cc=%T/clang-move/new_test.cpp -new_header=%T/clang-move/new_test.h -old_cc=%T/clang-move/src/test.cpp -old_header=%T/clang-move/include/test.h %T/clang-move/src/test.cpp
 // RUN: FileCheck -input-file=%T/clang-move/new_test.cpp -check-prefix=CHECK-NEW-TEST-CPP %s
 // RUN: FileCheck -input-file=%T/clang-move/new_test.h -check-prefix=CHECK-NEW-TEST-H %s
-// RUN: FileCheck -input-file=%T/clang-move/test.cpp -check-prefix=CHECK-OLD-TEST-CPP %s
-// RUN: FileCheck -input-file=%T/clang-move/test.h %s -implicit-check-not='{{namespace.*}}'
+// RUN: FileCheck -input-file=%T/clang-move/src/test.cpp -check-prefix=CHECK-OLD-TEST-CPP %s
+// RUN: FileCheck -input-file=%T/clang-move/include/test.h %s -implicit-check-not='{{namespace.*}}'
 //
 // CHECK-NEW-TEST-H: namespace a {
 // CHECK-NEW-TEST-H: class Foo {