[llvm] [llvm-lit] Fix `TypeError` string argument expected in lit's internal shell (PR #105925)

via llvm-commits llvm-commits at lists.llvm.org
Thu Sep 5 13:10:25 PDT 2024


================
@@ -776,8 +776,8 @@ def _executeShCmd(cmd, shenv, results, timeoutHelper):
         # FIXME: Standardize on the builtin echo implementation. We can use a
         # temporary file to sidestep blocking pipe write issues.
 
-        # Ensure args[0] is hashable.
-        args[0] = expand_glob(args[0], cmd_shenv.cwd)[0]
+        # Expand all glob expressions
+        args = expand_glob_expressions(args, cmd_shenv.cwd)
----------------
Harini0924 wrote:

I initially didn’t remove the call to `expand_glob_expressions` in `executeBuiltinMkdir` and `executeBuiltinRm` because removing it was causing an error. Here's the error I encountered:
```
  File "/usr/local/google/home/harinidonthula/llvm-project/llvm/utils/lit/lit/TestRunner.py", line 467, in executeBuiltinRm
    opts, args = getopt.gnu_getopt(args, "frR", ["--recursive"])

UnboundLocalError: cannot access local variable 'args' where it is not associated with a value
```

This error occurs because `args` wasn't being initialized correctly after I removed the `expand_glob_expressions(cmd.args, cmd_shenv.cwd)[1:]` line from these functions. The issue is that the functions still expect args to be expanded and processed, but when I rely solely on the expansion happening in `_executeShCmd`, the arguments aren't passed in the correct form to these built-in functions.

Specifically, in the context of commands like `mkdir` and `rm`, `args[0]` represents the command itself (e.g., mkdir or rm), and this should not be expanded. Expanding `args[0]` could cause the command to be mistakenly treated as a file or directory to expand, which would break the logic. Therefore, only `args[1:]` (the options or arguments) should undergo glob expansion. For example, in a command like mkdir -p folder*, we only want folder* to be expanded, not the command mkdir itself. Correct me if I am wrong, but that is what I was getting. 

In short, removing `expand_glob_expressions` from these functions without ensuring proper argument handling causes the built-in commands to fail when trying to access `args` later in the flow, especially since `args[0]` should not be expanded. 

I'm also working on writing the other test. Let me know what you think. 

https://github.com/llvm/llvm-project/pull/105925


More information about the llvm-commits mailing list