xiaolu / lz4

Automatically exported from code.google.com/p/lz4
1 stars 1 forks source link

Dynamic library and includes #103

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
Hello,

The current makefile allows to build and install lz4 and lz4c binaries.

Could you add the shared library and the headers files to the build and install 
process?

Resulting binaries (lz4 and lz4c) should be linked against this library.

Regards,

Original issue reported on code.google.com by sebastien.luttringer on 25 Dec 2013 at 8:08

GoogleCodeExporter commented 9 years ago

Original comment by yann.col...@gmail.com on 25 Dec 2013 at 8:22

GoogleCodeExporter commented 9 years ago
Sure.
I will have to learn how to do it properly, but since the latest cmake 
contribution seems to do the job correctly, it can serve as guidance in the 
learning process.

Original comment by yann.col...@gmail.com on 25 Dec 2013 at 8:23

GoogleCodeExporter commented 9 years ago
This can be done with a rule as simple as:

liblz4.so: lz4.c lz4hc.c lz4.h
       $(CC) -shared -fPIC -Wl,--soname=liblz4.so.1 $(CFLAGS) $(LDFLAGS) -o $@

Distributing the header file should be trivial. The "hard" part is 
understanding how to version and when/how to change the soname. Versioned 
symbols can make this easier (you'd need to create a .sym file and pass it to 
the linker as part of the compile). But, not all linkers support this.

I'd strongly consider learning autotools and libtool -- it will only make your 
life easier as your userspace grows.

Original comment by d...@falconindy.com on 25 Dec 2013 at 10:05

GoogleCodeExporter commented 9 years ago
The following attached file
is an attempt at providing
a lib builder and installer
within the official makefile (from root directory).

to build : make liblz4
to install : make install (it will also build, and install executables too).

The cmake file is currently broken, due to a change in directory structure.

As a minor comment : I'm not completely sure about the benefit of installing 
LZ4 libs. It seems to me that it creates a dependence on something created and 
installed by a 3rd party, which looks like a recipe for future problems. If it 
was me, I would always compile from the source.
So I'm probably missing something to understand why shared lib are so 
interesting, especially for a code as small as LZ4.

Original comment by yann.col...@gmail.com on 3 Jan 2014 at 4:04

Attachments:

GoogleCodeExporter commented 9 years ago
New version, with the cmake file corrected to the new directory tree.

Original comment by yann.col...@gmail.com on 3 Jan 2014 at 4:32

Attachments:

GoogleCodeExporter commented 9 years ago
libraries installation now part of r111

Original comment by yann.col...@gmail.com on 7 Jan 2014 at 6:52

GoogleCodeExporter commented 9 years ago
$ make all
gcc -O3 -c -I. -std=c99 -Wall -W -Wundef -DLZ4_VERSION=\"r111\" lz4.c lz4hc.c
ar rcs liblz4.a lz4.o lz4hc.o
gcc -shared -fPIC -Wl,--soname=liblz4.so.1 -I. -std=c99 -Wall -W -Wundef 
-DLZ4_VERSION=\"r111\" -o liblz4.so
make[1]: Entering directory '/home/seblu/scm/open/lz4/programs'
gcc      -O3 -I. -std=c99 -Wall -W -Wundef -DLZ4_VERSION=\"r111\" 
-DDISABLE_LZ4C_LEGACY_OPTIONS ../lz4.c ../lz4hc.c bench.c xxhash.c lz4cli.c -o 
lz4
bench.c:61:17: fatal error: lz4.h: No such file or directory
 #include "lz4.h"
                 ^
compilation terminated.
lz4cli.c:62:17: fatal error: lz4.h: No such file or directory
 #include "lz4.h"
                 ^
compilation terminated.
Makefile:57: recipe for target 'lz4' failed
make[1]: *** [lz4] Error 1
make[1]: Leaving directory '/home/seblu/scm/open/lz4/programs'
Makefile:70: recipe for target 'lz4programs' failed
make: *** [lz4programs] Error 2

Original comment by sebastien.luttringer on 7 Jan 2014 at 9:02

GoogleCodeExporter commented 9 years ago
Quick fix.

$ diff -u Makefile Makefile.2 
--- Makefile    2014-01-07 22:03:45.197517981 +0100
+++ Makefile.2  2014-01-07 22:03:43.126477362 +0100
@@ -34,7 +34,7 @@
 DESTDIR=
 PREFIX=/usr
 CC=gcc
-CFLAGS+= -I. -std=c99 -Wall -W -Wundef -DLZ4_VERSION=\"$(RELEASE)\"
+CFLAGS+= -I.. -std=c99 -Wall -W -Wundef -DLZ4_VERSION=\"$(RELEASE)\"

 BINDIR=$(PREFIX)/bin
 MANDIR=$(PREFIX)/share/man/man1

Original comment by sebastien.luttringer on 7 Jan 2014 at 9:04

GoogleCodeExporter commented 9 years ago
I suggest you to consider again using shared libraries into the programs.

In 2 bullets:
- Avoid code duplication (shared lib are mapped one time in memory by kernel 
for all lz4 executions)
- Avoid bugs between program and others tools using shared libs.

You have a full pros and cons of shared libs explained pretty well in wikipedia.

It also interessting to note that others recent compression tools do that:
$ ldd =bzip2|grep bz2
    libbz2.so.1.0 => /usr/lib/libbz2.so.1.0 (0x00007fcc390c3000)
$ ldd =xz|grep lz  
    liblzma.so.5 => /usr/lib/liblzma.so.5 (0x00007f5d88b72000)
$ ldd =lrzip|grep lzo
    liblzo2.so.2 => /usr/lib/liblzo2.so.2 (0x00007f9cee2fc000)

Copyrightly speaking, there is no difference to believe GNU GPL FAQ.

Original comment by sebastien.luttringer on 7 Jan 2014 at 9:50

GoogleCodeExporter commented 9 years ago
Another small comment about your current makefile workflow,

the default target of "make" build only the lib, where "make install" include 
program installation also programs.

So doing make && make install, leads to build binaries during make install. 
That's not a big deal, but it's a common expectation.

Original comment by sebastien.luttringer on 7 Jan 2014 at 10:06

GoogleCodeExporter commented 9 years ago
> +CFLAGS+= -I.. -std=c99 -Wall -W -Wundef -DLZ4_VERSION=\"$(RELEASE)\"

I'm baffled.
I could have swear this fix was already part of r111.
Besides, I tested the release directory just before commit...
Really, this is amazing this one got through.

OK, I guess I have to plan a quickfix then.

Original comment by yann.col...@gmail.com on 8 Jan 2014 at 9:34

GoogleCodeExporter commented 9 years ago
> doing make && make install, leads to build binaries during make install. 
That's not a big deal, but it's a common expectation.

OK, so make on the root directory should also trigger the default make into 
programs directory ?
Fair enough, it's quite simple to add.

Original comment by yann.col...@gmail.com on 8 Jan 2014 at 9:36

GoogleCodeExporter commented 9 years ago
The following attached release candidate should correct the 2 points mentioned 
above.

Shared library will have to wait, since it does not qualify as a "quickfix".

Original comment by yann.col...@gmail.com on 8 Jan 2014 at 10:08

Attachments:

GoogleCodeExporter commented 9 years ago
fixed into r112

Original comment by yann.col...@gmail.com on 8 Jan 2014 at 6:51

GoogleCodeExporter commented 9 years ago
Yann, issue is not really fixed as binaries are not linked against the library.

I was pinged by users about the BR 
https://code.google.com/p/lz4/issues/detail?id=107, which is a demonstration of 
my second bullet point of advantages to link lz4 to the shared library.

I could provide you a patch for that.

Original comment by sebastien.luttringer on 8 Jan 2014 at 8:45

GoogleCodeExporter commented 9 years ago
OK, let's give this issue another attempt.

What would be a correct Makefile instruction line
to build an LZ4 utility *using* the LZ4 shared library ?
I'm currently clueless on this point, and would greatly appreciate some help to 
move it forward.

Original comment by yann.col...@gmail.com on 28 Feb 2014 at 1:05