(0012519)
Bill Hoffman (manager)
2008-06-25 09:42

The problem seems to be the new handling of the -D stuff. In cmMakefile.cxx, we have this:

  // If this is really a definition, update COMPILE_DEFINITIONS.
  if(this->ParseDefineFlag(flag, false))
    {
    return;
    }

Later when we get the definition we have this:
  else if (!strcmp("DEFINITIONS",prop))
    {
// But this no longer has any -D stuff
    output = this->GetDefineFlags();
    return output.c_str();
    }

I have added this code:

  if(const char* cdefs = this->GetProperty("COMPILE_DEFINITIONS"))
      {
      // Expand the list.
      std::vector<std::string> defs;
      cmSystemTools::ExpandListArgument(cdefs, defs);
      for(std::vector<std::string>::iterator i = defs.begin();
          i != defs.end(); ++i)
        {
        output += "-D";
        output += *i;
        output += " ";
        }
      }

Not sure if this should be a policy or not, the only downside of the above code is that the original code, may not have used -D, it may have used /D, but -D should work for both, so I can't see how it would hurt... The other option would be to just store the string in some other place for this property.

(0012520)
Bill Hoffman (manager)
2008-06-25 09:45

$ cvs commit -m "BUG: fix for bug 7239, DEFINITIONS property not backwards compatible to 2.4" cmMakefile.cxx
Committer: Bill Hoffman
/cvsroot/CMake/CMake/Source/cmMakefile.cxx,v <-- cmMakefile.cxx
new revision: 1.474; previous revision: 1.473

(0012533)
Brad King (manager)
2008-06-26 10:59

I didn't even know about the DEFINITIONS property when I implemented COMPILE_DEFINITIONS. The only documentation for it is one entry in a list in the get_directory_property command documentation.

I've updated the command documentation and converted its list of properties to be in the property documentation.

/cvsroot/CMake/CMake/Source/cmGetDirectoryPropertyCommand.h,v <-- Source/cmGetDirectoryPropertyCommand.h
new revision: 1.9; previous revision: 1.8
/cvsroot/CMake/CMake/Source/cmMakefile.cxx,v <-- Source/cmMakefile.cxx
new revision: 1.476; previous revision: 1.475

(0012542)
Brad King (manager)
2008-06-26 13:23

Bill's previous fix may not preserve the old value in all cases (escapes, etc.). I'll commit a new fix shortly.

(0012543)
Brad King (manager)
2008-06-26 13:32

Okay, the following changes maintain a value the same way as CMake 2.4 did for use in returning the DEFINITIONS property.

/cvsroot/CMake/CMake/Source/cmMakefile.cxx,v <-- Source/cmMakefile.cxx
new revision: 1.477; previous revision: 1.476
/cvsroot/CMake/CMake/Source/cmMakefile.h,v <-- Source/cmMakefile.h
new revision: 1.233; previous revision: 1.232

The result is that old projects will work. Any values added to the COMPILE_DEFINITIONS property will not be reflected, but that means the code is new enough to know about it. I've documented the DEFINITIONS property as old/compatibility/debugging and explicitly stated that it only accounts for add_definitions.

(0012625)
Woebbeking (reporter)
2008-07-05 08:17

First, thanks for your fast replies/fixes!

Do I understand you correctly: we now have two properties for the same thing?

(0012631)
Brad King (manager)
2008-07-05 17:12

No.

COMPILE_DEFINITIONS is the (new) formal preprocessor definitions property. It does not have the -D part, and will properly escape definition values.

DEFINITIONS is the old property that tracks the results of ADD_DEFINITIONS/REMOVE_DEFINITIONS which are also abused for non-preprocessing flags.

If you can require 2.6 for your project and want to deal with preprocessor flags, use the COMPILE_DEFINITIONS property with set_property/get_property commands (especially the APPEND option to set_property).

(0012633)
Woebbeking (reporter)
2008-07-07 01:30

I see.

Issue History
Date Modified	Username	Field	Change
2008-06-25 05:33	Woebbeking	New Issue
2008-06-25 09:36	Bill Hoffman	Status	new => assigned
2008-06-25 09:36	Bill Hoffman	Assigned To	=> Brad King
2008-06-25 09:42	Bill Hoffman	Note Added: 0012519
2008-06-25 09:45	Bill Hoffman	Note Added: 0012520
2008-06-26 10:59	Brad King	Note Added: 0012533
2008-06-26 11:01	Brad King	Status	assigned => closed
2008-06-26 11:01	Brad King	Resolution	open => fixed
2008-06-26 13:23	Brad King	Note Added: 0012542
2008-06-26 13:23	Brad King	Status	closed => assigned
2008-06-26 13:32	Brad King	Note Added: 0012543
2008-07-05 08:17	Woebbeking	Note Added: 0012625
2008-07-05 17:12	Brad King	Note Added: 0012631
2008-07-07 01:30	Woebbeking	Note Added: 0012633
2008-07-07 09:07	Brad King	Status	assigned => closed

Notes
(0012519) Bill Hoffman (manager) 2008-06-25 09:42	The problem seems to be the new handling of the -D stuff. In cmMakefile.cxx, we have this: // If this is really a definition, update COMPILE_DEFINITIONS. if(this->ParseDefineFlag(flag, false)) { return; } Later when we get the definition we have this: else if (!strcmp("DEFINITIONS",prop)) { // But this no longer has any -D stuff output = this->GetDefineFlags(); return output.c_str(); } I have added this code: if(const char* cdefs = this->GetProperty("COMPILE_DEFINITIONS")) { // Expand the list. std::vector<std::string> defs; cmSystemTools::ExpandListArgument(cdefs, defs); for(std::vector<std::string>::iterator i = defs.begin(); i != defs.end(); ++i) { output += "-D"; output += *i; output += " "; } } Not sure if this should be a policy or not, the only downside of the above code is that the original code, may not have used -D, it may have used /D, but -D should work for both, so I can't see how it would hurt... The other option would be to just store the string in some other place for this property.

(0012520) Bill Hoffman (manager) 2008-06-25 09:45	$ cvs commit -m "BUG: fix for bug 7239, DEFINITIONS property not backwards compatible to 2.4" cmMakefile.cxx Committer: Bill Hoffman /cvsroot/CMake/CMake/Source/cmMakefile.cxx,v <-- cmMakefile.cxx new revision: 1.474; previous revision: 1.473

(0012533) Brad King (manager) 2008-06-26 10:59	I didn't even know about the DEFINITIONS property when I implemented COMPILE_DEFINITIONS. The only documentation for it is one entry in a list in the get_directory_property command documentation. I've updated the command documentation and converted its list of properties to be in the property documentation. /cvsroot/CMake/CMake/Source/cmGetDirectoryPropertyCommand.h,v <-- Source/cmGetDirectoryPropertyCommand.h new revision: 1.9; previous revision: 1.8 /cvsroot/CMake/CMake/Source/cmMakefile.cxx,v <-- Source/cmMakefile.cxx new revision: 1.476; previous revision: 1.475

(0012542) Brad King (manager) 2008-06-26 13:23	Bill's previous fix may not preserve the old value in all cases (escapes, etc.). I'll commit a new fix shortly.

(0012543) Brad King (manager) 2008-06-26 13:32	Okay, the following changes maintain a value the same way as CMake 2.4 did for use in returning the DEFINITIONS property. /cvsroot/CMake/CMake/Source/cmMakefile.cxx,v <-- Source/cmMakefile.cxx new revision: 1.477; previous revision: 1.476 /cvsroot/CMake/CMake/Source/cmMakefile.h,v <-- Source/cmMakefile.h new revision: 1.233; previous revision: 1.232 The result is that old projects will work. Any values added to the COMPILE_DEFINITIONS property will not be reflected, but that means the code is new enough to know about it. I've documented the DEFINITIONS property as old/compatibility/debugging and explicitly stated that it only accounts for add_definitions.

(0012625) Woebbeking (reporter) 2008-07-05 08:17	First, thanks for your fast replies/fixes! Do I understand you correctly: we now have two properties for the same thing?

(0012631) Brad King (manager) 2008-07-05 17:12	No. COMPILE_DEFINITIONS is the (new) formal preprocessor definitions property. It does not have the -D part, and will properly escape definition values. DEFINITIONS is the old property that tracks the results of ADD_DEFINITIONS/REMOVE_DEFINITIONS which are also abused for non-preprocessing flags. If you can require 2.6 for your project and want to deal with preprocessor flags, use the COMPILE_DEFINITIONS property with set_property/get_property commands (especially the APPEND option to set_property).

(0012633) Woebbeking (reporter) 2008-07-07 01:30	I see.

Relationships