Task #2243
closed
Allow building of shared library
Added by Alex Afanasyev over 9 years ago. Updated almost 9 years ago.
100%
Description
Right now ndn-cxx is compiled as a static library. This was done in attempt to reduce problems when library ABI changes (which is basically any time there is any change to header files). Right now, the library is relatively stable, so it is beneficial to compile the library as dynamic one.
We still need to keep compilation as a static library, at least as an option. This is needed on some platforms, for example on Android.
One subtle issue with static compilation affects ndnSIM effort. With static library, each linked unit (another shared lib, an app) has its own copy of the variables that controls selection of the custom clocks. As a result, setCustomClocks method called inside the scenario (an app) has different effect from calling the same method inside the helper class (part of ndnSIM shared lib).
Updated by Junxiao Shi over 9 years ago
- Subject changed from Switch compilation to shared library (keeping static library as an option) to Compile as shared library by default
- Start date deleted (
11/30/2014)
Updated by Alex Afanasyev almost 9 years ago
- Status changed from New to Code review
- % Done changed from 0 to 100
Updated by Junxiao Shi almost 9 years ago
Compiling as shared library is good.
But making shared library as the default is a breaking change.
https://travis-ci.org/yoursunny/ndn-cxx-breaks/builds/65342104
For a Ubuntu source code deployment, the ndn-cxx shared library is installed to /usr/local/lib.
When a program (eg. NFD) linked with this shared library is loaded, the system does not look for the shared library object in /usr/local/lib unless this path is set in LD_LIBRARY_PATH environment variable.
Therefore, I suggest proceeding in multiple steps:
- introduce the option to compile as shared library, but keep the default as static object
- adjust all affected deployments:
- send a "breaking change notification" to ndn-lib and nfd-dev to notify users
- modify instructions, manuals, documents, build scripts of all owned projects to set (or educate users to set)
LD_LIBRARY_PATH
- change the default to static compilation
Updated by Davide Pesavento almost 9 years ago
After this change, will the ndn-cxx-dev deb package continue to install the static lib?
Junxiao Shi wrote:
* modify instructions, manuals, documents, build scripts of all owned projects to set (or educate users to set) `LD_LIBRARY_PATH`
That, or rpath in the executable.
Updated by Alex Afanasyev almost 9 years ago
After this change, will the
ndn-cxx-devdeb package continue to install the static lib?
I'm planning to switch to shared library for the package.
* modify instructions, manuals, documents, build scripts of all owned projects to set (or educate users to set) `LD_LIBRARY_PATH`That, or rpath in the executable.
Based on some posts, rpath is not that recommended way. Just in case, on OSX each library already has "rpath" embedded, so things become partially simpler (no need to ldconfig or define LD_LIBRARY_PATH), but partially more complicated (need to run ./waf install before running unit tests).
Updated by Alex Afanasyev almost 9 years ago
Compiling as shared library is good.
But making shared library as the default is a breaking change.
https://travis-ci.org/yoursunny/ndn-cxx-breaks/builds/65342104
This is not a breaking change. The problem is with test script, not the dependent projects. Simply run ldconfig after installing the library and error will be gone.
For a Ubuntu source code deployment, the ndn-cxx shared library is installed to
/usr/local/lib.
When a program (eg. NFD) linked with this shared library is loaded, the system does not look for the shared library object in/usr/local/libunless this path is set inLD_LIBRARY_PATHenvironment variable.
/usr/local/lib is normally present in the library load path. The exception that I know is Fedora, which requires either what you said or adding /usr/local/lib into /etc/ld.so.conf.d/
Therefore, I suggest proceeding in multiple steps:
- introduce the option to compile as shared library, but keep the default as static object
- adjust all affected deployments:
- send a "breaking change notification" to ndn-lib and nfd-dev to notify users
- modify instructions, manuals, documents, build scripts of all owned projects to set (or educate users to set)
LD_LIBRARY_PATH- change the default to static compilation
I could go with this approach, but not 100% convinced yet that it is necessary.
Updated by Davide Pesavento almost 9 years ago
Alex Afanasyev wrote:
After this change, will the
ndn-cxx-devdeb package continue to install the static lib?I'm planning to switch to shared library for the package.
Sure, but libndn-cxx.so goes into ndn-cxx package... I was asking if you intended to keep the static archive in the -dev package or not.
That, or rpath in the executable.
Based on some posts, rpath is not that recommended way. Just in case, on OSX each library already has "rpath" embedded, so things become partially simpler (no need to ldconfig or define
LD_LIBRARY_PATH), but partially more complicated (need to run./waf installbefore running unit tests).
Yes, rpath is strongly discouraged by some distros for installed packages. But packages will install the lib into /usr/lib anyway, so there's no problem in this case. The rpath is still useful (and is more convenient than LD_LIBRARY_PATH) for running ndn-cxx unit tests for example.
For reverse dependencies not using binary packages (i.e. dev environments): the pkgconfig file should take care of supplying the build-time library path, and for the runtime path, as you said most distros already include /usr/local/lib in their ld.so.conf. So this will be an issue only for those running Fedora (or similar distros) and compiling everything themselves. I think documenting the need for LD_LIBRARY_PATH is enough to solve these cases.
Updated by Alex Afanasyev almost 9 years ago
After this change, will the
ndn-cxx-devdeb package continue to install the static lib?I'm planning to switch to shared library for the package.
Sure, but
libndn-cxx.sogoes intondn-cxxpackage... I was asking if you intended to keep the static archive in the-devpackage or not.
Sorry, I misread the question. I wasn't planning to include static library, but I don't have a strong preference.
Updated by Davide Pesavento almost 9 years ago
I'm fine either way. I just wanted to know so that I can do the same in the Gentoo ebuilds.
Updated by Junxiao Shi almost 9 years ago
But making shared library as the default is a breaking change.
https://travis-ci.org/yoursunny/ndn-cxx-breaks/builds/65342104This is not a breaking change. The problem is with test script, not the dependent projects. Simply run
ldconfigafter installing the library and error will be gone.
Original documentation of either ndn-cxx and NFD/NLSR does not mention ldconfig.
If ldconfig becomes necessary, this is a breaking change.
Updated by Alex Afanasyev almost 9 years ago
Davide:
what's the soname of the library?
The current logic that is built into waf scripts is
(for ELF) to assign SONAME as
<library-name>.so.<MAJOR>(libndn-cxx.so.0), which as I understand assumes that ABI stays the same with minor/patch changes.(for Mach-O) to use
install-name<install-path>/<library-name>.dylib(/usr/local/lib/libndn-cxx.dylib). I think this is a bug, which we can fix in our version and report upstream.
Now question is what is best for us. Obviously, using libndn-cxx.so.0 (or /usr/local/lib/libndn-cxx.0.dylib after the fix) is not a good solution for us, as we are breaking ABI all the time with each minor and "patch" release.
My suggestion, with assumption that we will always break ABI with each release, is to hack waf to set:
(for ELF) SONAME to
<library-name>.so.<MAJOR>.<MINOR>.<PATCH>(libndn-cxx.so.0.3.2)(for Mach-O)
install-nameto<install-path>/<library-name>.<MAJOR>.<MINOR>.<PATCH>.dylib(/usr/local/lib/libndn-cxx.0.3.2.dylib)
Updated by Davide Pesavento almost 9 years ago
Alex Afanasyev wrote:
My suggestion, with assumption that we will always break ABI with each release, is to hack waf to set:
- (for ELF) SONAME to
<library-name>.so.<MAJOR>.<MINOR>.<PATCH>(libndn-cxx.so.0.3.2)
Yes this is definitely doable (openssl does more or less the same). Alternatively we can use the "libtool approach" to soname versioning, but I find it less intuitive and it requires a little more maintenance in our case, so I prefer the openssl approach.
Btw, are you sure you have to hack waf to implement this? the soname is a very basic feature of shared libraries, I'd be surprised if waf doesn't allow client code to pass the soname as parameter...
- (for Mach-O)
install-nameto<install-path>/<library-name>.<MAJOR>.<MINOR>.<PATCH>.dylib(/usr/local/lib/libndn-cxx.0.3.2.dylib)
I don't care about OS X and I don't know much about the Mach-O format, so I don't know whether this is going to break something or not. But I guess not, since by putting the version before .dylib you're basically making it part of the library basename. If you want to be sure, use a hyphen before the version instead of a dot, i.e. libndn-cxx-0.3.2.dylib (glib does this, but for a different reason than ours).
Updated by Alex Afanasyev almost 9 years ago
- Target version changed from v0.3 to v0.4
I just submitted a pull request for waf: https://github.com/waf-project/waf/pull/1583
Based on my understanding, there were some hard-coded not user-configurable parts.
Preliminary, will change waf to this version (https://github.com/cawka/waf/commit/2b3bb0af33c07b93a72803a332ea8286427006bc), which is just adding my patch on top of previous version (no waf version upgrade).
Updated by Davide Pesavento almost 9 years ago
Btw, for several good reasons we should also add -fvisibility=hidden and -fvisibility-inlines-hidden to the shared lib's default CXXFLAGS, and explicitly export all public types (we can use a macro for this).
Updated by Chengyu Fan almost 9 years ago
- Related to Bug #2853: Global variable in static library causes double free or corruption error added
Updated by Alex Afanasyev almost 9 years ago
Just in case. My pull request was merged to the waf upstream.
Updated by Alex Afanasyev almost 9 years ago
Btw, for several good reasons we should also add
-fvisibility=hiddenand-fvisibility-inlines-hiddento the shared lib's default CXXFLAGS, and explicitly export all public types (we can use a macro for this).
I'm not sure I have expertise with this and cannot estimate how much work it would require. We can keep the current issue open or create another one specifically for these options.
Updated by Junxiao Shi almost 9 years ago
- Blocks Feature #2859: Explicitly export public symbols added
Updated by Alex Afanasyev almost 9 years ago
Huh.. Apparently, we need to use -fPIC for static library. With the change I made, ndnSIM stops compiling on some platforms:
/usr/bin/ld: /usr/local/lib/libndn-cxx.a(data.cpp.2.o): relocation R_X86_64_32 against `.rodata.str1.1' can not be used when making a shared object; recompile with -fPIC
/usr/local/lib/libndn-cxx.a: error adding symbols: Bad value
Updated by Davide Pesavento almost 9 years ago
Alex Afanasyev wrote:
Huh.. Apparently, we need to use
-fPICfor static library. With the change I made, ndnSIM stops compiling on some platforms:
You need to link against the shared version of ndn-cxx obviously. Non-PIC code cannot be used in shared libraries (except on x86_32, but even there it's a kludge).
Updated by Alex Afanasyev almost 9 years ago
Yeah. I'm updating the test scripts and documentation of ndnSIM for that.
Updated by Alex Afanasyev almost 9 years ago
- Status changed from Code review to Closed
Updated by Davide Pesavento almost 9 years ago
uh? the default is still static as far as I can see.
Updated by Alex Afanasyev almost 9 years ago
- Subject changed from Compile as shared library by default to Allow building of shared library
Oops. I'll separate the issue.