Build issues on Darwin: so vs dylib

zmwangx commented 8 years ago

On OS X 10.11.2:

> make libzopfli libzopflipng
gcc src/zopfli/blocksplitter.c src/zopfli/cache.c src/zopfli/deflate.c src/zopfli/gzip_container.c src/zopfli/hash.c src/zopfli/katajainen.c src/zopfli/lz77.c src/zopfli/squeeze.c src/zopfli/tree.c src/zopfli/util.c src/zopfli/zlib_container.c src/zopfli/zopfli_lib.c -W -Wall -Wextra -ansi -pedantic -lm -O2 -fPIC -c
gcc src/zopfli/blocksplitter.c src/zopfli/cache.c src/zopfli/deflate.c src/zopfli/gzip_container.c src/zopfli/hash.c src/zopfli/katajainen.c src/zopfli/lz77.c src/zopfli/squeeze.c src/zopfli/tree.c src/zopfli/util.c src/zopfli/zlib_container.c src/zopfli/zopfli_lib.c -W -Wall -Wextra -ansi -pedantic -lm -O2 -fPIC -c
clang: warning: -lm: 'linker' input unused
clang: warning: -lm: 'linker' input unused
g++ blocksplitter.o cache.o deflate.o gzip_container.o hash.o katajainen.o lz77.o squeeze.o tree.o util.o zlib_container.o zopfli_lib.o src/zopflipng/lodepng/lodepng.cpp src/zopflipng/lodepng/lodepng_util.cpp src/zopflipng/zopflipng_lib.cc -W -Wall -Wextra -ansi -pedantic -lm -O2 -fPIC --shared -Wl,-soname,libzopflipng.so.1 -o libzopflipng.so.1.0.0
gcc blocksplitter.o cache.o deflate.o gzip_container.o hash.o katajainen.o lz77.o squeeze.o tree.o util.o zlib_container.o zopfli_lib.o -W -Wall -Wextra -ansi -pedantic -lm -O2 -shared -Wl,-soname,libzopfli.so.1 -o libzopfli.so.1.0.1
clang: warning: argument unused during compilation: '-ansi'
ld: unknown option: -soname
clang: error: linker command failed with exit code 1 (use -v to see invocation)
make: *** [libzopfli] Error 1
make: *** Waiting for unfinished jobs....
src/zopflipng/zopflipng_lib.cc:34:5: warning: field 'lossy_transparent' will be initialized after field 'verbose' [-Wreorder]
  : lossy_transparent(false)
    ^
1 warning generated.
ld: unknown option: -soname
clang: error: linker command failed with exit code 1 (use -v to see invocation)
make: *** [libzopflipng] Error 1

This is because ld on Darwin doesn't recognize the -soname option, and instead use -install_name. Also, following Darwin's conventions, shared libraries should be named libfoo.ver.dylib, so the following changes should be applied when building on Darwin:

diff --git a/Makefile b/Makefile
index 26518ec..dbe20ef 100644
--- a/Makefile
+++ b/Makefile
@@ -25,7 +25,7 @@ zopfli:
 # Zopfli shared library
 libzopfli:
    $(CC) $(ZOPFLILIB_SRC) $(CFLAGS) -fPIC -c
-   $(CC) $(ZOPFLILIB_OBJ) $(CFLAGS) -shared -Wl,-soname,libzopfli.so.1 -o libzopfli.so.1.0.1
+   $(CC) $(ZOPFLILIB_OBJ) $(CFLAGS) -shared -Wl,-install_name,libzopfli.1.dylib -o libzopfli.1.0.1.dylib

 # ZopfliPNG binary
 zopflipng:
@@ -35,7 +35,7 @@ zopflipng:
 # ZopfliPNG shared library
 libzopflipng:
    $(CC) $(ZOPFLILIB_SRC) $(CFLAGS) -fPIC -c
-   $(CXX) $(ZOPFLILIB_OBJ) $(LODEPNG_SRC) $(ZOPFLIPNGLIB_SRC) $(CFLAGS) -fPIC --shared -Wl,-soname,libzopflipng.so.1 -o libzopflipng.so.1.0.0
+   $(CXX) $(ZOPFLILIB_OBJ) $(LODEPNG_SRC) $(ZOPFLIPNGLIB_SRC) $(CFLAGS) -fPIC --shared -Wl,-install_name,libzopflipng.1.dylib -o libzopflipng.1.0.0.dylib

 # Remove all libraries and binaries
 clean:

Hello71 commented 8 years ago

the "right" way to fix this is to move to a cross-platform build tool, but cmake sucks, and autotools... ugh.

zmwangx commented 8 years ago

I agree, but build failures suck even harder... Also there ought to be an install target...

CounterPillow commented 8 years ago

autotools would likely be the most appropriate choice for a project of this size. However, even if somebody made a pull request, it does currently not look like it'd be accepted; there are currently no less than three open PRs (#57, #58, and #69) which touch up or replace the very broken build system. The makefile already causes other issues such as #68 (which is why the Arch Linux package does a silly little dance with which order the make commands are executed in).

One of the things I wonder is whether there's a reason none of these PRs have gotten an official response or review - it feels like trying to fix the build system is an exercise in guessing what changes the contributors would feel okay with.

If somebody wants to take a crack at creating an autotools-based build system, https://autotools.io/ will probably be useful.

zmwangx commented 8 years ago

there are currently no less than three open PRs

Noticed.

One of the things I wonder is whether there's a reason none of these PRs have gotten an official response or review.

Same. Even a "no" would help.

In the end for Homebrew we just left out the shared libraries (although we could patch the Makefile).

lvandeve commented 8 years ago

Hello,

I agree getting build failures sucks.

Zopfli only depends on C90 and zopflipng requires only standard C++98, there are no external dependencies to make it portable. There are a few optional extras such as a recently added fix for the Windows terminal using win32. As far as I know, this makes it an easy case to build on most platforms with most build systems, and doesn't require a large tool.

I currently don't want to commit to a particular build system which is why I didn't accept the pull requests. In fact, I prefer plain make as it is the most widely available.

The dynamic libraries in the makefile are not needed to build the binaries, but for allowing others to depend on it in a standard way.

Providing multiple makefiles for the most popular platforms could be an option. I do not know how to make one for Darwin though. Would the above code in a different makefile work, and what filename can it have?

For now, on other platforms, I hope it is possible to build the binaries with the instructions given below. Commands for gcc (these also work for clang/clang++ of course) are given as well, for other compilers: please enable optimizations.

For zopfli, compile all .c source files under src/zopfli to a single binary with C, and link to the standard C math library: gcc src/zopfli/*.c -O2 -W -Wall -Wextra -ansi -pedantic -lm -o zopfli
For zopflipng, compile all .c, .cc and .cpp files from src/zopfli, src/zopflipng and src/zopflipng/lodepng, except src/zopfli/zopfli_bin.c, to a single binary with C++: g++ src/zopfli/{blocksplitter,cache,deflate,gzip_container,hash,katajainen,lz77,squeeze,tree,util,zlib_container,zopfli_lib}.c src/zopflipng/*.cc src/zopflipng/lodepng/*.cpp -O2 -W -Wall -Wextra -ansi -pedantic -o zopflipng

Does this work?

zmwangx commented 8 years ago

The current Makefile builds binaries just fine on OS X.

Providing multiple makefiles for the most popular platforms could be an option.

Actually one Makefile with conditionals goes a long way. The following patch (kudos to DomT4 https://github.com/Homebrew/homebrew/pull/47612#issuecomment-168534493) should make the current Makefile work on Darwin and Linux alike, and presumably other Unix or Unix-like systems (uname is defined by POSIX), and similar exceptions like the Darwin one can be easily made if it fails on some:

diff --git a/Makefile b/Makefile
index 26518ec..8d2b5dc 100644
--- a/Makefile
+++ b/Makefile
@@ -1,6 +1,8 @@
 CC = gcc
 CXX = g++

+UNAME := $(shell uname -s)
+
 CFLAGS = -W -Wall -Wextra -ansi -pedantic -lm -O2
 CXXFLAGS = -W -Wall -Wextra -ansi -pedantic -O2

@@ -18,14 +20,21 @@ ZOPFLIPNGBIN_SRC := src/zopflipng/zopflipng_bin.cc

 .PHONY: zopfli zopflipng

+all: zopfli libzopfli zopflipng libzopflipng
+
 # Zopfli binary
 zopfli:
    $(CC) $(ZOPFLILIB_SRC) $(ZOPFLIBIN_SRC) $(CFLAGS) -o zopfli

 # Zopfli shared library
 libzopfli:
+ifeq ($(UNAME),Darwin)
+   $(CC) $(ZOPFLILIB_SRC) $(CFLAGS) -fPIC -c
+   $(CC) $(ZOPFLILIB_OBJ) $(CFLAGS) -shared -Wl,-install_name,libzopfli.1.dylib -o libzopfli.1.0.1.dylib
+else
    $(CC) $(ZOPFLILIB_SRC) $(CFLAGS) -fPIC -c
    $(CC) $(ZOPFLILIB_OBJ) $(CFLAGS) -shared -Wl,-soname,libzopfli.so.1 -o libzopfli.so.1.0.1
+endif

 # ZopfliPNG binary
 zopflipng:
@@ -34,8 +43,13 @@ zopflipng:

 # ZopfliPNG shared library
 libzopflipng:
+ifeq ($(UNAME),Darwin)
+   $(CC) $(ZOPFLILIB_SRC) $(CFLAGS) -fPIC -c
+   $(CXX) $(ZOPFLILIB_OBJ) $(LODEPNG_SRC) $(ZOPFLIPNGLIB_SRC) $(CFLAGS) -fPIC --shared -Wl,-install_name,libzopflipng.1.dylib -o libzopflipng.1.0.0.dylib
+else
    $(CC) $(ZOPFLILIB_SRC) $(CFLAGS) -fPIC -c
    $(CXX) $(ZOPFLILIB_OBJ) $(LODEPNG_SRC) $(ZOPFLIPNGLIB_SRC) $(CFLAGS) -fPIC --shared -Wl,-soname,libzopflipng.so.1 -o libzopflipng.so.1.0.0
+endif

 # Remove all libraries and binaries
 clean:

An install target should also be added, which is pretty easy.

Of course this would break Windows, but (with limited knowledge of Windows) I suspect it wouldn't build on Windows in the first place, since Windows use DLLs. Maybe a separate Makefile could be made for Windows.

For now, on other platforms, I hope it is possible to build the binaries with the instructions given below.

Sounds good.

jibsen commented 8 years ago

The current Makefile works for building the executables on Windows using mingw-w64 or MSYS2 (probably Cygwin as well, I did not check). If you want to call out to uname, you could check for Windows first.

ifeq ($(OS),Windows_NT)
   UNAME = Windows_NT
else
   UNAME := $(shell uname -s)
endif

As far as building a DLL goes, I think there are multiple issues here,

The option structures are not packed, which could result in padding differences between the DLL and user
MSVC does not export symbols by default, and you would need a module-definition file, or suitable macros for declaring exported symbols.

It really is ridiculous how hard it is to be cross-platform enough to just cover the three most common platforms.

zmwangx commented 8 years ago

The current Makefile works for building the executables on Windows using mingw-w64 or MSYS2.

When talking about Windows, I'm not referring to Cygwin or MinGW, because they're inherently Unix systems, or at least strive to be Unix systems. I'm referring to vanilla Windows with a C compiler.

It really is ridiculous how hard it is to be cross-platform enough to just cover the three most common platforms.

Well, it's a well known and unsurprising fact that FOSS written for *ix requires (tremendous) effort to be ported to Windows. That's why to this day (AFAIK) Windows is still second class citizen in terms of git.

CounterPillow commented 8 years ago

I'm referring to vanilla Windows with a C compiler.

mingw-w64 produces native Windows executables with no compatibility layer. It's as vanilla as it gets. What you mean is MSVC.

Well, it's a well known and unsurprising fact that FOSS written for *ix requires (tremendous) effort to be ported to Windows.

I've created multiple (native) MSYS2 mingw-w64 packages and it's usually fairly easy, the biggest issue are build systems and preprocessor code which try to be smart and assume that all Windows runs MSVC.

lvandeve commented 8 years ago

Well, it's a well known and unsurprising fact that FOSS written for *ix requires (tremendous) effort to be ported to Windows.

Zopfli should be doable though. As mentioned before, it has no external dependencies, so e.g. in MSVC it requires adding all the mentioned source files in a new project and compiling.

google / zopfli

Build issues on Darwin: so vs dylib #82