[CMake] Bug #12189

David Cole david.cole at kitware.com
Thu Sep 8 15:08:55 EDT 2011 

On Thu, Sep 8, 2011 at 11:17 AM,  <aaron.meadows at thomsonreuters.com> wrote:
> The reason I didn't check _SBCS is that I introduced that as a control
> to CMake.  _MBCS and _UNICODE are defined by the compiler based on the
> CharacterSet setting.

OK, but since you're using it as a target level #define, it should be
defined in the test, right? So code something like:

  #ifndef _SBCS
    #error "_SBCS should be defined"
  #endif

...would be ok to have, too. The rest of the code verifies that
neither _UNICODE nor _MBCS is defined, but waits until runtime to
cause a failure.

An alternate approach (what I would do, since it fails earlier, at
compile time) would be:

#ifdef _UNICODE
#error "_UNICODE should not be defined"
#endif
#ifdef _MBCS
#error "_MBCS should not be defined"
#endif

// If it compiles and runs, it succeeds:
return 0;


> The test is being executed inside an
> IF(${CMAKE_TEST_GENERATOR} MATCHES "Visual Studio"). Is that
> insufficient to guarantee it only runs with VS?

Nope. That's sufficient. I simply failed to notice the "IF"... My bad.

However, the other tests in that block all start with "VS" in the
name. If the test is only going to run for VS generators, (and not the
NMake Makefiles generator), then we should name it with a leading VS.

One more note: are you using "git" -- if so, simply make this as a
commit on top of the existing CMake 'master' branch (any commit that's
contained in 'master'), and then do:

  $ git format-patch -1

And attach *that* patch. That will preserve your authorship
information in the patch file, and makes it a snap for us to apply
with '$ git am -3 0001-your-commit.patch' on our end.


Cheers (and thanks again),
David


>
> Aaron Meadows
>
> -----Original Message-----
> From: cmake-bounces at cmake.org [mailto:cmake-bounces at cmake.org] On Behalf
> Of Meadows, Aaron C.
> Sent: Tuesday, September 06, 2011 3:18 PM
> To: David Cole
> Cc: cmake at cmake.org
> Subject: Re: [CMake] Bug #12189
>
> Sounds good. I believe the test is already in an msvc block. The other
> is easily added. I'll get on it tomorrow.
>
> Any thoughts on where to document it?
>
> Sent from my iPhone
>
> On Sep 6, 2011, at 3:15 PM, "David Cole" <david.cole at kitware.com> wrote:
>
>> Thanks for your work on this Aaron. It looks almost good enough to
>> merge as is -- so close! Just two more things: I want to make sure the
>
>> test is conditional on MSVC (unless your intent is to make this work
>> everywhere?) and tweak the test slightly: I think it should also
>> verify that _SBCS is defined in addition to checking _UNICODE and
>> _MBCS.
>>
>> Unfortunately, I'm running out of time to apply patches, make sure
>> they work on all platforms, and then get the merged in for the
>> upcoming 2.8.6 release.
>>
>> I'll try to get to this one soon after the 2.8.6 release so that it
>> will be included in the next release.
>>
>>
>> Thanks,
>> David
>>
>>
>> On Thu, Sep 1, 2011 at 11:29 AM,  <aaron.meadows at thomsonreuters.com>
> wrote:
>>> Hi David,
>>>
>>> I've updated the bug with a patch which adds a test to check that
> defining _SBCS does set CharacterSet to 0 (Single Byte Character Set).
> I checked that it works when _SBCS is set and fails when _SBCS is not
> defined.
>>>
>>> I looked for where the _UNICODE option is documented, thinking to
>>> document _SBCS there, but was unable to find it in the documentation.
>
>>> If you want to point me to the right location to add documentation,
>>> I'm happy to write some. (I can write some for _UNICODE as well, if
>>> you like.)
>>>
>>> Aaron Meadows
>>>
>>>
>>> -----Original Message-----
>>> From: David Cole [mailto:david.cole at kitware.com]
>>> Sent: Wednesday, August 31, 2011 1:04 PM
>>> To: Meadows, Aaron C.
>>> Cc: cmake at cmake.org
>>> Subject: Re: [CMake] Bug #12189
>>>
>>> The CMake/Tests/CMakeLists.txt file lists most of the tests that
> execute on our dashboards.
>>>
>>> The directories under CMake/Tests are all the existing test source
> trees. If you want to modify one of the existing tests to have an
> "_SBCS" target compile definition, I'd start by looking at "Simple" or
> "COnly" as a simple basis for a new test.
>>>
>>> Or you may find another one where the code that's compiled and tested
> is compatible with "_SBCS".
>>>
>>> You may want to conditionalize it with code like:
>>> if (CMAKE_GENERATOR MATCHES "Visual Studio") .... or ....
>>> if (MSVC)
>>>
>>> since your changes only apply to the Visual Studio generators. It
> should work with "cl" and "nmake" too, though, to have an "_SBCS"
>>> definition. Other compilers may or may not like a -D_SBCS.
>>>
>>> Let me know if you need further guidance.
>>>
>>> Thanks,
>>> David
>>>
>>>
>>> On Wed, Aug 31, 2011 at 1:33 PM,  <aaron.meadows at thomsonreuters.com>
> wrote:
>>>> I'm happy to assist in any way I can.  Where do I need to add a
> test?  Also, where would it be appropriate to document this?
>>>>
>>>> Aaron Meadows
>>>>
>>>>
>>>> -----Original Message-----
>>>> From: David Cole [mailto:david.cole at kitware.com]
>>>> Sent: Wednesday, August 31, 2011 11:02 AM
>>>> To: Meadows, Aaron C.
>>>> Cc: cmake at cmake.org
>>>> Subject: Re: [CMake] Bug #12189
>>>>
>>>> Your patch has the "wrong sense" I think.... It looks like it's
> removing "_SBCS" code, but you want to add all that code, correct?
>>>>
>>>> I think (as long as my above assumption is correct) that this patch
> should be ok, even in a backwards compatibility sense, because only
> people who have "_SBCS" defined as a target property compile definition
> will have code generated differently than before.
>>>>
>>>> And I suspect your the only person in the world at this point for
>>>> whom this is true. :-)
>>>>
>>>> Of course, to accept it into CMake, it would be nice to have it
> documented somewhere, and to have a test that exercises the newly added
> code. Especially now that we've entered the release candidate phase of
> 2.8.6.
>>>>
>>>> Any chance you can add a test (or modify an existing one) to add the
> _SBCS compile definition as a target property?
>>>>
>>>>
>>>> On Wed, Aug 31, 2011 at 11:38 AM,
> <aaron.meadows at thomsonreuters.com> wrote:
>>>>> Any of you CMakers want to comment on this bug and patch?  I'd like
>
>>>>> to be able to switch my company back to the public version of CMake
>
>>>>> instead of compiling my own flavor every time there is a release...
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>
>>>>> Aaron Meadows
>>>>>
>>>>>
>>>>>
>>>>> From: Meadows, Aaron C.
>>>>> Sent: Thursday, June 23, 2011 2:23 PM
>>>>>
>>>>> To: Meadows, Aaron C.; 'cmake at cmake.org'
>>>>> Subject: RE: [CMake] Bug #12189
>>>>>
>>>>>
>>>>>
>>>>> I've updated the bug with an additional patch (had a  bug for some
>>>>> configuration cases).
>>>>>
>>>>>
>>>>>
>>>>> What is the procedure to get this patch considered and applied?  Do
>
>>>>> I need to contact the Visual Studio Generator maintainer directly?
>>>>>
>>>>>
>>>>>
>>>>> Aaron C. Meadows
>>>>>
>>>>> From: Meadows, Aaron C.
>>>>> Sent: Tuesday, June 21, 2011 9:49 AM
>>>>> To: Meadows, Aaron C.; cmake at cmake.org
>>>>> Subject: RE: [CMake] Bug #12189
>>>>>
>>>>>
>>>>>
>>>>> I updated the bug with this patch and some of the details from
> below.
>>>>>
>>>>>
>>>>>
>>>>> Aaron C. Meadows
>>>>>
>>>>> From: cmake-bounces at cmake.org [mailto:cmake-bounces at cmake.org] On
>>>>> Behalf Of Meadows, Aaron C.
>>>>> Sent: Monday, June 20, 2011 5:17 PM
>>>>> To: cmake at cmake.org
>>>>> Subject: [CMake] Bug #12189
>>>>>
>>>>>
>>>>>
>>>>> (link: http://public.kitware.com/Bug/view.php?id=12189)
>>>>>
>>>>>
>>>>>
>>>>> I just came across the above bug opened last month,  Summaraized
> as:
>>>>> "It is not possible to generate a Visual Studio project with
>>>>> ASCII/SBCS character set".  (Full Bug Text Following Message)
>>>>>
>>>>>
>>>>>
>>>>> I find myself in the situation where I need this to be set to "not
>>>>> set", so as to have ASCII/SBCS as the character set for all my
>>>>> projects.  I dug back through the last few years of the maillist
>>>>> and didn't see anything about this issue.
>>>>>
>>>>>
>>>>>
>>>>> Anyone have experience with this issue?  And better yet, know of a
> solution?
>>>>>
>>>>>
>>>>>
>>>>> I checked the source file he indicated (it's on line 702 of the
>>>>> 2.8.4
>>>>> source):
>>>>>
>>>>>   // If unicode is enabled change the character set to unicode, if
>>>>> not
>>>>>
>>>>>   // then default to MBCS.
>>>>>
>>>>>   if(targetOptions.UsingUnicode())
>>>>>
>>>>>     {
>>>>>
>>>>>     fout << "\t\t\tCharacterSet=\"1\">\n";
>>>>>
>>>>>     }
>>>>>
>>>>>   else
>>>>>
>>>>>     {
>>>>>
>>>>>     fout << "\t\t\tCharacterSet=\"2\">\n";
>>>>>
>>>>>     }
>>>>>
>>>>>
>>>>>
>>>>> Additionally, VS 2010 has a different mechanism (line 287 of the
>>>>> 2.8.4
>>>>> source):
>>>>>
>>>>>     if(this->Target->GetType() <= cmTarget::MODULE_LIBRARY &&
>>>>>
>>>>>        this->ClOptions[*i]->UsingUnicode())
>>>>>
>>>>>       {
>>>>>
>>>>>       this->WriteString("<CharacterSet>Unicode</CharacterSet>\n",
>>>>> 2);
>>>>>
>>>>>       }
>>>>>
>>>>>     else
>>>>>
>>>>>       {
>>>>>
>>>>>       this->WriteString("<CharacterSet>MultiByte</CharacterSet>\n",
>>>>> 2);
>>>>>
>>>>>       }
>>>>>
>>>>>
>>>>>
>>>>> In the case of VS 2010, the proper ASCII setting is:
>>>>>
>>>>>        <CharacterSet>NotSet</CharacterSet>
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>
>>>>> It would be fairly trivial to change both these cases to generate
>>>>> the "Not Set" case.  However, how should backward compatibility be
>>>>> maintained, if at all?
>>>>>
>>>>>
>>>>>
>>>>> I've created and attached a patch which resolves this bug by adding
>
>>>>> an "_SBCS" define which is checked.  If it is set and "_UNICODE" is
>
>>>>> not set, it will write the correct files for the "Not Set" ASCII
> case.
>>>>> This is slightly different than the suggestion in the Bug, but was
>>>>> very easy to write and provides identical behavior if "_SBCS" is
> not set.
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>
>>>>> -------------------------------------------------------------------
>>>>> --
>>>>> -
>>>>> ------------------------------------------------------------
>>>>>
>>>>> Full Bug Text:
>>>>>
>>>>> -------------------------------------------------------------------
>>>>> --
>>>>> -
>>>>> ------------------------------------------------------------
>>>>>
>>>>> In Visual Studio 9.0 (and prior, 10.0 i don't know) it is possible
>>>>> specify three different character sets for your project within the
>>>>> project
>>>>> properties:
>>>>>
>>>>> Not Set = ASCII/SBCS (Single Byte Character Set) Unicode Multi-Byte
>>>>>
>>>>> Depending on the option different preprocessor defines are set
>>>>> (http://msdn.microsoft.com/en-us/library/c426s321(v=vs.80).aspx
> [^]):
>>>>>
>>>>> SBCS: neither _UNICODE nor _MBCS defined
>>>>> Unicode: _UNICODE defined
>>>>> Multi_Byte: _MBCS defined
>>>>>
>>>>> The character set settings is stored within the vs proj files as an
>
>>>>> xml
>>>>> attribute:
>>>>>
>>>>> SBCS: CharacterSet="0"
>>>>> Unicode: CharacterSet="1"
>>>>> Multi-Byte: CharacterSet="2"
>>>>>
>>>>> However, the cmake visual studio generators do not support
>>>>> generating of projects with CharacterSet="0" (see
>>>>> cmLocalVisualStudio7Generator.cxx line 730). At the moment the
>>>>> generators select unicode if a _UNICODE macro has been set by
> add_definitions, otherwise multi-byte is selected.
>>>>>
>>>>> To solve the problem and to keep backwards compatability, i suggest
>
>>>>> to define the _MBCS macro by default for the visual studio
>>>>> generators and to set CharacterSet="2" only if this macro is still
>>>>> available and otherwise CharacterSet="0". In that case the user can
>
>>>>> remove the _MBCS macro by remove_definitions and select this way
>>>>> the SBCS. If the user adds _UNICODE by add_definitions
>>>>> CharacterSet="1" should be selected and the conflicting _MBCS macro
> must be deleted by the code generator.
>>>>>
>>>>> -------------------------------------------------------------------
>>>>> --
>>>>> -
>>>>> ------------------------------------------------------------
>>>>>
>>>>>
>>>>>
>>>>> Aaron Meadows
>>>>> Software Engineer
>>>>>
>>>>> Thomson Reuters
>>>>>
>>>>> Phone: 314.468.3530
>>>>> Mobile: 636.541.6139
>>>>> aaron.meadows at thomsonreuters.com
>>>>> thomsonreuters.com
>>>>>
>>>>> This email was sent to you by Thomson Reuters, the global news and
>>>>> information company. Any views expressed in this message are those
>>>>> of the individual sender, except where the sender specifically
>>>>> states them to be the views of Thomson Reuters.
>>>>> _______________________________________________
>>>>> Powered by www.kitware.com
>>>>>
>>>>> Visit other Kitware open-source projects at
>>>>> http://www.kitware.com/opensource/opensource.html
>>>>>
>>>>> Please keep messages on-topic and check the CMake FAQ at:
>>>>> http://www.cmake.org/Wiki/CMake_FAQ
>>>>>
>>>>> Follow this link to subscribe/unsubscribe:
>>>>> http://www.cmake.org/mailman/listinfo/cmake
>>>>>
>>>>
>>>> This email was sent to you by Thomson Reuters, the global news and
> information company. Any views expressed in this message are those of
> the individual sender, except where the sender specifically states them
> to be the views of Thomson Reuters.
>>>>
>>>
>>> This email was sent to you by Thomson Reuters, the global news and
> information company. Any views expressed in this message are those of
> the individual sender, except where the sender specifically states them
> to be the views of Thomson Reuters.
>>>
>
> This email was sent to you by Thomson Reuters, the global news and
> information company. Any views expressed in this message are those of
> the individual sender, except where the sender specifically states them
> to be the views of Thomson Reuters.
> _______________________________________________
> Powered by www.kitware.com
>
> Visit other Kitware open-source projects at
> http://www.kitware.com/opensource/opensource.html
>
> Please keep messages on-topic and check the CMake FAQ at:
> http://www.cmake.org/Wiki/CMake_FAQ
>
> Follow this link to subscribe/unsubscribe:
> http://www.cmake.org/mailman/listinfo/cmake
>
> This email was sent to you by Thomson Reuters, the global news and information company. Any views expressed in this message are those of the individual sender, except where the sender specifically states them to be the views of Thomson Reuters.
>

More information about the CMake mailing list