View Issue Details [ Jump to Notes ] [ Print ]
IDProjectCategoryView StatusDate SubmittedLast Update
0008779CMakeCMakepublic2009-03-23 11:472009-04-28 17:34
ReporterTwylite 
Assigned ToBrad King 
PrioritynormalSeveritymajorReproducibilityalways
StatusclosedResolutionfixed 
PlatformOSOS Version
Product VersionCMake-2-6 
Target VersionFixed in Version 
Summary0008779: Defines not correctly quoted in MSVC6 DSP
DescriptionWhen add_definitions() is used to add compile-time defines to a project, and the defined value includes double-quotes, the value is not correctly quoted in the generated DSP for the "Visual Studio 6" generator.

The value IS correctly quoted in an NMake Makefile, so it is not possible to generate both NMake makefiles and VS6 DSPs from the same CMakeLists.txt

Additional InformationExample:
Take the simple CMakeLists.txt file below
---
cmake_minimum_required (VERSION 2.6)
project (bug C)
add_definitions( -DSTR="str" )
add_executable( bug bug.c )
---

And bug.c is
---
#include <stdio.h>

void main()
{
  printf( STR );
}
---

In the NMake case:
cmake -G "NMake Makefiles" -DCMAKE_VERBOSE_MAKEFILE=true .
nmake
The output indicates that the cl command line includes '-DSTR=\"str\"', which is correct.

In the Visual Studio 6 case:
cmake -G "Visual Studio 6" .
The DSP contains several instances of '# ADD CPP -DSTR="str"', which is incorrect -- this substitutes 'str' rather than '"str"' into the .c file.

To get the desired result in MSVC6 IDE, change the CMakeLists.txt
---
add_definitions( -DSTR=\\"str\\" )
---

The DSP will now build correctly, but NMake won't work.


Using the preprocessor's stringify / token-pasting operator in the C file is not an acceptable work-around in this case -- I am creating a CMake build for an Open Source app that is autotools based in order to improve build support on Windows, but there's not going to be any support for source modifications to support a different build tool.




TagsNo tags attached.
Attached Files

 Relationships

  Notes
(0015762)
Twylite (reporter)
2009-03-23 11:57

 Severity is quite possibly wrong - I had intended to use "minor".

May be related to issue 0005099. Not checked with MSVC7+.
(0015766)
Brad King (manager)
2009-03-23 14:53

 As of CMake 2.6.3 all the generators support values with proper escaping except VS 6. This was implemented as part of creating the COMPILE_DEFINITIONS directory/target/source-file properties. See policy CMP0005 documentation:

  http://www.cmake.org/cmake/help/cmake2.6docs.html#policy:CMP0005 [^]

When I worked on that feature I mistakenly concluded after some testing that VS6 does not support preprocessor values very well. If you try to put a value in with COMPILE_DEFINITIONS and the VS6 generator you will get a warning. For compatibility CMake remains silent in this case for ADD_DEFINITIONS and just passes through the user value.

After further investigation I see that VS6 supports values that do not contain spaces. I've committed changes that allow all macros except those with spaces in their value:

ENH: Support preprocessor def values in VS6
/cvsroot/CMake/CMake/Source/cmLocalVisualStudio6Generator.cxx,v <-- Source/cmLocalVisualStudio6Generator.cxx
new revision: 1.151; previous revision: 1.150
/cvsroot/CMake/CMake/Source/cmMakefile.cxx,v <-- Source/cmMakefile.cxx
new revision: 1.507; previous revision: 1.506
/cvsroot/CMake/CMake/Source/cmSourceFile.cxx,v <-- Source/cmSourceFile.cxx
new revision: 1.56; previous revision: 1.55
/cvsroot/CMake/CMake/Source/cmTarget.cxx,v <-- Source/cmTarget.cxx
new revision: 1.237; previous revision: 1.236
/cvsroot/CMake/CMake/Tests/Preprocess/CMakeLists.txt,v <-- Tests/Preprocess/CMakeLists.txt
new revision: 1.11; previous revision: 1.10
/cvsroot/CMake/CMake/Tests/Preprocess/preprocess.c,v <-- Tests/Preprocess/preprocess.c
new revision: 1.3; previous revision: 1.2
/cvsroot/CMake/CMake/Tests/Preprocess/preprocess.cxx,v <-- Tests/Preprocess/preprocess.cxx
new revision: 1.3; previous revision: 1.2
(0015768)
Brad King (manager)
2009-03-23 15:05

 I've scheduled the fix for inclusion in the 2.6 release branch.
(0015769)
Twylite (reporter)
2009-03-23 17:24

 Wow - seriously impressed with the response time! :)

This has ended up being a rather long response. The summary:

1. MSVC6 DSPs handle defines with values with spaces just fine. The general form is -D"NAME=value" or -DNAME="value", where value has C-style backslash quoting (e.g. \" for a literal double-quote)

2. The use-case is for the substitution of a path via a compile-time defines, and the default path is under C:\Program Files\, so support for spaces is kindof important to me ;)

3. COMPILE_DEFINITIONS PATH="c:\\\\program files\\\\my app\\\\resources"
should become
  # ADD CPP /D PATH="\"c:\\program files\\my app\\resources\""
or
  # ADD CPP -D"PATH=\"c:\\program files\\my app\\resources\""
to behave the same as NMake and MSVC2008. The latter is identical to the substitution make for NMake (or at least how it ends up on the cl command line via the NMake generator), and conceptually the same as the approach used in MSVC2008.


---

I think I underestimated the complexity of this problem, so I'm going to have to get more thorough.

In the Tcl sources (specifically tclPkgConfig.c) there is an array that is initialised from compile-time defines; here's a snippet:

  static Tcl_Config const cfg[] = {
    {"libdir,runtime", CFG_RUNTIME_LIBDIR},
    {NULL, NULL}
  };

The normal Tcl build process uses automake/autoconf, which generates appropriate defines for the Unix environment. On Windows there is a hand-crafted Makefile for NMake that has appropriate defines for WIN32.

The default for CFG_RUNTIME_LIBDIR is "C:\\Program Files\\Tcl\\lib" (that is the literal that needs to be substituted in the C source, and includes C quoting of the backslashes).


So I need to use a compile-time define to set up certain constants, some of which are paths.
In an isolated experiment I could reduce this to e.g.
  printf( "c:\\program files\\my app\\resources" );
needs to be:
  printf( PATH );
so I need to define:
  PATH="c:\\program files\\my app\\resources"
but correctly quoted for the command line, tool or project file.

  
I'm going to update my bug.c to illustrate the problem:
---
#include <stdio.h>

void main()
{
  printf( PATH );
}
---

And I'll make a corresponding update to the CMakeLists.txt:
---
cmake_minimum_required (VERSION 2.6)
project (bug C)
cmake_policy( SET CMP0005 NEW )
add_executable( bug bug.c )
#add_definitions( -DPATH="c:\\program files\\my app\\resources" )
set_property( TARGET bug PROPERTY COMPILE_DEFINITIONS PATH="c:\\program files\\my app\\resources" )
---

Note on CMake quoting:
-
Using the more thoroughly quoted
  COMPILE_DEFINITIONS "PATH=\"c:\\program files\\my app\\resources\""
is no different to the CMakeLists.txt presented above.
-

Note on VS6 define support:
-
In the VS6 IDE I can go to Project Settings, C++, Preprocessor
Definitions and add:
  PATH="\"c:\\program files\\my app\\resources\""
Which appears in the .DSP as:
  # ADD CPP /D PATH="\"c:\\program files\\my app\\resources\""

There general format for a define in a VS6 DSP seems to be
  NAME="value"
where value can contain C-like escape sequences

The quotes can go around the entire define, so the general form
can also be described as
  "NAME=value"
which appears in the DSP as:
  # ADD CPP -D"PATH=\"c:\\program files\\my app\\resources\""
-


--- RESULTS WITH CMAKE 2.6.3 ---

Results for CMake 2.6.3, CMP0005 OLD, NMakefile

  cl ... -DPATH="c:\program files\my app\resources"
  bug.c(5) : error C2065: 'c' : undeclared identifier

Results for CMake 2.6.3, CMP0005 NEW, NMakefile

  cl ... -D"PATH=\"c:\program files\my app\resources\""
  bug.c(5) : warning C4129: 'p' : unrecognized character escape sequence

I get the same results for both the add_definitions() and set_property() approaches.

It is worth noting that the CMP0005 NEW escapes the double-quotes, but doesn't escape the backslash.

The intended outcome is

  cl ... -D"PATH=\"c:\\program files\\my app\\resources\""

To get that outcome with CMake 2.6.3 I will have to add extra escapes for the backslashes in the COMPILE_DEFINITIONS, which seems like the wrong thing to do? (UPDATE: I tried it below, and it seems to be the right thing to do ...)


For the Visual Studio 6 generator only add_definitions() can be used (COMPILE_DEFINITIONS gives "WARNING: The VS6 IDE does not support preprocessor definitions with values." as expected)

Results for CMake 2.6.3, Visual Studio 6

  # ADD CPP -DPATH="c:\program files\my app\resources"

The results do not differ for CMP0005 OLD / NEW.

The intended outcome is

  # ADD CPP /D PATH="\"c:\\program files\\my app\\resources\""


Results for CMake 2.6.3, CMP0005 NEW, Visual Studio 9 2008

  "PATH=\"c:\program files\my app\resources\""
  bug.c(5) : warning C4129: 'p' : unrecognized character escape sequence


Okay, the behaviour here is consistent with the NMake generator, but a little surprising (again, double-quotes are correctly quoted, but backslashes go into the .proj verbatim and end up being interpreted as escape characters).

--- TAKE 2: EXTRA QUOTING OF BACKSLASHES IN THE CMAKELISTS ---


I'll update my CMakeLists.txt and try again:
---
cmake_minimum_required (VERSION 2.6)
project (bug C)
cmake_policy( SET CMP0005 NEW )
add_executable( bug bug.c )
set_property( TARGET bug PROPERTY COMPILE_DEFINITIONS PATH="c:\\\\program files\\\\my app\\\\resources" )
---


This time NMake and VS2008 build correctly.

  nmake: cl ... -D"PATH=\"c:\\program files\\my app\\resources\""

  .proj: "PATH=\"c:\\program files\\my app\\resources\""

MSVC6 (using add_definitions()) still has problems, but is closer to the required value:

  # ADD CPP -DPATH="c:\\program files\\my app\\resources"
  bug.c(5) : error C2065: 'c' : undeclared identifier

what it should be:
  # ADD CPP /D PATH="\"c:\\program files\\my app\\resources\""


--- RESULTS WITH CMAKE 2.7.20090323 ---

Using the new CMakeLists.txt with
  COMPILE_DEFINITIONS PATH="c:\\\\program files\\\\my app\\\\resources"
resulted in warnings (as expected: The VS6 IDE does not support preprocessor definition values with spaces)

Using add_definitions() instead produced the same (buggy) result as CMake 2.6.3.

Tests with a value that did not contain spaces
  set_property( TARGET bug APPEND PROPERTY COMPILE_DEFINITIONS PATH="teststr" )
were successful - the value was correctly quoted in the .dsp.


--- CONCLUSION ---

To make the VS6 generator behave the same as NMake and VS2008, simply add quotes around the name=value (as with the other 2 generators), e.g.
  -D"NAME=VALUE"
(0016043)
Brad King (manager)
2009-04-16 15:29

 All generators know how to escape backslashes correctly for COMPILE_DEFINITIONS. The reason you need to put four in instead of two in the CMakeLists.txt file is because the CMake language processor evaluates one level of escapes, so the property value ends up with only one backslash for every pair written.

The only other issue is spaces in VS6 IDE. I'll investigate further.
(0016044)
Brad King (manager)
2009-04-16 15:52

 Okay, I create a simple project in VS6. Then I edit the settings, go to the list of preprocessor definitions, and add "STR=\"str with spaces\"". It builds as expected. I save the project, and in the .dsp file I see

  /D "STR=\"str with spaces\""

as you suggest. Next, I exit VS6, restart it, and then load the project. The definition has disappeared from the list, and the command line has

  /D /other /options "STR=\" str with spaces\"" /c

so the compiler looks for a source file with a strange name (note also the extra space before "str"). Clearly something goes wrong in the project file parser.
(0016049)
Twylite (reporter)
2009-04-17 05:49

 I can reproduce this. It's rather strange that I didn't experience this in my other development environment -- I'll investigate the discrepancy, but I suspect lucky chance ;(

To be accurate, the problem isn't a string with spaces, but a string with (quoted) double-quotes.

I'll investigate whether MSVC6 can in fact handle this case, and report back.
(0016081)
Brad King (manager)
2009-04-20 08:47
edited on: 2009-04-20 08:48

 Okay, I look forward to the results of your investigation.
(0016082)
Twylite (reporter)
2009-04-20 09:00

 What I've seen so far is that it's definitely the .DSP parser that's causing the problem, and that embedded double-quotes are an issue.

STR="str with spaces" is consistently fine.

STR=\"str with spaces\" seems fine, but I haven't looked at that much

STR="\"str with spaces\"" corrupts in all cases except where the define is the last thing on the #ADD CPP line other than the trailing /c. In that case the .DSP parser appears to pick up '/D STR="\' as the define, and then picks up some extra stuff '"str with spaces\""' which is appends to the command line to cl.

I have not yet found a way to quote the double-quotes in the value. Nothing that actually requires double-quotes (like \") appears to be compatible with the .DSP parser. I haven't identified any other escape mechanism in use by the parser, and there is no trigraph for a double-quote ;/

I should be able to come back with a conclusive answer by Thursday evening (GMT+2).
(0016178)
Twylite (reporter)
2009-04-24 09:28

 Or Friday afternoon, as the case may be.

I've tested a bunch of scenarios using the following round-trip:
(1) Add define(s) under Project -> Settings -> C/C++ (Tab) -> Category: Preprocessor (dropdown) -> Preprocessor definitions
(2) Compile (rebuild all), test, File -> Save All
(3) Exit & restart MSVC6
(4) Compile (rebuild all), test
(5) Remove definitions from project settings, save, exit
(6) Add definitions manually into DSP
(7) Start MSVC6, compile (rebuild all), test

The program source was:
--
#include <stdio.h>

#define __QUOTECLEAR(str) #str
#define QUOTE(str) __QUOTECLEAR(str)

int main(int argc, char* argv[])
{
  printf("Hello World!\n");
  printf( QUOTE(KEY) );
  printf("\n");
  return 0;
}
--

Note that the construction of the program to use the string pasting functionality of the preprocessor was to allow the program to compile in run in situations where a literal double-quote could not be introduced using /D in the project setting (as it turns out, all cases).

What works:

(A) /D KEY=val
(B) /D KEY="val"
(C) /D KEY=\"val\" -> output includes ""
(D) /D KEY="val with spaces"
(E) /D KEY="val \n with \t escapes and ??< trigraphs"
(F) Any combination of multiple defines quoted as above (e.g. I defined KEY, KEY2, KEY3 and adjusted the program to display all 3)

What doesn't work:

(G) /D KEY=\"val with spaces\"
(H) /D KEY="\"val\""
(I) /D KEY="\"val with spaces\""
(J) Any attempt whatsoever to include a literal '"' in the value of a define

* (G), (H) and (I) all work in the special case where the .DSP is modified such that there is only one define of this form, and it comes after all options on the #ADD CPP line other than the trailing /c. In all other cases the "Project Options" (i.e. the cl command line as displayed in Project Settings -> C/C++ -> General) is mangled by the presence of additional double-quotes.

In most cases that use double-quotes, the first quote can come before the key name or directly after the =. I didn't test all these cases, and this isn't how MSVC saves its settings.

My conclusion:

- Using the form '/D KEY="value"' is able to support values with or without spaces.

- There is no way to introduce a literal '"' into the value. Any attempt at quoting breaks the .DSP parser.

- The form '/D KEY=\"value\"' only works for a value without spaces.

Recommendation: When writing defines into an MSVC6 .DSP use the form '/D KEY="value"'. Allow spaces, backslashes, etc. but reject double-quotes. It may be worth indicating in the relevant documentation or policy help that this is an unresolvable problem, with a reference to this issue.
(0016179)
Twylite (reporter)
2009-04-24 09:33

 Additional note on the consequences of this problem with .dsp parsing:

If you need to support MSVC6, and you need to pass a string as a define on the command line (via /D) such that it will become a literal char*, then either:

(a) The string must have no spaces, and you can use /D KEY=\"val\"

OR

(b) The string may have spaces, you must use /D KEY="val with spaces", and you must use the preprocessor trick in the previous note to stringify the value.
(0016180)
Brad King (manager)
2009-04-24 11:21

 Thanks for the analysis. I've converted the definition command-line generator to use NAME="value" instead of "NAME=value" (since the NAME= part should never need quoting anyway). This enables many more values, so I ran some more experiments with values containing every character on the US keyboard.

I conclude that almost any value is valid if it does not have any spaces in combination with '"', '$', or ';'. All three characters require double quotes to escape them, which confuses the parser. I've committed changes that make the value check reject only these invalid combinations.

ENH: Support more preprocessor values in VS6
/cvsroot/CMake/CMake/Source/cmLocalGenerator.cxx,v <-- Source/cmLocalGenerator.cxx
new revision: 1.301; previous revision: 1.300
/cvsroot/CMake/CMake/Source/cmLocalVisualStudio6Generator.cxx,v <-- Source/cmLocalVisualStudio6Generator.cxx
new revision: 1.152; previous revision: 1.151
/cvsroot/CMake/CMake/Source/cmMakefile.cxx,v <-- Source/cmMakefile.cxx
new revision: 1.508; previous revision: 1.507
/cvsroot/CMake/CMake/Tests/Preprocess/CMakeLists.txt,v <-- Tests/Preprocess/CMakeLists.txt
new revision: 1.12; previous revision: 1.11

ENH: Test spaces in non-string preprocessor values
/cvsroot/CMake/CMake/Tests/Preprocess/CMakeLists.txt,v <-- Tests/Preprocess/CMakeLists.txt
new revision: 1.13; previous revision: 1.12
(0016181)
Twylite (reporter)
2009-04-24 11:32

 Great.

Just to confirm -- is '/D KEY="val $with ;spaces"' a valid combination? I've just done a round-trip test with that and '/D KEY="val $ with ; spaces"' and there are no apparent problems.
(0016183)
Brad King (manager)
2009-04-24 11:38

 No. While VS6 does support them I would have to introduce several special code paths in CMake to support them. Currently all escaping goes through one path, and that path writes $ as "$" and ; as ";" because this is necessary elsewhere in the IDE files. It's not worth the effort or maintenance for the corner-case.
(0016185)
Twylite (reporter)
2009-04-24 12:20

 Ah :) Okay, not a MSVC6 issue then - that's the part I was wondering about.

Sounds reasonable not to support that particular corner case then.

I'll try to find the time this weekend to work with the new revision; otherwise I consider this issue to be resolved. Thanks :)
(0016233)
Brad King (manager)
2009-04-28 17:34

 CMake 2.6.4 was just released. It has the first fix I did (allowing values without spaces). Unfortunately the second fix (allowing almost all values) was too late in the release cycle to make it. I've scheduled the second part for 2.6.5.

 Issue History
Date Modified Username Field Change
2009-03-23 11:47 Twylite New Issue
2009-03-23 11:57 Twylite Note Added: 0015762
2009-03-23 13:37 Bill Hoffman Status new => assigned
2009-03-23 13:37 Bill Hoffman Assigned To => Bill Hoffman
2009-03-23 14:53 Brad King Note Added: 0015766
2009-03-23 15:04 Brad King Assigned To Bill Hoffman => Brad King
2009-03-23 15:05 Brad King Note Added: 0015768
2009-03-23 15:05 Brad King Status assigned => closed
2009-03-23 15:05 Brad King Resolution open => fixed
2009-03-23 17:24 Twylite Note Added: 0015769
2009-03-23 17:24 Twylite Status closed => feedback
2009-03-23 17:24 Twylite Resolution fixed => reopened
2009-04-16 15:29 Brad King Note Added: 0016043
2009-04-16 15:52 Brad King Note Added: 0016044
2009-04-17 05:49 Twylite Note Added: 0016049
2009-04-20 08:47 Brad King Note Added: 0016081
2009-04-20 08:48 Brad King Note Edited: 0016081
2009-04-20 09:00 Twylite Note Added: 0016082
2009-04-24 09:28 Twylite Note Added: 0016178
2009-04-24 09:33 Twylite Note Added: 0016179
2009-04-24 11:21 Brad King Note Added: 0016180
2009-04-24 11:32 Twylite Note Added: 0016181
2009-04-24 11:38 Brad King Note Added: 0016183
2009-04-24 12:20 Twylite Note Added: 0016185
2009-04-28 17:34 Brad King Note Added: 0016233
2009-04-28 17:34 Brad King Status feedback => closed
2009-04-28 17:34 Brad King Resolution reopened => fixed

Copyright © 2000 - 2018 MantisBT Team