This is an archive of the discontinued LLVM Phabricator instance.

[lld-macho] Avoid requiring shell in tests
ClosedPublic

Authored by int3 on Mar 10 2021, 11:02 PM.

Details

Reviewers
alexander-shaposhnikov
Group Reviewers
Restricted Project
Commits
rGd1e57ee99aa8: [lld-macho] Avoid requiring shell in tests
Summary

There are 3 remaining tests that still have REQUIRE: shell:

  • color-diagnostics.test -- seems necessary for ANSI escape sequence support
  • stabs.s -- the shell part could be removed, but I don't think we can support the test on Windows anyway due to its reliance on touch to set the modtime
  • framework.s -- uses symlinks, I'm not sure this works on Windows

Addresses PR49512.

Diff Detail

Event Timeline

int3 created this revision.Mar 10 2021, 11:02 PM
Herald added a project: Restricted Project. · View Herald TranscriptMar 10 2021, 11:02 PM
int3 requested review of this revision.Mar 10 2021, 11:02 PM
Herald added a project: Restricted Project. · View Herald TranscriptMar 10 2021, 11:02 PM
This revision is now accepted and ready to land.Mar 10 2021, 11:05 PM
This revision was landed with ongoing or failed builds.Mar 11 2021, 10:28 AM
This revision was automatically updated to reflect the committed changes.
thakis added a subscriber: thakis.Mar 20 2021, 7:40 AM

Thanks for the fix! re touch: LLVM requires a whole bunch of unixy utilities for running tests even on Windows. http://commondatastorage.googleapis.com/chromium-browser-clang/tools/gnuwin-14.zip contains the ones that are currently needed. touch is among them, so that test should Just Work on Windows – many other tests use touch too.

The other two tests sound mostly fine as-is: The ANSI thing sounds like it might need shell for now (very new versions of windows apparently ship with the new win terminal that has ansi support, but we can't rely on this). For the symlink thing, maybe UNSUPPORTED: system-windows is clearer than REQUIRES: shell if it's for symlinks? But those are both nits.