fix link problem on win32 MSVC

[lifegpc]

On win32, LZ4LIB_STATIC_API will not exported to dll. So in this case, link static library first.

[Cyan4973]

This feels like a symptom of a larger problem.
Either your code only relies on public definitions, and it should link fine with the dynamic library (and if doesn't, there is a problem in the way the dynamic library is built).
Either it does rely on private definitions, and then it should be asked if this is a good idea.
Or, the API is not well defined, and something should be public but isn't.

Whatever the root cause, I suggest to investigate that first.

[lifegpc]

I am just wanna build lz4 command line tools.

The function starts with LZ4 is not exported by default. Of course, I can define LZ4LIB_STATIC_API to export it.
And XXH64 seems defined in xxhash.h and no function was exported.

[t-mat]

First of all, I could reproduce your issue with the following commands:

cd /d "%USERPROFILE%"
git clone https://github.com/lz4/lz4.git lz4-issue-1000
cd lz4-issue-1000
cd build\cmake
md msvc-2019
cd msvc-2019
cmake .. -G "Visual Studio 16 2019" -A x64
cmake --build . --config Release

But I think the root cause of this issue is current structure of CMakeLists.txt.
Please take a look at lz4/programs/Makefile first. Basically, Makefile rule for lz4 CLI is:

LZ4DIR   := ../lib

LIBFILES  = $(wildcard $(LZ4DIR)/*.c)
SRCFILES  = $(sort $(LIBFILES) $(wildcard *.c))
OBJFILES  = $(SRCFILES:.c=.o)

lz4: $(OBJFILES)
	$(CC) $(FLAGS) $(OBJFILES) -o $@$(EXT) $(LDLIBS)

It doesn't link any library file. It explicitly compiles and links all library and CLI files from source code directly.

On the other hand, CMakeLists.txt has the following lines:

...
if(BUILD_SHARED_LIBS)
  set(LZ4_LINK_LIBRARY lz4_shared)
else()
  set(LZ4_LINK_LIBRARY lz4_static)
endif()

# lz4
if (LZ4_BUILD_CLI)
  set(LZ4_PROGRAMS_BUILT lz4cli)
  add_executable(lz4cli ${LZ4_CLI_SOURCES})
  set_target_properties(lz4cli PROPERTIES OUTPUT_NAME lz4)
  target_link_libraries(lz4cli ${LZ4_LINK_LIBRARY})
endif()

It compiles (static or shared) library first. And it links the library to build lz4 CLI. Since this linking is the root cause of the issue, I think any linkage tweaking doesn't resolve it.

I'm not sure about convention of cmake community. But again, I suppose CMakeLists.txt should follow the basic structure of original Makefile.

[lifegpc]

I think if it have a static library, link to it first is a good idea because this don't need build same source files again.

[lifegpc]

Maybe I can change it to this. If it have a static library, link to it. If not, build library files again.

if(WIN32)
  if(BUILD_STATIC_LIBS)
    set(LZ4_LINK_LIBRARY lz4_static)
  else()
    list(APPEND LZ4_CLI_SOURCES ${LZ4_SOURCES})
  endif()
else()
# link to shared whenever possible, to static otherwise
  if(BUILD_SHARED_LIBS)
    set(LZ4_LINK_LIBRARY lz4_shared)
  else()
    set(LZ4_LINK_LIBRARY lz4_static)
  endif()
endif()

[t-mat]

I think if it have a static library, link to it first is a good idea because this don't need build same source files again.

You're right. I also realized bench.c has the following lines:

#define LZ4_STATIC_LINKING_ONLY
#include "lz4.h"
#define LZ4_HC_STATIC_LINKING_ONLY
#include "lz4hc.h"

Therefore, lz4 CLI must be linked with static library. So, I believe this issue is not only for WIN32 + MSVC.

---

Just in case, I checked difference of dependency of lz4 CLIs.

# make
cd
git clone https://github.com/lz4/lz4.git lz4-issue-1000
cd lz4-issue-1000
make
ldd ./lz4

linux-vdso.so.1 (0x00007ffd02ff2000)
libc.so.6 => /lib/x86_64-linux-gnu/libc.so.6 (0x00007f09c4eea000)
/lib64/ld-linux-x86-64.so.2 (0x00007f09c512e000)

# cmake
cd build/cmake
mkdir issue-1000
cd issue-1000
cmake ..
make
ldd ./lz4

linux-vdso.so.1 (0x00007ffe11565000)
liblz4.so.1 => /home/takayuki/lz4-issue-1000/build/cmake/issue-1000/liblz4.so.1 (0x00007febb3454000)
libc.so.6 => /lib/x86_64-linux-gnu/libc.so.6 (0x00007febb3256000)
/lib64/ld-linux-x86-64.so.2 (0x00007febb34e7000)

---

It seems 28db4ac (#296) introduced this change. @Cyan4973 do you know any additional context of it?

[Cyan4973]

I agree.
The lz4 CLI is not supposed to be linked to the dynamic library,
since it relies on a few STATIC_ONLY symbols.
The CLI is designed to be linked to the static library.

The safest solution would be to remove the recipe which tries to build the lz4 CLI as a program linked to the dynamic library.

[lifegpc]

So just

if(BUILD_STATIC_LIBS)
  set(LZ4_LINK_LIBRARY lz4_static)
else()
  list(APPEND LZ4_CLI_SOURCES ${LZ4_SOURCES})
endif()

[t-mat]

Thank you @lifegpc for taking your time! Now this PR fixes the issue which had been affected many platforms/distrubutions which use cmake.

---

The following note is not for this PR. Because it may be out of scope.

note: We need same change for lz4c.

[lifegpc]

note: We need same change for lz4c.

I see, I will add it.

[Cyan4973]

Looks good to me !