[cmake-developers] ExternalProject: Use native paths as substitute for directory tokens

[James Johnston]

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.kislinskiy at 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-bounces at cmake.org] On Behalf Of James Johnston 
 > Sent: Donnerstag, 20. August 2015 15:37 
 > To: cmake-developers at cmake.org 
 > Subject: Re: [cmake-developers] ExternalProject: Use native paths as substitute for directory tokens 
 >  
 > > -----Original Message----- 
 > > From: cmake-developers [mailto:cmake-developers-bounces at cmake.org] 
 > > On Behalf Of Kislinskiy, Stefan 
 > > Sent: Thursday, August 20, 2015 09:02 
 > > To: David Cole 
 > > Cc: cmake-developers at 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 
 >