lit: Improve %: normalization.

[pcc]

Summary

The new scheme should match the normalization of embedded paths in
linkrepro tar files.

[zturner]

llvm/utils/lit/lit/TestRunner.py

870–879 ↗

(On Diff #119397)

I think the %: substitutions are used outside of linkrepro tar files, so it seems odd to change the semantics of ALL substitutions just for the purposes of a small number of tests.

We already have %: (remove :), and %/ (convert \ to /). Maybe adding %:/ (remove : *and* convert \ to /) is a better choice? This way if for some reason someone still needs to use the existing semantics of %:, they can get it.

[ruiu]

llvm/utils/lit/lit/TestRunner.py

830 ↗

(On Diff #119397)

This returns /foo for C:\foo, no?

[ruiu]

I introduced %:X lit variables for linkrepro tests, and I doubt we use them in other tests.

[pcc]

llvm/utils/lit/lit/TestRunner.py

830 ↗

(On Diff #119397)

I think it returns C/foo. The replace will return C:/foo, and then the sub will match the C: and replace it with C.

870–879 ↗

(On Diff #119397)

I couldn't find any though (did a grep for %: over the monorepo).

[zturner]

In D39023#900213, @ruiu wrote:

I introduced %:X lit variables for linkrepro tests, and I doubt we use them in other tests.

In that case, I would suggest a couple of different alternatives:

1. If it's really not usable outside of LLD, add the substitutions to LLD's lit.cfg.py instead of to LLVM's TestRunner.py. A person writing a test for clang doesn't care about linkrepro TAR files and shouldn't have to understand how this comment applies to them. Worse, lit is downloadable as a public package from various package managesr, and these people using lit are not even doing anything related to LLVM (or even compilers) at all. It doesn't make sense for them to read a comment about linkrepro tar files. If it's in the core lit infrastructure, it has to be independent of how we (LLVM/LLD/Clang) decide to use it.

2. Re-write the comment to explain what it does, but don't mention linkrepro or tar files. For the same reason as above. (Basically: Just delete the second sentence).

I did a grep of the code, and you're right that these substitutions are not used outside of lld. For that reason, number 1 seems more "correct", but number 2 is less work to change.

[pcc]

• Remove the comment about linkrepro

[pcc]

Okay, I did 2.

llvm/trunk/utils/lit/lit/TestRunner.py

def getTempPaths(test):
     """Get the temporary location, this is always relative to the test suite
     root, not test source root."""
     execpath = test.getExecPath()
     execdir,execbase = os.path.split(execpath)
     tmpDir = os.path.join(execdir, 'Output')
     tmpBase = os.path.join(tmpDir, execbase)
     return tmpDir, tmpBase
 
+def colonNormalizePath(path):
+    if kIsWindows:
+        return re.sub(r'^(.):', r'\1', path.replace('\\', '/'))
+    else:
+        assert path[0] == '/'
+        return path[1:]
+
 def getDefaultSubstitutions(test, tmpDir, tmpBase, normalize_slashes=False):
     sourcepath = test.getSourcePath()
     sourcedir = os.path.dirname(sourcepath)
 
     # Normalize slashes, if requested.
     if normalize_slashes:
         sourcepath = sourcepath.replace('\\', '/')
         sourcedir = sourcedir.replace('\\', '/')
def getDefaultSubstitutions(test, tmpDir, tmpBase, normalize_slashes=False):
     substitutions.extend([
             ('%/s', sourcepath.replace('\\', '/')),
             ('%/S', sourcedir.replace('\\', '/')),
             ('%/p', sourcedir.replace('\\', '/')),
             ('%/t', tmpBase.replace('\\', '/') + '.tmp'),
             ('%/T', tmpDir.replace('\\', '/')),
             ])
 
-    # "%:[STpst]" are paths without colons.
-    if kIsWindows:
-        substitutions.extend([
-                ('%:s', re.sub(r'^(.):', r'\1', sourcepath)),
-                ('%:S', re.sub(r'^(.):', r'\1', sourcedir)),
-                ('%:p', re.sub(r'^(.):', r'\1', sourcedir)),
-                ('%:t', re.sub(r'^(.):', r'\1', tmpBase) + '.tmp'),
-                ('%:T', re.sub(r'^(.):', r'\1', tmpDir)),
-                ])
-    else:
-        substitutions.extend([
-                ('%:s', sourcepath),
-                ('%:S', sourcedir),
-                ('%:p', sourcedir),
-                ('%:t', tmpBase + '.tmp'),
-                ('%:T', tmpDir),
-                ])
+    # "%:[STpst]" are normalized paths without colons and without a leading
+    # slash.
+            ('%:s', colonNormalizePath(sourcepath)),
+            ('%:S', colonNormalizePath(sourcedir)),
+            ('%:p', colonNormalizePath(sourcedir)),
+            ('%:t', colonNormalizePath(tmpBase + '.tmp')),
+            ('%:T', colonNormalizePath(tmpDir)),
     return substitutions
 
 def applySubstitutions(script, substitutions):
     """Apply substitutions to the script.  Allow full regular expression syntax.
     Replace each matching occurrence of regular expression pattern a with
     substitution b in line ln."""
     def processLine(ln):
         # Apply substitutions