Thrift
  1. Thrift
  2. THRIFT-1750

Make compiler build cleanly under visual studio 10

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 0.9, 1.0
    • Fix Version/s: 0.9.2
    • Component/s: Compiler (General)
    • Labels:
      None
    • Environment:

      Windows, MSVC10

    • Patch Info:
      Patch Available

      Description

      Building the thrift compiler in Visual Studio 10 causes a lot of warnings.

        Issue Links

          Activity

          Hide
          Ben Craig added a comment -
          • No more MINGW flags, now testing for built-in _WIN32 and _MSCVER flags
          • Now using a config.h (and associated HAVE_CONFIG_H) for the compiler
          • No longer using visual studio's "force include" feature
          • Tweaked VS project invocation of flex and bison so that it doesn't rely on process current working directory
          • Fixing various integer truncation warnings. The scariest of these involves droppint field_ids from 64-bit to 32-bit. They were already being truncated, now the struct is smaller as well.
          • Squelching assorted Visual Studio build warnings in the associated files, instead of globally
          Show
          Ben Craig added a comment - No more MINGW flags, now testing for built-in _WIN32 and _MSCVER flags Now using a config.h (and associated HAVE_CONFIG_H) for the compiler No longer using visual studio's "force include" feature Tweaked VS project invocation of flex and bison so that it doesn't rely on process current working directory Fixing various integer truncation warnings. The scariest of these involves droppint field_ids from 64-bit to 32-bit. They were already being truncated, now the struct is smaller as well. Squelching assorted Visual Studio build warnings in the associated files, instead of globally
          Hide
          Roger Meier added a comment -

          please do not remove MINGW, we need it to cross compile the on linux for windows via contrib/mingw-cross-compile.sh

          Show
          Roger Meier added a comment - please do not remove MINGW, we need it to cross compile the on linux for windows via contrib/mingw-cross-compile.sh
          Hide
          Ben Craig added a comment -

          I will look in to making these changes work with that script. It is possible that they already do, but I don't know enough about mingw to be sure.

          Show
          Ben Craig added a comment - I will look in to making these changes work with that script. It is possible that they already do, but I don't know enough about mingw to be sure.
          Hide
          Ben Craig added a comment -

          MINGW still defines _WIN32, so that part seems to be fine. My patch does have one MINGW issue though. Windows uses "MAX_PATH", while *nix usis "PATH_MAX". MINGW takes the *nix approach. I will post an updated patch to address this, after I get an in-house review.

          My new patch will be tested against contrib/mingw-cross-compiler.sh before submission.

          Show
          Ben Craig added a comment - MINGW still defines _WIN32, so that part seems to be fine. My patch does have one MINGW issue though. Windows uses "MAX_PATH", while *nix usis "PATH_MAX". MINGW takes the *nix approach. I will post an updated patch to address this, after I get an in-house review. My new patch will be tested against contrib/mingw-cross-compiler.sh before submission.
          Hide
          Ben Craig added a comment -

          Updated to fix MINGW / MSVC PATH_MAX confusion. Tested with mingw compile script.

          Show
          Ben Craig added a comment - Updated to fix MINGW / MSVC PATH_MAX confusion. Tested with mingw compile script.
          Hide
          Ben Craig added a comment -

          Putting a newline at the end of platform.h that was accidentally omitted.

          Show
          Ben Craig added a comment - Putting a newline at the end of platform.h that was accidentally omitted.
          Hide
          Ben Craig added a comment -

          Ack! In my June 2013 mega patch upload, this basically got duplicated. 2045 has the more up to date patch, though it does not attempt to fix the project. Letting 2045 be the "real" bug.

          Show
          Ben Craig added a comment - Ack! In my June 2013 mega patch upload, this basically got duplicated. 2045 has the more up to date patch, though it does not attempt to fix the project. Letting 2045 be the "real" bug.
          Hide
          Jens Geyer added a comment - - edited

          So close this one as wontfix (or the like)?

          Show
          Jens Geyer added a comment - - edited So close this one as wontfix (or the like)?
          Hide
          Ben Craig added a comment -

          As soon as 2045 is wrapped up, I'll close it as fixed.

          Show
          Ben Craig added a comment - As soon as 2045 is wrapped up, I'll close it as fixed.

            People

            • Assignee:
              Ben Craig
              Reporter:
              Ben Craig
            • Votes:
              2 Vote for this issue
              Watchers:
              3 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development