MantisBT - CMake
View Issue Details
0016022CMakeModulespublic2016-03-16 17:272016-06-10 14:21
Andreas Schuh 
Brad King 
normalminoralways
closedfixed 
AnyAnyAny
CMake 3.4.1 
CMake 3.6CMake 3.6 
0016022: GenerateExportHeader DEFINE_NO_DEPRECATED define conflicts
The header file generated by generate_export_header adds a

    #define DEFINE_NO_DEPRECATED 0|1

macro which is used to decide whether or not to define the respective macro with the desired library prefix and base name. But this macro has the same name for all libraries and is not undefined when it is no longer needed. In my project, this for example created a conflict with the VTK library which uses such generated header file which must of course be included in the public header files.

To solve this conflict, I am using now (temporarily) a custom exportheader.cmake.in template file by changing the _GENERATE_EXPORT_HEADER_MODULE_DIR path to a directory in my project after including the GenerateExportHeader module. Find the modified template file attached.

There is certainly a better fix for this bug.
No tags attached.
? exportheader.cmake.in (1,098) 2016-03-16 17:27
https://public.kitware.com/Bug/file/5645/exportheader.cmake.in
patch cmake-master-811e297-fix-GenerateExportHeader-DEFINE_NO_DEPRECATED.patch (2,251) 2016-03-17 07:15
https://public.kitware.com/Bug/file/5646/cmake-master-811e297-fix-GenerateExportHeader-DEFINE_NO_DEPRECATED.patch
patch cmake-master-811e297-fix-GenerateExportHeader-DEFINE_NO_DEPRECATED-2.patch (14,163) 2016-03-17 10:18
https://public.kitware.com/Bug/file/5647/cmake-master-811e297-fix-GenerateExportHeader-DEFINE_NO_DEPRECATED-2.patch
Issue History
2016-03-16 17:27Andreas SchuhNew Issue
2016-03-16 17:28Andreas SchuhFile Added: exportheader.cmake.in
2016-03-17 07:15Andreas SchuhFile Added: cmake-master-811e297-fix-GenerateExportHeader-DEFINE_NO_DEPRECATED.patch
2016-03-17 07:16Andreas SchuhNote Added: 0040708
2016-03-17 08:58Brad KingNote Added: 0040710
2016-03-17 09:30Andreas SchuhNote Added: 0040711
2016-03-17 09:32Andreas SchuhNote Edited: 0040711bug_revision_view_page.php?bugnote_id=40711#r2064
2016-03-17 09:51Brad KingNote Added: 0040713
2016-03-17 10:17Andreas SchuhNote Added: 0040714
2016-03-17 10:18Andreas SchuhFile Added: cmake-master-811e297-fix-GenerateExportHeader-DEFINE_NO_DEPRECATED-2.patch
2016-03-17 10:42Brad KingNote Added: 0040715
2016-03-17 10:42Brad KingAssigned To => Brad King
2016-03-17 10:42Brad KingStatusnew => assigned
2016-03-17 10:42Brad KingResolutionopen => fixed
2016-03-17 10:42Brad KingFixed in Version => CMake 3.6
2016-03-17 10:42Brad KingTarget Version => CMake 3.6
2016-03-17 10:42Brad KingStatusassigned => resolved
2016-03-17 10:44Andreas SchuhNote Added: 0040716
2016-06-10 14:21Kitware RobotNote Added: 0041222
2016-06-10 14:21Kitware RobotStatusresolved => closed

Notes
(0040708)
Andreas Schuh   
2016-03-17 07:16   
I attached a patch generated with "git format-patch" against the CMake master branch at commit 811e2974545579d166396ca1ed4f91b0ce7c7990.

This patch avoids the definition of DEFINE_NO_DEPRECATED within the export header altogether. Instead, the export header will have the lines

#if 1 /* DEFINE_NO_DEPRECATED */
# ifndef MyLibrary_NO_DEPRECATED
# define MyLibrary_NO_DEPRECATED
# endif
#endif

The name of the MyLibrary_NO_DEPRECATED macro can be chosen freely using the NO_DEPRECATED_MACRO_NAME option of the generate_export_header function. It can thus also be set to a common macro name for a set of libraries belonging to one project and the defines in the export headers, one for each library target, would not cause an "already defined" preprocessor error with this patch.

I kept the possibility to either use the DEFINE_NO_DEPRECATED option flag of generate_export_header to enable the define of the respective macro in the export header file, or set a CMake variable named DEFINE_NO_DEPRECATED before the call of the generate_export_header function.
(0040710)
Brad King   
2016-03-17 08:58   
Thanks. The change looks good but it is not complete. The Module.GenerateExportHeader test in CMake's test suite fails after the change because it has some reference headers that need to be updated for comparison. Please update the patch to include fixes for the test.
(0040711)
Andreas Schuh   
2016-03-17 09:30   
(edited on: 2016-03-17 09:32)
Ok, I will have a look at the tests.

Before I resubmit a patch, maybe one question to clarify. The documentation of the GenerateExportHeader module specifies that the DEFINE_NO_DEPRECATED option flag of the generate_export_header would decide whether or not the macro is defined with an example on how to set it given a user option in the GUI. But the current module actually does not set DEFINE_NO_DEPRECATED explicitly to FALSE when the option flag was not given. I am wondering if that is deliberate? As noted before, with the previous patch I still consider also a DEFINE_NO_DEPRECATED CMake variable set beforehand.

My question, should the module be rather changed to

  if(_GHE_DEFINE_NO_DEPRECATED)
    set(DEFINE_NO_DEPRECATED 1)
  else()
    set(DEFINE_NO_DEPRECATED 0)
  endif()

or should the module documentation also mention the alternatively considered CMake variable
or should there be a DEFINE_NO_DEPRECATED target property similar to DEFINE_SYMBOL?

This could of course be another issue to be dealt with separately...

(0040713)
Brad King   
2016-03-17 09:51   
DEFINE_NO_DEPRECATED is an argument to the function, not a variable to be checked or set by the project code. The function sets the variable as an implementation detail for configuring the header file. It should not escape the function's scope on the CMake language side. Therefore one should use just

  if(_GHE_DEFINE_NO_DEPRECATED)
    set(DEFINE_NO_DEPRECATED 1)
  else()
    set(DEFINE_NO_DEPRECATED 0)
  endif()
(0040714)
Andreas Schuh   
2016-03-17 10:17   
Ok, I also fixed this. I thought already it was not intended before. With the new patch, the Module.GenerateExportHeader test passes.
(0040715)
Brad King   
2016-03-17 10:42   
Thanks. I squashed some of the changes and applied the series:

 GenerateExportHeader: Do not define DEFINE_NO_DEPRECATED
 https://cmake.org/gitweb?p=cmake.git;a=commitdiff;h=be5a8973 [^]

 GenerateExportHeader: Allow common NO_DEPRECATED_MACRO_NAME for multiple libs
 https://cmake.org/gitweb?p=cmake.git;a=commitdiff;h=6a6e5d89 [^]
(0040716)
Andreas Schuh   
2016-03-17 10:44   
Thanks! Glad there was a quick resolution for this.
(0041222)
Kitware Robot   
2016-06-10 14:21   
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.