I'm going to let other CMake developers chime in on this one.
It's better than the first patch, because it's opt-in, and you have to
add something to get "different than previous" behavior, but I'm still
of the opinion that this is very command specific, and you won't
always want all _DIR references as native. In my opinion, it's better
left to the person constructing the ExternalProject_Add call. But I am
curious to hear other CMake devs give their opinions.
David C.
On Tue, Aug 25, 2015 at 3:18 AM, Kislinskiy, Stefan
<s.kislins...@dkfz-heidelberg.de> wrote:
> Dear CMake developers,
>
> any thoughts on the fix? :)
>
> Best regards,
> Stefan Kislinskiy
> ________________________________________
> Von: cmake-developers [cmake-developers-boun...@cmake.org] im Auftrag von
> Kislinskiy, Stefan [s.kislins...@dkfz-heidelberg.de]
> Gesendet: Freitag, 21. August 2015 23:56
> An: David Cole; James Johnston
> Cc: cmake-developers@cmake.org
> Betreff: Re: [cmake-developers] ExternalProject: Use native paths as
> substitute for directory tokens
>
> What do you think about the new patch I attached to this mail? It adds an
> option NATIVE_DIR_TOKENS to ExternalProjects_Add. I also attached a CMake
> script file which tests/shows this feature.
>
> Stefan Kislinskiy
> ________________________________________
> Von: cmake-developers [cmake-developers-boun...@cmake.org] im Auftrag von
> David Cole via cmake-developers [cmake-developers@cmake.org]
> Gesendet: Donnerstag, 20. August 2015 23:20
> An: James Johnston
> Cc: cmake-developers@cmake.org
> Betreff: Re: [cmake-developers] ExternalProject: Use native paths as
> substitute for directory tokens
>
> It's exactly what I am concerned about:
>
> You're asking to change the behavior of something for EVERYONE to
> solve a problem which you have encountered. If you change it the way
> you have proposed, you will cause problems for others. It has worked
> the way it is now since ExternalProject_Add was introduced in CMake
> 2.8. Changing it unconditionally the way you propose is simply not
> feasible for backwards compatibility.
>
> I think commands that take native paths ought NOT to use the <*_DIR>
> replacement values, and instead, ought to pass in variables that
> contain the native paths in the first place.
>
>
> David C.
>
>
>
> On Thu, Aug 20, 2015 at 2:58 PM, James Johnston
> <johnstonj.pub...@codenest.com> wrote:
>> Hi,
>>
>> Funny you are mailing the list about this, since I just ran into this same
>> issue today building something totally different from Boost... In this case
>> it's a build tool that thinks the "/U" in "C:/Users" is a new command line
>> argument, that isn't recognized - and then the subsequent "s" also ends up
>> unrecognized... and it all fails... And it has nothing to do with the
>> working directory, so _Add_Step(WORKING_DIRECTORY) isn't a possible
>> workaround for me.
>>
>> I think the issue with globally making this change to the existing tokens is
>> that there could be some external tool/program that is EXPECTING to get
>> CMake paths, not native paths. Who knows? I am guessing that is what David
>> Cole was concerned about.
>>
>> Maybe the right answer is to introduce some NEW tokens while leaving the
>> behavior of the old ones unchanged. E.g. <BINARY_DIR_NATIVE> etc. It would
>> be good if the patch updates the documentation of ExternalProject and
>> clearly states the path format of <BINARY_DIR> vs <BINARY_DIR_NATIVE>. Then
>> the user can pick whichever one suits them best, depending on the tool being
>> invoked.
>>
>> Furthermore, sometimes <BINARY_DIR_NATIVE> still needs to be replaced with a
>> CMake path, not native path. For example, if the token is being found in a
>> property like WORKING_DIRECTORY that eventually gets passed to
>> add_custom_command(WORKING_DIRECTORY) then I'm guessing still has to be a
>> CMake path. I am guessing this is what David Cole was also concerned about.
>>
>> I still think your original method of building Boost is a bit unusual and
>> would be better served by _Add_Step with a custom working directory -
>> because that's the publicly documented/standard way of changing the working
>> directory, but that is up to you. :)
>>
>> Best regards,
>>
>> James Johnston
>>
>>
>> ---- On Thu, 20 Aug 2015 14:37:08 +0000 Stefan Kislinskiy
>> <s.kislins...@dkfz-heidelberg.de> wrote ----
>>
>> > Hi,
>> >
>> > thank you for our suggestions. I am aware that I can solve my example
>> differently and that it might look not directly connected the proposal, but
>> well, it is an example just to show a single case and why it matters. :) I
>> did not want to discuss the example itself. Working around here would just
>> resolve a symptom.
>> >
>> > My point is the overall problem that would persist: A big part of
>> ExternalProject is to issue commands for predefined and custom steps. Those
>> commands are supposed to be executed by the shell/command line. According to
>> the documentation and the source code of ExternalProject, directory tokens
>> are mainly supposed to be replaced in commands. It is my understanding, that
>> it is a bug, if CMake isn't able to assemble these commands correctly. This
>> would include usage of the correct path style of the OS for shell/command
>> line commands. As directory tokens are replaced internally right before a
>> shell/command line command is assembled, I can't see why this would be kind
>> of "API-breaking". You cannot interfere in your CMake code with these
>> internal replacements.
>> >
>> > Therefore I would still prefer my solution as it is pretty simple without
>> adding even more features to ExternalProject and in my opinion without
>> breaking code in the wild. It is a true bug fix instead of a feature request
>> for working directories, which is a different topic that just coincidentally
>> arised because of my specific example I guess. The features you described
>> wouldn't fix the actual bug.
>> >
>> > As you were not sure if my approach would even fix my problems: It does
>> of course and this is what I am currently doing and what I tested
>> extensively before creating the patch. :) Regarding your quote from the
>> add_custom_command documentation I can tell you that this is how things are
>> currently done in ExternalProject and always were as far as I know, for
>> example (from ExternalProject.cmake):
>> >
>> > add_custom_command(
>> > OUTPUT ${stamp_file}
>> > BYPRODUCTS ${byproducts}
>> > COMMENT ${comment}
>> > COMMAND ${command}
>> > COMMAND ${touch}
>> > DEPENDS ${depends}
>> > WORKING_DIRECTORY ${work_dir}
>> > VERBATIM
>> > )
>> >
>> > Best regards,
>> > Stefan
>> >
>> >
>> > -----Original Message-----
>> > From: cmake-developers [mailto:cmake-developers-boun...@cmake.org] On
>> Behalf Of James Johnston
>> > Sent: Donnerstag, 20. August 2015 15:37
>> > To: cmake-developers@cmake.org
>> > Subject: Re: [cmake-developers] ExternalProject: Use native paths as
>> substitute for directory tokens
>> >
>> > > -----Original Message-----
>> > > From: cmake-developers [mailto:cmake-developers-boun...@cmake.org]
>> > > On Behalf Of Kislinskiy, Stefan
>> > > Sent: Thursday, August 20, 2015 09:02
>> > > To: David Cole
>> > > Cc: cmake-developers@cmake.org
>> > > Subject: Re: [cmake-developers] ExternalProject: Use native paths as
>> > > substitute for directory tokens
>> > >
>> > > Hi David,
>> > >
>> > > Example excerpt (it is not possible to change the working directory
>> > > for
>> > the
>> > > CONFIGURE_COMMAND as it is fixed to the BUILD_DIR, which might not be
>> > > sufficient):
>> >
>> > This doesn't really directly have to do with your proposal, but what if
>> an option was added to change the working dir of the CONFIGURE_COMMAND? E.g.
>> > WORKING_DIRECTORY_CONFIGURE. And suppose you'd have it recognize the
>> various tags like <SOURCE_DIR>, etc. This might be useful to add to other
>> steps as well, and would be more portable than your solution which is using
>> cmd.exe-specific commands. You'd want to audit for any resulting breakage
>> (e.g. does ExternalProject make assumptions that the working directory of
>> CONFIGURE is always the binary dir? - e.g. a relative path being used
>> somewhere. And probably only allow specification of
>> WORKING_DIRECTORY_CONFIGURE if a CONFIGURE_COMMAND was also specified, as
>> the built-in commands certainly assume the default working dir.)
>> >
>> > In your situation though, I'm not sure it's strictly needed. From your
>> sample, it looks like you're building boost. In your case what if you:
>> >
>> > * Use ExternalProject_Add_Step to bootstrap. You can specify a
>> WORKING_DIRECTORY here. Note one problem: you can't do out of source build
>> of b2, which breaks user expectations.
>> > * Then use ExternalProject_Add_Step to build Boost.
>> >
>> > Yes, using _Add_Step is somewhat of a workaround, but in this case, I've
>> found it wasn't much of a burden at all. In fact the only case I can think
>> of where it WOULD be a burden would be if the configure step is CMake. But
>> then you wouldn't need to change the working directory; changing it would
>> break CMake. In practice nobody will want to change WORKING_DIRECTORY
>> unless it's a custom command and then it's easy to use _Add_Step anyway.
>> > That said, it might still be considered a little undesired and so maybe
>> my proposal above would be a better way to handle it.
>> >
>> > Corrections from maintainers and others on the above commentary are
>> welcome...
>> >
>> > >
>> > > set(bootstrap_cmd "<SOURCE_DIR>/bootstrap${shell_ext}"
>> > > ${bootstrap_toolset})
>> > >
>> > > if(WIN32)
>> > > set(bootstrap_cmd pushd "<SOURCE_DIR>" COMMAND ${bootstrap_cmd}
>> > > COMMAND popd)
>> > > endif()
>> > >
>> > > ExternalProject_Add(Boost
>> > > ...
>> > > CONFIGURE_COMMAND ${bootstrap_cmd}
>> > > ...
>> > > )
>> >
>> > From add_custom_command: "If more than one COMMAND is specified they
>> will be executed in order, but not necessarily composed into a stateful
>> shell or batch script."
>> >
>> > So I am not sure your approach will work for you even if you fix the
>> issue with path slashes.
>> >
>> > James
>> >
>>
>> --
>>
>> 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
>> information on each offering, please visit:
>>
>> CMake Support: http://cmake.org/cmake/help/support.html
>> CMake Consulting: http://cmake.org/cmake/help/consulting.html
>> CMake Training Courses: http://cmake.org/cmake/help/training.html
>>
>> Visit other Kitware open-source projects at
>> http://www.kitware.com/opensource/opensource.html
>>
>> Follow this link to subscribe/unsubscribe:
>> http://public.kitware.com/mailman/listinfo/cmake-developers
> --
>
> 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
> information on each offering, please visit:
>
> CMake Support: http://cmake.org/cmake/help/support.html
> CMake Consulting: http://cmake.org/cmake/help/consulting.html
> CMake Training Courses: http://cmake.org/cmake/help/training.html
>
> Visit other Kitware open-source projects at
> http://www.kitware.com/opensource/opensource.html
>
> Follow this link to subscribe/unsubscribe:
> http://public.kitware.com/mailman/listinfo/cmake-developers
--
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
information on each offering, please visit:
CMake Support: http://cmake.org/cmake/help/support.html
CMake Consulting: http://cmake.org/cmake/help/consulting.html
CMake Training Courses: http://cmake.org/cmake/help/training.html
Visit other Kitware open-source projects at
http://www.kitware.com/opensource/opensource.html
Follow this link to subscribe/unsubscribe:
http://public.kitware.com/mailman/listinfo/cmake-developers
