[PATCH] Add update target to ninja

[Alp Toker]

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