Re: Review Request: Fix cmake based build of nss based ssl support on ubuntu

2013-04-05 Thread Alan Conway

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/10299/#review18697
---


There is no flag-ordering code in the diff, I think it's from a different issue 
(missing nss libraries). 

- Alan Conway


On April 5, 2013, 1:14 p.m., Gordon Sim wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/10299/
 ---
 
 (Updated April 5, 2013, 1:14 p.m.)
 
 
 Review request for qpid, Andrew Stitcher, Alan Conway, Steve Huston, and Rob 
 Godfrey.
 
 
 Description
 ---
 
 The problem is that when the --as-needed flag is used with the linker (the 
 default for recent versions of ubuntu), libraries must follow any .o file 
 that needs them or they get ignored as not needed. At present the libraries 
 are included before the .o files. This is an update to the initial patch that 
 avoids the duplicated libraries on the link command line.
 
 
 This addresses bug QPID-4702.
 https://issues.apache.org/jira/browse/QPID-4702
 
 
 Diffs
 -
 
   /trunk/qpid/cpp/src/ssl.cmake 1464148 
 
 Diff: https://reviews.apache.org/r/10299/diff/
 
 
 Testing
 ---
 
 make test
 
 
 Thanks,
 
 Gordon Sim
 




Re: Review Request: Fix cmake based build of nss based ssl support on ubuntu

2013-04-05 Thread Gordon Sim


 On April 5, 2013, 1:51 p.m., Alan Conway wrote:
  There is no flag-ordering code in the diff, I think it's from a different 
  issue (missing nss libraries).

No, the libraries were there (and were on the link command, just that they were 
before the .o files). The ordering change here comes from the way in which the 
libraries are set. Rather than having them added through the LINKFLAGS, they 
are explictly added as target_link_libraries.

Now, this may very well not be the right fix. It was driven by trial and error 
rather than in-depth understanding of cmake. So if there are better solutions, 
I'd be delighted to see them.


- Gordon


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/10299/#review18697
---


On April 5, 2013, 1:14 p.m., Gordon Sim wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/10299/
 ---
 
 (Updated April 5, 2013, 1:14 p.m.)
 
 
 Review request for qpid, Andrew Stitcher, Alan Conway, Steve Huston, and Rob 
 Godfrey.
 
 
 Description
 ---
 
 The problem is that when the --as-needed flag is used with the linker (the 
 default for recent versions of ubuntu), libraries must follow any .o file 
 that needs them or they get ignored as not needed. At present the libraries 
 are included before the .o files. This is an update to the initial patch that 
 avoids the duplicated libraries on the link command line.
 
 
 This addresses bug QPID-4702.
 https://issues.apache.org/jira/browse/QPID-4702
 
 
 Diffs
 -
 
   /trunk/qpid/cpp/src/ssl.cmake 1464148 
 
 Diff: https://reviews.apache.org/r/10299/diff/
 
 
 Testing
 ---
 
 make test
 
 
 Thanks,
 
 Gordon Sim
 




Re: Review Request: Fix cmake based build of nss based ssl support on ubuntu

2013-04-05 Thread Alan Conway


 On April 5, 2013, 1:51 p.m., Alan Conway wrote:
  There is no flag-ordering code in the diff, I think it's from a different 
  issue (missing nss libraries).
 
 Gordon Sim wrote:
 No, the libraries were there (and were on the link command, just that 
 they were before the .o files). The ordering change here comes from the way 
 in which the libraries are set. Rather than having them added through the 
 LINKFLAGS, they are explictly added as target_link_libraries.
 
 Now, this may very well not be the right fix. It was driven by trial and 
 error rather than in-depth understanding of cmake. So if there are better 
 solutions, I'd be delighted to see them.

I get it now. I think that is the right solution: tell cmake about the 
dependency and let it figure out how to order the link flags - which might be 
different on different platformsShi

Ship it!


- Alan


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/10299/#review18697
---


On April 5, 2013, 1:14 p.m., Gordon Sim wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/10299/
 ---
 
 (Updated April 5, 2013, 1:14 p.m.)
 
 
 Review request for qpid, Andrew Stitcher, Alan Conway, Steve Huston, and Rob 
 Godfrey.
 
 
 Description
 ---
 
 The problem is that when the --as-needed flag is used with the linker (the 
 default for recent versions of ubuntu), libraries must follow any .o file 
 that needs them or they get ignored as not needed. At present the libraries 
 are included before the .o files. This is an update to the initial patch that 
 avoids the duplicated libraries on the link command line.
 
 
 This addresses bug QPID-4702.
 https://issues.apache.org/jira/browse/QPID-4702
 
 
 Diffs
 -
 
   /trunk/qpid/cpp/src/ssl.cmake 1464148 
 
 Diff: https://reviews.apache.org/r/10299/diff/
 
 
 Testing
 ---
 
 make test
 
 
 Thanks,
 
 Gordon Sim
 




Re: Review Request: Fix cmake based build of nss based ssl support on ubuntu

2013-04-05 Thread Andrew Stitcher


 On April 5, 2013, 1:51 p.m., Alan Conway wrote:
  There is no flag-ordering code in the diff, I think it's from a different 
  issue (missing nss libraries).
 
 Gordon Sim wrote:
 No, the libraries were there (and were on the link command, just that 
 they were before the .o files). The ordering change here comes from the way 
 in which the libraries are set. Rather than having them added through the 
 LINKFLAGS, they are explictly added as target_link_libraries.
 
 Now, this may very well not be the right fix. It was driven by trial and 
 error rather than in-depth understanding of cmake. So if there are better 
 solutions, I'd be delighted to see them.
 
 Alan Conway wrote:
 I get it now. I think that is the right solution: tell cmake about the 
 dependency and let it figure out how to order the link flags - which might be 
 different on different platformsShi
 
 Ship it!

I haven't looked at this in enough depth yet, but I'm surprised at the 
circumlocution needed considering that what you are doing is the basic use case 
for the nss3 detection logic!

So I'd expect that there would be some variable it directly outputs that avoid 
you needing the foreach.


- Andrew


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/10299/#review18697
---


On April 5, 2013, 1:14 p.m., Gordon Sim wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/10299/
 ---
 
 (Updated April 5, 2013, 1:14 p.m.)
 
 
 Review request for qpid, Andrew Stitcher, Alan Conway, Steve Huston, and Rob 
 Godfrey.
 
 
 Description
 ---
 
 The problem is that when the --as-needed flag is used with the linker (the 
 default for recent versions of ubuntu), libraries must follow any .o file 
 that needs them or they get ignored as not needed. At present the libraries 
 are included before the .o files. This is an update to the initial patch that 
 avoids the duplicated libraries on the link command line.
 
 
 This addresses bug QPID-4702.
 https://issues.apache.org/jira/browse/QPID-4702
 
 
 Diffs
 -
 
   /trunk/qpid/cpp/src/ssl.cmake 1464148 
 
 Diff: https://reviews.apache.org/r/10299/diff/
 
 
 Testing
 ---
 
 make test
 
 
 Thanks,
 
 Gordon Sim
 




Re: Review Request: Fix cmake based build of nss based ssl support on ubuntu

2013-04-05 Thread Andrew Stitcher


 On April 5, 2013, 1:51 p.m., Alan Conway wrote:
  There is no flag-ordering code in the diff, I think it's from a different 
  issue (missing nss libraries).
 
 Gordon Sim wrote:
 No, the libraries were there (and were on the link command, just that 
 they were before the .o files). The ordering change here comes from the way 
 in which the libraries are set. Rather than having them added through the 
 LINKFLAGS, they are explictly added as target_link_libraries.
 
 Now, this may very well not be the right fix. It was driven by trial and 
 error rather than in-depth understanding of cmake. So if there are better 
 solutions, I'd be delighted to see them.
 
 Alan Conway wrote:
 I get it now. I think that is the right solution: tell cmake about the 
 dependency and let it figure out how to order the link flags - which might be 
 different on different platformsShi
 
 Ship it!
 
 Andrew Stitcher wrote:
 I haven't looked at this in enough depth yet, but I'm surprised at the 
 circumlocution needed considering that what you are doing is the basic use 
 case for the nss3 detection logic!
 
 So I'd expect that there would be some variable it directly outputs that 
 avoid you needing the foreach.

What I mean to say is that what we need to achieve here isn't some strange 
unusual use case it's a basic use case and so I expect that there is a direct 
solution to it.


- Andrew


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/10299/#review18697
---


On April 5, 2013, 1:14 p.m., Gordon Sim wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/10299/
 ---
 
 (Updated April 5, 2013, 1:14 p.m.)
 
 
 Review request for qpid, Andrew Stitcher, Alan Conway, Steve Huston, and Rob 
 Godfrey.
 
 
 Description
 ---
 
 The problem is that when the --as-needed flag is used with the linker (the 
 default for recent versions of ubuntu), libraries must follow any .o file 
 that needs them or they get ignored as not needed. At present the libraries 
 are included before the .o files. This is an update to the initial patch that 
 avoids the duplicated libraries on the link command line.
 
 
 This addresses bug QPID-4702.
 https://issues.apache.org/jira/browse/QPID-4702
 
 
 Diffs
 -
 
   /trunk/qpid/cpp/src/ssl.cmake 1464148 
 
 Diff: https://reviews.apache.org/r/10299/diff/
 
 
 Testing
 ---
 
 make test
 
 
 Thanks,
 
 Gordon Sim