You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@qpid.apache.org by Alan Conway <ac...@redhat.com> on 2016/10/18 17:24:33 UTC
Re: [Proton-c][Solaris][SunStudio-2.1] Compiling Qpid Proton-c and
C++ bindings on Solaris
Hi Adel,
I've got your Solaris fixes and my (minor) changes up on
�https://github.com/alanconway/qpid-proton/tree/absolaris
I'd like Andrew to take a look (he's back tomorrow), unless he has
anything to add I'll commit them.
Cheers,
Alan.
On Mon, 2016-10-17 at 20:03 +0000, Adel Boutros wrote:
> Hello Alan,
>
> No, it isn't a necessary fix (change from T*).
>
> However, we compiled this code on linux using GCC 4.9.1 and we didn't
> get any error on the master. I wonder which version of gcc you are
> using which revealed this error?
>
> Regards,
> Adel
>
> Ps: The original issue here is that CC doesn't handle constructors
> with "..." correctly which is here the case of sfinae::wildcard(...)
>
> Get Outlook for Android
>
>
>
> On Mon, Oct 17, 2016 at 9:48 PM +0200, "Alan Conway" <aconway@redhat.
> com> wrote:
>
> Hi Adel,
>
> I've put a workaround for one GCC problem on�https://github.com/alanc
> on
> way/qpid-proton/tree/absolaris, otherwise I'm happy with this. I'll
> wait for Andrew's OK and then commit if he's happy.
>
> One question: this makes GCC choke:
>
> index b1aff89..5285e4b 100644
> --- a/proton-c/bindings/cpp/include/proton/codec/encoder.hpp
> +++ b/proton-c/bindings/cpp/include/proton/codec/encoder.hpp
> @@ -175,14 +175,14 @@ namespace is_encodable_impl {���// Protected
> the
> world from wildcard operator<<
> �
> �using namespace internal;
> �
> -sfinae::no operator<<(sfinae::wildcard, sfinae::wildcard); //
> Fallback
> +sfinae::no operator<<(encoder const&, const sfinae::any_t &); //
> Fallback
> �
> �template<typename T> struct is_encodable : public sfinae {
> -����static yes test(encoder);
> +����static yes test(encoder&);
> �����static no test(...);���������// Failed test, no match.
> -����static encoder* e;
> -����static const T* t;
> -����static bool const value = sizeof(test(*e << *t)) == sizeof(yes);
> +����static encoder& e;
> +����static const T& t;
> ^^^^^^^THIS LINE^^^^^^^
> +����static bool const value = sizeof(test(e << t)) == sizeof(yes);
> �};
> �
> Is the change from T* to T& required to compile on Solaris? If not I
> would probably revert it, but if it is needed then the workaround
> seems
> to be OK. Windows & clang compilers don't have any problem so I
> suspect
> this is GCC's fault.
>
> Cheers,
> Alan.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
Re: [Proton-c][Solaris][SunStudio-2.1] Compiling Qpid Proton-c and
C++ bindings on Solaris
Posted by Alan Conway <ac...@redhat.com>.
On Thu, 2016-10-20 at 12:52 +0000, Adel Boutros wrote:
> Thanks alot Alan and Andrew for your help!!
>
>
> However, it seems one patch has slipped. It is about "int8_t".
>
>
> PROTON-1324: Interpretation of "int8_t" on Solaris using SunStudio is
> different from GCC one
>
> https://issues.apache.org/jira/browse/PROTON-1324
>
>�
Comitted a slightly different patch, should have the same effect on
Solaris but is a bit more general:
modified���proton-
c/bindings/cpp/include/proton/internal/type_traits.hpp
@@ -30,6 +30,7 @@
�#include "./config.hpp"
�#include "../types_fwd.hpp"
�#include "../type_id.hpp"
+#include <limits>
�
�namespace proton {
�namespace internal {
@@ -47,7 +48,7 @@ template <class T> struct is_integral : public
false_type {};
�template <class T> struct is_signed : public false_type {};
�
�template <> struct is_integral<char> : public true_type {};
-template <> struct is_signed<char> : public false_type {};
+template <> struct is_signed<char> { static const bool value =
std::numeric_limits<char>::is_signed; };
�
�template <> struct is_integral<unsigned char> : public true_type {};
�template <> struct is_integral<unsigned short> : public true_type {};
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
Re: [Proton-c][Solaris][SunStudio-2.1] Compiling Qpid Proton-c and
C++ bindings on Solaris
Posted by Adel Boutros <Ad...@live.com>.
Thanks alot Alan and Andrew for your help!!
However, it seems one patch has slipped. It is about "int8_t".
PROTON-1324: Interpretation of "int8_t" on Solaris using SunStudio is different from GCC one
https://issues.apache.org/jira/browse/PROTON-1324
Regards,
Adel
________________________________
From: Alan Conway <ac...@redhat.com>
Sent: Thursday, October 20, 2016 2:27:10 PM
To: Adel Boutros; Andrew Stitcher; dev
Cc: rabih.promail@gmail.com
Subject: Re: [Proton-c][Solaris][SunStudio-2.1] Compiling Qpid Proton-c and C++ bindings on Solaris
On Thu, 2016-10-20 at 07:27 +0000, Adel Boutros wrote:
>
> >>> Symbol hiding: PROTON-1316 removes symbol hiding for gcc
> completely, so that change can't go in as is. I'm sure that wasn't
> intentional, just a side effect of doing what was necessary for
> Solaris without checking properly again on Linux.
>
> This is not true, the patch is protected by "if" which will only
> execute the code for SunStudio compiler and will not affect GCC in
> any way (elseif (${CMAKE_CXX_COMPILER_ID} STREQUAL "SunPro" in
> CMakeLists.txt and elif defined(__SUNPRO_C) || defined(__SUNPRO_CC)
> in code).
>
The original patch did delete the HIDE lines in the GCC section - I'm
sure this was an accident, I restored them as intended.
> >>> This change doesn't quite address my concerns: For SunPro the
> ENABLE_HIDE_UNEXPORTED_SYMBOLS toggle is not honoured.
>
> I apologise for the error in the CMakeLists.txt. We missed the "if
> (ENABLE_HIDE_UNEXPORTED_SYMBOLS) " for the code submitted.
> @Alan
> Will you handle updating the patch here?
Done. I made the minimal change to respect the ENABLE_HIDE... flag for
solaris, I haven't re-factored the flag-setting section.
I've committed the patches, any further improvements can be made
directly on master.
Cheers,
Alan.
Re: [Proton-c][Solaris][SunStudio-2.1] Compiling Qpid Proton-c and
C++ bindings on Solaris
Posted by Andrew Stitcher <as...@redhat.com>.
On Thu, 2016-10-20 at 16:23 +0000, Adel Boutros wrote:
> One last question: will the fixes be released in a patch (0.15.1) or
> a minor (0.16.0)?
>�
The changes have been committed to the master branch of proton so will
be part of the next release - 0.16.0.
They don't fit our usual criterea for inclusion into a point release
(in fact I doubt there will be a 0.15 point release anyway).
As we have just released 0.15 it'll probably be around 3 months until
we release 0.16.
Andrew
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
Re: [Proton-c][Solaris][SunStudio-2.1] Compiling Qpid Proton-c and
C++ bindings on Solaris
Posted by Adel Boutros <ad...@live.com>.
One last question: will the fixes be released in a patch (0.15.1) or a minor (0.16.0)?
On Thu, Oct 20, 2016 at 2:27 PM +0200, "Alan Conway" <ac...@redhat.com>> wrote:
On Thu, 2016-10-20 at 07:27 +0000, Adel Boutros wrote:
>
> >>> Symbol hiding: PROTON-1316 removes symbol hiding for gcc
> completely, so that change can't go in as is. I'm sure that wasn't
> intentional, just a side effect of doing what was necessary for
> Solaris without checking properly again on Linux.
>
> This is not true, the patch is protected by "if" which will only
> execute the code for SunStudio compiler and will not affect GCC in
> any way (elseif (${CMAKE_CXX_COMPILER_ID} STREQUAL "SunPro" in
> CMakeLists.txt and elif defined(__SUNPRO_C) || defined(__SUNPRO_CC)
> in code).
>
The original patch did delete the HIDE lines in the GCC section - I'm
sure this was an accident, I restored them as intended.
> >>> This change doesn't quite address my concerns: For SunPro the
> ENABLE_HIDE_UNEXPORTED_SYMBOLS toggle is not honoured.
>
> I apologise for the error in the CMakeLists.txt. We missed the "if
> (ENABLE_HIDE_UNEXPORTED_SYMBOLS) " for the code submitted.
> @Alan
> Will you handle updating the patch here?
Done. I made the minimal change to respect the ENABLE_HIDE... flag for
solaris, I haven't re-factored the flag-setting section.
I've committed the patches, any further improvements can be made
directly on master.
Cheers,
Alan.
Re: [Proton-c][Solaris][SunStudio-2.1] Compiling Qpid Proton-c and
C++ bindings on Solaris
Posted by Alan Conway <ac...@redhat.com>.
On Thu, 2016-10-20 at 07:27 +0000, Adel Boutros wrote:
>�
> >>>�Symbol hiding: PROTON-1316 removes symbol hiding for gcc
> completely,�so that change can't go in as is. I'm sure that wasn't
> intentional,�just a side effect of doing what was necessary for
> Solaris without�checking properly again on Linux.
>
> This is not true, the patch is protected by "if" which will only
> execute the code for SunStudio compiler and will not affect GCC in
> any way (elseif (${CMAKE_CXX_COMPILER_ID} STREQUAL "SunPro" in
> CMakeLists.txt and�elif defined(__SUNPRO_C) || defined(__SUNPRO_CC)
> in code).
>
The original patch did delete the HIDE lines in the GCC section - I'm
sure this was an accident, I restored them as intended.
> >>>�This change doesn't quite address my concerns:�For SunPro the
> ENABLE_HIDE_UNEXPORTED_SYMBOLS toggle is not honoured.
>
> I apologise for the error in the CMakeLists.txt. We missed the�"if
> (ENABLE_HIDE_UNEXPORTED_SYMBOLS)�"�for the code submitted.
> @Alan
> Will you handle updating the patch here?
Done. I made the minimal change to respect the ENABLE_HIDE... flag for
solaris, I haven't re-factored the flag-setting section.
I've committed the patches, �any further improvements can be made
directly on master.
Cheers,
Alan.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
Re: [Proton-c][Solaris][SunStudio-2.1] Compiling Qpid Proton-c and
C++ bindings on Solaris
Posted by Adel Boutros <Ad...@live.com>.
Hello Andrew,
Before I reply to your comments, I would like to clarify that although the patches seem easy and minor, it took us (Rabih and myself) 2 weeks full time to find the root causes of the issues, prepare a clean patch which will be accepted by the community and has no side effects. This is because we have no expertise on the Proton code and the errors were not straightforward.
>>> Solaris should have SO_NOSIGPIPE, so the new code in PROTON-1314 should not be needed (although there's nothing wrong with the code as far as I can see).
Apologies for not mentioning it, but we are using SunOs (x86 and sparc) which are not BSD OS. If you google about SO_NOSIGPIPE and sunos (or even Solaris), you will find a lot of articles mentioning this issue. Also doing a quick grep will show this signal is not supported.
[host] / # man getsockopt | grep SO_NOSIGPIPE
Reformatting page. Please Wait... done
[host] / # man signal | grep SIGPIPE
Reformatting page. Please Wait... done
[host] / #
>>> Symbol hiding: PROTON-1316 removes symbol hiding for gcc completely, so that change can't go in as is. I'm sure that wasn't intentional, just a side effect of doing what was necessary for Solaris without checking properly again on Linux.
This is not true, the patch is protected by "if" which will only execute the code for SunStudio compiler and will not affect GCC in any way (elseif (${CMAKE_CXX_COMPILER_ID} STREQUAL "SunPro" in CMakeLists.txt and elif defined(__SUNPRO_C) || defined(__SUNPRO_CC) in code).
>>> This change doesn't quite address my concerns: For SunPro the ENABLE_HIDE_UNEXPORTED_SYMBOLS toggle is not honoured.
I apologise for the error in the CMakeLists.txt. We missed the "if (ENABLE_HIDE_UNEXPORTED_SYMBOLS) " for the code submitted.
@Alan
Will you handle updating the patch here?
Just a bit of context for this issue (You can correct me if I said something wrong):
When we asked on the mailing list, we were told there are 2 parts here: The 1st is setting export of symbols to hidden by default (implemented in CMakeLists.txt) and the 2nd part is explicitly exporting symbols (implemented in the code)
Hide Symbols
------------------
GCC: "-fvisibility=hidden": This causes all inlined class member functions to have hidden visibility (https://gcc.gnu.org/wiki/Visibility)
SunStudio: "-xldscope=hidden": Hidden scope is easiest to describe as it just means that the symbol can only be seen within the module and is not exported for applications or libraries to use(https://blogs.oracle.com/d/entry/using_symbol_scoping_libraries_and)
Export Symbols explicitly
----------------------------------
GCC: "__attribute ((visibility ("default")))" : In your header files, wherever you want an interface or API made public outside the current DSO (https://gcc.gnu.org/wiki/Visibility)
SunStudio: "define PN_CPP_EXPORT __global" __declspec(dllimport) is equivalent to __global (https://docs.oracle.com/cd/E24457_01/html/E21991/bkaee.html)
Regards,
Adel
________________________________
From: Andrew Stitcher <as...@redhat.com>
Sent: Thursday, October 20, 2016 12:45:14 AM
To: Alan Conway; Adel Boutros; dev
Cc: rabih.promail@gmail.com
Subject: Re: [Proton-c][Solaris][SunStudio-2.1] Compiling Qpid Proton-c and C++ bindings on Solaris
On Wed, 2016-10-19 at 17:30 -0400, Alan Conway wrote:
> On Tue, 2016-10-18 at 13:24 -0400, Alan Conway wrote:
> >
> > Hi Adel,
> >
> > I've got your Solaris fixes and my (minor) changes up on
> > https://github.com/alanconway/qpid-proton/tree/absolaris
[https://avatars3.githubusercontent.com/u/7780958?v=3&s=400]<https://github.com/alanconway/qpid-proton/tree/absolaris>
alanconway/qpid-proton<https://github.com/alanconway/qpid-proton/tree/absolaris>
github.com
qpid-proton - Mirror of Apache Qpid Proton
> >
> > I'd like Andrew to take a look (he's back tomorrow), unless he has
> > anything to add I'll commit them.
>
> I think I've addressed all Andrew's comments on the github branch,
> will
> wait for one more go-round from him.
I'm happy except for PROTON-1316. I've made a comment on the new github
branch.
Andrew
Re: [Proton-c][Solaris][SunStudio-2.1] Compiling Qpid Proton-c and
C++ bindings on Solaris
Posted by Andrew Stitcher <as...@redhat.com>.
On Wed, 2016-10-19 at 17:30 -0400, Alan Conway wrote:
> On Tue, 2016-10-18 at 13:24 -0400, Alan Conway wrote:
> >
> > Hi Adel,
> >
> > I've got your Solaris fixes and my (minor) changes up on
> > �https://github.com/alanconway/qpid-proton/tree/absolaris
> >
> > I'd like Andrew to take a look (he's back tomorrow), unless he has
> > anything to add I'll commit them.
>
> I think I've addressed all Andrew's comments on the github branch,
> will
> wait for one more go-round from him.
I'm happy except for PROTON-1316. I've made a comment on the new github
branch.
Andrew
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
Re: [Proton-c][Solaris][SunStudio-2.1] Compiling Qpid Proton-c and
C++ bindings on Solaris
Posted by Alan Conway <ac...@redhat.com>.
On Tue, 2016-10-18 at 13:24 -0400, Alan Conway wrote:
> Hi Adel,
>
> I've got your Solaris fixes and my (minor) changes up on
> �https://github.com/alanconway/qpid-proton/tree/absolaris
>
> I'd like Andrew to take a look (he's back tomorrow), unless he has
> anything to add I'll commit them.
I think I've addressed all Andrew's comments on the github branch, will
wait for one more go-round from him.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
Re: [Proton-c][Solaris][SunStudio-2.1] Compiling Qpid Proton-c and
C++ bindings on Solaris
Posted by Adel Boutros <ad...@live.com>.
Hello Andrew,
I will check tomorrow all your comments because I work in France so I am no longer in the office now.
I let you choose where to place the comments and we will be happy to reply to them by tomorrow. Just let us know where you put them.
Regards,
Adel
Get Outlook for Android<https://aka.ms/ghei36>
On Wed, Oct 19, 2016 at 9:21 PM +0200, "Andrew Stitcher" <as...@redhat.com>> wrote:
Being clear, these are all fairly minor issues:
Thank you both for doing this work, I'm very happy to be expanding the
scope of portability for proton,
Andrew
Re: [Proton-c][Solaris][SunStudio-2.1] Compiling Qpid Proton-c and
C++ bindings on Solaris
Posted by Andrew Stitcher <as...@redhat.com>.
Being clear, these are all fairly minor issues:
Thank you both for doing this work, I'm very happy to be expanding the
scope of portability for proton,
Andrew
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
Re: [Proton-c][Solaris][SunStudio-2.1] Compiling Qpid Proton-c and
C++ bindings on Solaris
Posted by Andrew Stitcher <as...@redhat.com>.
On Wed, 2016-10-19 at 14:56 -0400, Andrew Stitcher wrote:
> How would you guys like my comments?
>
> I can put them in this email thread (with no external visibility), in
> the Jiras, or as comments to the branch in alan's repo.
>
> Any preferences (I'd prefer to keep the comments public).
So I've made a number of comments both on github and on the Jiras:
* Solaris should have SO_NOSIGPIPE, so the new code in PROTON-1314
should be needed (although there's nothing wrong with the code as far
as I can see).
I think the necessary include might not be getting included.
* Symbol hiding: PROTON-1316 removes symbol hiding for gcc completely,
so that change can't go in as is. I'm sure that wasn't intentional,
just a side effect of doing what was necessary for Solaris without
checking properly again on Linux.
* I agree with Alan's modifications of PROTON-1318/PROTON-1319: I
thionk the branch needs to be rebased before merging to squash these
into one commit each.
Andrew
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
Re: [Proton-c][Solaris][SunStudio-2.1] Compiling Qpid Proton-c and
C++ bindings on Solaris
Posted by Andrew Stitcher <as...@redhat.com>.
How would you guys like my comments?
I can put them in this email thread (with no external visibility), in
the Jiras, or as comments to the branch in alan's repo.
Any preferences (I'd prefer to keep the comments public).
Andrew
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
Re: [Proton-c][Solaris][SunStudio-2.1] Compiling Qpid Proton-c and
C++ bindings on Solaris
Posted by Adel Boutros <Ad...@live.com>.
Great news!! Thanks for your efforts.
Once they are merged, we can test them with the following configuration:
* Linux: GCC 4.9.1
* Solaris SunOS x86: SunStudio 12.1
* Solaris SunOS sparc: SunStudio 12.1
Regards,
Adel
________________________________
From: Alan Conway <ac...@redhat.com>
Sent: Tuesday, October 18, 2016 7:24:33 PM
To: Adel Boutros; dev
Cc: Andrew Stitcher; rabih.promail@gmail.com
Subject: Re: [Proton-c][Solaris][SunStudio-2.1] Compiling Qpid Proton-c and C++ bindings on Solaris
Hi Adel,
I've got your Solaris fixes and my (minor) changes up on
https://github.com/alanconway/qpid-proton/tree/absolaris
I'd like Andrew to take a look (he's back tomorrow), unless he has
anything to add I'll commit them.
Cheers,
Alan.
On Mon, 2016-10-17 at 20:03 +0000, Adel Boutros wrote:
> Hello Alan,
>
> No, it isn't a necessary fix (change from T*).
>
> However, we compiled this code on linux using GCC 4.9.1 and we didn't
> get any error on the master. I wonder which version of gcc you are
> using which revealed this error?
>
> Regards,
> Adel
>
> Ps: The original issue here is that CC doesn't handle constructors
> with "..." correctly which is here the case of sfinae::wildcard(...)
>
> Get Outlook for Android
>
>
>
> On Mon, Oct 17, 2016 at 9:48 PM +0200, "Alan Conway" <aconway@redhat.
> com> wrote:
>
> Hi Adel,
>
> I've put a workaround for one GCC problem on https://github.com/alanc
> on
> way/qpid-proton/tree/absolaris, otherwise I'm happy with this. I'll
> wait for Andrew's OK and then commit if he's happy.
>
> One question: this makes GCC choke:
>
> index b1aff89..5285e4b 100644
> --- a/proton-c/bindings/cpp/include/proton/codec/encoder.hpp
> +++ b/proton-c/bindings/cpp/include/proton/codec/encoder.hpp
> @@ -175,14 +175,14 @@ namespace is_encodable_impl { // Protected
> the
> world from wildcard operator<<
>
> using namespace internal;
>
> -sfinae::no operator<<(sfinae::wildcard, sfinae::wildcard); //
> Fallback
> +sfinae::no operator<<(encoder const&, const sfinae::any_t &); //
> Fallback
>
> template<typename T> struct is_encodable : public sfinae {
> - static yes test(encoder);
> + static yes test(encoder&);
> static no test(...); // Failed test, no match.
> - static encoder* e;
> - static const T* t;
> - static bool const value = sizeof(test(*e << *t)) == sizeof(yes);
> + static encoder& e;
> + static const T& t;
> ^^^^^^^THIS LINE^^^^^^^
> + static bool const value = sizeof(test(e << t)) == sizeof(yes);
> };
>
> Is the change from T* to T& required to compile on Solaris? If not I
> would probably revert it, but if it is needed then the workaround
> seems
> to be OK. Windows & clang compilers don't have any problem so I
> suspect
> this is GCC's fault.
>
> Cheers,
> Alan.