Xxhash: Makefile in release tarball does not contain the install targets

[amadio]

Please do not strip the install targets from the Makefile. I am packaging xxHash for Gentoo Linux, and noticed that the Makefile is incomplete compared to what's in the git repository.

[Cyan4973]

Which change, or patch, would you like ?
As far as I can tell, make install is expected to work on Linux ?

[Cyan4973]

ah ok, since the change in the title, the issue is clearer.

Indeed, v0.6.2 is dated 13 Aug 2016.
Back then, there was no install target in Makefile. It was added later on.

[amadio]

Yes, I'm picking the release tarball. Maybe just creating a new release would be fine, then. Thanks!

[Cyan4973]

done : https://github.com/Cyan4973/xxHash/releases/tag/v0.6.3

[amadio]

Thanks! As a side note, shouldn't the header be installed on make install? How is xxhash supposed to be used by projects depending on it (e.g., LZ4)?

[amadio]

Maybe creating a small library with the hash functions would be useful for other projects.

[Cyan4973]

Right,
currently, there is no library created.
It could be added, though, to be fair, I haven't received any request in this direction so far.
I guess most users include the source code directly.

As a reference point, it would be good to know how it works for some other common checksum such as md5sum. The cli side is fine, it's the library side which is still mysterious to me.

[atweiden]

I haven't received any request in this direction so far

I use the xxHash shared library in a Perl6 package. 0.6.3 is broken for me, as it does not include 0.6.2's options to build a shared library.

[Cyan4973]

I'm surprised because xxHash release tarball never offered a Makefile target to build a shared library :
https://github.com/Cyan4973/xxHash/blob/v0.6.2/Makefile

However, the unofficial community-supported cmake script did :
https://github.com/Cyan4973/xxHash/blob/v0.6.2/cmake_unofficial/CMakeLists.txt

It was updated in v0.6.3, maybe that's when it was broken ?
https://github.com/Cyan4973/xxHash/blob/v0.6.3/cmake_unofficial/CMakeLists.txt
https://github.com/Cyan4973/xxHash/commit/5ab73ee82245b586f69762411edc7824d778ee2a

_Edit_ : I tested the new cmake script, indeed, it only produces the static library, not the dynamic one as it used to in 0.6.2.

_Edit 2_ : Actually, it's possible to build dynamic library with this script, it requires using flag -DBUILD_SHARED_LIBS=ON. According to patch's author, @ChrisKitching, this is standard behavior for cmake scripts. Though it's unclear to me how users can discover it...

[atweiden]

Thanks, -DBUILD_SHARED_LIBS did the trick.

[ChrisKitching]

Though it's unclear to me how users can discover it...

https://cmake.org/cmake/help/v3.0/variable/BUILD_SHARED_LIBS.html

It's in the manual.

It's also the default behaviour. If you're only getting static libraries by default, then something in your cmake script must be explicitly setting this to off (effectively changing the default).

[Cyan4973]

It's also the default behaviour. If you're only getting static libraries by default, then something in your cmake script must be explicitly setting this to off (effectively changing the default).

The cmake script has not been modified since your patch.
There is no variable BUILD_SHARED_LIBS mentioned in the file.

According to your statement, it means that it should be set to ON by default.
But tests show that it only produces static library.

Now, if it's enough to just add an option line with BUILD_SHARED_LIBS set to ON, I can certainly do it.

[Cyan4973]

dev branch update :

cmake script was modified to also create shared library by default.

Since there are users of the library in binary format,
I've also added a lib target to Makefile.
make install now also installs static and shared libraries.

[amadio]

@Cyan4973 Could you please tag a new release? I'd like to use the updated Makefile when packaging xxHash for Gentoo. Cheers,

[Cyan4973]

xxHash v0.6.4 has been released

[amadio]

Thank you!