Makefile build system does not with MSYS and CYGWIN as well as having issues with MINGW-W64

[JPeterMugaas]

When attempting to build lz4 for MINGW using the MSYS2 bash shell, encountered some issues. The install does not work, the export library has a .lib suffix instead of the standard .dll.a (using gcc for mingw), and the dll has a version number in it (not usual with the MSYS2 MINGW-W64 distribution). If building for MSYS2 itself, the Makefile generates the liblz4-$version name instead of msys-lz4-$version.dll. It's not unusual for this stuff to be patched by the MSYS2 and Cygwin mainters. I deally, I would like see a fix here since cmake depends indirectly on lz4.

There's several issues that I see here:

1. THe proper library suffixes and prefixes are NOT specified in the Makefiles.
2. The Makefile does not take into account Poxis-like environments such as MSYS2 and Cygwin which have version of the uname command. The OS venvironment variable would probably always be Windows_NT even in those environments. I prefer using "uname" rather than $(OS) in a Makefile. Interestingly enough, the MSYS2 version of uname will return a different value based the MSYSTEM environment variable. Examples are:

MSYSTEM=MINGW32 uname

MINGW32_NT-10.0

MSYSTEM=MINGW64 uname

MINGW64_NT-10.0

uname

MSYS_NT-10.0

3. The Makefiles probably would not be able to compile binaries for Windows on a Unix-based operating system where the target is NOT the same as the host.
4. Cygwin and MSYS2 binaries are compiled similarly to MINGW in that a seperate .dll export is usually generated and the .dll is placed in the bin directory instead of the lib directory.
5. The MSYS, MINGW, and CYGWIN uname values might vary based upon the version of Windows where they are being built on.

I have attached a diff I made showing what I did to try address these things. Incidentally, I would also think it's possible to embed version info in the .EXE's and .DLL's as well as an icon for the .EXE's. It's a good practice in Windows.

001-mingw-msys-cygwin-build-install.diff.txt

[Cyan4973]

Thanks for the insightful suggestions @JPeterMugaas .
I'm completely opened to a PR on this topic.

I see you have already attached a diff file in this issue.
If you can, it would be preferable to present the same work as a PR, since it makes it much easier to review, test and discuss the proposed changes, and the amount of changes suggested in this patch is large enough to deserve a good review.

[JPeterMugaas]

Alright, here you go. #689 . I might work on a little more but please feel free to comment. Some of this work is consolidating various patches from MSYS2 and CygWin. I'm also going to look at the cmake invocation and I may consider properly adding version information in the .DLL's. But I want to do this stuff properly because the .DLL names can change and that's a good thing because we don't want to mix a MSYS2 or Cygwin .DLL with something for MINGW32.

[Cyan4973]

Thanks @JPeterMugaas for your patches which make Windows builds much nicer !
Is there more you would like to add, or can this issue be closed ?

[JPeterMugaas]

I think that it can be closed. If I have anything further, I will add it. We'll see how things evolve.

[Silarn]

This change has caused some issues in my case. I assume it's intentionally incompatible with nmake, as nmake can't interpret the Makefile.inc, but previously we were able to use the build output .lib file to link with MSVC projects. Now the release only includes a .dll.a, which is not compatible. It would be nice if both formats could be included.

Yes, I can manually build a lib file to make everything work, but this will necessitate nontrivial changes in our build process to accommodate this change.

Edit:
After some testing, it would seem that simply renaming the file allows it to link successfully. As long as we don't run into any further problems doing this, we can probably just rename the file after extracting the release.

Does not work in practice, so building our own .lib file is the only way.

[Cyan4973]

I'm all fine with a version which could work for both.

The first thing to do is to design a test that can be run on Travis CI, so that the issue can be observed, the fix tested, and then continuously maintained.

[Silarn]

Actually, I think the problem is that there's no provided DLL in the last two releases. You can, in fact, link that .dll.a file but then there's no DLL present to link with at runtime, which causes the application to crash.

I'm now compiling against the provided static lib. This should work, but it also inflates the sizes of each of our libraries that was using LZ4 and the overall size of the application.

[JPeterMugaas]

Alright, I have noticed something similar with C++ Builder when attempting that. I think the best way to to try to approach the issue is to consider mingw32 and Visual C++ (or what you refer to as nmake) as separate targets. I have noticed some similar issues with C++Builder although there some code differences as well. The C++Builder make does have a different syntax for quite a number of things. And yes, I suspect that the .dll.a is different than a .lib not only in filename but format. In addition, the way the version information is generated is different in MINGW and Visual C++ is different. But with C++Builder, I was using the unofficial cmake CMakeLists and I even found an issue or two with it. The cygwin and MSYS .DLL's should still have the prefixes and suffixes as I had set in the Makefile.

The dll should probably have the name "liblz4.dll" for both the MINGW32 and Visual C++. The big issue is that we both are probably doing to completely INCOMPATIBLE things for Visual C++ and MINGW with totally different build systems and probably syntax as well.

[Silarn]

The provided static lib is not compatible with MSVC either, referencing GCC-style .o files. I would guess that the 1.9.x releases would work in theory, using the dynamic lib, but there's no base DLL file present to load at runtime. That's assuming that the the 1.8.x and 1.9.x GitHub releases didn't completely change the build system, and it's more about how we're handling the build output.

I'll go back to my custom built static lib for now.

[JPeterMugaas]

I doubt that all is really lost here because we have another option,
contrib/cmake_unofficial/CMakeLists.txt. This is something I'm working on now and I think that this might help if I can refine things. Now, maybe with the "Makefiles", I didn't abstract enough to cover both MSVC and MINGW (goodness knows I've tried).

[Silarn]

It's just that the Makefile.inc isn't compatible with MSVC, and more specifically nmake. I think there's also some sort of bug where it isn't grabbing the generated liblz4.dll during the release builds.

[Cyan4973]

It's just that the Makefile.inc isn't compatible with MSVC, and more specifically nmake.

Maybe we could try to be more specific on this point ?

Is it the include statement that's problematic ?
nmake documentation states it supports it, but maybe there are some subtle parsing issue ? (lowercase, uppercase, indentation, etc.)

Is it the name of the file Makefile.inc ? Is it its content ?

(That's where a test using nmake on Windows, through AppveyorCI, could be handy to observe the error and ensure it's fixed)

[Silarn]

I'm not able to pull it up right at the moment but as I recall the error I got was about there being a missing colon between the source and the target. Unfortunately I don't think it specified a line.

I just looked up the error, which is u1034: syntax error : separator missing

[JPeterMugaas]

Actually, it did specify a line. Here's what I found from the Visual Studio Prompt (I actually had to download that just for this).

Microsoft (R) Program Maintenance Utility Version 14.20.27508.1
Copyright (C) Microsoft Corporation. All rights reserved.

Makefile.inc(1) : fatal error U1034: syntax error : separator missing
Stop.

so that tells us that it's line 1 and that is:

ifeq ($(V), 1)

[Silarn]

Right. The core issue is that nmake conditionals are just a different format:

!IF
!IFDEF
!IFNDEF

etc...

So if the long term goal is to make compilation compatible with MSVC (out of the box), CMake might be the best option.

That being said, I think all I really need is a DLL and shared LIB file I can link against. This worked before, and I think it would be fine still if all the files were present. I don't mind renaming the .dll.a in the build process.

[Cyan4973]

Quick question :
did nmake ever worked before, with a previous version of the Makefile ?

[JPeterMugaas]

unless GNU Make and something like the FreeBSD Make have can support that format, we have an incompatability issue that would raise it's ugly head in way or another. And at this point, for a .DLL, I also added another complexity by embedding version info right into the .DLL and that too would be different for Visual C++ and binutils (where windres is being used). Add to this that I had to add a complexity to support Cygwin, MSYS, and MINGW better through a series of ifeqs.

[Silarn]

Probably not, we've just been linking against the release builds until now. (Which is what the included example MSVC solution is essentially doing, which is probably broken now.)

[JPeterMugaas]

Alright, I don't know how helpful this is but there is a directory, visual and a "Solution" for Visual C++ 2010 and Visual C++ 2017. Perhaps, we might also need a "Solution" for VS2019.

But we could also consider doing some refinements of cmake. The reason I hesitated before about that was that cmake itself depends upon libarchive and that depends upon lz4. I happen to have some experience with cmake

[Silarn]

I missed those VS projects at first glance. We can transition to building the library ourselves, which we do for some other dependencies. I just tried it out with the VS 2017 solution, and it's a simple conversion for VS 2019 which can be done on-the-fly since there is no incompatible code.

So that can work for us as an alternative.

[JPeterMugaas]

Actually, it might also be a good idea if we did have a solution for VS2019 (the conversion could be a start). The thing is that the CMakeLists.txt when being used for Visual C++ would have to be in sync with the solution is actually doing. It might generate a .dll named lz4.dll and I don't like that because it doesn't match the liblz4.dll generated by the solution but we also don't want a libliblz4.dll generated for MINGW32 or MINGW64. In addition, I would like the .DLL's to ALWAYS have the proper version information included with them.

[Silarn]

Either way. I think CMake simplifies the project in the long run as you shouldn't need to maintain a bunch of standalone VS projects, but it will take some work to replicate all of the current projects and versioning data. The VS projects work for now, though. However, if the intent for the releases is to provide a linkable DLL you can compile against, I'm guessing it's the mingw project that would need some adjustment. In that case, however, there is a CMake config argument (CMAKE_GNUtoMS) that would cause it to generate MSVC lib equivalents during the build, provided a local MSVC SDK is available. So that could provide a solution there as well.

[JPeterMugaas]

I agree that cmake is good in the long run here particularly as MSVC and MINGW both have different conventions for filenames and different formats for export files and static libs and we really should be isolated from having to discuss things such as lib prefix and suffixes (those differ based on tools and platforms). What I'm concerned about is making sure that things are being done properly the first time with that. I still have some work to do on the CMakeLists.txt but I think this is a separate discussion.

In the meantime, I have taken the liberty of making a PR that has Visual Studio files for 2019 so we should be happy for the moment.