(0040708)
Andreas Schuh (reporter)
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 (manager)
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 (reporter)
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 (manager)
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 (reporter)
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 (manager)
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 (reporter)
2016-03-17 10:44

Thanks! Glad there was a quick resolution for this.

(0041222)
Kitware Robot (administrator)
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.

Notes
(0040708) Andreas Schuh (reporter) 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 (manager) 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 (reporter) 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 (manager) 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 (reporter) 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 (manager) 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 (reporter) 2016-03-17 10:44	Thanks! Glad there was a quick resolution for this.

(0041222) Kitware Robot (administrator) 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.

Issue History
Date Modified	Username	Field	Change
2016-03-16 17:27	Andreas Schuh	New Issue
2016-03-16 17:28	Andreas Schuh	File Added: exportheader.cmake.in
2016-03-17 07:15	Andreas Schuh	File Added: cmake-master-811e297-fix-GenerateExportHeader-DEFINE_NO_DEPRECATED.patch
2016-03-17 07:16	Andreas Schuh	Note Added: 0040708
2016-03-17 08:58	Brad King	Note Added: 0040710
2016-03-17 09:30	Andreas Schuh	Note Added: 0040711
2016-03-17 09:32	Andreas Schuh	Note Edited: 0040711
2016-03-17 09:51	Brad King	Note Added: 0040713
2016-03-17 10:17	Andreas Schuh	Note Added: 0040714
2016-03-17 10:18	Andreas Schuh	File Added: cmake-master-811e297-fix-GenerateExportHeader-DEFINE_NO_DEPRECATED-2.patch
2016-03-17 10:42	Brad King	Note Added: 0040715
2016-03-17 10:42	Brad King	Assigned To	=> Brad King
2016-03-17 10:42	Brad King	Status	new => assigned
2016-03-17 10:42	Brad King	Resolution	open => fixed
2016-03-17 10:42	Brad King	Fixed in Version	=> CMake 3.6
2016-03-17 10:42	Brad King	Target Version	=> CMake 3.6
2016-03-17 10:42	Brad King	Status	assigned => resolved
2016-03-17 10:44	Andreas Schuh	Note Added: 0040716
2016-06-10 14:21	Kitware Robot	Note Added: 0041222
2016-06-10 14:21	Kitware Robot	Status	resolved => closed

Relationships