View Issue Details Jump to Notes ] Print ]
IDProjectCategoryView StatusDate SubmittedLast Update
0013120CMakeCMakepublic2012-04-11 06:252013-01-09 10:55
ReporterLeonid Yurchenko 
Assigned ToBrad King 
PrioritynormalSeverityminorReproducibilityalways
StatusclosedResolutionfixed 
PlatformPCOSWindowsOS VersionXP / 7 / 8
Product Version 
Target VersionCMake 2.8.9Fixed in VersionCMake 2.8.9 
Summary0013120: include_external_msproject features are limited
DescriptionFeatures of include_external_msproject are limited. There is no possibility to provide project type, guid and platform.

Provided patch adds this with adding keywords TYPE, GUID, PLATFORM.
This patch is used successfully in our company for a while, so we want to share it with the community.
TagsNo tags attached.
Attached Filespatch file icon custom_guid_in_external_prj.patch [^] (13,730 bytes) 2012-04-11 06:25 [Show Content]
patch file icon include_external_msproject.patch [^] (14,530 bytes) 2012-04-13 04:57 [Show Content]
patch file icon 0001-include_external_msproject-Add-TYPE-GUID-PLATFORM-op.patch [^] (15,414 bytes) 2012-04-16 10:19 [Show Content]
zip file icon include_external_msproject-test.zip [^] (3,259 bytes) 2012-04-18 09:55
zip file icon include_external_msproject-test_v2.zip [^] (3,324 bytes) 2012-04-19 07:52

 Relationships

  Notes
(0029125)
Brad King (manager)
2012-04-11 08:09

Thanks for the patch. Here are a few comments. Please attach a new patch addressing them.

- What is the purpose of the commented-out custom command at the bottom of the patch?

- Please add to the command's documentation to explain the new interface. Include limitations like behavior on VS 6 and VS >= 10. It is not clear from the patch whether it addresses those versions.

- To parse keyword arguments we use an enumeration value to track the current state. See http://cmake.org/gitweb?p=cmake.git;a=blob;f=Source/cmInstallCommand.cxx;hb=v2.8.7#l846 [^] for example.

- I prefer the property prefix to be "VS_" instead of "MSVS_" because we already have several other properties with the former prefix.
(0029146)
Leonid Yurchenko (reporter)
2012-04-13 04:58

Thanks for the feedback.

Added new patch that addresses this requests: include_external_msproject.patch

Tested compatibility with VS 10 and VS 11, and appeared that support of VS 10+ was broken. This new patch includes fix for this.
(0029167)
Leonid Yurchenko (reporter)
2012-04-16 07:04

Anything else we can improve in the patch to help it be included in the future releases?
(0029176)
Brad King (manager)
2012-04-16 10:21

Please test patch "0001-include_external_msproject-Add-TYPE-GUID-PLATFORM-op.patch" which I adapted from your patch to adjust some documentation phrasing and fix keyword argument handling.
(0029179)
Leonid Yurchenko (reporter)
2012-04-16 11:08

It works the same way as previous did. So nothing was broken, everything is fine. Thanks for the documentation update.
(0029180)
Brad King (manager)
2012-04-16 11:21

Great, thanks for testing.

In order to ensure this feature works and keeps working please extend the test case in Tests/VSExternalInclude to cover the optional arguments.
(0029200)
Leonid Yurchenko (reporter)
2012-04-17 06:45

Thanks. Is it possible for you to elaborate a bit, how you expect test case to check there optional arguments?

We are new to cmake unit tests, so it's unclear how to test if custom guid was set correctly (It may require VS plugin to be installed - it's the main purpose of this patch, to allow work with VS plugins, like wix projects).

Easiest solution that comes to mind is to check the solution manually with file(STRINGS REGEX) to whether guid was set correctly. Does it sounds ok? Or you had something different in mind?
(0029201)
Brad King (manager)
2012-04-17 08:07

Re 0013120:0029200: We do need the VSExternalInclude test project to load and build to continue to cover the current cases it checks, so if a custom GUID can't load without a VS plugin then we'll need to put the test somewhere else.

The file(STRINGS) approach sounds reasonable so it can verify that the generator at least puts something in there. This would be the first test to do such direct testing on the generated solution so I'm not sure off the top of my head where it would best fit in the test suite. Instead please just provide a sample CMakeLists.txt file that covers a few cases using the optional arguments and a .cmake script that uses file(STRINGS) to validate the .sln files. I'll find a place to add them to the automated tests.
(0029211)
Leonid Yurchenko (reporter)
2012-04-18 09:58

Added include_external_msproject-test.zip, that contains:

- check_utils.cmake - helper functions to check vs solutions for certain values
- validate.cmake - validation script. It can be started with the command mode (cmake -P):
cmake -DTARGET_FILE=test.sln -DPROJECT_NAME=external -DPROJECT_GUID=111 -P validate.cmake

- simple_test folder - folder that contains test projects and cmake lists that uses this external projects and generates solution that should be tested.

Looking forward for the feedback, if we can change or improve something there.
(0029213)
Brad King (manager)
2012-04-18 11:30

Thanks.

In validate.cmake shouldn't the test for project type call find_project_with_type instead of find_project_with_guid?

Also the TYPE/PLATFORM test without GUID cannot be checked with the tools provided. We need to find the project with the given type, extract its GUID, and then check the platform for that GUID later. Please add that capability to check_utils. Otherwise we cannot verify that setting a TYPE/PLATFORM works with an auto-generated GUID.
(0029230)
Leonid Yurchenko (reporter)
2012-04-19 07:53

Thanks for the feedback. Added include_external_msproject-test_v2.zip. Updated files are only: check_utils.cmake and validate.cmake
(0029241)
Brad King (manager)
2012-04-19 09:37

Thanks for the contribution and help with the test! I've added them:

 http://cmake.org/gitweb?p=cmake.git;a=commitdiff;h=59139031 [^]
 http://cmake.org/gitweb?p=cmake.git;a=commitdiff;h=4a30258d [^]
(0032018)
Robert Maynard (manager)
2013-01-09 10:55

Closing resolved issues that have not been updated in more than 4 months.

 Issue History
Date Modified Username Field Change
2012-04-11 06:25 Leonid Yurchenko New Issue
2012-04-11 06:25 Leonid Yurchenko File Added: custom_guid_in_external_prj.patch
2012-04-11 08:09 Brad King Note Added: 0029125
2012-04-13 04:57 Leonid Yurchenko File Added: include_external_msproject.patch
2012-04-13 04:58 Leonid Yurchenko Note Added: 0029146
2012-04-16 07:04 Leonid Yurchenko Note Added: 0029167
2012-04-16 10:19 Brad King File Added: 0001-include_external_msproject-Add-TYPE-GUID-PLATFORM-op.patch
2012-04-16 10:21 Brad King Note Added: 0029176
2012-04-16 11:08 Leonid Yurchenko Note Added: 0029179
2012-04-16 11:21 Brad King Note Added: 0029180
2012-04-17 06:45 Leonid Yurchenko Note Added: 0029200
2012-04-17 08:07 Brad King Note Added: 0029201
2012-04-18 09:55 Leonid Yurchenko File Added: include_external_msproject-test.zip
2012-04-18 09:58 Leonid Yurchenko Note Added: 0029211
2012-04-18 11:30 Brad King Note Added: 0029213
2012-04-19 07:52 Leonid Yurchenko File Added: include_external_msproject-test_v2.zip
2012-04-19 07:53 Leonid Yurchenko Note Added: 0029230
2012-04-19 09:20 Brad King Assigned To => Brad King
2012-04-19 09:20 Brad King Status new => assigned
2012-04-19 09:37 Brad King Note Added: 0029241
2012-04-19 09:37 Brad King Status assigned => resolved
2012-04-19 09:37 Brad King Resolution open => fixed
2012-08-09 16:56 David Cole Fixed in Version => CMake 2.8.9
2012-08-09 16:56 David Cole Target Version => CMake 2.8.9
2013-01-09 10:55 Robert Maynard Note Added: 0032018
2013-01-09 10:55 Robert Maynard Status resolved => closed


Copyright © 2000 - 2018 MantisBT Team