From b36631de947fbea590702e4f2e45d2d877741558 Mon Sep 17 00:00:00 2001
From: Jean-Christophe Fillion-Robin <jchris.fillionr@kitware.com>
Date: Wed, 12 Oct 2016 08:36:11 -0400
Subject: [PATCH 1/5] STYLE: UseSlicer: Simplify how list of optional extension
variable is obtained
---
CMake/UseSlicer.cmake.in | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/CMake/UseSlicer.cmake.in b/CMake/UseSlicer.cmake.in
index a214b81986..30c39c5f36 100644
--- a/CMake/UseSlicer.cmake.in
+++ b/CMake/UseSlicer.cmake.in
@@ -393,10 +393,9 @@ if(NOT Slicer_DONT_USE_EXTENSION)
slicer_setting_variable_message("MIDAS_PACKAGE_EMAIL" OBFUSCATE)
slicer_setting_variable_message("MIDAS_PACKAGE_API_KEY" OBFUSCATE)
- set(optional_vars HOMEPAGE CATEGORY ICONURL CONTRIBUTORS DESCRIPTION
- SCREENSHOTURLS DEPENDS LICENSE_FILE README_FILE
- )
- foreach(var ${optional_vars})
+ include(SlicerExtensionDescriptionSpec)
+
+ foreach(var IN LISTS Slicer_EXT_OPTIONAL_METADATA_NAMES)
slicer_setting_variable_message(EXTENSION_${var})
endforeach()
--
GitLab
From 1447fa7f8129dee2a1622ad4336bcaf70e836637 Mon Sep 17 00:00:00 2001
From: Jean-Christophe Fillion-Robin <jchris.fillionr@kitware.com>
Date: Wed, 12 Oct 2016 10:03:27 -0400
Subject: [PATCH 2/5] STYLE: UseSlicerMacros: Use CMakeParseArguments with
setting_variable_message
This is done anticipating a other change.
---
CMake/UseSlicerMacros.cmake | 20 +++++++++++++-------
1 file changed, 13 insertions(+), 7 deletions(-)
diff --git a/CMake/UseSlicerMacros.cmake b/CMake/UseSlicerMacros.cmake
index 19bd9ab870..ccb11d5959 100644
--- a/CMake/UseSlicerMacros.cmake
+++ b/CMake/UseSlicerMacros.cmake
@@ -66,16 +66,22 @@ endfunction()
# -- Setting SHORTNAME ........: This is short variable name
# -- Setting LONGLONGNAME .....: This is a longer variable name
#
+include(CMakeParseArguments)
function(slicer_setting_variable_message varname)
+ set(options OBFUSCATE SKIP_TRUNCATE)
+ set(oneValueArgs )
+ set(multiValueArgs )
+ CMAKE_PARSE_ARGUMENTS(LOCAL
+ "${options}"
+ "${oneValueArgs}"
+ "${multiValueArgs}"
+ ${ARGN}
+ )
set(truncate TRUE)
- set(obfuscate FALSE)
- foreach(arg ${ARGN})
- if(arg STREQUAL "SKIP_TRUNCATE")
- set(truncate FALSE)
- elseif(arg STREQUAL "OBFUSCATE")
- set(obfuscate TRUE)
+ if(LOCAL_SKIP_TRUNCATE)
+ set(truncate FALSE)
endif()
- endforeach()
+ set(obfuscate ${LOCAL_OBFUSCATE})
set(pretext_right_jusitfy_length 45)
set(fill_char ".")
set(truncated_text_length 120)
--
GitLab
From 7eab9f4f5e85382a1c039c30625390fc5f47ad33 Mon Sep 17 00:00:00 2001
From: Jean-Christophe Fillion-Robin <jchris.fillionr@kitware.com>
Date: Wed, 12 Oct 2016 09:53:51 -0400
Subject: [PATCH 3/5] STYLE: UseSlicerMacros: setting_variable_message function
learn PRETEXT arg
It is now possible to override the default pretext, specifying
PRETEXT argument.
---
CMake/UseSlicerMacros.cmake | 21 +++++++++++++++++----
1 file changed, 17 insertions(+), 4 deletions(-)
diff --git a/CMake/UseSlicerMacros.cmake b/CMake/UseSlicerMacros.cmake
index ccb11d5959..15f026b141 100644
--- a/CMake/UseSlicerMacros.cmake
+++ b/CMake/UseSlicerMacros.cmake
@@ -47,12 +47,15 @@ endfunction()
# If the variable is not defined, it will display:
# "-- Setting <varname> ........: <NOT DEFINED>"
#
-# If the optional argument 'SKIP_TRUNCATE' is provided, the
+# If the option 'SKIP_TRUNCATE' is provided, the
# text will NOT be truncated it too long.
#
-# If the optional argument 'OBFUSCATE' is provided, 'OBFUSCATED' will
+# If the option 'OBFUSCATE' is provided, 'OBFUSCATED' will
# be displayed instead of the variable value.
#
+# If the optional argument `PRETEXT <pretext>` is provided,
+# `<pretext` will be displaying instead of `Setting <varname>`.
+#
# In the current implementation, the padding is hardcoded to a length of 40
# and the total text will be truncated if longer than 120 characters.
#
@@ -69,7 +72,7 @@ endfunction()
include(CMakeParseArguments)
function(slicer_setting_variable_message varname)
set(options OBFUSCATE SKIP_TRUNCATE)
- set(oneValueArgs )
+ set(oneValueArgs PRETEXT)
set(multiValueArgs )
CMAKE_PARSE_ARGUMENTS(LOCAL
"${options}"
@@ -82,6 +85,12 @@ function(slicer_setting_variable_message varname)
set(truncate FALSE)
endif()
set(obfuscate ${LOCAL_OBFUSCATE})
+
+ set(pretext "Setting ${varname}")
+ if(DEFINED LOCAL_PRETEXT)
+ set(pretext ${LOCAL_PRETEXT})
+ endif()
+
set(pretext_right_jusitfy_length 45)
set(fill_char ".")
set(truncated_text_length 120)
@@ -96,7 +105,6 @@ function(slicer_setting_variable_message varname)
set(_value "OBFUSCATED")
endif()
- set(pretext "Setting ${varname}")
string(LENGTH ${pretext} pretext_length)
math(EXPR pad_length "${pretext_right_jusitfy_length} - ${pretext_length} - 1")
if(pad_length GREATER 0)
@@ -145,6 +153,11 @@ function(slicer_setting_variable_message_test)
set(YOU_SHOULD_NOT_SEE_THE_VALUE "You should not see this")
slicer_setting_variable_message("YOU_SHOULD_NOT_SEE_THE_VALUE" OBFUSCATE)
+ set(YOU_SHOULD_SEE_DIFFERENT_PRETEXT "good")
+ slicer_setting_variable_message(
+ "YOU_SHOULD_SEE_DIFFERENT_PRETEXT"
+ PRETEXT "This is different pretext")
+
message("SUCCESS")
endfunction()
if(TEST_slicer_setting_variable_message_test)
--
GitLab
From 866bdfe788ea7a0fb124cc54e19c5643bab761a4 Mon Sep 17 00:00:00 2001
From: Jean-Christophe Fillion-Robin <jchris.fillionr@kitware.com>
Date: Wed, 12 Oct 2016 11:30:50 -0400
Subject: [PATCH 4/5] BUG: Extension upload: Ensure missing optional metatdata
do not cause failure
This commit improves the Extension build system tests to check that:
(1) optional extension metatdata NOT specified in the CMakeLists do NOT
prevent the configure/build/test/upload from happening
(2) metadata submitted to the extension server during the upload are the
expected ones.
Fixes #4276
---
CMake/UseSlicer.cmake.in | 13 ++
.../CMake/MIDASAPIUploadExtension.cmake | 6 +-
.../Testing/SlicerExtensionBuildSystemTest.py | 114 +++++++++++++++++-
3 files changed, 129 insertions(+), 4 deletions(-)
diff --git a/CMake/UseSlicer.cmake.in b/CMake/UseSlicer.cmake.in
index 30c39c5f36..06d14f6082 100644
--- a/CMake/UseSlicer.cmake.in
+++ b/CMake/UseSlicer.cmake.in
@@ -399,6 +399,19 @@ if(NOT Slicer_DONT_USE_EXTENSION)
slicer_setting_variable_message(EXTENSION_${var})
endforeach()
+ #
+ # If not defined, initialize optional variables with their default values.
+ #
+ foreach(var IN LISTS Slicer_EXT_OPTIONAL_METADATA_NAMES)
+ if(NOT DEFINED EXTENSION_${var} AND
+ NOT ${var} STREQUAL "SVNUSERNAME" AND
+ NOT ${var} STREQUAL "SVNPASSWORD")
+ set(EXTENSION_${var} "${Slicer_EXT_METADATA_${var}_DEFAULT}")
+ slicer_setting_variable_message(EXTENSION_${var}
+ PRETEXT "Setting default for EXTENSION_${var}")
+ endif()
+ endforeach()
+
#-----------------------------------------------------------------------------
if(NOT DEFINED Slicer_SKIP_SlicerEnableExtensionTesting)
set(Slicer_SKIP_SlicerEnableExtensionTesting FALSE)
diff --git a/Extensions/CMake/MIDASAPIUploadExtension.cmake b/Extensions/CMake/MIDASAPIUploadExtension.cmake
index 2e0dd33781..643afb090b 100644
--- a/Extensions/CMake/MIDASAPIUploadExtension.cmake
+++ b/Extensions/CMake/MIDASAPIUploadExtension.cmake
@@ -31,9 +31,6 @@
function(midas_api_upload_extension)
set(expected_nonempty_args
ARCHITECTURE
- EXTENSION_CATEGORY
- EXTENSION_ENABLED
- EXTENSION_HOMEPAGE
EXTENSION_NAME
EXTENSION_REPOSITORY_TYPE
EXTENSION_REPOSITORY_URL
@@ -48,8 +45,11 @@ function(midas_api_upload_extension)
SUBMISSION_TYPE
)
set(optional_args
+ EXTENSION_CATEGORY
EXTENSION_CONTRIBUTORS
EXTENSION_DESCRIPTION
+ EXTENSION_ENABLED
+ EXTENSION_HOMEPAGE
EXTENSION_ICONURL
EXTENSION_SCREENSHOTURLS
)
diff --git a/Extensions/CMake/Testing/SlicerExtensionBuildSystemTest.py b/Extensions/CMake/Testing/SlicerExtensionBuildSystemTest.py
index 1bf7919121..97271a8f74 100644
--- a/Extensions/CMake/Testing/SlicerExtensionBuildSystemTest.py
+++ b/Extensions/CMake/Testing/SlicerExtensionBuildSystemTest.py
@@ -21,6 +21,10 @@ _server = None
_midas_token = 'TestTokenWithExpectedLengthxxxxxxxxxxxxx'
_requests = []
+_midas_upload_query_data = {}
+"""Keep track of Midas upload query parameters for each submitted Extension.
+"""
+
def get_cmakecache_values(file_path, variables):
result = dict.fromkeys(variables)
with open(file_path) as cmakecache:
@@ -131,6 +135,13 @@ class Handler(BaseHTTPServer.BaseHTTPRequestHandler):
response_type = "application/json"
query_data = parse_qs(urlparse(self.path).query)
if query_data['method'][0] == 'midas.slicerpackages.extension.upload':
+
+ extension_name = query_data['productname'][0]
+
+ # Keep track of method parameters
+ global _midas_upload_query_data
+ _midas_upload_query_data[extension_name] = query_data
+
# Send back extension data
response_data = {
'stat': 'ok',
@@ -138,7 +149,7 @@ class Handler(BaseHTTPServer.BaseHTTPRequestHandler):
'message': '',
'data': {
'extension': {
- 'productname': query_data['productname'][0]
+ 'productname': extension_name
# XXX Real server sends back all properties of extension DAO
}
}
@@ -223,6 +234,20 @@ class SlicerExtensionBuildSystemTest(unittest.TestCase):
if _server:
_server.stop()
+ def _remove_matching_lines(self, filepath, patterns=[]):
+ """Given an input ``filepath`` and a list of ``patterns``, this function
+ will update the file in place removing all lines matching any of the
+ listed ``patterns``.
+ """
+ updated_content = []
+ with open(filepath) as content:
+ for line in content:
+ if any([pattern in line for pattern in patterns]):
+ continue
+ updated_content.append(line)
+ with open(filepath, 'w') as output:
+ output.write("".join(updated_content))
+
def _prepare_test_binary_dir(self, test_binary_dir):
# Remove binary directory
@@ -274,6 +299,29 @@ class SlicerExtensionBuildSystemTest(unittest.TestCase):
with open(extension_description_dir + '/TestExt%s.s4ext' % suffix, 'w') as description_file:
description_file.write(description)
+ if suffix == 'A':
+ #
+ # Strip all optional metadata from Extension A. This allows to check
+ # that the full process can effectively succeed without them.
+ # See issue #4276
+ #
+ # Extension A is chosen because it has no dependency. This mean we
+ # can even strip the DEPENDS option.
+ #
+ # Finally, note this is done after generating the description file
+ # because the ExtensionWizard currently expects them to be set.
+ #
+ # The list of optional metadata has been copied from
+ # SlicerExtensionDescriptionSpec.cmake CMake module.
+ #
+ patterns = ['EXTENSION_' + pattern for pattern in [
+ 'SVNUSERNAME', 'SVNPASSWORD', 'DEPENDS', 'BUILD_SUBDIRECTORY',
+ 'HOMEPAGE', 'CONTRIBUTORS', 'CATEGORY', 'ICONURL',
+ 'DESCRIPTION', 'SCREENSHOTURLS', 'ENABLED', 'STATUS'
+ ]]
+ extension_dir = test_binary_dir + '/TestExt%s' % suffix
+ self._remove_matching_lines(extension_dir + '/CMakeLists.txt', patterns)
+
def test_index_build_with_upload(self):
self._test_index_build('build_with_upload', True)
@@ -394,6 +442,7 @@ include({slicer_source_dir}/Extensions/CMake/SlicerExtensionsDashboardDriverScri
# Check
self._check_caches(test_binary_dir + '/S-E')
self._check_queries(test_upload, with_ctest=True)
+ self._check_midas_upload_query_parameters(test_upload, with_ctest=True)
def _test_index_build(self, test_name, test_upload):
@@ -450,6 +499,7 @@ include({slicer_source_dir}/Extensions/CMake/SlicerExtensionsDashboardDriverScri
# Check
self._check_caches(test_binary_dir)
self._check_queries(test_upload, with_ctest=False)
+ self._check_midas_upload_query_parameters(test_upload, with_ctest=False)
def _check_caches(self, test_binary_dir):
@@ -553,3 +603,65 @@ include({slicer_source_dir}/Extensions/CMake/SlicerExtensionsDashboardDriverScri
# Upload top-level build results and notes to CDash
check_cdash_request(parse_request(requests.next()), 'PUT', r'.+Build\.xml')
check_cdash_request(parse_request(requests.next()), 'PUT', r'.+Notes\.xml')
+
+ def _check_midas_upload_query_parameters(self, test_upload, with_ctest=False):
+ if not test_upload:
+ for extensionName in ['TestExtA', 'TestExtB', 'TestExtC']:
+ self.assertTrue(extensionName not in _midas_upload_query_data)
+
+ else:
+
+ def _expected_parameters(extensionName):
+ expected = {
+ 'arch': ['amd64'],
+ 'codebase': ['Slicer4'],
+ 'method': ['midas.slicerpackages.extension.upload'],
+ 'name': None, # ['25430-linux-amd64-TestExtB-local0-0000-00-00.tar.gz'],
+ 'os': None, # ['linux'],
+ 'packagetype': ['archive'],
+ 'productname': [extensionName],
+ 'repository_type': ['local'],
+ 'repository_url': ['NA'],
+ 'revision': ['0'],
+ 'slicer_revision': None, #['25430'],
+ 'submissiontype': ['nightly'],
+ 'token': ['TestTokenWithExpectedLengthxxxxxxxxxxxxx']
+ }
+ if extensionName != 'TestExtA':
+ expected.update({
+ 'category': ['Examples'],
+ 'contributors': ['John Doe (AnyWare Corp.)'],
+ 'description': ['This is an example of a simple extension'],
+ 'enabled': ['1'],
+ 'homepage': ['http://slicer.org/slicerWiki/index.php/Documentation/Nightly/Extensions/%s' % extensionName],
+ 'icon_url': ['http://www.example.com/Slicer/Extensions/%s.png' % extensionName],
+ 'screenshots': ['http://www.example.com/Slicer/Extensions/%s/Screenshots/1.png' % extensionName],
+ })
+ else:
+ # XXX We should have a way to get the default value from
+ # SlicerExtensionDescriptionSpec.cmake
+ # XXX Empty parameter are not sent to the server
+ expected.update({
+ #'category': [''],
+ #'contributors': [''],
+ #'description': [''],
+ 'enabled': ['1'],
+ #'homepage': [''],
+ #'icon_url': [''],
+ #'screenshots': [''],
+ })
+ return expected
+
+ for extensionName in ['TestExtA', 'TestExtB', 'TestExtC']:
+ query_data = _midas_upload_query_data[extensionName]
+ expected_query_data = _expected_parameters(extensionName)
+
+ self.assertEqual(sorted(query_data.keys()), sorted(expected_query_data.keys()))
+
+ for key, expected_value in expected_query_data.items():
+ current_value = query_data[key]
+ if expected_value is None:
+ # XXX For now only consider values that do not change with platform
+ # and Slicer revision.
+ continue
+ self.assertEqual(current_value, expected_value)
--
GitLab
From f0eb21f82bbea391a92864e3a9e448dd53018bc1 Mon Sep 17 00:00:00 2001
From: Jean-Christophe Fillion-Robin <jchris.fillionr@kitware.com>
Date: Wed, 12 Oct 2016 13:30:52 -0400
Subject: [PATCH 5/5] BUG: Ensure ExtensionBuildSystem tests propagate
CMAKE_OSX_* vars
This commit fixes error like this one:
```
7: -- Setting OSX_ARCHITECTURES to 'x86_64' as none was specified.
7: -- Setting OSX_SYSROOT to latest '/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.12.sdk' as none was specified.
7: CMake Error at /Users/inorton/git/slcr/r4/CMake/SlicerBlockSetCMakeOSXVariables.cmake:102 (message):
7: The OSX_SYSROOT is set to version 10.12 (>10.8) and OSX_DEPLOYMENT_TARGET
7: is not explicitly set!
7:
7: Since:
7:
7: (1) the default runtime associated with >=10.9 deployment target is 'libc++'.[1]
7: (2) the default runtime associated with <=10.8 deployment target is 'libstdc++'.
7: (3) Qt support for 'macx-clang-libc++' is listed as 'unsupported' mkspecs.
7: (4) Qt binaries may be build against 'libstdc++' or 'libc++'.
7: (5) Mixing the two different runtime in binaries is unstable.
7: [1]http://stackoverflow.com/questions/19637164/c-linking-error-after-upgrading-to-mac-os-x-10-9-xcode-5-0-1/19637199#19637199
7:
7: --------------------------------
7:
7: Run '$otool -L $(which qmake) |grep lib.\*c++' to check what library Qt is
7: built against:
7:
7: (1) if it is libstdc++ then add '-DCMAKE_OSX_DEPLOYMENT_TARGET:STRING=10.8' (or older) to the cmake command line.
7: (2) if it is libc++ then add '-DCMAKE_OSX_DEPLOYMENT_TARGET:STRING=10.9' (or newer) to the cmake command line.
7:
7: Call Stack (most recent call first):
7: CMakeLists.txt:14 (include)
7:
```
Reported-by: Isaiah Norton <isaiah.norton@gmail.com>
Tested-by: Isaiah Norton <isaiah.norton@gmail.com>
---
.../CMake/Testing/SlicerExtensionBuildSystemTest.py | 11 ++++++++++-
.../SlicerExtensionBuildSystemTestConfig.py.in | 4 ++++
2 files changed, 14 insertions(+), 1 deletion(-)
diff --git a/Extensions/CMake/Testing/SlicerExtensionBuildSystemTest.py b/Extensions/CMake/Testing/SlicerExtensionBuildSystemTest.py
index 97271a8f74..564aa0b3e4 100644
--- a/Extensions/CMake/Testing/SlicerExtensionBuildSystemTest.py
+++ b/Extensions/CMake/Testing/SlicerExtensionBuildSystemTest.py
@@ -376,6 +376,9 @@ Slicer_EXTENSION_DESCRIPTION_DIR:PATH={extension_description_dir}
Slicer_LOCAL_EXTENSIONS_DIR:PATH={local_extensions_dir}
CMAKE_C_COMPILER:PATH={cmake_c_compiler}
CMAKE_CXX_COMPILER:PATH={cmake_cxx_compiler}
+CMAKE_OSX_DEPLOYMENT_TARGET={cmake_osx_deployment_target}
+CMAKE_OSX_ARCHITECTURES= {cmake_osx_architectures}
+CMAKE_OSX_SYSROOT={cmake_osx_sysroot}
")
set(CTEST_GIT_COMMAND "{git_executable}")
@@ -411,6 +414,9 @@ include({slicer_source_dir}/Extensions/CMake/SlicerExtensionsDashboardDriverScri
cmake_generator=config.CMAKE_GENERATOR,
cmake_generator_platform=config.CMAKE_GENERATOR_PLATFORM,
cmake_generator_toolset=config.CMAKE_GENERATOR_TOOLSET,
+ cmake_osx_deployment_target=config.CMAKE_OSX_DEPLOYMENT_TARGET,
+ cmake_osx_architectures=config.CMAKE_OSX_ARCHITECTURES,
+ cmake_osx_sysroot=config.CMAKE_OSX_SYSROOT,
compiler_name=compiler_name,
ctest_drop_site=self.ctest_drop_site,
ctest_build_configuration=build_config,
@@ -461,7 +467,10 @@ include({slicer_source_dir}/Extensions/CMake/SlicerExtensionsDashboardDriverScri
'-DCMAKE_C_COMPILER:PATH=' + config.CMAKE_C_COMPILER,
'-DCMAKE_CXX_COMPILER:PATH=' + config.CMAKE_CXX_COMPILER,
'-DGIT_EXECUTABLE:PATH=' + config.GIT_EXECUTABLE,
- '-DSubversion_SVN_EXECUTABLE:PATH=' + config.Subversion_SVN_EXECUTABLE
+ '-DSubversion_SVN_EXECUTABLE:PATH=' + config.Subversion_SVN_EXECUTABLE,
+ '-DCMAKE_OSX_DEPLOYMENT_TARGET:STRING=' + config.CMAKE_OSX_DEPLOYMENT_TARGET,
+ '-DCMAKE_OSX_ARCHITECTURES:STRING=' + config.CMAKE_OSX_ARCHITECTURES,
+ '-DCMAKE_OSX_SYSROOT:PATH=' + config.CMAKE_OSX_SYSROOT
]
if not config.CMAKE_CONFIGURATION_TYPES:
diff --git a/Extensions/CMake/Testing/SlicerExtensionBuildSystemTestConfig.py.in b/Extensions/CMake/Testing/SlicerExtensionBuildSystemTestConfig.py.in
index 11aa31c3c4..a71373723c 100644
--- a/Extensions/CMake/Testing/SlicerExtensionBuildSystemTestConfig.py.in
+++ b/Extensions/CMake/Testing/SlicerExtensionBuildSystemTestConfig.py.in
@@ -13,6 +13,10 @@ CMAKE_GENERATOR = '@CMAKE_GENERATOR@'
CMAKE_GENERATOR_PLATFORM = '@CMAKE_GENERATOR_PLATFORM@'
CMAKE_GENERATOR_TOOLSET = '@CMAKE_GENERATOR_TOOLSET@'
+CMAKE_OSX_DEPLOYMENT_TARGET = '@CMAKE_OSX_DEPLOYMENT_TARGET@'
+CMAKE_OSX_ARCHITECTURES = '@CMAKE_OSX_ARCHITECTURES@'
+CMAKE_OSX_SYSROOT = '@CMAKE_OSX_SYSROOT@'
+
MIDAS_PACKAGE_API_KEY = 'ThisIsATestApiKeyOfExpectedLengthxxxxxxx'
MIDAS_PACKAGE_EMAIL = 'test@slicer.org'
--
GitLab