This is an archive of the discontinued LLVM Phabricator instance.

[dotest] Avoid the need for LEVEL= makefile boilerplate
ClosedPublic

Authored by labath on Sep 2 2019, 10:41 AM.

Details

Summary

Instead of each test case knowing its depth relative to the test root,
we can just have dotest add the folder containing Makefile.rules to the
include path.

This was motivated by r370616, though I have been wanting to do this
ever since we moved to building tests out-of-tree.

The only manually modified files in this patch are lldbinline.py and
plugins/builder_base.py. The rest of the patch has been produced by this
shell command:

find . \( -name Makefile -o -name '*.mk' \)  -exec sed --in-place -e '/LEVEL *:\?=/d' -e '1,2{/^$/d}' -e 's,\$(LEVEL)/,,' {} +

Diff Detail

Repository
rL LLVM

Event Timeline

labath created this revision.Sep 2 2019, 10:41 AM

Thanks! LGTM, but Jonas probably should also sign this off.

labath edited reviewers, added: JDevlieghere; removed: espindola, jfb.Sep 3 2019, 12:14 AM
aprantl accepted this revision.Sep 3 2019, 9:26 AM

This looks like a very good idea!

packages/Python/lldbsuite/test/lldbinline.py
116 ↗(On Diff #218383)

wait.. lldbinline can auto-generate a Makefile? Is that feature used by any tests?

This revision is now accepted and ready to land.Sep 3 2019, 9:26 AM
JDevlieghere accepted this revision.Sep 3 2019, 9:27 AM

This has always bothered me, but never enough to look into. Thanks for doing this, Pavel!

labath marked an inline comment as done.Sep 3 2019, 9:49 AM
labath added inline comments.
packages/Python/lldbsuite/test/lldbinline.py
116 ↗(On Diff #218383)

I would guess "all of them", because otherwise, how would the inline test executable get built?

aprantl added inline comments.Sep 3 2019, 10:12 AM
packages/Python/lldbsuite/test/lldbinline.py
116 ↗(On Diff #218383)

I see. My only exposure to inline test was in the Swift branch and there they all ship their own Makefile, but that is likely because they need to set SWIFT_SOURCES instead of the default CXX_SOURCES.

amccarth accepted this revision.Sep 3 2019, 1:10 PM
amccarth added a subscriber: amccarth.

Nice!

labath marked an inline comment as done.Sep 4 2019, 12:25 AM
labath added inline comments.
packages/Python/lldbsuite/test/lldbinline.py
116 ↗(On Diff #218383)

Aha, interesting. I did some stats, and it seems that (in the lldb repo), most (41) inline tests don't come with a Makefile, but there are some (14) that do. I am going to separately check if those 14 can be removed too.

It sounds like it might be interesting to make swift inline tests auto generate makefiles too, but that's up to you guys...

This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptSep 4 2019, 12:48 AM