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

Clean up the Makefile #35

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Clean up the Makefile #35

wants to merge 3 commits into from

Conversation

kousu
Copy link

@kousu kousu commented May 16, 2015

  • use := with $(shell), because = is lazily evaluated
  • build the lib by default, not just the programs (the Windows version already did this, it must have been an oversight)
  • follow the Windows naming conventions (this is important to me because I'm trying to write a cross-platform build which depends partially on libsvm and partially on other cross-platform libraries, and I want to be able to use a single declaration in my Makefile)
  • use make's features to get the platform detection out of the shell script
  • make the library file a real target and properly mark lib as a .PHONY one. Now make knows when it has already built libsvm.so.2.
  • don't explicitly embed -soname (Linux) or -install_name (OS X); rather, let the platform decide.
    Because of this, the target is built with its final name and then linked to its versioned name, instead of the other way around.
  • build the static library

OS X auto-embeds -install_name:

sasgradmac:libsvm nguenthe$ otool -L libsvm.dylib
libsvm.dylib:
    libsvm.dylib (compatibility version 0.0.0, current version 0.0.0)
    /usr/lib/libstdc++.6.dylib (compatibility version 7.0.0, current version 56.0.0)
    /usr/lib/libSystem.B.dylib (compatibility version 1.0.0, current version 169.3.0)

Linux doesn't embed -soname (but maybe it should??):

[kousu@galleon libsvm]$ readelf -d libsvm.so

Dynamic section at offset 0x103d8 contains 27 entries:
  Tag        Type                         Name/Value
 0x0000000000000001 (NEEDED)             Shared library: [libstdc++.so.6]
 0x0000000000000001 (NEEDED)             Shared library: [libm.so.6]
 0x0000000000000001 (NEEDED)             Shared library: [libgcc_s.so.1]
 0x0000000000000001 (NEEDED)             Shared library: [libc.so.6]
 0x000000000000000c (INIT)               0x2f18
 0x000000000000000d (FINI)               0xe728
 0x0000000000000019 (INIT_ARRAY)         0x2100c0
 0x000000000000001b (INIT_ARRAYSZ)       8 (bytes)
 0x000000000000001a (FINI_ARRAY)         0x2100c8
 0x000000000000001c (FINI_ARRAYSZ)       8 (bytes)
 0x000000006ffffef5 (GNU_HASH)           0x1b8
 0x0000000000000005 (STRTAB)             0x13c0
 0x0000000000000006 (SYMTAB)             0x568
 0x000000000000000a (STRSZ)              2742 (bytes)
 0x000000000000000b (SYMENT)             24 (bytes)
 0x0000000000000003 (PLTGOT)             0x210658
 0x0000000000000002 (PLTRELSZ)           1632 (bytes)
 0x0000000000000014 (PLTREL)             RELA
 0x0000000000000017 (JMPREL)             0x28b8
 0x0000000000000007 (RELA)               0x2048
 0x0000000000000008 (RELASZ)             2160 (bytes)
 0x0000000000000009 (RELAENT)            24 (bytes)
 0x000000006ffffffe (VERNEED)            0x1fa8
 0x000000006fffffff (VERNEEDNUM)         4
 0x000000006ffffff0 (VERSYM)             0x1e76
 0x000000006ffffff9 (RELACOUNT)          14
 0x0000000000000000 (NULL)               0x0

But this is inconsistent with what I've observed the system libraries on my Linux. Hence, I am not entirely sold on the idea of removing -soname, and if you think it is important I can easily revert it.

Also, please investigate what downstream is doing to your package. You can get clues from them, or you need to beat them over the head with a cluestick. I am not familiar enough to judge which.

macports added .dylib-specific flags: -current_version and -compatibility_version. I am not sure how important these are to OS X. The .dylib works without them, but it might be worthwhile to play by Apple's rules.

Debian/Ubuntu thinks LVER=3 and packages libsvm as "libsvm3", but your SHVER=2; what gives?

In good news, libsvm installs almost unmodified on ArchLinux. All they had to add was make lib and where files get finally installed.

kousu and others added 3 commits May 13, 2015 19:30
- use make's features to get the platform detection out of the shell script
- make the library file a real target: now use 'make libsvm.so.2' instead of 'make lib' (but the latter is still there for backwards compatibility)
- don't explicitly embed -soname (Linux) or -install_name (OS X); rather, let the platform decide.
  Because of this, the target is built with its final name and then linked to its versioned name, instead of the other way around.
MinGW describes the differences well:
http://www.mingw.org/wiki/Specify_the_libraries_for_the_linker_to_use
By following this convention, libsvm should be compatible with MinGW.
@kousu kousu mentioned this pull request May 16, 2015
svm.o: svm.cpp svm.h
$(CXX) $(CFLAGS) -c svm.cpp
$(CXX) $(CFLAGS) -c $<

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

out of curiosity: Couldn't we use implicit make rules for the 4 targets above?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. 100%. I can add these, or you can fork my branch, PR me, and I'll pull, if you want the credit in the log for it.

Unfortunately I don't think the authors pay close attention to this repo---their official one seems private---so we will have to wait a while to see what they think.

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

Successfully merging this pull request may close these issues.

2 participants