You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@qpid.apache.org by Kenneth Giusti <kg...@apache.org> on 2012/11/01 15:24:38 UTC
Review Request: Proton: cmake changes to support MS Visual C++ Express 2010
(WIP)
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/7826/
-----------------------------------------------------------
Review request for qpid, Andrew Stitcher and Cliff Jansen.
Description
-------
Work In Progress: changes to cmake configuration to support Microsoft Visual C++ Express 2010 IDE.
Diffs
-----
/proton/trunk/proton-c/CMakeLists.txt 1404604
/proton/trunk/proton-c/README 1404604
/proton/trunk/proton-c/src/codec/encodings.h.py 1404604
/proton/trunk/proton-c/src/protocol.h.py 1404604
Diff: https://reviews.apache.org/r/7826/diff/
Testing
-------
Thanks,
Kenneth Giusti
Re: Review Request: Proton: cmake changes to support MS Visual C++ Express
2010 (WIP)
Posted by Steve Huston <sh...@riverace.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/7826/#review13060
-----------------------------------------------------------
Ship it!
Ship It!
- Steve Huston
On Nov. 2, 2012, 5:19 p.m., Kenneth Giusti wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/7826/
> -----------------------------------------------------------
>
> (Updated Nov. 2, 2012, 5:19 p.m.)
>
>
> Review request for qpid, Andrew Stitcher and Cliff Jansen.
>
>
> Description
> -------
>
> Work In Progress: changes to cmake configuration to support Microsoft Visual C++ Express 2010 IDE.
>
>
> Diffs
> -----
>
> /proton/trunk/proton-c/CMakeLists.txt 1404284
> /proton/trunk/proton-c/README 1404284
> /proton/trunk/proton-c/env.py PRE-CREATION
>
> Diff: https://reviews.apache.org/r/7826/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Kenneth Giusti
>
>
Re: Review Request: Proton: cmake changes to support MS Visual C++ Express
2010 (WIP)
Posted by Andrew Stitcher <as...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/7826/#review13049
-----------------------------------------------------------
Ship it!
I like this much better.
Personally I would split out the compiler warning flags from the rest, but that is just quibling
- Andrew Stitcher
On Nov. 2, 2012, 5:19 p.m., Kenneth Giusti wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/7826/
> -----------------------------------------------------------
>
> (Updated Nov. 2, 2012, 5:19 p.m.)
>
>
> Review request for qpid, Andrew Stitcher and Cliff Jansen.
>
>
> Description
> -------
>
> Work In Progress: changes to cmake configuration to support Microsoft Visual C++ Express 2010 IDE.
>
>
> Diffs
> -----
>
> /proton/trunk/proton-c/CMakeLists.txt 1404284
> /proton/trunk/proton-c/README 1404284
> /proton/trunk/proton-c/env.py PRE-CREATION
>
> Diff: https://reviews.apache.org/r/7826/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Kenneth Giusti
>
>
Re: Review Request: Proton: cmake changes to support MS Visual C++ Express
2010 (WIP)
Posted by Chug Rolke <cr...@redhat.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/7826/#review13050
-----------------------------------------------------------
/proton/trunk/proton-c/README
<https://reviews.apache.org/r/7826/#comment28087>
The cmake -G "Visual Studio 10" builds a 32-bit solution. You may mention that "Visual Studio 10 Win64" builds a 64-bit solution.
- Chug Rolke
On Nov. 2, 2012, 5:19 p.m., Kenneth Giusti wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/7826/
> -----------------------------------------------------------
>
> (Updated Nov. 2, 2012, 5:19 p.m.)
>
>
> Review request for qpid, Andrew Stitcher and Cliff Jansen.
>
>
> Description
> -------
>
> Work In Progress: changes to cmake configuration to support Microsoft Visual C++ Express 2010 IDE.
>
>
> Diffs
> -----
>
> /proton/trunk/proton-c/CMakeLists.txt 1404284
> /proton/trunk/proton-c/README 1404284
> /proton/trunk/proton-c/env.py PRE-CREATION
>
> Diff: https://reviews.apache.org/r/7826/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Kenneth Giusti
>
>
Re: Review Request: Proton: cmake changes to support MS Visual C++ Express
2010 (WIP)
Posted by Kenneth Giusti <kg...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/7826/
-----------------------------------------------------------
(Updated Nov. 2, 2012, 5:19 p.m.)
Review request for qpid, Andrew Stitcher and Cliff Jansen.
Changes
-------
Revised the patch a bit to reduce scope. This patch merely provides:
1) Fixes running the python build scripts in the windows environment
2) Fixes a syntax error in the CMakeLists.txt file
3) Introduces a platform-specific "extra compiler flags" variable - necessary as the GNU-specific flags passed in the COMPILER_OPTIONS is hosing the Windows build
Opinions?
Description
-------
Work In Progress: changes to cmake configuration to support Microsoft Visual C++ Express 2010 IDE.
Diffs (updated)
-----
/proton/trunk/proton-c/CMakeLists.txt 1404284
/proton/trunk/proton-c/README 1404284
/proton/trunk/proton-c/env.py PRE-CREATION
Diff: https://reviews.apache.org/r/7826/diff/
Testing
-------
Thanks,
Kenneth Giusti
Re: Review Request: Proton: cmake changes to support MS Visual C++ Express
2010 (WIP)
Posted by Andrew Stitcher <as...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/7826/#review12997
-----------------------------------------------------------
/proton/trunk/proton-c/CMakeLists.txt
<https://reviews.apache.org/r/7826/#comment27976>
I don't understand the point of most of these lines:
The only ones that I agree with are setting WARNING_FLAGS.
Cmake already sets up different compile flags for the different builds, just leave the differences to cmake. If the developer really wants to change the flags they can do it themselves. Any flags that are actually necessary for the build would be in common between the builds so shouldn't be here anyway.
/proton/trunk/proton-c/CMakeLists.txt
<https://reviews.apache.org/r/7826/#comment27977>
Instead of getting rid of this it should read -
COMPILE_FLAGS "${WARNINGS_FLAGS} ${OTHER_STUFF}"
where OTHER_STUFF (or a better name) is set to -std=c99 for gcc and the flag to compile C as C++ for MSVC.
/proton/trunk/proton-c/CMakeLists.txt
<https://reviews.apache.org/r/7826/#comment27978>
As previous comment
/proton/trunk/proton-c/src/protocol.h.py
<https://reviews.apache.org/r/7826/#comment27979>
I don't like the way the python changes have been implemented: I think it would be much better to have a new separate python script that takes as arguments the additional python path and the rest of the command line to run and then set up the python path and run the next script.
This would avoid repeating the same, but slightly different, code twice. And would be extensible to any new pythons script we have to add. And would avoid changing the existing scripts into something less pythonic (IMO)
- Andrew Stitcher
On Nov. 1, 2012, 2:24 p.m., Kenneth Giusti wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/7826/
> -----------------------------------------------------------
>
> (Updated Nov. 1, 2012, 2:24 p.m.)
>
>
> Review request for qpid, Andrew Stitcher and Cliff Jansen.
>
>
> Description
> -------
>
> Work In Progress: changes to cmake configuration to support Microsoft Visual C++ Express 2010 IDE.
>
>
> Diffs
> -----
>
> /proton/trunk/proton-c/CMakeLists.txt 1404604
> /proton/trunk/proton-c/README 1404604
> /proton/trunk/proton-c/src/codec/encodings.h.py 1404604
> /proton/trunk/proton-c/src/protocol.h.py 1404604
>
> Diff: https://reviews.apache.org/r/7826/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Kenneth Giusti
>
>