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 <[email protected]>
Sent: Thursday, October 20, 2016 12:45:14 AM
To: Alan Conway; Adel Boutros; dev
Cc: [email protected]
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
