MantisBT - CMake
View Issue Details
IDProjectCategoryView StatusDate SubmittedLast Update
0013659CMakeCMakepublic2012-11-07 13:072016-06-10 14:31
ReporterAndreas Mohr 
Assigned ToEric NOULARD 
PriorityhighSeverityminorReproducibilityN/A
StatusclosedResolutionmoved 
PlatformPCOSLinuxOS VersionDebian
Product VersionCMake 2.8.10 
Target VersionFixed in Version 
Summary0013659: [PATCH] Patchset git master: several documentation improvements/corrections, typos, bug fixes
Descriptiongit format-patchset against current master, containing assorted fixes that piled up in my notes over the years.

I did not do the official git maintainer setup (see http://www.itk.org/Wiki/CMake/Git [^]), but I hope a very informal loose set of patches is ok anyway ;)

Note that the content is UNTESTED - it was created on a working-copy-only machine, no compile done.

Contains patches:
0001-CPACK_SET_DESTDIR-bug-fix-Wrong-variable-used-for-te.patch
0002-Correct-string-literal-typo-have-NULL-like-all-other.patch
0003-Remove-seemingly-bogus-duplicate-CPACK_PACKAGE_FILE_.patch
0004-Correct-typos-grammar-in-CMake-docs.patch
0005-Add-some-new-improved-documentation-parts.patch


Patch 0001 might have wider effects - it's a rather old possible(?) bug in CMake sources, thus it might require special care.

About attribution: I don't care overly much - if commit logs end up largely mangled (or completely custom-recreated on your side), then so be it.
If some of the patches are judged towards getting skipped completely, then... my loss :)

And it was about time to finally get this submitted, since I've had so many benefits from CMake infrastructure!

Priority "high" since it contains several very useful docs additions.

Thank you!
Steps To Reproduce
Additional Information
TagsNo tags attached.
Relationships
Attached Filesgz git_cmake_master_format_patch_set_20121107.tar.gz (5,772) 2012-11-07 13:07
https://public.kitware.com/Bug/file/4557/git_cmake_master_format_patch_set_20121107.tar.gz
Issue History
Date ModifiedUsernameFieldChange
2012-11-07 13:07Andreas MohrNew Issue
2012-11-07 13:07Andreas MohrFile Added: git_cmake_master_format_patch_set_20121107.tar.gz
2012-11-07 13:13Eric NOULARDNote Added: 0031478
2012-11-07 14:37Brad KingNote Added: 0031479
2012-11-07 14:38Brad KingNote Added: 0031480
2012-11-07 14:38Brad KingAssigned To => Eric NOULARD
2012-11-07 14:38Brad KingStatusnew => assigned
2012-11-07 14:47Andreas MohrNote Added: 0031481
2012-11-07 14:53Brad KingNote Added: 0031482
2012-11-10 10:00Eric NOULARDNote Added: 0031521
2012-11-11 16:04Eric NOULARDNote Added: 0031525
2016-06-10 14:28Kitware RobotNote Added: 0042143
2016-06-10 14:28Kitware RobotStatusassigned => resolved
2016-06-10 14:28Kitware RobotResolutionopen => moved
2016-06-10 14:31Kitware RobotStatusresolved => closed

Notes
(0031478)
Eric NOULARD   
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   
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   
2012-11-07 14:38   
Assigning to Eric for the CPack changes.
(0031481)
Andreas Mohr   
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   
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   
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   
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   
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.