Page MenuHomePhabricator

Make FileCheck be a common dependency, not an ASan one.
ClosedPublic

Authored by filcab on Feb 18 2015, 5:13 PM.

Details

Summary

It still gets picked up by ASan, but it also gets picked up by the other
test suites.

Otherwise, some test suites (e.g: UBSan) would complain they had no
dependencies, and wouldn't run.

Diff Detail

Repository
rL LLVM

Event Timeline

filcab updated this revision to Diff 20240.Feb 18 2015, 5:13 PM
filcab retitled this revision from to Make FileCheck be a common dependency, not an ASan one..
filcab updated this object.
filcab edited the test plan for this revision. (Show Details)
filcab added reviewers: samsonov, eugenis.
filcab added a subscriber: Unknown Object (MLST).
pcc added a subscriber: pcc.Feb 18 2015, 6:51 PM

Was this fixed by r229753?

It might have been, thanks for pointing it out.
I will check it and report back or re-submit as a fix to remove our
workaround.

Thanks,

Filipe

Yes, r229753 makes it work without the workaround.

I'm just not sure what would be preferred:

  • To have the explicit FileCheck (and not, at the very least) dependency
  • Or to simply not have those dependencies and let lit error out if it

can't find them;

Filipe

samsonov added inline comments.Feb 19 2015, 5:36 PM
test/CMakeLists.txt
13 ↗(On Diff #20240)

Shouldn't you append it to SANITIZER_COMMON_LIT_TEST_DEPS instead?

filcab updated this revision to Diff 20362.Feb 19 2015, 6:13 PM

Fixed a copy-paste problem spotted by Alexey

Submitted a new patch with the fixed variable name

test/CMakeLists.txt
13 ↗(On Diff #20240)

Yes, of course.

samsonov accepted this revision.Feb 19 2015, 7:14 PM
samsonov edited edge metadata.

Please submit after fixing the error below:

test/CMakeLists.txt
13 ↗(On Diff #20362)

I believe this is still wrong =)
Should be SANITIZER_COMMON_LIT_TEST_DEPS

This revision is now accepted and ready to land.Feb 19 2015, 7:14 PM
This revision was automatically updated to reflect the committed changes.

Wow, my face is red.
Thanks :-)