trendmicro / tlsh

Other
732 stars 136 forks source link

Use tlsh with cmake, missing include "version.h" #91

Closed adib0u closed 3 years ago

adib0u commented 4 years ago

When I use the c++ library in my project and include it with #include tlsh.h then i have the following error :

/usr/local/include/tlsh.h:66:10 fatal error: version.h: No file found
#include "version.h"

It seems like the file /usr/local/include/version.h doesn't exist. It's generated when i run make.sh but is not installed when using make install in the build/release directory. Am I doing something wrong during the installation ?

jonjoliver commented 4 years ago

There is no need to install any .h files in /usr/local/include/

There is no need to go into build/release directory and do the command $ make install

The binary "tlsh_unittest" in the bin directory is a stand-alone program

Should I add installation instructions? all the installation instructions will say is to copy the binary to the directory where you want it...

Dkapps commented 4 years ago

@jonjoliver Just giving my 2 cents here, but if we aren't supposed to install anything, I think we should either disable the install target or provide instructions on what exactly that target is for.

As it stands, it is kind of confusing because make install is AFAIK generally used to install libraries in /usr/local/lib + /usr/local/include (to allow for standardised library includes in build systems), or install programs in /usr/local/bin (or both, of course)

jonjoliver commented 3 years ago

@Dkapps I agree. That is a historical accident. I don't want to break other projects that use tlsh as a library. What is the safest way forwards here - that minimizes the impact on dependent projects.

juju4 commented 3 years ago

It seems problem is still current and appearing on centos8 as it does in my tlsh ansible role https://github.com/juju4/ansible-tlsh/runs/2809688106?check_suite_focus=true#step:6:376

any fix/workaround?

Thanks

Dkapps commented 3 years ago

@jonjoliver I opened the PR #103 to propose a fix for this situation.
Like I said in the description of the PR, if we do not want to remove the make install target, we might as well fix it so that it works as intended

Let me know if this works for you