Re: [cmake-developers] [PATCH 3/6] Resolve replace @rpath placeholders

2014-09-18 Thread Adam Strzelecki
After that change is dropped, I give a +1 for the patch set. Done, both in attached 3/5 patch and GitHub branch of mine. --Adam -- Powered by www.kitware.com Please keep messages on-topic and check the CMake FAQ at: http://www.cmake.org/Wiki/CMake_FAQ Kitware offers various services to

Re: [cmake-developers] [PATCH 3/6] Resolve replace @rpath placeholders

2014-09-18 Thread Adam Strzelecki
FYI I pushed stage/fix-OSX-bundle-rpaths-and-Qt5 topic for final review. Regards, -- Adam -- Powered by www.kitware.com Please keep messages on-topic and check the CMake FAQ at: http://www.cmake.org/Wiki/CMake_FAQ Kitware offers various services to support the CMake community. For more

Re: [cmake-developers] [PATCH 3/6] Resolve replace @rpath placeholders

2014-09-18 Thread Clinton Stimpson
On Thursday, September 18, 2014 06:15:51 PM Adam Strzelecki wrote: FYI I pushed stage/fix-OSX-bundle-rpaths-and-Qt5 topic for final review. Regards, Does the functionality you add allow us to modify CMake/Tests/BundleUtilities/CMakeLists.txt such that the last part in the tests where @rpath

Re: [cmake-developers] [PATCH 3/6] Resolve replace @rpath placeholders

2014-09-17 Thread Clinton Stimpson
On Wednesday, September 17, 2014 01:13:45 AM Adam Strzelecki wrote: What is the new optional parameter to gp_file_type() used for? My intention was to pass exepath rather than take it from the path of original_file. But this is in fact not necessary. I don't see any code in your branch

[cmake-developers] [PATCH 3/6] Resolve replace @rpath placeholders

2014-09-16 Thread Adam Strzelecki
This is done by gathering LC_RPATH commands for main bundle executable and using it for @rpath lookup in dependent frameworks. To achieve this all utility functions now take path to executable rather than path to its directory. This enabled apps using @rpath to be bundled correctly, which will

Re: [cmake-developers] [PATCH 3/6] Resolve replace @rpath placeholders

2014-09-16 Thread Clinton Stimpson
This patch has problems. You are calling BundleUtilities::get_item_key() from GetPrerequisites::gp_resolve_item(). There are users, including myself, which sometimes use GetPrerequisites.cmake but don't use BundleUtilities.cmake. So you cannot call BundleUtilities functions from

[cmake-developers] [PATCH 3/6] Resolve replace @rpath placeholders

2014-09-16 Thread Adam Strzelecki
This is done by gathering LC_RPATH commands for main bundle executable and using it for @rpath lookup in dependent frameworks. To achieve this all utility functions now take path to executable rather than path to its directory. This enabled apps using @rpath to be bundled correctly, which will

Re: [cmake-developers] [PATCH 3/6] Resolve replace @rpath placeholders

2014-09-16 Thread Adam Strzelecki
Instead, can you extract rpaths for a binary in BundleUtilities and pass that into gp_resolve_item via the existing dirs argument? Okay, fixed this in the new 3/6 + 4/6 patches, attached to previous patch post. FYI I cannot use existing dirs arguments because other replacements search paths

Re: [cmake-developers] [PATCH 3/6] Resolve replace @rpath placeholders

2014-09-16 Thread Clinton Stimpson
On Tuesday, September 16, 2014 08:48:31 PM Adam Strzelecki wrote: Instead, can you extract rpaths for a binary in BundleUtilities and pass that into gp_resolve_item via the existing dirs argument? Okay, fixed this in the new 3/6 + 4/6 patches, attached to previous patch post. FYI I

Re: [cmake-developers] [PATCH 3/6] Resolve replace @rpath placeholders

2014-09-16 Thread Adam Strzelecki
No? These functions are called by other codes and we can't just change the meaning of the arguments. You are completely right. I am changing all stuff back, once I pass rpath as optional argument we can have exepath everywhere. Thanks for your feedback. Regards, -- Adam -- Powered by

Re: [cmake-developers] [PATCH 3/6] Resolve replace @rpath placeholders

2014-09-16 Thread Adam Strzelecki
I have sent [PATCH 3/5] Resolve replace @rpath placeholders which replaces previous 3/6 and obsoletes 4/6. Since it is getting messy like checking fixing maybe stage account and topic branch would be more accurate. --Adam -- Powered by www.kitware.com Please keep messages on-topic and

Re: [cmake-developers] [PATCH 3/6] Resolve replace @rpath placeholders

2014-09-16 Thread Clinton Stimpson
On Tuesday, September 16, 2014 10:53:14 PM Adam Strzelecki wrote: I have sent [PATCH 3/5] Resolve replace @rpath placeholders which replaces previous 3/6 and obsoletes 4/6. Since it is getting messy like checking fixing maybe stage account and topic branch would be more accurate. --Adam

Re: [cmake-developers] [PATCH 3/6] Resolve replace @rpath placeholders

2014-09-16 Thread Adam Strzelecki
Yes, it would be easier to review on stage or on github. Thanks. Here it is: https://github.com/nanoant/CMake/commits/fix-bundle-rpaths I would love to get stage access though ;) Cheers, -- Adam -- Powered by www.kitware.com Please keep messages on-topic and check the CMake FAQ at:

Re: [cmake-developers] [PATCH 3/6] Resolve replace @rpath placeholders

2014-09-16 Thread Clinton Stimpson
On Tuesday, September 16, 2014 11:01:33 PM Adam Strzelecki wrote: Yes, it would be easier to review on stage or on github. Thanks. Here it is: https://github.com/nanoant/CMake/commits/fix-bundle-rpaths I would love to get stage access though ;) Cheers, What is the new optional

[cmake-developers] [PATCH 3/6] Resolve replace @rpath placeholders

2014-09-05 Thread Adam Strzelecki
This is done by gathering LC_RPATH commands for main bundle executable and using it for @rpath lookup in dependent frameworks. To achieve this all utility functions now take path to executable rather than path to its directory. This enabled apps using @rpath to be bundled correctly, which will

Re: [cmake-developers] [PATCH 3/6] Resolve replace @rpath placeholders

2014-09-05 Thread clinton
I think it would be nice to move get_item_rpaths() from BundleUtilities.cmake to gp_item_get_rpaths() in GetPrerequisites.cmake. And how does your patch or patch set handle @loader_path being used in the rpath? Also, I think the function signature should include the rpath vs runpath

Re: [cmake-developers] [PATCH 3/6] Resolve replace @rpath placeholders

2014-09-05 Thread Adam Strzelecki
I think it would be nice to move get_item_rpaths() from BundleUtilities.cmake to gp_item_get_rpaths() in GetPrerequisites.cmake. Well, this function is generic enough to (possible) serve other routines or even in CMakeLists.txt, so I'd leave it as it is. And how does your patch or patch

[cmake-developers] [PATCH 3/6] Resolve replace @rpath placeholders

2014-09-04 Thread Adam Strzelecki
This is done by gathering LC_RPATH commands for main bundle executable and using it for @rpath lookup in dependent frameworks. To achieve this all utility functions now take path to executable rather than path to its directory. This enabled apps using @rpath to be bundled correctly, which will