Skip to content

Commit 52fae62

Browse files
shahmishalshahmishal
authored
Merge pull request #7513 from bnbarham/cherry-subtitution-drive-fixes
[5.9] Cherry-pick substitution drive fixes for Windows
2 parents 070d386 + fda6d9e commit 52fae62

File tree

16 files changed

+165
-92
lines changed

16 files changed

+165
-92
lines changed

‎clang/include/clang/Basic/FileManager.h

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -339,6 +339,13 @@ class FileManager : public RefCountedBase<FileManager> {
339339
/// required, which is (almost) never.
340340
StringRef getCanonicalName(const FileEntry *File);
341341

342+
private:
343+
/// Retrieve the canonical name for a given file or directory.
344+
///
345+
/// The first param is a key in the CanonicalNames array.
346+
StringRef getCanonicalName(const void *Entry, StringRef Name);
347+
348+
public:
342349
void PrintStats() const;
343350
};
344351

‎clang/lib/Basic/FileManager.cpp

Lines changed: 35 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -651,34 +651,48 @@ void FileManager::GetUniqueIDMapping(
651651
}
652652

653653
StringRef FileManager::getCanonicalName(const DirectoryEntry *Dir) {
654-
llvm::DenseMap<const void *, llvm::StringRef>::iterator Known
655-
= CanonicalNames.find(Dir);
656-
if (Known != CanonicalNames.end())
657-
return Known->second;
658-
659-
StringRef CanonicalName(Dir->getName());
660-
661-
SmallString<4096> CanonicalNameBuf;
662-
if (!FS->getRealPath(Dir->getName(), CanonicalNameBuf))
663-
CanonicalName = CanonicalNameBuf.str().copy(CanonicalNameStorage);
664-
665-
CanonicalNames.insert({Dir, CanonicalName});
666-
return CanonicalName;
654+
return getCanonicalName(Dir, Dir->getName());
667655
}
668656

669657
StringRef FileManager::getCanonicalName(const FileEntry *File) {
670-
llvm::DenseMap<const void *, llvm::StringRef>::iterator Known
671-
= CanonicalNames.find(File);
658+
return getCanonicalName(File, File->getName());
659+
}
660+
661+
StringRef FileManager::getCanonicalName(const void *Entry, StringRef Name) {
662+
llvm::DenseMap<const void *, llvm::StringRef>::iterator Known =
663+
CanonicalNames.find(Entry);
672664
if (Known != CanonicalNames.end())
673665
return Known->second;
674666

675-
StringRef CanonicalName(File->getName());
676-
677-
SmallString<4096> CanonicalNameBuf;
678-
if (!FS->getRealPath(File->getName(), CanonicalNameBuf))
679-
CanonicalName = CanonicalNameBuf.str().copy(CanonicalNameStorage);
667+
// Name comes from FileEntry/DirectoryEntry::getName(), so it is safe to
668+
// store it in the DenseMap below.
669+
StringRef CanonicalName(Name);
670+
671+
SmallString<256> AbsPathBuf;
672+
SmallString<256> RealPathBuf;
673+
if (!FS->getRealPath(Name, RealPathBuf)) {
674+
if (is_style_windows(llvm::sys::path::Style::native)) {
675+
// For Windows paths, only use the real path if it doesn't resolve
676+
// a substitute drive, as those are used to avoid MAX_PATH issues.
677+
AbsPathBuf = Name;
678+
if (!FS->makeAbsolute(AbsPathBuf)) {
679+
if (llvm::sys::path::root_name(RealPathBuf) ==
680+
llvm::sys::path::root_name(AbsPathBuf)) {
681+
CanonicalName = RealPathBuf.str().copy(CanonicalNameStorage);
682+
} else {
683+
// Fallback to using the absolute path.
684+
// Simplifying /../ is semantically valid on Windows even in the
685+
// presence of symbolic links.
686+
llvm::sys::path::remove_dots(AbsPathBuf, /*remove_dot_dot=*/true);
687+
CanonicalName = AbsPathBuf.str().copy(CanonicalNameStorage);
688+
}
689+
}
690+
} else {
691+
CanonicalName = RealPathBuf.str().copy(CanonicalNameStorage);
692+
}
693+
}
680694

681-
CanonicalNames.insert({File, CanonicalName});
695+
CanonicalNames.insert({Entry, CanonicalName});
682696
return CanonicalName;
683697
}
684698

‎clang/lib/ExtractAPI/ExtractAPIConsumer.cpp

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -184,9 +184,7 @@ struct LocationFileChecker {
184184
if (ExternalFileEntries.count(File))
185185
return false;
186186

187-
StringRef FileName = File->tryGetRealPathName().empty()
188-
? File->getName()
189-
: File->tryGetRealPathName();
187+
StringRef FileName = SM.getFileManager().getCanonicalName(File);
190188

191189
// Try to reduce the include name the same way we tried to include it.
192190
bool IsQuoted = false;

‎clang/test/Lexer/case-insensitive-include-win.c

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,9 +2,15 @@
22
// This file should only include code that really needs a Windows host OS to
33
// run.
44

5+
// Note: We must use the real path here, because the logic to detect case
6+
// mismatches must resolve the real path to figure out the original casing.
7+
// If we use %t and we are on a substitute drive S: mapping to C:\subst,
8+
// then we will compare "S:\test.dir\FOO.h" to "C:\subst\test.dir\foo.h"
9+
// and avoid emitting the diagnostic because the structure is different.
10+
511
// REQUIRES: system-windows
612
// RUN: mkdir -p %t.dir
713
// RUN: touch %t.dir/foo.h
8-
// RUN: not %clang_cl /FI\\?\%t.dir\FOO.h /WX -fsyntax-only %s 2>&1 | FileCheck %s
14+
// RUN: not %clang_cl /FI \\?\%{t:real}.dir\FOO.h /WX -fsyntax-only %s 2>&1 | FileCheck %s
915

1016
// CHECK: non-portable path to file '"\\?\

‎llvm/cmake/modules/AddLLVM.cmake

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1688,10 +1688,15 @@ endfunction()
16881688
# use it and can't be in a lit module. Use with make_paths_relative().
16891689
string(CONCAT LLVM_LIT_PATH_FUNCTION
16901690
"# Allow generated file to be relocatable.\n"
1691-
"from pathlib import Path\n"
1691+
"import os\n"
1692+
"import platform\n"
16921693
"def path(p):\n"
16931694
" if not p: return ''\n"
1694-
" return str((Path(__file__).parent / p).resolve())\n"
1695+
" # Follows lit.util.abs_path_preserve_drive, which cannot be imported here.\n"
1696+
" if platform.system() == 'Windows':\n"
1697+
" return os.path.abspath(os.path.join(os.path.dirname(__file__), p))\n"
1698+
" else:\n"
1699+
" return os.path.realpath(os.path.join(os.path.dirname(__file__), p))\n"
16951700
)
16961701

16971702
# This function provides an automatic way to 'configure'-like generate a file

‎llvm/docs/CommandGuide/lit.rst

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -541,6 +541,16 @@ TestRunner.py:
541541
%/p %p but ``\`` is replaced by ``/``
542542
%/t %t but ``\`` is replaced by ``/``
543543
%/T %T but ``\`` is replaced by ``/``
544+
%{s:real} %s after expanding all symbolic links and substitute drives
545+
%{S:real} %S after expanding all symbolic links and substitute drives
546+
%{p:real} %p after expanding all symbolic links and substitute drives
547+
%{t:real} %t after expanding all symbolic links and substitute drives
548+
%{T:real} %T after expanding all symbolic links and substitute drives
549+
%{/s:real} %/s after expanding all symbolic links and substitute drives
550+
%{/S:real} %/S after expanding all symbolic links and substitute drives
551+
%{/p:real} %/p after expanding all symbolic links and substitute drives
552+
%{/t:real} %/t after expanding all symbolic links and substitute drives
553+
%{/T:real} %/T after expanding all symbolic links and substitute drives
544554
%{/s:regex_replacement} %/s but escaped for use in the replacement of a ``s@@@`` command in sed
545555
%{/S:regex_replacement} %/S but escaped for use in the replacement of a ``s@@@`` command in sed
546556
%{/p:regex_replacement} %/p but escaped for use in the replacement of a ``s@@@`` command in sed

‎llvm/docs/TestingGuide.rst

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -581,7 +581,7 @@ RUN lines:
581581
``${fs-sep}``
582582
Expands to the file system separator, i.e. ``/`` or ``\`` on Windows.
583583

584-
``%/s, %/S, %/t, %/T:``
584+
``%/s, %/S, %/t, %/T``
585585

586586
Act like the corresponding substitution above but replace any ``\``
587587
character with a ``/``. This is useful to normalize path separators.
@@ -590,7 +590,17 @@ RUN lines:
590590

591591
Example: ``%/s: C:/Desktop Files/foo_test.s.tmp``
592592

593-
``%:s, %:S, %:t, %:T:``
593+
``%{s:real}, %{S:real}, %{t:real}, %{T:real}``
594+
``%{/s:real}, %{/S:real}, %{/t:real}, %{/T:real}``
595+
596+
Act like the corresponding substitution, including with ``/``, but use
597+
the real path by expanding all symbolic links and substitute drives.
598+
599+
Example: ``%s: S:\foo_test.s.tmp``
600+
601+
Example: ``%{/s:real}: C:/SDrive/foo_test.s.tmp``
602+
603+
``%:s, %:S, %:t, %:T``
594604

595605
Act like the corresponding substitution above but remove colons at
596606
the beginning of Windows paths. This is useful to allow concatenation

‎llvm/utils/lit/lit/TestRunner.py

Lines changed: 52 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,7 @@ def change_dir(self, newdir):
7070
if os.path.isabs(newdir):
7171
self.cwd = newdir
7272
else:
73-
self.cwd = os.path.realpath(os.path.join(self.cwd, newdir))
73+
self.cwd = lit.util.abs_path_preserve_drive(os.path.join(self.cwd, newdir))
7474

7575
class TimeoutHelper(object):
7676
"""
@@ -401,7 +401,7 @@ def executeBuiltinMkdir(cmd, cmd_shenv):
401401
dir = to_unicode(dir) if kIsWindows else to_bytes(dir)
402402
cwd = to_unicode(cwd) if kIsWindows else to_bytes(cwd)
403403
if not os.path.isabs(dir):
404-
dir = os.path.realpath(os.path.join(cwd, dir))
404+
dir = lit.util.abs_path_preserve_drive(os.path.join(cwd, dir))
405405
if parent:
406406
lit.util.mkdir_p(dir)
407407
else:
@@ -446,7 +446,7 @@ def on_rm_error(func, path, exc_info):
446446
path = to_unicode(path) if kIsWindows else to_bytes(path)
447447
cwd = to_unicode(cwd) if kIsWindows else to_bytes(cwd)
448448
if not os.path.isabs(path):
449-
path = os.path.realpath(os.path.join(cwd, path))
449+
path = lit.util.abs_path_preserve_drive(os.path.join(cwd, path))
450450
if force and not os.path.exists(path):
451451
continue
452452
try:
@@ -1153,58 +1153,59 @@ def getDefaultSubstitutions(test, tmpDir, tmpBase, normalize_slashes=False):
11531153
substitutions.extend(test.config.substitutions)
11541154
tmpName = tmpBase + '.tmp'
11551155
baseName = os.path.basename(tmpBase)
1156-
substitutions.extend([('%s', sourcepath),
1157-
('%S', sourcedir),
1158-
('%p', sourcedir),
1159-
('%{pathsep}', os.pathsep),
1160-
('%t', tmpName),
1161-
('%basename_t', baseName),
1162-
('%T', tmpDir)])
1163-
1164-
substitutions.extend([
1165-
('%{fs-src-root}', pathlib.Path(sourcedir).anchor),
1166-
('%{fs-tmp-root}', pathlib.Path(tmpBase).anchor),
1167-
('%{fs-sep}', os.path.sep),
1168-
])
1169-
1170-
# "%/[STpst]" should be normalized.
1171-
substitutions.extend([
1172-
('%/s', sourcepath.replace('\\', '/')),
1173-
('%/S', sourcedir.replace('\\', '/')),
1174-
('%/p', sourcedir.replace('\\', '/')),
1175-
('%/t', tmpBase.replace('\\', '/') + '.tmp'),
1176-
('%/T', tmpDir.replace('\\', '/')),
1177-
('%/et',tmpName.replace('\\', '\\\\\\\\\\\\\\\\')),
1178-
])
1179-
1180-
# "%{/[STpst]:regex_replacement}" should be normalized like "%/[STpst]" but we're
1181-
# also in a regex replacement context of a s@@@ regex.
1156+
1157+
substitutions.append(("%{pathsep}", os.pathsep))
1158+
substitutions.append(("%basename_t", baseName))
1159+
1160+
substitutions.extend(
1161+
[
1162+
("%{fs-src-root}", pathlib.Path(sourcedir).anchor),
1163+
("%{fs-tmp-root}", pathlib.Path(tmpBase).anchor),
1164+
("%{fs-sep}", os.path.sep),
1165+
]
1166+
)
1167+
1168+
substitutions.append(("%/et", tmpName.replace("\\", "\\\\\\\\\\\\\\\\")))
1169+
11821170
def regex_escape(s):
11831171
s = s.replace('@', r'\@')
11841172
s = s.replace('&', r'\&')
11851173
return s
1186-
substitutions.extend([
1187-
('%{/s:regex_replacement}',
1188-
regex_escape(sourcepath.replace('\\', '/'))),
1189-
('%{/S:regex_replacement}',
1190-
regex_escape(sourcedir.replace('\\', '/'))),
1191-
('%{/p:regex_replacement}',
1192-
regex_escape(sourcedir.replace('\\', '/'))),
1193-
('%{/t:regex_replacement}',
1194-
regex_escape(tmpBase.replace('\\', '/')) + '.tmp'),
1195-
('%{/T:regex_replacement}',
1196-
regex_escape(tmpDir.replace('\\', '/'))),
1197-
])
1198-
1199-
# "%:[STpst]" are normalized paths without colons and without a leading
1200-
# slash.
1201-
substitutions.extend([
1202-
('%:s', colonNormalizePath(sourcepath)),
1203-
('%:S', colonNormalizePath(sourcedir)),
1204-
('%:p', colonNormalizePath(sourcedir)),
1205-
('%:t', colonNormalizePath(tmpBase + '.tmp')),
1206-
('%:T', colonNormalizePath(tmpDir)),
1207-
])
1174+
1175+
path_substitutions = [
1176+
("s", sourcepath), ("S", sourcedir), ("p", sourcedir),
1177+
("t", tmpName), ("T", tmpDir)
1178+
]
1179+
for path_substitution in path_substitutions:
1180+
letter = path_substitution[0]
1181+
path = path_substitution[1]
1182+
1183+
# Original path variant
1184+
substitutions.append(("%" + letter, path))
1185+
1186+
# Normalized path separator variant
1187+
substitutions.append(("%/" + letter, path.replace("\\", "/")))
1188+
1189+
# realpath variants
1190+
# Windows paths with substitute drives are not expanded by default
1191+
# as they are used to avoid MAX_PATH issues, but sometimes we do
1192+
# need the fully expanded path.
1193+
real_path = os.path.realpath(path)
1194+
substitutions.append(("%{" + letter + ":real}", real_path))
1195+
substitutions.append(("%{/" + letter + ":real}",
1196+
real_path.replace("\\", "/")))
1197+
1198+
# "%{/[STpst]:regex_replacement}" should be normalized like
1199+
# "%/[STpst]" but we're also in a regex replacement context
1200+
# of a s@@@ regex.
1201+
substitutions.append(
1202+
("%{/" + letter + ":regex_replacement}",
1203+
regex_escape(path.replace("\\", "/"))))
1204+
1205+
# "%:[STpst]" are normalized paths without colons and without
1206+
# a leading slash.
1207+
substitutions.append(("%:" + letter, colonNormalizePath(path)))
1208+
12081209
return substitutions
12091210

12101211
def _memoize(f):

‎llvm/utils/lit/lit/builtin_commands/diff.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -229,7 +229,7 @@ def main(argv):
229229
try:
230230
for file in args:
231231
if file != "-" and not os.path.isabs(file):
232-
file = os.path.realpath(os.path.join(os.getcwd(), file))
232+
file = util.abs_path_preserve_drive(file)
233233

234234
if flags.recursive_diff:
235235
if file == "-":

‎llvm/utils/lit/lit/discovery.py

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
import sys
88

99
from lit.TestingConfig import TestingConfig
10-
from lit import LitConfig, Test
10+
from lit import LitConfig, Test, util
1111

1212
def chooseConfigFileFromDir(dir, config_names):
1313
for name in config_names:
@@ -52,7 +52,7 @@ def search1(path):
5252
# configuration to load instead.
5353
config_map = litConfig.params.get('config_map')
5454
if config_map:
55-
cfgpath = os.path.realpath(cfgpath)
55+
cfgpath = util.abs_path_preserve_drive(cfgpath)
5656
target = config_map.get(os.path.normcase(cfgpath))
5757
if target:
5858
cfgpath = target
@@ -63,13 +63,13 @@ def search1(path):
6363

6464
cfg = TestingConfig.fromdefaults(litConfig)
6565
cfg.load_from_path(cfgpath, litConfig)
66-
source_root = os.path.realpath(cfg.test_source_root or path)
67-
exec_root = os.path.realpath(cfg.test_exec_root or path)
66+
source_root = util.abs_path_preserve_drive(cfg.test_source_root or path)
67+
exec_root = util.abs_path_preserve_drive(cfg.test_exec_root or path)
6868
return Test.TestSuite(cfg.name, source_root, exec_root, cfg), ()
6969

7070
def search(path):
7171
# Check for an already instantiated test suite.
72-
real_path = os.path.realpath(path)
72+
real_path = util.abs_path_preserve_drive(path)
7373
res = cache.get(real_path)
7474
if res is None:
7575
cache[real_path] = res = search1(path)

‎llvm/utils/lit/lit/util.py

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -127,6 +127,23 @@ def usable_core_count():
127127

128128
return n
129129

130+
def abs_path_preserve_drive(path):
131+
"""Return the absolute path without resolving drive mappings on Windows.
132+
133+
"""
134+
if platform.system() == "Windows":
135+
# Windows has limitations on path length (MAX_PATH) that
136+
# can be worked around using substitute drives, which map
137+
# a drive letter to a longer path on another drive.
138+
# Since Python 3.8, os.path.realpath resolves sustitute drives,
139+
# so we should not use it. In Python 3.7, os.path.realpath
140+
# was implemented as os.path.abspath.
141+
return os.path.abspath(path)
142+
else:
143+
# On UNIX, the current directory always has symbolic links resolved,
144+
# so any program accepting relative paths cannot preserve symbolic
145+
# links in paths and we should always use os.path.realpath.
146+
return os.path.realpath(path)
130147

131148
def mkdir(path):
132149
try:

‎llvm/utils/lit/tests/Inputs/config-map-discovery/driver.py

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,7 @@
22
import os
33
import sys
44

5-
main_config = sys.argv[1]
6-
main_config = os.path.realpath(main_config)
5+
main_config = lit.util.abs_path_preserve_drive(sys.argv[1])
76
main_config = os.path.normcase(main_config)
87

98
config_map = {main_config : sys.argv[2]}

‎llvm/utils/lit/tests/Inputs/config-map-discovery/lit.alt.cfg

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,5 +5,5 @@ config.suffixes = ['.txt']
55
config.test_format = lit.formats.ShTest()
66

77
import os
8-
config.test_exec_root = os.path.realpath(os.path.dirname(__file__))
8+
config.test_exec_root = os.path.dirname(lit.util.abs_path_preserve_drive(__file__))
99
config.test_source_root = os.path.join(config.test_exec_root, "tests")

0 commit comments

Comments
 (0)