-
Notifications
You must be signed in to change notification settings - Fork 14.9k
[Utils] fix diff_test_updater on Windows #158235
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
@llvm/pr-subscribers-testing-tools Author: Henrik G. Olsson (hnrklssn) ChangesFull diff: https://github.com/llvm/llvm-project/pull/158235.diff 1 Files Affected:
diff --git a/llvm/utils/lit/lit/DiffUpdater.py b/llvm/utils/lit/lit/DiffUpdater.py
index fefcdcc99f3f2..a29c46fb8508f 100644
--- a/llvm/utils/lit/lit/DiffUpdater.py
+++ b/llvm/utils/lit/lit/DiffUpdater.py
@@ -62,17 +62,19 @@ def __str__(self):
@staticmethod
def get_target_dir(commands, test_path):
+ # posix=True breaks Windows paths because \ is treated as an escaping character
for cmd in commands:
- split = shlex.split(cmd)
+ split = shlex.split(cmd, posix=False)
if "split-file" not in split:
continue
start_idx = split.index("split-file")
split = split[start_idx:]
if len(split) < 3:
continue
- if split[1].strip() != test_path:
+ p = unquote(split[1].strip())
+ if not test_path.samefile(p):
continue
- return split[2].strip()
+ return unquote(split[2].strip())
return None
@staticmethod
@@ -104,6 +106,12 @@ def _get_split_line_path(l):
return l.rstrip()
+def unquote(s):
+ if len(s) > 1 and s[0] == s[-1] and (s[0] == '"' or s[0] == "'"):
+ return s[1:-1]
+ return s
+
+
def get_source_and_target(a, b, test_path, commands):
"""
Try to figure out which file is the test output and which is the reference.
@@ -145,7 +153,7 @@ def diff_test_updater(result, test, commands):
[cmd, a, b] = args
if cmd != "diff":
return None
- res = get_source_and_target(a, b, test.getFilePath(), commands)
+ res = get_source_and_target(a, b, pathlib.Path(test.getFilePath()), commands)
if not res:
return f"update-diff-test: could not deduce source and target from {a} and {b}"
source, target = res
|
Hi @hnrklssn, the test unfortunately still seems to be failing on our Windows bot: https://lab.llvm.org/buildbot/#/builders/46/builds/23206 Can you take a look? |
Was asleep, sorry! But I see you fixed it with the mismatching CRs, thanks. Not sure why that didn't end up being an issue on my Windows machine; I don't use Windows very often, but I would've expected it to behave consistently. |
(cherry picked from commit 1b05212)
No worries, I figured as much when you did not respond and decided to take a look. How are you running the tests on your Windows machine? If inside WSL, that might be the reason why. (I run everything through the regular Windows command prompt) |
I launched It seems like python scripts on my Windows machine somehow emits Unix line endings. The |
(cherry picked from commit 1b05212)
(cherry picked from commit 1b05212)
No description provided.