Skip to content

Commit

Permalink
ENH: Create a functional install target
Browse files Browse the repository at this point in the history 
Update Artichoke to commontk/Artichoke@b0cf6fc85d5a. Use new
ExternalProject_Install_CMake to install external projects when doing a
superbuild build of slicer, so that there is a functional install target
for the same.

This change is made synchronously with the merge of commontk/CTK#466
which no longer installs QtTesting to the build tree, and instead
installs it as part of the install target for CTK; thus,
SlicerBlockInstallQtTesting.cmake is (and indeed, must be, as there is
no longer an install path being exported) removed.

At least on Fedora, this is sufficient to produce a Slicer that can be
successfully run after building the 'install' target.
  • Loading branch information
@mwoehlke-kitware
mwoehlke-kitware committed May 21, 2014
1 parent be6b4d4 commit 97679dd
Show file tree
Hide file tree
Showing 13 changed files with 215 additions and 191 deletions.
293 changes: 172 additions & 121 deletions CMake/ExternalProjectDependency.cmake
View file
Original file line number Diff line number Diff line change
@@ -1,33 +1,43 @@
################################################################################
#.rst:
# ExternalProjectDependency
# -------------------------

###########################################################################
#
# Program: 3D Slicer
# Library: CTK
#
# Copyright (c) Kitware Inc.
#
# See COPYRIGHT.txt
# or http://www.slicer.org/copyright/copyright.txt for details.
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0.txt
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.
#
# This file was originally developed by Jean-Christophe Fillion-Robin, Kitware Inc.
# and was partially funded by NIH grant 3P41RR013218-12S1
#
################################################################################
###########################################################################

include(CMakeParseArguments)

if(NOT DEFINED EP_LIST_SEPARATOR)
set(EP_LIST_SEPARATOR "^^")
endif()

#.rst:
# .. cmake:variable:: EXTERNAL_PROJECT_DIR
#
if(NOT EXISTS "${EXTERNAL_PROJECT_DIR}")
set(EXTERNAL_PROJECT_DIR ${CMAKE_SOURCE_DIR}/SuperBuild)
endif()

#.rst:
# .. cmake:variable:: EXTERNAL_PROJECT_FILE_PREFIX
#
if(NOT DEFINED EXTERNAL_PROJECT_FILE_PREFIX)
set(EXTERNAL_PROJECT_FILE_PREFIX "External_")
endif()
Expand All @@ -39,28 +49,36 @@ else()
set(EP_CMAKE_GENERATOR "${CMAKE_GENERATOR}")
endif()

#!
#! mark_as_superbuild(<varname1>[:<vartype1>] [<varname2>[:<vartype2>] [...]])
#!
#! mark_as_superbuild(
#! VARS <varname1>[:<vartype1>] [<varname2>[:<vartype2>] [...]]
#! [PROJECTS <projectname> [<projectname> [...]] | ALL_PROJECTS]
#! [LABELS <label1> [<label2> [...]]]
#! )
#!
#! PROJECTS corresponds to a list of <projectname> that will be added using 'ExternalProject_Add' function.
#! If not specified and called within a project file, it defaults to the value of 'SUPERBUILD_TOPLEVEL_PROJECT'
#! Otherwise, it defaults to 'CMAKE_PROJECT_NAME'.
#! If instead 'ALL_PROJECTS' is specified, the variables and labels will be passed to all projects.
#!
#! VARS is an expected list of variables specified as <varname>:<vartype> to pass to <projectname>
#!
#!
#! LABELS is an optional list of label to associate with the variable names specified using 'VARS' and passed to
#! the <projectname> as CMake CACHE args of the form:
#! -D<projectname>_EP_LABEL_<label1>=<varname1>;<varname2>[...]
#! -D<projectname>_EP_LABEL_<label2>=<varname1>;<varname2>[...]
#!
#.rst:
# .. cmake:function:: mark_as_superbuild
#
# .. code-block:: cmake
#
# mark_as_superbuild(<varname1>[:<vartype1>] [<varname2>[:<vartype2>] [...]])
#
# .. code-block:: cmake
#
# mark_as_superbuild(
# VARS <varname1>[:<vartype1>] [<varname2>[:<vartype2>] [...]]
# [PROJECTS <projectname> [<projectname> [...]] | ALL_PROJECTS]
# [LABELS <label1> [<label2> [...]]]
# )
#
# .. code-block:: cmake
#
# PROJECTS corresponds to a list of <projectname> that will be added using 'ExternalProject_Add' function.
# If not specified and called within a project file, it defaults to the value of 'SUPERBUILD_TOPLEVEL_PROJECT'
# Otherwise, it defaults to 'CMAKE_PROJECT_NAME'.
# If instead 'ALL_PROJECTS' is specified, the variables and labels will be passed to all projects.
#
# VARS is an expected list of variables specified as <varname>:<vartype> to pass to <projectname>
#
#
# LABELS is an optional list of label to associate with the variable names specified using 'VARS' and passed to
# the <projectname> as CMake CACHE args of the form:
# -D<projectname>_EP_LABEL_<label1>=<varname1>;<varname2>[...]
# -D<projectname>_EP_LABEL_<label2>=<varname1>;<varname2>[...]
#
function(mark_as_superbuild)
set(options ALL_PROJECTS CMAKE_CMD)
set(oneValueArgs)
Expand Down Expand Up @@ -112,14 +130,15 @@ function(mark_as_superbuild)
VARS ${_vars_with_type} LABELS ${_sb_LABELS} ${optional_arg_ALL_PROJECTS})
endfunction()

#!
#! _sb_extract_varname_and_vartype(<cmake_varname_and_type> <varname_var> [<vartype_var>])
#!
#! <cmake_varname_and_type> corresponds to variable name and variable type passed as "<varname>:<vartype>"
#!
#! <varname_var> will be set to "<varname>"
#!
#! <vartype_var> is an optional variable name that will be set to "<vartype>"
#
# _sb_extract_varname_and_vartype(<cmake_varname_and_type> <varname_var> [<vartype_var>])
#
# <cmake_varname_and_type> corresponds to variable name and variable type passed as "<varname>:<vartype>"
#
# <varname_var> will be set to "<varname>"
#
# <vartype_var> is an optional variable name that will be set to "<vartype>"
#
function(_sb_extract_varname_and_vartype cmake_varname_and_type varname_var)
set(_vartype_var "${ARGV2}")
string(REPLACE ":" ";" varname_and_vartype ${cmake_varname_and_type})
Expand Down Expand Up @@ -158,23 +177,23 @@ function(_sb_list_to_string separator input_list output_string_var)
cmake_policy(POP)
endfunction()


#!
#! _sb_cmakevar_to_cmakearg(<cmake_varname_and_type> <cmake_arg_var> <has_cfg_intdir_var> [<varname_var> [<vartype_var>]])
#!
#! <cmake_varname_and_type> corresponds to variable name and variable type passed as "<varname>:<vartype>"
#!
#! <cmake_arg_var> is a variable name that will be set to "-D<varname>:<vartype>=${<varname>}"
#!
#! <has_int_dir_var> is set to either TRUE or FALSE.
#! FALSE means that the value does NOT reference ${CMAKE_CFG_INTDIR} and
#! the generated cmake argument should be passed to ExternalProject_Add as CMAKE_CACHE_ARGS.
#! TRUEmeans that the value does reference ${CMAKE_CFG_INTDIR} and
#! the generated cmake argument should be passed to ExternalProject_Add as CMAKE_ARGS.
#!
#! <varname_var> is an optional variable name that will be set to "<varname>"
#!
#! <vartype_var> is an optional variable name that will be set to "<vartype>"
#
# _sb_cmakevar_to_cmakearg(<cmake_varname_and_type> <cmake_arg_var> <has_cfg_intdir_var> [<varname_var> [<vartype_var>]])
#
# <cmake_varname_and_type> corresponds to variable name and variable type passed as "<varname>:<vartype>"
#
# <cmake_arg_var> is a variable name that will be set to "-D<varname>:<vartype>=${<varname>}"
#
# <has_int_dir_var> is set to either TRUE or FALSE.
# FALSE means that the value does NOT reference ${CMAKE_CFG_INTDIR} and
# the generated cmake argument should be passed to ExternalProject_Add as CMAKE_CACHE_ARGS.
# TRUEmeans that the value does reference ${CMAKE_CFG_INTDIR} and
# the generated cmake argument should be passed to ExternalProject_Add as CMAKE_ARGS.
#
# <varname_var> is an optional variable name that will be set to "<varname>"
#
# <vartype_var> is an optional variable name that will be set to "<vartype>"
#
function(_sb_cmakevar_to_cmakearg cmake_varname_and_type cmake_arg_var has_cfg_intdir_var)
set(_varname_var "${ARGV3}")
set(_vartype_var "${ARGV4}")
Expand Down Expand Up @@ -210,26 +229,26 @@ endfunction()

set(_ALL_PROJECT_IDENTIFIER "ALLALLALL")

#!
#! _sb_append_to_cmake_args(
#! VARS <varname1>:<vartype1> [<varname2>:<vartype2> [...]]
#! [PROJECTS <projectname> [<projectname> [...]] | ALL_PROJECTS]
#! [LABELS <label1> [<label2> [...]]]
#! )
#!
#! PROJECTS corresponds to a list of <projectname> that will be added using 'ExternalProject_Add' function.
#! If not specified and called within a project file, it defaults to the value of 'SUPERBUILD_TOPLEVEL_PROJECT'
#! Otherwise, it defaults to 'CMAKE_PROJECT_NAME'.
#! If instead 'ALL_PROJECTS' is specified, the variables and labels will be passed to all projects.
#!
#! VARS is an expected list of variables specified as <varname>:<vartype> to pass to <projectname>
#!
#!
#! LABELS is an optional list of label to associate with the variable names specified using 'VARS' and passed to
#! the <projectname> as CMake CACHE args of the form:
#! -D<projectname>_EP_LABEL_<label1>=<varname1>;<varname2>[...]
#! -D<projectname>_EP_LABEL_<label2>=<varname1>;<varname2>[...]
#!
#
# _sb_append_to_cmake_args(
# VARS <varname1>:<vartype1> [<varname2>:<vartype2> [...]]
# [PROJECTS <projectname> [<projectname> [...]] | ALL_PROJECTS]
# [LABELS <label1> [<label2> [...]]]
# )
#
# PROJECTS corresponds to a list of <projectname> that will be added using 'ExternalProject_Add' function.
# If not specified and called within a project file, it defaults to the value of 'SUPERBUILD_TOPLEVEL_PROJECT'
# Otherwise, it defaults to 'CMAKE_PROJECT_NAME'.
# If instead 'ALL_PROJECTS' is specified, the variables and labels will be passed to all projects.
#
# VARS is an expected list of variables specified as <varname>:<vartype> to pass to <projectname>
#
#
# LABELS is an optional list of label to associate with the variable names specified using 'VARS' and passed to
# the <projectname> as CMake CACHE args of the form:
# -D<projectname>_EP_LABEL_<label1>=<varname1>;<varname2>[...]
# -D<projectname>_EP_LABEL_<label2>=<varname1>;<varname2>[...]
#
function(_sb_append_to_cmake_args)
set(options ALL_PROJECTS)
set(oneValueArgs)
Expand Down Expand Up @@ -330,9 +349,13 @@ function(_sb_update_indent proj)
set_property(GLOBAL PROPERTY SUPERBUILD_${proj}_INDENT ${_indent})
endfunction()

#!
#! ExternalProject_Message(<project_name> <msg> [condition])
#!
#.rst:
# .. cmake:function:: ExternalProject_Message
#
# .. code-block:: cmake
#
# ExternalProject_Message(<project_name> <msg> [condition])
#
function(ExternalProject_Message proj msg)
set(_display 1)
if("${ARGV2}" MATCHES ".+")
Expand All @@ -344,48 +367,52 @@ function(ExternalProject_Message proj msg)
endif()
endfunction()

#!
#! superbuild_stack_content(<stack_name> <output_var>)
#!
#! <stack_name> corresponds to the name of stack.
#!
#! <output_var> is the name of CMake variable that will be set with the content
#! of the stack identified by <stack_name>.
#
# superbuild_stack_content(<stack_name> <output_var>)
#
# <stack_name> corresponds to the name of stack.
#
# <output_var> is the name of CMake variable that will be set with the content
# of the stack identified by <stack_name>.
#
function(superbuild_stack_content stack_name output_var)
get_property(_stack GLOBAL PROPERTY ${stack_name})
set(${output_var} ${_stack} PARENT_SCOPE)
endfunction()

#!
#! superbuild_stack_size(<stack_name> <output_var>)
#!
#! <stack_name> corresponds to the name of stack.
#!
#! <output_var> is the name of CMake variable that will be set with the size
#! of the stack identified by <stack_name>.
#
# superbuild_stack_size(<stack_name> <output_var>)
#
# <stack_name> corresponds to the name of stack.
#
# <output_var> is the name of CMake variable that will be set with the size
# of the stack identified by <stack_name>.
#
function(superbuild_stack_size stack_name output_var)
get_property(_stack GLOBAL PROPERTY ${stack_name})
list(LENGTH _stack _stack_size)
set(${output_var} ${_stack_size} PARENT_SCOPE)
endfunction()

#!
#! superbuild_stack_push(<stack_name> <value>)
#!
#! <stack_name> corresponds to the name of stack.
#!
#! <value> is appended to the stack identified by <stack_name>.
#
# superbuild_stack_push(<stack_name> <value>)
#
# <stack_name> corresponds to the name of stack.
#
# <value> is appended to the stack identified by <stack_name>.
#
function(superbuild_stack_push stack_name value)
set_property(GLOBAL APPEND PROPERTY ${stack_name} ${value})
endfunction()

#!
#! superbuild_stack_pop(<stack_name> <item_var>)
#!
#! <stack_name> corresponds to the name of stack.
#!
#! <item_var> names a CMake variable that will be set with the item
#! removed from the stack identified by <stack_name>.
#
# superbuild_stack_pop(<stack_name> <item_var>)
#
# <stack_name> corresponds to the name of stack.
#
# <item_var> names a CMake variable that will be set with the item
# removed from the stack identified by <stack_name>
#
function(superbuild_stack_pop stack_name item_var)
get_property(_stack GLOBAL PROPERTY ${stack_name})
list(LENGTH _stack _stack_size)
Expand All @@ -410,16 +437,19 @@ function(_sb_is_optional proj output_var)
set(${output_var} ${optional} PARENT_SCOPE)
endfunction()


#!
#! ExternalProject_Include_Dependencies(<project_name>
#! [PROJECT_VAR <project_var>]
#! [EP_ARGS_VAR <external_project_args_var>]
#! [DEPENDS_VAR <depends_var>]
#! [USE_SYSTEM_VAR <use_system_var>]
#! [SUPERBUILD_VAR <superbuild_var>]
#! )
#!
#.rst:
# .. cmake:function:: ExternalProject_Include_Dependencies
#
# .. code-block:: cmake
#
# ExternalProject_Include_Dependencies(<project_name>
# [PROJECT_VAR <project_var>]
# [EP_ARGS_VAR <external_project_args_var>]
# [DEPENDS_VAR <depends_var>]
# [USE_SYSTEM_VAR <use_system_var>]
# [SUPERBUILD_VAR <superbuild_var>]
# )
#
macro(ExternalProject_Include_Dependencies project_name)
set(options)
set(oneValueArgs PROJECT_VAR DEPENDS_VAR EP_ARGS_VAR USE_SYSTEM_VAR SUPERBUILD_VAR)
Expand Down Expand Up @@ -645,12 +675,15 @@ macro(ExternalProject_Include_Dependencies project_name)
#message("[${_sb_proj}] Setting ${_sb_USE_SYSTEM_VAR}:${_sb_USE_SYSTEM}")
endmacro()


#!
#! ExternalProject_Add_Empty(<project_name>
#! DEPENDS <depends>
#! )
#!
#.rst:
# .. cmake:function:: ExternalProject_Add_Empty
#
# .. code-block:: cmake
#
# ExternalProject_Add_Empty(<project_name>
# DEPENDS <depends>
# )
#
macro(ExternalProject_Add_Empty project_name)
set(options)
set(oneValueArgs)
Expand All @@ -675,3 +708,21 @@ macro(ExternalProject_Add_Empty project_name)
DEPENDS ${_sb_DEPENDS}
)
endmacro()

#.rst:
# .. cmake:function:: ExternalProject_Install_CMake
#
# Install an external CMake-based project as part of the ``install`` target.
#
# .. code-block:: cmake
#
# ExternalProject_Install_CMake(<project_name>)
#
# This causes building the main project's ``install`` target to also execute
# the CMake install script for the specified external project.
#
function(ExternalProject_Install_CMake project_name)
ExternalProject_Get_Property(${project_name} binary_dir)

install(SCRIPT ${binary_dir}/cmake_install.cmake)
endfunction()

8 comments on commit 97679dd

@jcfr
Copy link

@jcfr jcfr commented on 97679dd May 21, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You mentioned no longer installs QtTesting to the build tree, and instead installs it as part of the install target for CTK, it probably mean that CTK install rule like the one in [1] should be changed to deal with the superbuild folder instead of the inner build one. Indeed, the install script statement like [2] are only added in the superbuild.

# Install CTK Apps and Plugins (PythonQt modules, QtDesigner plugins ...)
if(NOT "${CTK_DIR}" STREQUAL "" AND EXISTS "${CTK_DIR}/CTK-build/CMakeCache.txt")
  set(CPACK_INSTALL_CMAKE_PROJECTS "${CPACK_INSTALL_CMAKE_PROJECTS};${CTK_DIR}/CTK-build;CTK;RuntimeApplications;/")
  set(CPACK_INSTALL_CMAKE_PROJECTS "${CPACK_INSTALL_CMAKE_PROJECTS};${CTK_DIR}/CTK-build;CTK;RuntimePlugins;/")
endif()

should be changed into

# Install CTK Apps and Plugins (PythonQt modules, QtDesigner plugins ...)
if(NOT "${CTK_DIR}" STREQUAL "" AND EXISTS "${CTK_DIR}/CMakeCache.txt")
  set(CPACK_INSTALL_CMAKE_PROJECTS "${CPACK_INSTALL_CMAKE_PROJECTS};${CTK_DIR};CTK;RuntimeApplications;/")
  set(CPACK_INSTALL_CMAKE_PROJECTS "${CPACK_INSTALL_CMAKE_PROJECTS};${CTK_DIR};CTK;RuntimePlugins;/")
endif()

Same remarks for

Slicer/CMake/SlicerBlockInstallCMakeProjects.cmake

Line 65 in 97679dd

set(CPACK_INSTALL_CMAKE_PROJECTS "${CPACK_INSTALL_CMAKE_PROJECTS};${CTK_DIR}/CTK-build;CTK;RuntimeLibraries;/")

[1]

Slicer/CMake/SlicerCPack.cmake

Line 24 in 97679dd

set(CPACK_INSTALL_CMAKE_PROJECTS "${CPACK_INSTALL_CMAKE_PROJECTS};${CTK_DIR}/CTK-build;CTK;RuntimeApplications;/")

[2] https://github.com/mwoehlke-kitware/CTK/blob/8a0a9c7d96495d946f2f6913be13a20b61779498/CMakeExternals/QtTesting.cmake#L71

@jcfr
Copy link

@jcfr jcfr commented on 97679dd May 21, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would suggest we keep the installation of QtTesting. Use of ExternalProject_Install_CMake should impact the only the superbuild project and not the inner build one.

@mwoehlke-kitware
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would suggest we keep the installation of QtTesting.

Obviously we want it installed. Do you mean to keep SlicerBlockInstallQtTesting.cmake and instead rewrite it to work from the build dir like e.g. SlicerBlockInstallPythonQt.cmake? Or are you still proposing to make the changes as in your previous comment?

If the latter, should SlicerBlockInstallPythonQt.cmake also go away? (And similar for anything else we get via CTK superbuild?)

@mwoehlke-kitware
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, right... PythonQt is still locally installed. In general, I'm not in favor of doing that unnecessarily (i.e. I'm not fond of CTK doing local installs only for Slicer's benefit). IIRC it wasn't as trivial to use PythonQt from the build dir, but now I'm not sure how much time I actually spent looking at it, vs. that I knew QtTesting is usable from the build dir.

@jcfr
Copy link

@jcfr jcfr commented on 97679dd May 21, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. What do you mean by having SlicerBlockInstallQtTesting.cmake work from the build dir ?
    Both [SlicerBlockInstallQtTesting.cmake](https://github.com/Slicer/Slicer/blob/master/CMake/SlicerBlockInstallQtTesting.cmake and) and SlicerBlockInstallPythonQt.cmake are already doing the same things.

  2. For the approach where we installed from Superbuild tree, SlicerBlockInstallPythonQt.cmake would have to be removed. This implies that both QtTesting and PythonQt have their libraries associated with components.

To clarify things, are you trying to make install working from:

  • Slicer superbuild folder
  • Slicer inner build
  • both

I am assuming you addressed the Superbuild folder case ?

@jcfr
Copy link

@jcfr jcfr commented on 97679dd May 21, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re 97679dd#commitcomment-6406562

Let's not change how project are used, can we keep that for later ? That means we would have to an insane amount of testing on MacOSX and windows ... and I don't feel comfortable doing such change now.

@mwoehlke-kitware
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. What do you mean by having SlicerBlockInstallQtTesting.cmake work from the build dir ?

Disregard this (seem my other comment).

To clarify things, are you trying to make install working from:

Slicer superbuild folder

Yes; to install slicer and all requisite dependencies built as part of the superbuild.

Slicer inner build

I don't expect this to install any external dependencies, but only slicer itself. To at least that extent, it is already working, and at least VTK and CTK must be separately installed (as expected). However, I'm not entirely clear where the package target picks up external dependencies, since it exists here and not in the superbuild.

@jcfr
Copy link

@jcfr jcfr commented on 97679dd May 21, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not entirely clear where the package target picks up external dependencies, since it exists here and not in the superbuild.

CPack provide you with a variable named CPACK_INSTALL_CMAKE_PROJECTS allowing to specify additional project to install. For example see [1] and [2]

On MacOSX, we install only the executable and plugins and we let the fixup script copying over dependencies.

[1] https://github.com/Slicer/Slicer/blob/daeeacad1b60300f866fe3ae2a1172609ea32052/CMake/SlicerCPack.cmake#L24
[2] https://github.com/Slicer/Slicer/blob/master/CMake/SlicerBlockInstallCMakeProjects.cmake

Please sign in to comment.