Changes in Avogadro master break linking in Kalzium

On Saturday 22 May 2010 14:39:56 Konstantin Tokarev wrote:

You
don’t see Qt splitting into hundreds of libraries for each widget,
and you certainly don’t see them moving public API between libraries
in minor releases.

AFAIK, for Qt 4.5,4.6,4.7 could be called “major” releases. Why we
can’t call 1.2 “major”? And it’s better to make 1.1.x compatible with
1.2 than 1.0 (only my humble opinion)

You are incorrect - they are minor releases. Qt also maintains API and ABI
compatibility with previous minor releases. Only would they do what you
suggest in a major release, i.e. 5.0. That is not to say that they do not add
large amounts of new features in each minor release.

We used Qt as a model, something to aspire to, when designing Avogadro. They
are the reason we have things like d-pointers in the public API. 1.2 in its
current form should just be called 2.0.

Marcus

1.2 in its current form should just be called 2.0.

Yeah. If we’ll release 1.1.x once in 0.5 year and make 10 releases as
in 0.9, it’ll be a good time for 2.0 :slight_smile:


Regards,
Konstantin

On Saturday 22 May 2010 14:35:26 Konstantin Tokarev wrote:

That would preserve compatibility. Also, in general you want to
change the target name to reflect the library name, rather than
changing the output name. It makes it much easier to relate the
target to the compiled binary. OUTPUT_NAME should only really be
changed in situations where you have things like avogadro and
avogadro-app (both would naturally have the same target name, but
cannot).

Thanks for clarifying this thing. I thought it vice versa: use
“avogadro” for AvogadroWidget to simplify porting, since “AvogadroCore”
will be linked as interface library

What do you did doesn’t really help, as the symbols in the library are
different, so if you used everything, you already need to add logic,

if (Avogadro 1.0)
link to avogadro
else if (Avogadro 1.1)
link to core, widget…
else if (Avogadro 1.3)

endif()

See my concern about the changes. Kalzium would already have required an if
else, thus making starting the fall down the slippery slope. This is why good
libraries tend to remain conservative in what they change until major
releases.

Hence why I am saying if we keep the changes, then call the next release 2.0.
Most developers will then know that they should expect to do some porting
(amount of porting would be undefined). Also, if you have different libs in
Avogadro, their include files should be logically separated. See Qt for an
example - each module has an include directory. There is no point in keeping
the include directory layout, and changing the actual binary layout.

Marcus

On Saturday 22 May 2010 14:48:36 Konstantin Tokarev wrote:

1.2 in its current form should just be called 2.0.

Yeah. If we’ll release 1.1.x once in 0.5 year and make 10 releases as
in 0.9, it’ll be a good time for 2.0 :slight_smile:

Why do you think we made 10 releases in 0.9? Didn’t want to commit to API
until we were sure. Then there is timing, resources, funding. This is one of
the major reasons Avogadro has been less active, currently it is a volunteeer
effort and many of the core developers have limited free time/new, young
families.

Marcus

Why do you think we made 10 releases in 0.9?

0.9.0 - 0.9.9 (didn’t see anything from 0.9 before 0.9.3 though)

Didn’t want to commit to
API until we were sure. Then there is timing, resources, funding.
This is one of the major reasons Avogadro has been less active,
currently it is a volunteeer effort and many of the core developers
have limited free time/new, young families.

Sorry, I didn’t want to blame anyone…
I only see there’s nobody except for you willing to do releases (I
don’t want because I’ll certainly break something in API or ABI or
something else and it’s 100x worse if it would be made public)


Regards,
Konstantin

What do you did doesn’t really help, as the symbols in the library
are different, so if you used everything, you already need to add
logic,

if (Avogadro 1.0)
link to avogadro
else if (Avogadro 1.1)
link to core, widget…
else if (Avogadro 1.3)

endif()

OK. Is it safe to link our own stuff (plugins, avogadro-app) to new
modules?


Regards,
Konstantin

On Saturday 22 May 2010 15:28:51 Konstantin Tokarev wrote:

What do you did doesn’t really help, as the symbols in the library
are different, so if you used everything, you already need to add
logic,

if (Avogadro 1.0)

link to avogadro

else if (Avogadro 1.1)

link to core, widget…

else if (Avogadro 1.3)

endif()

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.

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. 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.

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. 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.

Marcus

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?

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

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

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

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.

Marcus


Regards,
Konstantin

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.

AFAIK, in such situation needed symbols are taken from library which
was loaded first. I remember effect of LD_PRELOAD, there’s also a
possibility to replace library function defining own function with th
same name inside application, etc. Correct me if I’m wrong


Regards,
Konstantin

Given more time I have hundreds of ideas, many of which involve
redesigning our API. How about building a full scene graph for
rendering, and using that with point sprites for highly scalable
OpenGL 2.0 rendering. I have some local code where I have
experimented with that.

If I split out rendering, and do it in another separate library, I
will also use a separate namespace, so that the old Avogadro
rendering code would still be valid.

Great! But replacement of code will break compatibility with video
cards not capable of OpenGL2

Have you read my proposal about RenderWidget? Maind idea is that client
doesn’t need to know what backend is used: different OpenGL-based
widgets or something else. I think your new code could be painlessly
added to that model because nobody will have to used it as “main”
rendering backend and switch backends in run time.

If you have a possibility to incorporate your new developments, maybe
it’s worth to create a branch targeted on 2.0 release?


Regards,
Konstantin

I personally would like to begin following more coherent development
patterns. Read the gitworkflows man page, this is something we are
adopting at Kitware and it shows great promise. All features are
developed in topic branches, those topic branches are worked on, and
when ready, they are merged into next. If, after some peer
review/testing, they are found to be good they are merged into
master. For more experimental changes, a pu (proposed updates) branch
is used.

Sound great, but I think we aren’t Qt for such complicated development
cycle to be reasonable. Keeping too many separate significantlydifferent
branches under development will inevitably add some cost of their
merging in future. But I think we could have “next” in addition to
master and 1.0


Regards,
Konstantin

Kalzium links to the
original libavogadro, loads a few plugins (which it does)

Question: is there any way for client of libavogadro to specify list of
loaded plugins? If I understand correctly, currently PluginManager loads
everything it can find


Regards,
Konstantin

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

On Sunday 23 May 2010 10:40:36 Konstantin Tokarev wrote:

Given more time I have hundreds of ideas, many of which involve
redesigning our API. How about building a full scene graph for
rendering, and using that with point sprites for highly scalable
OpenGL 2.0 rendering. I have some local code where I have
experimented with that.

If I split out rendering, and do it in another separate library, I
will also use a separate namespace, so that the old Avogadro
rendering code would still be valid.

Great! But replacement of code will break compatibility with video
cards not capable of OpenGL2

Fallbacks if it is worth the time/using the code already in place.

Have you read my proposal about RenderWidget? Maind idea is that client
doesn’t need to know what backend is used: different OpenGL-based
widgets or something else. I think your new code could be painlessly
added to that model because nobody will have to used it as “main”
rendering backend and switch backends in run time.

I have not had time to read it, other things are more pressing at present. The
whole point of the painters was to abstract the rendering, the code in the
widget is relatively thin. I can see how we could share most of the
interaction code, but you are still creating/destroying widgets at some level,
in order to use the specialized API for any given backend.

Our GLWidget has grown into a bit of a god class, I have been meaning to move
more of the code into the painters. Doing something about that would be good.
Definitely 2.0 material. There is also picking - what we do is deprecated in
the GL spec, and quite slow.

If you have a possibility to incorporate your new developments, maybe
it’s worth to create a branch targeted on 2.0 release?

At present this is experimental code. If I find more time to work on it I can
push something to look at. I tend to work in topic branches, and can push one
when it is doing something useful. Right now I have a lot on, but hope to make
more time to do some more meaningful development for Avogadro.

Marcus

  • Doubles the compilation time for libavogadro

You’re incorrect - a lot of code lives in plugins+avogadro-app, they
are compiled once.

Also, I don’t fully understand how double compilation helps - objects
are bit-equal because generated by one compiler with the same options,
aren’t them?

  • Increae memory footprint (loading duplicate - split+monolithic)

Not fully correct - Avogadro uses only one version. And I think if all
symbols are resolved using modular libraries, monolithic library will
not be loaded at all (needs investigation)

  • Redundant shared objects with duplicate symbols
  • Multiple ways of doing things - split development, duplication
  • Have you verified the new layout works everywhere?

No, I haven’t chance.

Regards,
Konstantin

On Sunday 23 May 2010 10:47:09 Konstantin Tokarev wrote:

I personally would like to begin following more coherent development
patterns. Read the gitworkflows man page, this is something we are
adopting at Kitware and it shows great promise. All features are
developed in topic branches, those topic branches are worked on, and
when ready, they are merged into next. If, after some peer
review/testing, they are found to be good they are merged into
master. For more experimental changes, a pu (proposed updates) branch
is used.

Sound great, but I think we aren’t Qt for such complicated development
cycle to be reasonable. Keeping too many separate significantlydifferent
branches under development will inevitably add some cost of their
merging in future. But I think we could have “next” in addition to
master and 1.0

It is not that complicated, I would start out with master, next and 1.0. It
does require people to actually do development on topic branches though (not
hard, but does require some discipline), and think about the feature they are
working on in that branch. If topic branches are not used, then this workflow
is not useful.

It sounds like you are thinking of CVS/SVN branches, if Git is used properly
then topic branches are used a lot. I am not sure that Qt is using this model,
I never stated they were…

Marcus

On Sunday 23 May 2010 11:36:34 Konstantin Tokarev wrote:

  • Doubles the compilation time for libavogadro

You’re incorrect - a lot of code lives in plugins+avogadro-app, they
are compiled once.

I think I am correct, but you did not follow what I meant. It looks like it
doubles the compile time for “libavogadro”, i.e. all of the sources in that
target would be compiled twice. I am incorrect though, as I use ccache and
that will cache the majority of those objects.

I am well aware that most of the code lives in the plugins and app - I wrote
quite a bit of it :slight_smile:

Also, I don’t fully understand how double compilation helps - objects
are bit-equal because generated by one compiler with the same options,
aren’t them?

  • Increae memory footprint (loading duplicate - split+monolithic)

Not fully correct - Avogadro uses only one version. And I think if all
symbols are resolved using modular libraries, monolithic library will
not be loaded at all (needs investigation)

I should have been more explicit - for users such as Kalzium…

  • Redundant shared objects with duplicate symbols
  • Multiple ways of doing things - split development, duplication
  • Have you verified the new layout works everywhere?

No, I haven’t chance.

The export macro will not work on Windows as it is, like I said…

Marcus

This will only import symbols. On Linux/Mac that is fine (no
difference), but on Windows this will likely not compile.

Sorry, not an expert in DLL construction. Hope you can fix it.

All of these changes concern me because they are
making large changes to the library, removing API etc without
discussion.

I’ve tried, but received no answers. I’ve posted “Refactoring Ideas”,
and Geoffrey said they’re good. Than I’ve tried to separate periodic
table and found it’s safe. Than I step by step splitted other things,
and I thought there will be no great difference for users (it’s not
really hard to add another library, right?).

Now I realize that these changes are not so safe as I thought. I didn’t
thought that Kalzium people will use trunk while they have supported
1.0.x. But I think changes are logical and they are needed, at least for
2.0


Regards,
Konstantin

On Sunday 23 May 2010 11:55:33 Konstantin Tokarev wrote:

This will only import symbols. On Linux/Mac that is fine (no
difference), but on Windows this will likely not compile.

Sorry, not an expert in DLL construction. Hope you can fix it.

This is why changes like this end up creating more work, more maintenance.
Personally I would rather be working on other things, but likely could fix it
up.

All of these changes concern me because they are
making large changes to the library, removing API etc without
discussion.

I’ve tried, but received no answers. I’ve posted “Refactoring Ideas”,
and Geoffrey said they’re good. Than I’ve tried to separate periodic
table and found it’s safe. Than I step by step splitted other things,
and I thought there will be no great difference for users (it’s not
really hard to add another library, right?).

Now I realize that these changes are not so safe as I thought. I didn’t
thought that Kalzium people will use trunk while they have supported
1.0.x. But I think changes are logical and they are needed, at least for
2.0

My point is that we spent all this time setting certain rules once we hit 1.0.
All of us are constantly learning, but pulling things out of public API is an
absolute no no. There are links to the KDE policies, which provide great
guidance here.

The changes are needed, in your opinion. I think we can get by without them.
Personally, I think they are good, but only for a 2.0 release, and only if
done properly.

Avogadro should not be treated as a personal sandbox, unless that is all we
want it to be. I have not had time to follow the lists all the time, and I am
sorry for not being as responsive. Answering all of your emails takes up quite
a bit of time too though, so when you send five in a day I could not always find
the time to look over each one and reply.

I will likely remain silent the rest of the day, as there is stuff I need to
get done. Git offers the opportunity to experiment, share those experiments and
merge when appropriate. I would rather see well tested changes going into
master, and stop merging everything.

If I take the time to set up a Windows and Linux dashboard for master and
next, will anyone actually do development on topic branches though?

http://my.cdash.org/index.php?project=Avogadro&display=project

The Mac dashboard is failing, the Windows one stopped at some point. I don’t
think master is in a great state right now. I would rather move away from
CVS/SVN development practices to something where tested features are landed on
next, and then master after some testing/verification.

I am not going to push hard for this, right now at least, but it makes
maintenance easier, history more useful and bug fixes can easily be merged into
both master and maint.

I will check my mail some time this evening, and try to reply if necessary.

Marcus

The Mac dashboard is failing

Maybe something is wrong on build host (e.g., fresh clone is needed)?
Successful build of master on Mac was reported recently


Regards,
Konstantin