This is an archive of the discontinued LLVM Phabricator instance.

[ELF][test] Change referenceds of %T to %t.dir
ClosedPublic

Authored by MaskRay on Oct 29 2019, 10:32 AM.

Details

Summary

Test files in the same directory share the same %T. %T is easy to
misuse and cause race conditions (when running concurrently) so it has
been deprecated since D48842 (see docs/CommandGuide/lit.rst).

While here, add rm -rf %t.dir so that tests cannot depend on old files lying around.

Event Timeline

MaskRay created this revision.Oct 29 2019, 10:32 AM
MaskRay updated this revision to Diff 226936.Oct 29 2019, 10:40 AM

Missed a few %T

ruiu accepted this revision.Oct 30 2019, 2:08 AM

LGTM

This revision is now accepted and ready to land.Oct 30 2019, 2:08 AM
jhenderson accepted this revision.Oct 30 2019, 2:11 AM

LGTM. I don't know if you did, but this might be a good case for deleting the test output directory and then rerunning the tests, to make sure you didn't miss anything subtle in the tests that are working only because of old files lying around.

lld/test/ELF/reproduce-backslash.s
1

Aside: Shouldn't this be REQUIRES: x86, system-linux (or whatever the equivalent is), rather than REQUIRES: x86, shell?

MaskRay updated this revision to Diff 227121.Oct 30 2019, 9:20 AM
MaskRay edited the summary of this revision. (Show Details)

Add rm -rf %t.dir; shell -> system-linux

This revision was automatically updated to reflect the committed changes.