[llvm] [llvm-lit] Add redirection handling for `env` command without args and write a lit test to check behavior with lit internal shell (PR #106629)

Alexander Richardson via llvm-commits llvm-commits at lists.llvm.org
Thu Aug 29 23:37:46 PDT 2024


================
@@ -746,11 +746,25 @@ def _executeShCmd(cmd, shenv, results, timeoutHelper):
                     env_str = "\n".join(
                         f"{key}={value}" for key, value in sorted(cmd_shenv.env.items())
                     )
-                    results.append(
-                        ShellCommandResult(
-                            j, env_str, "", 0, timeoutHelper.timeoutReached(), []
-                        )
+                    # Process redirections.
+                    stdin, stdout, stderr = processRedirects(
+                        j, default_stdin, cmd_shenv, opened_files
                     )
+                    if stdout != default_stdin:
----------------
arichardson wrote:

I looked at processRedirects() and could not see the `stdin_source` parameter (what you are passing as `default_stdin`) ever being returned as the second tuple element.

As you say the check is actually `if stdout != subprocess.PIPE` and the way it is spelled now is very confusing since it seems to imply some relationship between stdout and stdin. 

As I understand your comment the `!= default_stdin` check isn't actually checking anything related to what was stdin value for the process, but just relies on the fact that `default_stdin` holds `subprocess.PIPE`. With the following check the code makes a lot more sense to me:
```suggestion
                    if stdout != subprocess.PIPE:
```

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


More information about the llvm-commits mailing list