Page MenuHomePhabricator

unittests: Don't install TestPlugin.so
AbandonedPublic

Authored by tstellar on Aug 13 2018, 2:59 PM.

Details

Summary

add_llvm_loadable_module adds an install target by default, but this
module is only used for a unit test, so we don't need to instal it.

This patch adds a NO_INSTALL option to add_llvm_loadable_module that
can be used to disable installation of modules.

Diff Detail

Repository
rL LLVM

Event Timeline

tstellar created this revision.Aug 13 2018, 2:59 PM

This makes complete sense to me, but I'm wondering whether add_llvm_loadable_module is the right macro to use here in the first place. All it does is call add_llvm_library with a MODULE argument, and set up the install.

So it seems to me that add_llvm_library(TestPlugin MODULE BUILDTREE_ONLY ...) should work just as well, without changing the setup. Does that make sense?

tstellar updated this revision to Diff 160659.Aug 14 2018, 11:59 AM

Use add_llvm_library() instead of add_llvm_loadable_module()

This makes complete sense to me, but I'm wondering whether add_llvm_loadable_module is the right macro to use here in the first place. All it does is call add_llvm_library with a MODULE argument, and set up the install.

So it seems to me that add_llvm_library(TestPlugin MODULE BUILDTREE_ONLY ...) should work just as well, without changing the setup. Does that make sense?

That's a better idea. I've updated the patch.

philip.pfaffe accepted this revision.Aug 15 2018, 2:42 AM

Cool, LGTM!

This revision is now accepted and ready to land.Aug 15 2018, 2:42 AM
This revision was automatically updated to reflect the committed changes.
tstellar reopened this revision.Aug 16 2018, 10:30 AM

I had to revert this, because it broke the Windows build. I am working on a new patch.

This revision is now accepted and ready to land.Aug 16 2018, 10:30 AM

I had to revert this, because it broke the Windows build. I am working on a new patch.

http://lab.llvm.org:8011/builders/sanitizer-windows/builds/33474

We probably should just disable the unittest on windows, in particular if(NOT LLVM_ENABLE_PLUGINS AND NOT (ARG_PLUGIN_TOOL AND LLVM_EXPORT_SYMBOLS_FOR_PLUGINS)).

There are a few other modules in the tree that are unnecessarily installed. So I made an attempt to fix this in a more generic way: D51748.

tstellar abandoned this revision.Dec 20 2018, 3:39 PM

This has been superseded by D55965.