Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Build issues on Darwin: so vs dylib #82

Open
zmwangx opened this issue Jan 2, 2016 · 10 comments
Open

Build issues on Darwin: so vs dylib #82

zmwangx opened this issue Jan 2, 2016 · 10 comments

Comments

@zmwangx
Copy link

zmwangx commented Jan 2, 2016

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
Copy link
Contributor

Hello71 commented Jan 3, 2016

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

@zmwangx
Copy link
Author

zmwangx commented Jan 3, 2016

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

@CounterPillow
Copy link

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
Copy link
Author

zmwangx commented Jan 5, 2016

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
Copy link
Collaborator

lvandeve commented Jan 6, 2016

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
Copy link
Author

zmwangx commented Jan 6, 2016

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 Homebrew/legacy-homebrew#47612 (comment)) 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
Copy link
Contributor

jibsen commented Jan 6, 2016

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
Copy link
Author

zmwangx commented Jan 6, 2016

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
Copy link

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
Copy link
Collaborator

lvandeve commented Jan 6, 2016

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants