(0031478)
Eric NOULARD (developer)
2012-11-07 13:13

Thank you Andreas.

I'll try to handle the CPack related patch over the week end.

May be I can handle the other as well, I let you and CMake dev know.

(0031479)
Brad King (manager)
2012-11-07 14:37

I've handled these two:

0004-Correct-typos-grammar-in-CMake-docs.patch
0005-Add-some-new-improved-documentation-parts.patch

They are now:

http://cmake.org/gitweb?p=cmake.git;a=commitdiff;h=965de974 [^]
http://cmake.org/gitweb?p=cmake.git;a=commitdiff;h=07d5e4b8 [^]

I dropped this hunk from 0005:

+ "VS2005-specific hint: current include path config can be determined "
+ "via an #include \"nonexistent.h\" "
+ "and then right-click, selecting 'Open Header' item.";

because IMO that does not belong in reference documentation of a
cross-platform feature of a cross-platform tool.

I dropped the configure_file change from 0005 because this the hunk

+ "Note that to achieve an empty-definition i.e. '#define VAR', "
+ "the CMake-side variable must be set to single-space (\" \") "
+ "rather than empty. "

is misleading. Instead I added a new commit to clarify things:

http://cmake.org/gitweb?p=cmake.git;a=commitdiff;h=fa046735 [^]

Thanks!

(0031480)
Brad King (manager)
2012-11-07 14:38

Assigning to Eric for the CPack changes.

(0031481)
Andreas Mohr (reporter)
2012-11-07 14:47

> because IMO that does not belong in reference documentation of a
> cross-platform feature of a cross-platform tool.

"Why am I not surprised?" ;)
Yeah, that was at least bordering on context-unsuitable, if a quite interesting nice hint.

> I dropped the configure_file change from 0005 because this the hunk

[...]

> is misleading.

"misleading", why? No idea, *shrug*

Other than that, your new description is better since it describes the mechanism more transparently, rather than creating additional blurb about the overly-special " " case.

Thank you for your prompt response!

(0031482)
Brad King (manager)
2012-11-07 14:53

> "misleading", why? No idea, *shrug*

If I were looking to get "#define VAR" your text tells me I "must" do

set(VAR " ")

which is not correct. Many other values will work. Perhaps the term "misleading" was too strong but please don't read anything into it.

(0031521)
Eric NOULARD (developer)
2012-11-10 10:00

This one
0002-Correct-string-literal-typo-have-NULL-like-all-other.patch
has been merged to next:
Merge topic 'CPack-fixTypoInErrorMsg' into next
572d9e1 Correct string literal typo (have "(NULL)" like all other cases).

working on the 2 remaining ones afterwards.

(0031525)
Eric NOULARD (developer)
2012-11-11 16:04

0003-Remove-seemingly-bogus-duplicate-CPACK_PACKAGE_FILE_.patch
is ok and merged às well:
Merge topic 'CPack-BugFixesSet' into next

b6f7881 Remove seemingly bogus duplicate CPACK_PACKAGE_FILE_NAME call.

concerning the remaining one:
0001-CPACK_SET_DESTDIR-bug-fix-Wrong-variable-used-for-te.patch

I have some doubt and it need more thinking and testing.
Because 'bareTempInstallDirectory' is used at lines 255-256 as well:
if ( !this->InstallProjectViaInstallCMakeProjects(
setDestDir, bareTempInstallDirectory.c_str()) )

and it may be that both usage are linked...
I will process that one later.

(0042143)
Kitware Robot (administrator)
2016-06-10 14:28

Resolving issue as `moved`.

This issue tracker is no longer used. Further discussion of this issue may take place in the current CMake Issues page linked in the banner at the top of this page.

Issue History
Date Modified	Username	Field	Change
2012-11-07 13:07	Andreas Mohr	New Issue
2012-11-07 13:07	Andreas Mohr	File Added: git_cmake_master_format_patch_set_20121107.tar.gz
2012-11-07 13:13	Eric NOULARD	Note Added: 0031478
2012-11-07 14:37	Brad King	Note Added: 0031479
2012-11-07 14:38	Brad King	Note Added: 0031480
2012-11-07 14:38	Brad King	Assigned To	=> Eric NOULARD
2012-11-07 14:38	Brad King	Status	new => assigned
2012-11-07 14:47	Andreas Mohr	Note Added: 0031481
2012-11-07 14:53	Brad King	Note Added: 0031482
2012-11-10 10:00	Eric NOULARD	Note Added: 0031521
2012-11-11 16:04	Eric NOULARD	Note Added: 0031525
2016-06-10 14:28	Kitware Robot	Note Added: 0042143
2016-06-10 14:28	Kitware Robot	Status	assigned => resolved
2016-06-10 14:28	Kitware Robot	Resolution	open => moved
2016-06-10 14:31	Kitware Robot	Status	resolved => closed

Notes
(0031478) Eric NOULARD (developer) 2012-11-07 13:13	Thank you Andreas. I'll try to handle the CPack related patch over the week end. May be I can handle the other as well, I let you and CMake dev know.

(0031479) Brad King (manager) 2012-11-07 14:37	I've handled these two: 0004-Correct-typos-grammar-in-CMake-docs.patch 0005-Add-some-new-improved-documentation-parts.patch They are now: http://cmake.org/gitweb?p=cmake.git;a=commitdiff;h=965de974 [^] http://cmake.org/gitweb?p=cmake.git;a=commitdiff;h=07d5e4b8 [^] I dropped this hunk from 0005: + "VS2005-specific hint: current include path config can be determined " + "via an #include \"nonexistent.h\" " + "and then right-click, selecting 'Open Header' item."; because IMO that does not belong in reference documentation of a cross-platform feature of a cross-platform tool. I dropped the configure_file change from 0005 because this the hunk + "Note that to achieve an empty-definition i.e. '#define VAR', " + "the CMake-side variable must be set to single-space (\" \") " + "rather than empty. " is misleading. Instead I added a new commit to clarify things: http://cmake.org/gitweb?p=cmake.git;a=commitdiff;h=fa046735 [^] Thanks!

(0031480) Brad King (manager) 2012-11-07 14:38	Assigning to Eric for the CPack changes.

(0031481) Andreas Mohr (reporter) 2012-11-07 14:47	> because IMO that does not belong in reference documentation of a > cross-platform feature of a cross-platform tool. "Why am I not surprised?" ;) Yeah, that was at least bordering on context-unsuitable, if a quite interesting nice hint. > I dropped the configure_file change from 0005 because this the hunk [...] > is misleading. "misleading", why? No idea, shrug Other than that, your new description is better since it describes the mechanism more transparently, rather than creating additional blurb about the overly-special " " case. Thank you for your prompt response!

(0031482) Brad King (manager) 2012-11-07 14:53	> "misleading", why? No idea, shrug If I were looking to get "#define VAR" your text tells me I "must" do set(VAR " ") which is not correct. Many other values will work. Perhaps the term "misleading" was too strong but please don't read anything into it.

(0031521) Eric NOULARD (developer) 2012-11-10 10:00	This one 0002-Correct-string-literal-typo-have-NULL-like-all-other.patch has been merged to next: Merge topic 'CPack-fixTypoInErrorMsg' into next 572d9e1 Correct string literal typo (have "(NULL)" like all other cases). working on the 2 remaining ones afterwards.

(0031525) Eric NOULARD (developer) 2012-11-11 16:04	0003-Remove-seemingly-bogus-duplicate-CPACK_PACKAGE_FILE_.patch is ok and merged às well: Merge topic 'CPack-BugFixesSet' into next b6f7881 Remove seemingly bogus duplicate CPACK_PACKAGE_FILE_NAME call. concerning the remaining one: 0001-CPACK_SET_DESTDIR-bug-fix-Wrong-variable-used-for-te.patch I have some doubt and it need more thinking and testing. Because 'bareTempInstallDirectory' is used at lines 255-256 as well: if ( !this->InstallProjectViaInstallCMakeProjects( setDestDir, bareTempInstallDirectory.c_str()) ) and it may be that both usage are linked... I will process that one later.

(0042143) Kitware Robot (administrator) 2016-06-10 14:28	Resolving issue as `moved`. This issue tracker is no longer used. Further discussion of this issue may take place in the current CMake Issues page linked in the banner at the top of this page.

Relationships