intercept / intercept-plugin-template

A template plugin for Intercept
MIT License
1 stars 6 forks source link

Download Intercept library to vendor folder on build if needed #2

Open Dahlgren opened 6 years ago

jonpas commented 6 years ago

It's an intercept plugin, of course it'll need intercept library?

Dahlgren commented 6 years ago

Yes, using this feature of CMake it will automatically download Intercept when building if not already available. It removes the need to have git installed (when downloading the template as an archive) or knowledge about how git submodules work. Not all clients will automatically initialize submodules which can be confusing to users.

A new version of Intercept will also be automatically downloaded if version number if changed in the CMake file

dedmen commented 6 years ago

Can you also change https://github.com/intercept/intercept-plugin-template/pull/2/files#diff-af3b638bc2a3e6c650974192a53c7291R1 to require version 3.8? Just tried building with CMake 3.7.2 on debian and it fails because it doesn't know CXX17. Which gets added in CMake 3.8 I don't want to make a seperate commit for that 😀

Target "template-plugin_x64" requires the language dialect "CXX17" , but CMake does not know the compile flags to use to enable it.

Dahlgren commented 6 years ago

Sure, I'll fix it @dedmen

dedmen commented 6 years ago

:D trying to build on linux right now.. Guess what.. It didn't clone the submodule and thus the build fails.. duh. With your PR added.

-- Downloading...
   dst='/home/wolf/projects/intercept-plugin-template/vendor/intercept/src/f283e2fd3ce1024e496c6c17eb27f81693f05ca8.tar.gz'
   timeout='none'
-- Using src='https://github.com/intercept/intercept/archive/f283e2fd3ce1024e496c6c17eb27f81693f05ca8.tar.gz'
-- Downloading... done
-- extracting...
     src='/home/wolf/projects/intercept-plugin-template/vendor/intercept/src/f283e2fd3ce1024e496c6c17eb27f81693f05ca8.tar.gz'
     dst='/home/wolf/projects/intercept-plugin-template/vendor/intercept/src/intercept'

get's the libs and builds just fi........ waaait..

[ 70%] Performing install step for 'intercept'
make[3]: *** No rule to make target 'install'.  Stop.
CMakeFiles/intercept.dir/build.make:73: recipe for target 'vendor/intercept/src/intercept-stamp/intercept-install' failed
make[2]: *** [vendor/intercept/src/intercept-stamp/intercept-install] Error 2
CMakeFiles/Makefile2:67: recipe for target 'CMakeFiles/intercept.dir/all' failed
make[1]: *** [CMakeFiles/intercept.dir/all] Error 2
Makefile:83: recipe for target 'all' failed
make: *** [all] Error 2

Where did it get that install step from. There is no install step.

intercept core doesn't have a install step. This apparently has for some reason. Do you know how to disable it? I found https://cmake.org/pipermail/cmake/2012-April/050023.html

# overwrite install() command with a dummy macro that is a nop
macro (install)
endmacro ()

Google tells me the ExternalProject adds that.

Ahh https://stackoverflow.com/questions/37838786/how-to-not-make-install-step-when-building-external-project-with-cmake/37855224

~Add~

  STEP_TARGETS build
  EXCLUDE_FROM_ALL TRUE

~to the External project.~

Didn't even notice before. But I see that the plugin is also compiling the intercept host binaries. Which ofcause isn't needed. This disables that. too.

dedmen commented 6 years ago

Okey that didn't work. There is

    BUILD_COMMAND ""
    INSTALL_COMMAND ""

to disable the build and install steps. But that causes the plugin template to fail to find the intercept include which... Makes total sense https://github.com/Dahlgren/intercept-plugin-template/blob/3d7e2faa819151bc3934512111d2270494b0fe0a/src/CMakeLists.txt#L19 it expects it in /intercept. but with this it is in /vendor/intercept/src/intercept I honestly would prefer it to stay in /intercept but I don't think ExternalProject can do that. Unless you make a install step for the project that moves the files.

Also hardcoded commit hash is a bad idea. Tons of people will grab this as zip. And then later on compile their plugins with a completly outdated intercept version while they don't know they have to update the hash. But you can just enter master branch instead of hash right? And I asked that before if it automatically downloads new master branch if there is one available and you kinda answered that here with

A new version of Intercept will also be automatically downloaded if version number if changed in the CMake file

The actual file link changing causing a new download is rather clear. But what about the branch download?

Dahlgren commented 6 years ago

I'll give it a go on Windows tonight and resolve any issues. I'm unable to get it to even start building with clang on macOS but the CMake parts works for me.