[Test][Driver] Remove bashisms from filelists test #84923
Conversation
Replace (cd %t && ...) with cd %t && ...
Replace env PATH manipulation with Swift driver's -tools-directory flag to specify location of fake linker. This seems to work with lit's internal shell, though I'm not 100% certain it's the right approach.
|
Let me know if you'd like me to squash these commits or make any other changes. |
|
@swift-ci please smoke test |
| // RUN: cat $(cat %t/output-filelist) | %FileCheck -check-prefix OUTPUT-FILELIST-CONTENTS %s | ||
| // RUN: cat $(cat %t/input-filelist) | %FileCheck -check-prefix INPUT-FILELIST-CONTENTS %s | ||
| // RUN: cat %t/output-filelist | xargs cat | %FileCheck -check-prefix OUTPUT-FILELIST-CONTENTS %s | ||
| // RUN: cat %t/input-filelist | xargs cat | %FileCheck -check-prefix INPUT-FILELIST-CONTENTS %s |
There was a problem hiding this comment.
No, so while this gives lit shell compatibility, it still requires that xargs is on the PATH. Do you have a suggestion for a Windows compatible approach?
There was a problem hiding this comment.
I thought it would be okay since tests have used Unix specific commands, as in this with // REQUIRES: OS=macosx. I'm just learning that might not be the same for UNSUPPORTED: OS=windows-msvc 🤦♂️
There was a problem hiding this comment.
I think it's safe to assume (for the foreseeable future) that UNSUPPORTED: OS=windows-msvc implies POSIX compatibility. The question is why UNSUPPORTED: OS=windows-msvc is there in the first place. If it's just because of the bash-isms, then maybe we can enable this for Windows with some work.
There was a problem hiding this comment.
No, I don't think that UNSUPPORTED: OS=windows-msvc implies POSIX compatibility. We do have some of the tools, not all of them. I think that this might've been one of the early ones that I didn't find. A lot of tests were improperly annotated as not supporting Windows and we should fix that. As to how to support this on Windows, I would say that instead of using xargs we use temporary files and redirection.
There was a problem hiding this comment.
I'll make a note to create a patch and remove UNSUPPORTED: OS=windows-msvc.
There was a problem hiding this comment.
Oh are you referring to building on Windows, but not targeting Windows? Or is there some other supported OS without POSIX?
|
@swift-ci please smoke test windows platform |
|
I'll merge this for now, as internal shell support is still an improvement even without Windows support. Either of you are very welcome to file a follow-up to add Windows support if you feel like it @ramonasuncion @compnerd |
3 participants
Remove subshells, command substitution, and env PATH.
Partially address: #84407