On Sunday 23 May 2010 05:22:17 Konstantin Tokarev wrote:
OK. Is it safe to link our own stuff (plugins, avogadro-app) to new
modules?
No, because then you have duplicate symbols. Kalzium links to the
original libavogadro, loads a few plugins (which it does), those
plugins link to split libraries and then you have a problem.
I’ve tried to link several plugins against monolithic library and
application to split ones. Everything seems to work. What I’m doing
wrong?
I was incorrect here, I should have prefixed that with “I think…” It is
something most of us avoid doing - bringing in multiple libraries with the
same symbols. In this case it is much less risky (same code, compiled twice),
but I would still have a few concerns with it:
- Does it work consistently across all platforms (probably)
- Doubles the compilation time for libavogadro
- Increae memory footprint (loading duplicate - split+monolithic)
- Redundant shared objects with duplicate symbols
- Multiple ways of doing things - split development, duplication
- Have you verified the new layout works everywhere?
I doubt it works on Windows, it looks like you copied parts of the libavoagdro
global header, without investigating how it works. For example,
// This macro should be used to export parts of the API
#ifndef A_EXPORT
#ifdef avogadro_EXPORTS
#define A_EXPORT A_DECL_EXPORT
#else
#define A_EXPORT A_DECL_IMPORT
#endif
#endif
This will only import symbols. On Linux/Mac that is fine (no difference), but on
Windows this will likely not compile. Part of this is my fault for not finding
more time to add some dashboards to catch this early. All of these changes
concern me because they are making large changes to the library, removing API
etc without discussion.
class A_EXPORT ElementTranslator : public QObject
On Windows I suspect we require multiple export macros for each module, that
is how it is normally done. This allows one module to call code from another
module and get the import/export logic correct.
Not to mention that you are doubling compile time. It should be an
option, defaulting to off, to build experimental split libraries.
Those who wish to experiment use it, those who are not interested/do
not know about it do not.
If you are going to split properly, then the headers should also be
split so that it is clear which headers belong to which libraries.
They are split
So in that case you have removed public API from avogadro/*, and code that
once compiled would not. This looks like a hack, and is why Kalzium compiled,
but could not link. You have headers in avogadro to “ease porting” but no
symbols that go along with that header.
install(FILES {periodictable_HDRS}
DESTINATION "{INCLUDE_INSTALL_DIR}/avogadro")
install(FILES {periodictable_HDRS} global.h
DESTINATION "{INCLUDE_INSTALL_DIR}/qperiodictable")
Look at Qt’s header directory layout. The build system can gloss over
their layout, but it is there for a reason, and if you are coding
against Qt and being strict you explicitly include the header file in
the specified directory.
include/avogadro is something like include/qt4 from were everything
can be included. Modules’ headers are in their own directories
They are, but they aren’t. This is another thing I do not agree with doing.
See above. In the build tree they are not, in the install tree they are.
Having multiple ways of doing things in the same release can leaded
to added confusion, having multiple layouts then means if both become
common enough you have to check for both.
Yes, but (in my opinion) it can simplify migration.
For example, David already ported his extension to new layout
I disagree.
We should be aiming for
least surprise, hence why I was advocating making a 2.0 release if
everyone feels this is the way we need to go. So far I have only
heard what you want, and am not sure how many others have an opinion.
We talked about living with an ABI break, and I agreed it was necessary and
would allow you to make some of the changes you wanted to. We never discussed
breaking API/linking/module layout in such major ways. This should have been
discussed before it was merged into master, I do not think now is the right
time to do it for minimal gain.
If we want to do it, then we should relax into the fact that the next release
will be some alphas/betas and then a 2.0. This at least lets our users and
developers know that should they upgrade there may be some work to do. It
allows the distros to realize that that will not be a simple bump, but
(possibly) a major change in the library.
There is no saying we ever have to release a 1.2, I would have liked to, but
if we must change the layout of the API in such a way, then let’s at least do
it properly. Otherwise, we back out some of these structural changes, and
shelve them for later.
I do not think the payoff is worth it in this case. That is my opinion. If
there is a general feeling that we must move down this path then ditch the
original library, split everything properly, don’t install headers multiple
times, test everything on all of our supported platforms and work on getting a
2.0 ready.
For KDE I can maintain a 1.y line, with minimal releases for bug fixes and
occasional backported features. At some point we bump the KDE requirement, and
port the code to use 2.y. So Kalzium is not a blocker, but that community
helped us a lot early on, and I want to make sure their interests are also
taken care of.
Thanks,
Marcus