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


I'm not convinced there is a good reason for this change.

If you want to use the executables in the build tree you need to run the 
config.sh or config.bat scripts there to set this up in any case - not doing 
this may cause some confusing issues down the line.

On Windows this script will include the DLLs in PATH so I don't really see what 
this change brings. It also has the downside (IMO) of making the layout of the 
Windows build tree different from the Unix build tree (of course this is the 
main purpose of the change in some ways). I'm not convinced there are 
sufficient benefits to warrant the inconsistent tree layout.

Be that as it may: I think the overall strategy here is flawed because of the 
way that CMake strings and lists work. It may work on the versions of CMake 
that you have tested it with, but it won't work with other versions (I'm 
guessing older versions).

when you so set(BLAH FOO "/path/to/something") then BLAH gets set to 
"FOO;/path/to/something" so when you later do set_target_property(file ${BLAH}) 
you actually get set_target_property(file "FOO;/path/to/something") whereas the 
syntax you are looking for is set_target_property(file FOO /path/to/something).

It seems that the later versions of CMake may account for this somehow, earlier 
ones do not.

- Andrew Stitcher


On June 29, 2015, 9:07 p.m., Chug Rolke wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36020/
> -----------------------------------------------------------
> 
> (Updated June 29, 2015, 9:07 p.m.)
> 
> 
> Review request for qpid, Andrew Stitcher and Steve Huston.
> 
> 
> Bugs: proton-926
>     https://issues.apache.org/jira/browse/proton-926
> 
> 
> Repository: qpid-proton-git
> 
> 
> Description
> -------
> 
> Same as https://issues.apache.org/jira/browse/QPID-6611 for qpid this patch 
> creates .exe files in common place. It is windows only with no change for 
> linux.
> 
> The patch moves intermediate files around in the build tree only. It does not 
> change the layout of the install directory.
> 
> 
> Diffs
> -----
> 
>   examples/c/messenger/CMakeLists.txt 1b32d0c 
>   proton-c/CMakeLists.txt a1cf0e4 
>   proton-c/src/tests/CMakeLists.txt ac749e1 
>   tests/tools/apps/c/CMakeLists.txt 9507c1f 
> 
> Diff: https://reviews.apache.org/r/36020/diff/
> 
> 
> Testing
> -------
> 
> Passes all tests on windows and linux.
> 
> 
> Thanks,
> 
> Chug Rolke
> 
>

Reply via email to