You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@qpid.apache.org by Ted Ross <tr...@redhat.com> on 2012/02/20 22:23:19 UTC
Re: svn commit: r1291436 - in /qpid/trunk/qpid/cpp: include/qpid/framing/
include/qpid/types/ managementgen/qmfgen/ managementgen/qmfgen/templates/
src/qmf/ src/qpid/broker/ src/qpid/ha/
The scope of this fix seems to be much greater than that of the commit
that broke Windows.
A couple of questions:
1) Why add "BrokerImportExport.h" to cpp/src/qmf? There's already a
cpp/include/qmf/ImportExport.h that serves the same purpose.
2) Why all the changes to the generated code? The old generation
templates worked fine under Windows.
The main thing you need to be concerned with is that any classes and
class methods that need to be accessible from outside the
DLL/shared-object are prefixed with the proper EXTERN symbol *for the
library*. Furthermore, there's a different EXTERN symbol for inline
methods.
-Ted
On 02/20/2012 03:45 PM, aconway@apache.org wrote:
> Author: aconway
> Date: Mon Feb 20 20:45:22 2012
> New Revision: 1291436
>
> URL: http://svn.apache.org/viewvc?rev=1291436&view=rev
> Log:
> NO-JIRA: Fix missing EXTERN declarations, broke windows build.
>
> Added:
> qpid/trunk/qpid/cpp/src/qmf/BrokerImportExport.h (with props)
> Modified:
> qpid/trunk/qpid/cpp/include/qpid/framing/SequenceNumber.h
> qpid/trunk/qpid/cpp/include/qpid/framing/SequenceSet.h
> qpid/trunk/qpid/cpp/include/qpid/types/Variant.h
> qpid/trunk/qpid/cpp/managementgen/qmfgen/schema.py
> qpid/trunk/qpid/cpp/managementgen/qmfgen/templates/Class.h
> qpid/trunk/qpid/cpp/managementgen/qmfgen/templates/Event.h
> qpid/trunk/qpid/cpp/managementgen/qmfgen/templates/Package.h
> qpid/trunk/qpid/cpp/src/qpid/broker/Broker.h
> qpid/trunk/qpid/cpp/src/qpid/broker/ExchangeRegistry.h
> qpid/trunk/qpid/cpp/src/qpid/broker/Link.h
> qpid/trunk/qpid/cpp/src/qpid/broker/LinkRegistry.cpp
> qpid/trunk/qpid/cpp/src/qpid/broker/LinkRegistry.h
> qpid/trunk/qpid/cpp/src/qpid/broker/Queue.h
> qpid/trunk/qpid/cpp/src/qpid/broker/SemanticState.h
> qpid/trunk/qpid/cpp/src/qpid/ha/ReplicatingSubscription.h
>
> [SNIP...]
>
> ---------------------------------------------------------------------
> Apache Qpid - AMQP Messaging Implementation
> Project: http://qpid.apache.org
> Use/Interact: mailto:commits-subscribe@qpid.apache.org
>
---------------------------------------------------------------------
Apache Qpid - AMQP Messaging Implementation
Project: http://qpid.apache.org
Use/Interact: mailto:dev-subscribe@qpid.apache.org
Re: svn commit: r1291436 - in /qpid/trunk/qpid/cpp:
include/qpid/framing/ include/qpid/types/ managementgen/qmfgen/
managementgen/qmfgen/templates/ src/qmf/ src/qpid/broker/ src/qpid/ha/
Posted by Alan Conway <ac...@redhat.com>.
On Tue, 2012-02-21 at 11:07 -0500, Ted Ross wrote:
> On 02/21/2012 11:05 AM, Alan Conway wrote:
> > On Tue, 2012-02-21 at 09:29 -0600, Steve Huston wrote:
> >>> -----Original Message-----
> >>> From: Alan Conway [mailto:aconway@redhat.com]
> >>> Sent: Tuesday, February 21, 2012 10:23 AM
> >>> To: dev@qpid.apache.org; Ted Ross; astitcher@redhat.com
> >>> Subject: Re: svn commit: r1291436 - in /qpid/trunk/qpid/cpp:
> >>> include/qpid/framing/ include/qpid/types/ managementgen/qmfgen/
> >>> managementgen/qmfgen/templates/ src/qmf/ src/qpid/broker/
> >>> src/qpid/ha/
> >>>
> >>> On Mon, 2012-02-20 at 18:21 -0500, Andrew Stitcher wrote:
> >>>> On Mon, 2012-02-20 at 17:28 -0500, Alan Conway wrote:
> >>>>> ...
> >>>>> That is because there was no code outside of libqpidbroker calling
> >>>>> the generated functions (except for cluster.so but that is never
> >>>>> built on
> >>>>> windows.) The ha plugin now also calls some QMF generated functions.
> >>>>>
> >>>>> Perhaps a better solution is to move the QMF generated code for each
> >>>>> plugin into the plugin library itself, rather than putting it all
> >>>>> into libqpidbroker. If that's preferable I can do that.
> >>>>>
> >>>> I'd say that would be the most preferable idea - any generated code
> >>>> should only be included with the code that actually uses it. This is
> >>>> particular irritation with the current cluster implementation IMO.
> >>> So if we move the cluster& ha generated QMF code into their respective
> >>> plugins, do we then want to remove all the EXTERNs in the remaining broker
> >>> QMF code or leave it there for possible future use?
> >> Does the generated code to be in the plugins call the remaining broker QMF
> >> code?
> > Good point, I had overlooked that. The ha plugin does use the
> > definitions for broker events such as queue-create etc. So we would
> > still need EXTERNS in the qmf code even if we move the plugin-specific
> > qmf code into the plugin directories.
> >
> >
> Another possibility is to have the code generator create the proper raw
> externs for the environment (gcc, MS, mingw32) rather than use the macros.
I'd prefer to use the macros. We can't use the raw externs in
non-generated code, and I think it's best to be consistent between
generated and non-generated code.
---------------------------------------------------------------------
Apache Qpid - AMQP Messaging Implementation
Project: http://qpid.apache.org
Use/Interact: mailto:dev-subscribe@qpid.apache.org
Re: svn commit: r1291436 - in /qpid/trunk/qpid/cpp: include/qpid/framing/
include/qpid/types/ managementgen/qmfgen/ managementgen/qmfgen/templates/
src/qmf/ src/qpid/broker/ src/qpid/ha/
Posted by Ted Ross <tr...@redhat.com>.
On 02/21/2012 11:05 AM, Alan Conway wrote:
> On Tue, 2012-02-21 at 09:29 -0600, Steve Huston wrote:
>>> -----Original Message-----
>>> From: Alan Conway [mailto:aconway@redhat.com]
>>> Sent: Tuesday, February 21, 2012 10:23 AM
>>> To: dev@qpid.apache.org; Ted Ross; astitcher@redhat.com
>>> Subject: Re: svn commit: r1291436 - in /qpid/trunk/qpid/cpp:
>>> include/qpid/framing/ include/qpid/types/ managementgen/qmfgen/
>>> managementgen/qmfgen/templates/ src/qmf/ src/qpid/broker/
>>> src/qpid/ha/
>>>
>>> On Mon, 2012-02-20 at 18:21 -0500, Andrew Stitcher wrote:
>>>> On Mon, 2012-02-20 at 17:28 -0500, Alan Conway wrote:
>>>>> ...
>>>>> That is because there was no code outside of libqpidbroker calling
>>>>> the generated functions (except for cluster.so but that is never
>>>>> built on
>>>>> windows.) The ha plugin now also calls some QMF generated functions.
>>>>>
>>>>> Perhaps a better solution is to move the QMF generated code for each
>>>>> plugin into the plugin library itself, rather than putting it all
>>>>> into libqpidbroker. If that's preferable I can do that.
>>>>>
>>>> I'd say that would be the most preferable idea - any generated code
>>>> should only be included with the code that actually uses it. This is
>>>> particular irritation with the current cluster implementation IMO.
>>> So if we move the cluster& ha generated QMF code into their respective
>>> plugins, do we then want to remove all the EXTERNs in the remaining broker
>>> QMF code or leave it there for possible future use?
>> Does the generated code to be in the plugins call the remaining broker QMF
>> code?
> Good point, I had overlooked that. The ha plugin does use the
> definitions for broker events such as queue-create etc. So we would
> still need EXTERNS in the qmf code even if we move the plugin-specific
> qmf code into the plugin directories.
>
>
Another possibility is to have the code generator create the proper raw
externs for the environment (gcc, MS, mingw32) rather than use the macros.
---------------------------------------------------------------------
Apache Qpid - AMQP Messaging Implementation
Project: http://qpid.apache.org
Use/Interact: mailto:dev-subscribe@qpid.apache.org
RE: svn commit: r1291436 - in /qpid/trunk/qpid/cpp:
include/qpid/framing/ include/qpid/types/ managementgen/qmfgen/
managementgen/qmfgen/templates/ src/qmf/ src/qpid/broker/ src/qpid/ha/
Posted by Alan Conway <ac...@redhat.com>.
On Tue, 2012-02-21 at 09:29 -0600, Steve Huston wrote:
> > -----Original Message-----
> > From: Alan Conway [mailto:aconway@redhat.com]
> > Sent: Tuesday, February 21, 2012 10:23 AM
> > To: dev@qpid.apache.org; Ted Ross; astitcher@redhat.com
> > Subject: Re: svn commit: r1291436 - in /qpid/trunk/qpid/cpp:
> > include/qpid/framing/ include/qpid/types/ managementgen/qmfgen/
> > managementgen/qmfgen/templates/ src/qmf/ src/qpid/broker/
> > src/qpid/ha/
> >
> > On Mon, 2012-02-20 at 18:21 -0500, Andrew Stitcher wrote:
> > > On Mon, 2012-02-20 at 17:28 -0500, Alan Conway wrote:
> > > > ...
> > > > That is because there was no code outside of libqpidbroker calling
> > > > the generated functions (except for cluster.so but that is never
> > > > built on
> > > > windows.) The ha plugin now also calls some QMF generated functions.
> > > >
> > > > Perhaps a better solution is to move the QMF generated code for each
> > > > plugin into the plugin library itself, rather than putting it all
> > > > into libqpidbroker. If that's preferable I can do that.
> > > >
> > >
> > > I'd say that would be the most preferable idea - any generated code
> > > should only be included with the code that actually uses it. This is
> > > particular irritation with the current cluster implementation IMO.
> >
> > So if we move the cluster & ha generated QMF code into their respective
> > plugins, do we then want to remove all the EXTERNs in the remaining broker
> > QMF code or leave it there for possible future use?
>
> Does the generated code to be in the plugins call the remaining broker QMF
> code?
Good point, I had overlooked that. The ha plugin does use the
definitions for broker events such as queue-create etc. So we would
still need EXTERNS in the qmf code even if we move the plugin-specific
qmf code into the plugin directories.
---------------------------------------------------------------------
Apache Qpid - AMQP Messaging Implementation
Project: http://qpid.apache.org
Use/Interact: mailto:dev-subscribe@qpid.apache.org
RE: svn commit: r1291436 - in /qpid/trunk/qpid/cpp: include/qpid/framing/ include/qpid/types/ managementgen/qmfgen/ managementgen/qmfgen/templates/ src/qmf/ src/qpid/broker/ src/qpid/ha/
Posted by Steve Huston <sh...@riverace.com>.
> -----Original Message-----
> From: Alan Conway [mailto:aconway@redhat.com]
> Sent: Tuesday, February 21, 2012 10:23 AM
> To: dev@qpid.apache.org; Ted Ross; astitcher@redhat.com
> Subject: Re: svn commit: r1291436 - in /qpid/trunk/qpid/cpp:
> include/qpid/framing/ include/qpid/types/ managementgen/qmfgen/
> managementgen/qmfgen/templates/ src/qmf/ src/qpid/broker/
> src/qpid/ha/
>
> On Mon, 2012-02-20 at 18:21 -0500, Andrew Stitcher wrote:
> > On Mon, 2012-02-20 at 17:28 -0500, Alan Conway wrote:
> > > ...
> > > That is because there was no code outside of libqpidbroker calling
> > > the generated functions (except for cluster.so but that is never
> > > built on
> > > windows.) The ha plugin now also calls some QMF generated functions.
> > >
> > > Perhaps a better solution is to move the QMF generated code for each
> > > plugin into the plugin library itself, rather than putting it all
> > > into libqpidbroker. If that's preferable I can do that.
> > >
> >
> > I'd say that would be the most preferable idea - any generated code
> > should only be included with the code that actually uses it. This is
> > particular irritation with the current cluster implementation IMO.
>
> So if we move the cluster & ha generated QMF code into their respective
> plugins, do we then want to remove all the EXTERNs in the remaining broker
> QMF code or leave it there for possible future use?
Does the generated code to be in the plugins call the remaining broker QMF
code?
-Steve
---------------------------------------------------------------------
Apache Qpid - AMQP Messaging Implementation
Project: http://qpid.apache.org
Use/Interact: mailto:dev-subscribe@qpid.apache.org
Re: svn commit: r1291436 - in /qpid/trunk/qpid/cpp:
include/qpid/framing/ include/qpid/types/ managementgen/qmfgen/
managementgen/qmfgen/templates/ src/qmf/ src/qpid/broker/ src/qpid/ha/
Posted by Alan Conway <ac...@redhat.com>.
On Mon, 2012-02-20 at 18:21 -0500, Andrew Stitcher wrote:
> On Mon, 2012-02-20 at 17:28 -0500, Alan Conway wrote:
> > ...
> > That is because there was no code outside of libqpidbroker calling the
> > generated functions (except for cluster.so but that is never built on
> > windows.) The ha plugin now also calls some QMF generated functions.
> >
> > Perhaps a better solution is to move the QMF generated code for each
> > plugin into the plugin library itself, rather than putting it all into
> > libqpidbroker. If that's preferable I can do that.
> >
>
> I'd say that would be the most preferable idea - any generated code
> should only be included with the code that actually uses it. This is
> particular irritation with the current cluster implementation IMO.
So if we move the cluster & ha generated QMF code into their respective
plugins, do we then want to remove all the EXTERNs in the remaining
broker QMF code or leave it there for possible future use?
---------------------------------------------------------------------
Apache Qpid - AMQP Messaging Implementation
Project: http://qpid.apache.org
Use/Interact: mailto:dev-subscribe@qpid.apache.org
Re: svn commit: r1291436 - in /qpid/trunk/qpid/cpp:
include/qpid/framing/ include/qpid/types/ managementgen/qmfgen/
managementgen/qmfgen/templates/ src/qmf/ src/qpid/broker/ src/qpid/ha/
Posted by Andrew Stitcher <as...@redhat.com>.
On Mon, 2012-02-20 at 17:28 -0500, Alan Conway wrote:
> ...
> That is because there was no code outside of libqpidbroker calling the
> generated functions (except for cluster.so but that is never built on
> windows.) The ha plugin now also calls some QMF generated functions.
>
> Perhaps a better solution is to move the QMF generated code for each
> plugin into the plugin library itself, rather than putting it all into
> libqpidbroker. If that's preferable I can do that.
>
I'd say that would be the most preferable idea - any generated code
should only be included with the code that actually uses it. This is
particular irritation with the current cluster implementation IMO.
Andrew
---------------------------------------------------------------------
Apache Qpid - AMQP Messaging Implementation
Project: http://qpid.apache.org
Use/Interact: mailto:dev-subscribe@qpid.apache.org
Re: svn commit: r1291436 - in /qpid/trunk/qpid/cpp:
include/qpid/framing/ include/qpid/types/ managementgen/qmfgen/
managementgen/qmfgen/templates/ src/qmf/ src/qpid/broker/ src/qpid/ha/
Posted by Alan Conway <ac...@redhat.com>.
On Mon, 2012-02-20 at 16:23 -0500, Ted Ross wrote:
> The scope of this fix seems to be much greater than that of the commit
> that broke Windows.
It looks that way because in the original commit you only see the new
code for the HA plugin being added. You don't see the set of functions
in libqpidbroker that are being called by that new code - which is what
you see in the fix commit.
> A couple of questions:
>
> 1) Why add "BrokerImportExport.h" to cpp/src/qmf? There's already a
> cpp/include/qmf/ImportExport.h that serves the same purpose.
It doesn't. qmf/ImportExport.h defines EXTERNS for the qmf.so library.
The generated code doesn't go in that library, it goes in
libqpidbroker.so. Hence the awkward BrokerImportExport for the generated
code.
> 2) Why all the changes to the generated code? The old generation
> templates worked fine under Windows.
That is because there was no code outside of libqpidbroker calling the
generated functions (except for cluster.so but that is never built on
windows.) The ha plugin now also calls some QMF generated functions.
Perhaps a better solution is to move the QMF generated code for each
plugin into the plugin library itself, rather than putting it all into
libqpidbroker. If that's preferable I can do that.
> The main thing you need to be concerned with is that any classes and
> class methods that need to be accessible from outside the
> DLL/shared-object are prefixed with the proper EXTERN symbol *for the
> library*. Furthermore, there's a different EXTERN symbol for inline
> methods.
Correct, and as things are currently linked the ha plugin needs to call
QMF generated symbols in libqpidbroker so they need EXTERNs. Why do we
need EXTERNs for inline code? From what I saw making the changes, we
don't currently have EXTERNS on many/most inline functions in
libqpidbroker.
I'll be happy to fix this up however we decide is best.
Cheers,
Alan.
---------------------------------------------------------------------
Apache Qpid - AMQP Messaging Implementation
Project: http://qpid.apache.org
Use/Interact: mailto:dev-subscribe@qpid.apache.org