Avro
  1. Avro
  2. AVRO-551

C: Build and pass tests on Win32

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 1.6.3, 1.7.0
    • Fix Version/s: 1.7.0
    • Component/s: c
    • Labels:
      None
    • Release Note:
      Hide
      Port Avro-C to Win32, using the Microsoft Visual C++ 2008 compiler.
      Show
      Port Avro-C to Win32, using the Microsoft Visual C++ 2008 compiler.

      Description

      Avro C does not currently build on Win32. We need to address that.

      1. AVRO-551-patch2.patch
        3 kB
        Vivek Nadkarni
      2. AVRO-551-license.patch
        3 kB
        Vivek Nadkarni
      3. AVRO-551.patch
        298 kB
        Vivek Nadkarni

        Issue Links

        There are no Sub-Tasks for this issue.

          Activity

          Hide
          Douglas Creager added a comment -

          Committed to SVN

          Show
          Douglas Creager added a comment - Committed to SVN
          Hide
          Douglas Creager added a comment -

          All tests pass for me on Visual Studio 2010 Premium. I don't have objections to anything in the patch. The only thing that might be controversial, from what I can see, is that since we're compiling using VC++ under Windows, we have to include many more pointer casts. In particular, we have to cast the result of all of our malloc-like calls, since C++ doesn't automatically cast from void *. In C, the best practice is to not include a cast, so that the compiler can yell at you if you forgot to include the header file that defines malloc. I think that since we're using avro_malloc and friends, rather than the system malloc, this is not an issue that we have to worry about.

          So, short version, +1. If I don't hear any objections from Bruce or others by Friday, I'll commit this is SVN.

          Show
          Douglas Creager added a comment - All tests pass for me on Visual Studio 2010 Premium. I don't have objections to anything in the patch. The only thing that might be controversial, from what I can see, is that since we're compiling using VC++ under Windows, we have to include many more pointer casts. In particular, we have to cast the result of all of our malloc-like calls, since C++ doesn't automatically cast from void * . In C, the best practice is to not include a cast, so that the compiler can yell at you if you forgot to include the header file that defines malloc. I think that since we're using avro_malloc and friends, rather than the system malloc, this is not an issue that we have to worry about. So, short version, +1. If I don't hear any objections from Bruce or others by Friday, I'll commit this is SVN.
          Hide
          Vivek Nadkarni added a comment -

          All the work I had planned to do for the Win32 port of Avro-C is complete.

          All the tests pass under Windows, Linux and Cygwin.

          Doug and/or Bruce - I was wondering if one of you could take the time
          to test my patches, and integrate them in the codebase, before the
          patches get stale.

          Also, if there are any additional changes to be made, it will be
          easier for me to make those changes when the code is still fresh in my
          mind.

          Thanks,
          Vivek Nadkarni

          Show
          Vivek Nadkarni added a comment - All the work I had planned to do for the Win32 port of Avro-C is complete. All the tests pass under Windows, Linux and Cygwin. Doug and/or Bruce - I was wondering if one of you could take the time to test my patches, and integrate them in the codebase, before the patches get stale. Also, if there are any additional changes to be made, it will be easier for me to make those changes when the code is still fresh in my mind. Thanks, Vivek Nadkarni
          Hide
          Vivek Nadkarni added a comment -

          This patch (AVRO-551-patch2.patch) should be applied on top of
          AVRO-551.patch.

          This patch contains minor updates – fixed some warnings and fixed the
          AVRO_NON_ATOMIC_REFCOUNT implementation. The previous implementation
          of AVRO_NON_ATOMIC_REFCOUNT didn't work with some versions of GCC 4.6
          for (some) ARM processors.

          Show
          Vivek Nadkarni added a comment - This patch ( AVRO-551 -patch2.patch) should be applied on top of AVRO-551 .patch. This patch contains minor updates – fixed some warnings and fixed the AVRO_NON_ATOMIC_REFCOUNT implementation. The previous implementation of AVRO_NON_ATOMIC_REFCOUNT didn't work with some versions of GCC 4.6 for (some) ARM processors.
          Hide
          Vivek Nadkarni added a comment -

          Doug Cutting - Thanks for the licensing instructions.

          This patch file (AVRO-551-license.patch) updates the top level
          LICENSE.txt file with the licenses for the source files that I
          added. This patch file does not contain my source code changes, which
          have already been uploaded in AVRO-551.patch.

          Doug Creager - Thanks for testing out my Win32 changes.

          Cheers,
          Vivek

          Show
          Vivek Nadkarni added a comment - Doug Cutting - Thanks for the licensing instructions. This patch file ( AVRO-551 -license.patch) updates the top level LICENSE.txt file with the licenses for the source files that I added. This patch file does not contain my source code changes, which have already been uploaded in AVRO-551 .patch. Doug Creager - Thanks for testing out my Win32 changes. Cheers, Vivek
          Hide
          Douglas Creager added a comment -

          Thanks for the clarification on the licensing parts, Doug.

          I have a Windows VM that I can test these changes on — I'll try to do that in the next couple of days.

          Bruce had also done some work on a Windows build awhile back, so he might have some useful comments on the patch, too.

          Show
          Douglas Creager added a comment - Thanks for the clarification on the licensing parts, Doug. I have a Windows VM that I can test these changes on — I'll try to do that in the next couple of days. Bruce had also done some work on a Windows build awhile back, so he might have some useful comments on the patch, too.
          Hide
          Doug Cutting added a comment -

          It's fine to include MIT and BSD licensed code, but the licenses for these files should be appended to the end of Avro's top-level LICENSE.txt file. For more information see:

          http://apache.org/legal/resolved.html

          Show
          Doug Cutting added a comment - It's fine to include MIT and BSD licensed code, but the licenses for these files should be appended to the end of Avro's top-level LICENSE.txt file. For more information see: http://apache.org/legal/resolved.html
          Hide
          Vivek Nadkarni added a comment -

          Hi -

          This patchset (AVRO-551.patch) ports Avro-C version 1.6.3 and version
          1.7.0 to compile under Microsoft Visual C++ 2008 (MSVC++ 2008). I
          compiled this code using the Express edition.

          I have tested that after the AVRO-551.patch is applied:

          • All code continues to compile and all tests pass under Linux and
            Cygwin.
          • The Avro-C static library and all tests pass under Windows, when
            compiled using the C++ compiler under MSVC++ 2008.

          A file README.maintaining_win32.txt is included with the
          patchset. This file contains a detailed description of the changes
          required, and the process used, to port Avro-C to Win32. It also lists
          limitations of the Windows build and instructions on how to maintain
          the code so that the Win32 build continues to compile and pass all
          tests.

          This patchset contains the BSD Licensed inttypes.h and stdint.h from
          http://code.google.com/p/msinttypes/ and the MIT Licensed dirent.h
          from http://www.softagalleria.net/download/dirent/dirent-1.11.zip .

          Please let me know if there is anything else I need to do to get this
          patchset incorporated into Avro-C.

          Thanks,
          Vivek

          Show
          Vivek Nadkarni added a comment - Hi - This patchset ( AVRO-551 .patch) ports Avro-C version 1.6.3 and version 1.7.0 to compile under Microsoft Visual C++ 2008 (MSVC++ 2008). I compiled this code using the Express edition. I have tested that after the AVRO-551 .patch is applied: All code continues to compile and all tests pass under Linux and Cygwin. The Avro-C static library and all tests pass under Windows, when compiled using the C++ compiler under MSVC++ 2008. A file README.maintaining_win32.txt is included with the patchset. This file contains a detailed description of the changes required, and the process used, to port Avro-C to Win32. It also lists limitations of the Windows build and instructions on how to maintain the code so that the Win32 build continues to compile and pass all tests. This patchset contains the BSD Licensed inttypes.h and stdint.h from http://code.google.com/p/msinttypes/ and the MIT Licensed dirent.h from http://www.softagalleria.net/download/dirent/dirent-1.11.zip . Please let me know if there is anything else I need to do to get this patchset incorporated into Avro-C. Thanks, Vivek
          Hide
          Vivek Nadkarni added a comment -

          Hi -

          I am in the process of porting Avro-C, version 1.6.3, to Microsoft
          Visual C++ 2008. I would like to submit my changes for inclusion in
          the Avro-C project.

          In the porting process, I needed to include a few header files that
          are not present in the Microsoft environment. I have included the
          following files:

          1. inttypes.h & stdint.h
          Obtained from
          http://code.google.com/p/msinttypes/
          This project uses the New BSD License:
          http://www.opensource.org/licenses/bsd-license.php

          2. dirent.h
          Obtained from
          http://www.softagalleria.net/dirent.php
          http://www.softagalleria.net/download/dirent/dirent-1.11.zip
          The file copyright header matches the MIT-License.

          Can I just include these files in my "patchset" when I upload my
          patches? Is there anything special I need to do to include these files
          in the project?

          Thanks,
          Vivek

          Show
          Vivek Nadkarni added a comment - Hi - I am in the process of porting Avro-C, version 1.6.3, to Microsoft Visual C++ 2008. I would like to submit my changes for inclusion in the Avro-C project. In the porting process, I needed to include a few header files that are not present in the Microsoft environment. I have included the following files: 1. inttypes.h & stdint.h Obtained from http://code.google.com/p/msinttypes/ This project uses the New BSD License: http://www.opensource.org/licenses/bsd-license.php 2. dirent.h Obtained from http://www.softagalleria.net/dirent.php http://www.softagalleria.net/download/dirent/dirent-1.11.zip The file copyright header matches the MIT-License. Can I just include these files in my "patchset" when I upload my patches? Is there anything special I need to do to include these files in the project? Thanks, Vivek

            People

            • Assignee:
              Unassigned
              Reporter:
              Bruce Mitchener
            • Votes:
              0 Vote for this issue
              Watchers:
              0 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development