[PATCH] D96795: [FileCollector] Fix that the file system case-sensitivity check was inverted

Raphael Isemann via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Feb 16 09:10:34 PST 2021


teemperor created this revision.
teemperor added a reviewer: JDevlieghere.
Herald added subscribers: dexonsmith, hiraditya.
teemperor requested review of this revision.
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

real_path returns an `std::error_code` which evaluates to `true` in case an error
happens and `false` if not. This code was checking the inverse, so case-insensitive
file systems ended up being detected as case sensitive.

Tested using an LLDB reproducer test as we anyway need a real file system and
also some matching logic to detect whether the respective file system is case-sensitive
(which the test is doing via some Python checks that we can't really emulate with
the usual FileCheck logic).

Fixes rdar://67003004


https://reviews.llvm.org/D96795

Files:
  lldb/test/API/functionalities/reproducers/fs-case-sensitivity/TestReproducerFSCaseSensitivity.py
  llvm/lib/Support/FileCollector.cpp


Index: llvm/lib/Support/FileCollector.cpp
===================================================================
--- llvm/lib/Support/FileCollector.cpp
+++ llvm/lib/Support/FileCollector.cpp
@@ -35,7 +35,7 @@
   SmallString<256> TmpDest = Path, UpperDest, RealDest;
 
   // Remove component traversals, links, etc.
-  if (!sys::fs::real_path(Path, TmpDest))
+  if (sys::fs::real_path(Path, TmpDest))
     return true; // Current default value in vfs.yaml
   Path = TmpDest;
 
@@ -44,7 +44,7 @@
   // sensitive in the absence of real_path, since this is the YAMLVFSWriter
   // default.
   UpperDest = Path.upper();
-  if (sys::fs::real_path(UpperDest, RealDest) && Path.equals(RealDest))
+  if (!sys::fs::real_path(UpperDest, RealDest) && Path.equals(RealDest))
     return false;
   return true;
 }
Index: lldb/test/API/functionalities/reproducers/fs-case-sensitivity/TestReproducerFSCaseSensitivity.py
===================================================================
--- /dev/null
+++ lldb/test/API/functionalities/reproducers/fs-case-sensitivity/TestReproducerFSCaseSensitivity.py
@@ -0,0 +1,57 @@
+"""
+Test if the reproducer correctly detects whether the file system is case sensitive.
+"""
+
+import lldb
+import tempfile
+from lldbsuite.test import lldbtest_config
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+class ReproducerFileSystemSensitivityTestCase(TestBase):
+
+    mydir = TestBase.compute_mydir(__file__)
+    NO_DEBUG_INFO_TESTCASE = True
+
+    @skipIfNetBSD
+    @skipIfWindows
+    @skipIfRemote
+    @skipIfiOSSimulator
+    @skipIfReproducer
+    def test_reproducer_attach(self):
+        # The reproducer output path. Note that this is on purpose a lower-case
+        # file name. See the case-sensitivity check below.
+        reproducer = self.getBuildArtifact('test.reproducer')
+        try:
+            shutil.rmtree(reproducer)
+        except OSError:
+            pass
+        # Use Popen because pexpect is overkill and spawnSubprocess is
+        # asynchronous.
+        capture = subprocess.Popen([
+            lldbtest_config.lldbExec, '-b', '--no-lldbinit', '--no-use-colors']
+            + sum(map(lambda x: ['-O', x], self.setUpCommands()), [])
+            + ['--capture', '--capture-path', reproducer,
+            '-o', 'reproducer generate'
+        ],
+                                   stdin=subprocess.PIPE,
+                                   stdout=subprocess.PIPE,
+                                   stderr=subprocess.PIPE)
+        outs, _ = capture.communicate()
+        outs = outs.decode('utf-8')
+        self.assertIn('Reproducer written', outs)
+
+        # Read in the YAML file. We only care about a single value, so no
+        # need to parse the full file.
+        with open(os.path.join(reproducer, "files.yaml"), 'r') as file:
+            files_yaml = file.read()
+
+        # Detect the file system case sensitivity by checking if we can
+        # find the reproducer path after converting it to upper case (the
+        # file name is lower case before conversion, so this only works
+        # on case insensitive file systems).
+        case_sensitive = "false" if os.path.exists(reproducer.upper()) else "true"
+
+        self.assertIn("'case-sensitive': '" + case_sensitive + "'", files_yaml)


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D96795.324027.patch
Type: text/x-patch
Size: 3352 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20210216/8e9b2500/attachment.bin>


More information about the llvm-commits mailing list