diff --git a/clang/include/clang/Basic/FileManager.h b/clang/include/clang/Basic/FileManager.h index e83a61d6ff00c..769cfedae6018 100644 --- a/clang/include/clang/Basic/FileManager.h +++ b/clang/include/clang/Basic/FileManager.h @@ -319,13 +319,6 @@ class FileManager : public RefCountedBase { /// required, which is (almost) never. StringRef getCanonicalName(FileEntryRef File); -private: - /// Retrieve the canonical name for a given file or directory. - /// - /// The first param is a key in the CanonicalNames array. - StringRef getCanonicalName(const void *Entry, StringRef Name); - -public: void PrintStats() const; /// Import statistics from a child FileManager and add them to this current diff --git a/clang/lib/Basic/FileManager.cpp b/clang/lib/Basic/FileManager.cpp index 86fe352df0461..f0285f56c9476 100644 --- a/clang/lib/Basic/FileManager.cpp +++ b/clang/lib/Basic/FileManager.cpp @@ -618,48 +618,33 @@ void FileManager::GetUniqueIDMapping( } StringRef FileManager::getCanonicalName(DirectoryEntryRef Dir) { - return getCanonicalName(Dir, Dir.getName()); -} + auto Known = CanonicalNames.find(Dir); + if (Known != CanonicalNames.end()) + return Known->second; -StringRef FileManager::getCanonicalName(FileEntryRef File) { - return getCanonicalName(File, File.getName()); + StringRef CanonicalName(Dir.getName()); + + SmallString<4096> CanonicalNameBuf; + if (!FS->getRealPath(Dir.getName(), CanonicalNameBuf)) + CanonicalName = CanonicalNameBuf.str().copy(CanonicalNameStorage); + + CanonicalNames.insert({Dir, CanonicalName}); + return CanonicalName; } -StringRef FileManager::getCanonicalName(const void *Entry, StringRef Name) { +StringRef FileManager::getCanonicalName(FileEntryRef File) { llvm::DenseMap::iterator Known = - CanonicalNames.find(Entry); + CanonicalNames.find(File); if (Known != CanonicalNames.end()) return Known->second; - // Name comes from FileEntry/DirectoryEntry::getName(), so it is safe to - // store it in the DenseMap below. - StringRef CanonicalName(Name); - - SmallString<256> AbsPathBuf; - SmallString<256> RealPathBuf; - if (!FS->getRealPath(Name, RealPathBuf)) { - if (is_style_windows(llvm::sys::path::Style::native)) { - // For Windows paths, only use the real path if it doesn't resolve - // a substitute drive, as those are used to avoid MAX_PATH issues. - AbsPathBuf = Name; - if (!FS->makeAbsolute(AbsPathBuf)) { - if (llvm::sys::path::root_name(RealPathBuf) == - llvm::sys::path::root_name(AbsPathBuf)) { - CanonicalName = RealPathBuf.str().copy(CanonicalNameStorage); - } else { - // Fallback to using the absolute path. - // Simplifying /../ is semantically valid on Windows even in the - // presence of symbolic links. - llvm::sys::path::remove_dots(AbsPathBuf, /*remove_dot_dot=*/true); - CanonicalName = AbsPathBuf.str().copy(CanonicalNameStorage); - } - } - } else { - CanonicalName = RealPathBuf.str().copy(CanonicalNameStorage); - } - } + StringRef CanonicalName(File.getName()); + + SmallString<4096> CanonicalNameBuf; + if (!FS->getRealPath(File.getName(), CanonicalNameBuf)) + CanonicalName = CanonicalNameBuf.str().copy(CanonicalNameStorage); - CanonicalNames.insert({Entry, CanonicalName}); + CanonicalNames.insert({File, CanonicalName}); return CanonicalName; } diff --git a/clang/test/Lexer/case-insensitive-include-absolute.c b/clang/test/Lexer/case-insensitive-include-absolute.c index ef9e131c45df5..4274d05045ecf 100644 --- a/clang/test/Lexer/case-insensitive-include-absolute.c +++ b/clang/test/Lexer/case-insensitive-include-absolute.c @@ -1,8 +1,8 @@ // REQUIRES: case-insensitive-filesystem // RUN: rm -rf %t && split-file %s %t -// RUN: sed "s|DIR|%{/t:real}|g" %t/tu.c.in > %t/tu.c -// RUN: %clang_cc1 -fsyntax-only %t/tu.c 2>&1 | FileCheck %s -DDIR=%{/t:real} +// RUN: sed "s|DIR|%/t|g" %t/tu.c.in > %t/tu.c +// RUN: %clang_cc1 -fsyntax-only %t/tu.c 2>&1 | FileCheck %s -DDIR=%/t //--- header.h //--- tu.c.in diff --git a/clang/test/Lexer/case-insensitive-include-win.c b/clang/test/Lexer/case-insensitive-include-win.c index ed722feb3d994..6a17d0b552448 100644 --- a/clang/test/Lexer/case-insensitive-include-win.c +++ b/clang/test/Lexer/case-insensitive-include-win.c @@ -2,15 +2,9 @@ // This file should only include code that really needs a Windows host OS to // run. -// Note: We must use the real path here, because the logic to detect case -// mismatches must resolve the real path to figure out the original casing. -// If we use %t and we are on a substitute drive S: mapping to C:\subst, -// then we will compare "S:\test.dir\FOO.h" to "C:\subst\test.dir\foo.h" -// and avoid emitting the diagnostic because the structure is different. - // REQUIRES: system-windows // RUN: mkdir -p %t.dir // RUN: touch %t.dir/foo.h -// RUN: not %clang_cl /FI \\?\%{t:real}.dir\FOO.h /WX -fsyntax-only %s 2>&1 | FileCheck %s +// RUN: not %clang_cl /FI\\?\%t.dir\FOO.h /WX -fsyntax-only %s 2>&1 | FileCheck %s // CHECK: non-portable path to file '"\\?\ diff --git a/llvm/cmake/modules/AddLLVM.cmake b/llvm/cmake/modules/AddLLVM.cmake index 52bf7ca0906a3..2360f72e59d08 100644 --- a/llvm/cmake/modules/AddLLVM.cmake +++ b/llvm/cmake/modules/AddLLVM.cmake @@ -1887,15 +1887,10 @@ endfunction() # use it and can't be in a lit module. Use with make_paths_relative(). string(CONCAT LLVM_LIT_PATH_FUNCTION "# Allow generated file to be relocatable.\n" - "import os\n" - "import platform\n" + "from pathlib import Path\n" "def path(p):\n" " if not p: return ''\n" - " # Follows lit.util.abs_path_preserve_drive, which cannot be imported here.\n" - " if platform.system() == 'Windows':\n" - " return os.path.abspath(os.path.join(os.path.dirname(__file__), p))\n" - " else:\n" - " return os.path.realpath(os.path.join(os.path.dirname(__file__), p))\n" + " return str((Path(__file__).parent / p).resolve())\n" ) # This function provides an automatic way to 'configure'-like generate a file diff --git a/llvm/docs/CommandGuide/lit.rst b/llvm/docs/CommandGuide/lit.rst index 2a0ddd0ea04b4..46c5b9936239e 100644 --- a/llvm/docs/CommandGuide/lit.rst +++ b/llvm/docs/CommandGuide/lit.rst @@ -616,16 +616,6 @@ TestRunner.py: %/T %T but ``\`` is replaced by ``/`` %{s:basename} The last path component of %s %{t:stem} The last path component of %t but without the ``.tmp`` extension (alias for %basename_t) - %{s:real} %s after expanding all symbolic links and substitute drives - %{S:real} %S after expanding all symbolic links and substitute drives - %{p:real} %p after expanding all symbolic links and substitute drives - %{t:real} %t after expanding all symbolic links and substitute drives - %{T:real} %T after expanding all symbolic links and substitute drives - %{/s:real} %/s after expanding all symbolic links and substitute drives - %{/S:real} %/S after expanding all symbolic links and substitute drives - %{/p:real} %/p after expanding all symbolic links and substitute drives - %{/t:real} %/t after expanding all symbolic links and substitute drives - %{/T:real} %/T after expanding all symbolic links and substitute drives %{/s:regex_replacement} %/s but escaped for use in the replacement of a ``s@@@`` command in sed %{/S:regex_replacement} %/S but escaped for use in the replacement of a ``s@@@`` command in sed %{/p:regex_replacement} %/p but escaped for use in the replacement of a ``s@@@`` command in sed diff --git a/llvm/docs/TestingGuide.rst b/llvm/docs/TestingGuide.rst index b6dda6a732405..3a555d6bea5aa 100644 --- a/llvm/docs/TestingGuide.rst +++ b/llvm/docs/TestingGuide.rst @@ -757,7 +757,7 @@ RUN lines: ``%{fs-sep}`` Expands to the file system separator, i.e. ``/`` or ``\`` on Windows. -``%/s, %/S, %/t, %/T`` +``%/s, %/S, %/t, %/T:`` Act like the corresponding substitution above but replace any ``\`` character with a ``/``. This is useful to normalize path separators. @@ -766,17 +766,7 @@ RUN lines: Example: ``%/s: C:/Desktop Files/foo_test.s.tmp`` -``%{s:real}, %{S:real}, %{t:real}, %{T:real}`` -``%{/s:real}, %{/S:real}, %{/t:real}, %{/T:real}`` - - Act like the corresponding substitution, including with ``/``, but use - the real path by expanding all symbolic links and substitute drives. - - Example: ``%s: S:\foo_test.s.tmp`` - - Example: ``%{/s:real}: C:/SDrive/foo_test.s.tmp`` - -``%:s, %:S, %:t, %:T`` +``%:s, %:S, %:t, %:T:`` Act like the corresponding substitution above but remove colons at the beginning of Windows paths. This is useful to allow concatenation diff --git a/llvm/utils/lit/lit/TestRunner.py b/llvm/utils/lit/lit/TestRunner.py index 16e9c7fbf45c5..48650f5cda9c7 100644 --- a/llvm/utils/lit/lit/TestRunner.py +++ b/llvm/utils/lit/lit/TestRunner.py @@ -96,7 +96,7 @@ def change_dir(self, newdir): if os.path.isabs(newdir): self.cwd = newdir else: - self.cwd = lit.util.abs_path_preserve_drive(os.path.join(self.cwd, newdir)) + self.cwd = os.path.realpath(os.path.join(self.cwd, newdir)) class TimeoutHelper(object): @@ -455,7 +455,7 @@ def executeBuiltinMkdir(cmd, cmd_shenv): dir = to_unicode(dir) if kIsWindows else to_bytes(dir) cwd = to_unicode(cwd) if kIsWindows else to_bytes(cwd) if not os.path.isabs(dir): - dir = lit.util.abs_path_preserve_drive(os.path.join(cwd, dir)) + dir = os.path.realpath(os.path.join(cwd, dir)) if parent: lit.util.mkdir_p(dir) else: @@ -501,7 +501,7 @@ def on_rm_error(func, path, exc_info): path = to_unicode(path) if kIsWindows else to_bytes(path) cwd = to_unicode(cwd) if kIsWindows else to_bytes(cwd) if not os.path.isabs(path): - path = lit.util.abs_path_preserve_drive(os.path.join(cwd, path)) + path = os.path.realpath(os.path.join(cwd, path)) if force and not os.path.exists(path): continue try: @@ -1399,11 +1399,19 @@ def getDefaultSubstitutions(test, tmpDir, tmpBase, normalize_slashes=False): tmpBaseName = os.path.basename(tmpBase) sourceBaseName = os.path.basename(sourcepath) - substitutions.append(("%{pathsep}", os.pathsep)) - substitutions.append(("%basename_t", tmpBaseName)) - - substitutions.append(("%{s:basename}", sourceBaseName)) - substitutions.append(("%{t:stem}", tmpBaseName)) + substitutions.extend( + [ + ("%s", sourcepath), + ("%S", sourcedir), + ("%p", sourcedir), + ("%{pathsep}", os.pathsep), + ("%t", tmpName), + ("%basename_t", tmpBaseName), + ("%T", tmpDir), + ("%{s:basename}", sourceBaseName), + ("%{t:stem}", tmpBaseName), + ] + ) substitutions.extend( [ @@ -1413,47 +1421,49 @@ def getDefaultSubstitutions(test, tmpDir, tmpBase, normalize_slashes=False): ] ) - substitutions.append(("%/et", tmpName.replace("\\", "\\\\\\\\\\\\\\\\"))) + # "%/[STpst]" should be normalized. + substitutions.extend( + [ + ("%/s", sourcepath.replace("\\", "/")), + ("%/S", sourcedir.replace("\\", "/")), + ("%/p", sourcedir.replace("\\", "/")), + ("%/t", tmpBase.replace("\\", "/") + ".tmp"), + ("%/T", tmpDir.replace("\\", "/")), + ("%/et", tmpName.replace("\\", "\\\\\\\\\\\\\\\\")), + ] + ) + # "%{/[STpst]:regex_replacement}" should be normalized like "%/[STpst]" but we're + # also in a regex replacement context of a s@@@ regex. def regex_escape(s): s = s.replace("@", r"\@") s = s.replace("&", r"\&") return s - path_substitutions = [ - ("s", sourcepath), ("S", sourcedir), ("p", sourcedir), - ("t", tmpName), ("T", tmpDir) - ] - for path_substitution in path_substitutions: - letter = path_substitution[0] - path = path_substitution[1] - - # Original path variant - substitutions.append(("%" + letter, path)) - - # Normalized path separator variant - substitutions.append(("%/" + letter, path.replace("\\", "/"))) - - # realpath variants - # Windows paths with substitute drives are not expanded by default - # as they are used to avoid MAX_PATH issues, but sometimes we do - # need the fully expanded path. - real_path = os.path.realpath(path) - substitutions.append(("%{" + letter + ":real}", real_path)) - substitutions.append(("%{/" + letter + ":real}", - real_path.replace("\\", "/"))) - - # "%{/[STpst]:regex_replacement}" should be normalized like - # "%/[STpst]" but we're also in a regex replacement context - # of a s@@@ regex. - substitutions.append( - ("%{/" + letter + ":regex_replacement}", - regex_escape(path.replace("\\", "/")))) - - # "%:[STpst]" are normalized paths without colons and without - # a leading slash. - substitutions.append(("%:" + letter, colonNormalizePath(path))) + substitutions.extend( + [ + ("%{/s:regex_replacement}", regex_escape(sourcepath.replace("\\", "/"))), + ("%{/S:regex_replacement}", regex_escape(sourcedir.replace("\\", "/"))), + ("%{/p:regex_replacement}", regex_escape(sourcedir.replace("\\", "/"))), + ( + "%{/t:regex_replacement}", + regex_escape(tmpBase.replace("\\", "/")) + ".tmp", + ), + ("%{/T:regex_replacement}", regex_escape(tmpDir.replace("\\", "/"))), + ] + ) + # "%:[STpst]" are normalized paths without colons and without a leading + # slash. + substitutions.extend( + [ + ("%:s", colonNormalizePath(sourcepath)), + ("%:S", colonNormalizePath(sourcedir)), + ("%:p", colonNormalizePath(sourcedir)), + ("%:t", colonNormalizePath(tmpBase + ".tmp")), + ("%:T", colonNormalizePath(tmpDir)), + ] + ) return substitutions diff --git a/llvm/utils/lit/lit/builtin_commands/diff.py b/llvm/utils/lit/lit/builtin_commands/diff.py index fbf4eb0e137b3..3a91920f9b5ed 100644 --- a/llvm/utils/lit/lit/builtin_commands/diff.py +++ b/llvm/utils/lit/lit/builtin_commands/diff.py @@ -281,7 +281,7 @@ def main(argv): try: for file in args: if file != "-" and not os.path.isabs(file): - file = util.abs_path_preserve_drive(file) + file = os.path.realpath(os.path.join(os.getcwd(), file)) if flags.recursive_diff: if file == "-": diff --git a/llvm/utils/lit/lit/discovery.py b/llvm/utils/lit/lit/discovery.py index 2e7f90c6bb0c9..a0a2549d8df99 100644 --- a/llvm/utils/lit/lit/discovery.py +++ b/llvm/utils/lit/lit/discovery.py @@ -7,7 +7,7 @@ import sys from lit.TestingConfig import TestingConfig -from lit import LitConfig, Test, util +from lit import LitConfig, Test def chooseConfigFileFromDir(dir, config_names): @@ -56,7 +56,7 @@ def search1(path): # configuration to load instead. config_map = litConfig.params.get("config_map") if config_map: - cfgpath = util.abs_path_preserve_drive(cfgpath) + cfgpath = os.path.realpath(cfgpath) target = config_map.get(os.path.normcase(cfgpath)) if target: cfgpath = target @@ -67,13 +67,13 @@ def search1(path): cfg = TestingConfig.fromdefaults(litConfig) cfg.load_from_path(cfgpath, litConfig) - source_root = util.abs_path_preserve_drive(cfg.test_source_root or path) - exec_root = util.abs_path_preserve_drive(cfg.test_exec_root or path) + source_root = os.path.realpath(cfg.test_source_root or path) + exec_root = os.path.realpath(cfg.test_exec_root or path) return Test.TestSuite(cfg.name, source_root, exec_root, cfg), () def search(path): # Check for an already instantiated test suite. - real_path = util.abs_path_preserve_drive(path) + real_path = os.path.realpath(path) res = cache.get(real_path) if res is None: cache[real_path] = res = search1(path) diff --git a/llvm/utils/lit/lit/util.py b/llvm/utils/lit/lit/util.py index b03fd8bc22693..bd32d760f8ff1 100644 --- a/llvm/utils/lit/lit/util.py +++ b/llvm/utils/lit/lit/util.py @@ -128,23 +128,6 @@ def usable_core_count(): return n -def abs_path_preserve_drive(path): - """Return the absolute path without resolving drive mappings on Windows. - - """ - if platform.system() == "Windows": - # Windows has limitations on path length (MAX_PATH) that - # can be worked around using substitute drives, which map - # a drive letter to a longer path on another drive. - # Since Python 3.8, os.path.realpath resolves sustitute drives, - # so we should not use it. In Python 3.7, os.path.realpath - # was implemented as os.path.abspath. - return os.path.abspath(path) - else: - # On UNIX, the current directory always has symbolic links resolved, - # so any program accepting relative paths cannot preserve symbolic - # links in paths and we should always use os.path.realpath. - return os.path.realpath(path) def mkdir(path): try: diff --git a/llvm/utils/lit/tests/Inputs/config-map-discovery/driver.py b/llvm/utils/lit/tests/Inputs/config-map-discovery/driver.py index 50ad2cd65cb1b..d22b84d6a15cf 100644 --- a/llvm/utils/lit/tests/Inputs/config-map-discovery/driver.py +++ b/llvm/utils/lit/tests/Inputs/config-map-discovery/driver.py @@ -2,7 +2,8 @@ import os import sys -main_config = lit.util.abs_path_preserve_drive(sys.argv[1]) +main_config = sys.argv[1] +main_config = os.path.realpath(main_config) main_config = os.path.normcase(main_config) config_map = {main_config: sys.argv[2]} diff --git a/llvm/utils/lit/tests/Inputs/config-map-discovery/lit.alt.cfg b/llvm/utils/lit/tests/Inputs/config-map-discovery/lit.alt.cfg index 27860e192fdc1..c7b303f50a05c 100644 --- a/llvm/utils/lit/tests/Inputs/config-map-discovery/lit.alt.cfg +++ b/llvm/utils/lit/tests/Inputs/config-map-discovery/lit.alt.cfg @@ -5,5 +5,5 @@ config.suffixes = ['.txt'] config.test_format = lit.formats.ShTest() import os -config.test_exec_root = os.path.dirname(lit.util.abs_path_preserve_drive(__file__)) +config.test_exec_root = os.path.realpath(os.path.dirname(__file__)) config.test_source_root = os.path.join(config.test_exec_root, "tests") diff --git a/llvm/utils/lit/tests/Inputs/use-llvm-tool-required/lit.cfg b/llvm/utils/lit/tests/Inputs/use-llvm-tool-required/lit.cfg index 5062e38c82f14..e41207bc2f05d 100644 --- a/llvm/utils/lit/tests/Inputs/use-llvm-tool-required/lit.cfg +++ b/llvm/utils/lit/tests/Inputs/use-llvm-tool-required/lit.cfg @@ -1,5 +1,4 @@ import lit.formats -import lit.util config.name = "use-llvm-tool-required" config.suffixes = [".txt"] @@ -8,7 +7,7 @@ config.test_source_root = None config.test_exec_root = None import os.path -config.llvm_tools_dir = os.path.dirname(lit.util.abs_path_preserve_drive(__file__)) +config.llvm_tools_dir = os.path.realpath(os.path.dirname(__file__)) import lit.llvm lit.llvm.initialize(lit_config, config) diff --git a/llvm/utils/lit/tests/Inputs/use-llvm-tool/lit.cfg b/llvm/utils/lit/tests/Inputs/use-llvm-tool/lit.cfg index 2a52db2183edf..8fe62d98c1349 100644 --- a/llvm/utils/lit/tests/Inputs/use-llvm-tool/lit.cfg +++ b/llvm/utils/lit/tests/Inputs/use-llvm-tool/lit.cfg @@ -1,5 +1,4 @@ import lit.formats -import lit.util config.name = "use-llvm-tool" config.suffixes = [".txt"] @@ -8,7 +7,7 @@ config.test_source_root = None config.test_exec_root = None import os.path -this_dir = os.path.dirname(lit.util.abs_path_preserve_drive(__file__)) +this_dir = os.path.realpath(os.path.dirname(__file__)) config.llvm_tools_dir = os.path.join(this_dir, "build") import lit.llvm diff --git a/llvm/utils/llvm-lit/llvm-lit.in b/llvm/utils/llvm-lit/llvm-lit.in index 0b6683b6b6230..33ec8017cf05f 100755 --- a/llvm/utils/llvm-lit/llvm-lit.in +++ b/llvm/utils/llvm-lit/llvm-lit.in @@ -8,7 +8,7 @@ config_map = {} def map_config(source_dir, site_config): global config_map - source_dir = os.path.abspath(source_dir) + source_dir = os.path.realpath(source_dir) source_dir = os.path.normcase(source_dir) site_config = os.path.normpath(site_config) config_map[source_dir] = site_config