[PATCH] D65697: [lit] Fix internal env calling internal commands

Michał Górny via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Aug 19 19:31:44 PDT 2019


mgorny added inline comments.


================
Comment at: llvm/utils/lit/lit/TestRunner.py:810
+    cmd0_args_pruned = list(cmd.commands[0].args)
+    while len(cmd0_args_pruned) and cmd0_args_pruned[0] == 'env':
+        cmd0_args_pruned = updateEnv(None, cmd0_args_pruned)
----------------
You don't need `len()` here, list will evaluate to whether it's non-empty in boolean context.


================
Comment at: llvm/utils/lit/lit/TestRunner.py:814
+
+    if cmd0_args_pruned[0] == 'cd':
+        if cmd0_has_envs:
----------------
You are not checking whether it's empty here. I guess this means it's gonna crash if you pass just `env`.


================
Comment at: llvm/utils/lit/lit/TestRunner.py:836
     # temporary file to sidestep blocking pipe write issues.
-    if cmd.commands[0].args[0] == 'echo' and len(cmd.commands) == 1:
+    if cmd0_args_pruned[0] == 'echo' and len(cmd.commands) == 1:
+        if cmd0_has_envs:
----------------
The RHS of this condition is verifying the original command, so I suppose it's never going to trigger with `env`.


================
Comment at: llvm/utils/lit/lit/TestRunner.py:907
         args = list(j.args)
-        if j.args[0] == 'env':
+        while len(args) and args[0] == 'env':
             # Create a copy of the global environment and modify it for this one
----------------
Same about `len()` here.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D65697/new/

https://reviews.llvm.org/D65697





More information about the llvm-commits mailing list