[PATCH] Add update target to ninja

[Gautier DI FOLCO]

2014-03-02 10:29 GMT+01:00 Alp Toker <alp at nuanti.com>:

>
> On 01/03/2014 15:46, Gautier DI FOLCO wrote:
>
>> Hi chandlerc, hansw, chapuni,
>>
>> I just added an update command to ninja, now we can do $ ninja update, as
>> we can do $ make update.
>>
>
> 'make update' is currently only available in the traditional non-CMake
> makefiles.
>
>
>
>> http://llvm-reviews.chandlerc.com/D2908
>>
>> Files:
>>    CMakeLists.txt
>>
>> Index: CMakeLists.txt
>> ===================================================================
>> --- CMakeLists.txt
>> +++ CMakeLists.txt
>> @@ -569,3 +569,8 @@
>>     endif()
>>   endif()
>>   +if(CMAKE_GENERATOR MATCHES "Ninja")
>> +  add_custom_target(update
>> +      COMMAND find "${CMAKE_CURRENT_SOURCE_DIR}" -name .svn -type d |
>> sed -e 's/\\.svn//' | xargs svn update
>> +  )
>> +endif()
>>
>
> The patch isn't correct to conditionalize on Ninja alone -- if the feature
> is desirable it should be enabled with all CMake generators including
> Makefiles.
>
> However the invocation as proposed will fail to update source modules
> unless they're nested in the LLVM core source directory (ignoring e.g.
> side-by-side modules), and the shell invocations won't work at all on
> non-Unix platforms. So there are deeper problems here.
>
> Moreover the feature feels like an anachronism now that there are various
> version control systems in use. It also breaks the invariant that build
> commands issued from the build output directory can never modify the source
> tree.
>
> I won't weigh in on the value of keeping 'make update' support in the
> traditional Makefile build system but it doesn't feel right to bring it
> forward to CMake.
>
> Alp.
>
> --
> http://www.nuanti.com
> the browser experts
>
>
In fact I have asked on ninja IRC channel and some time after that I have
sent this patch, I was told that that it could be a problem if we call $
ninja update all (he also said that the same problem occurs with make).
So that is really a bad idea.

Thanks for your time,
Sorry for that.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140302/36663ce9/attachment.html>