Avro
  1. Avro
  2. AVRO-700

Change C++ build system to CMake

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 1.5.0
    • Fix Version/s: 1.5.0
    • Component/s: c++
    • Labels:
      None

      Description

      If we move from the current automake to CMake, build becomes portable across multiple platforms. Prior to this patch, Avro C++ was building on Cygwin but was crashing. I've tested this on Ubuntu 10.04 and Cygwin.
      There are problems making it work with Visual Studio 2008 Express, not because of CMake, but because the current build procedure needs Flex, Bison and python. Visual Studio seems to have trouble with these three.

      1. AVRO-700.patch
        12 kB
        Thiruvalluvan M. G.
      2. apply-patch.sh
        0.1 kB
        Thiruvalluvan M. G.
      3. apply-patch.sh
        0.2 kB
        Thiruvalluvan M. G.
      4. ignored_files
        0.1 kB
        Thiruvalluvan M. G.
      5. AVRO-700.patch
        14 kB
        Thiruvalluvan M. G.
      6. AVRO-700.patch
        15 kB
        Thiruvalluvan M. G.
      7. AVRO-700.patch
        15 kB
        Thiruvalluvan M. G.

        Activity

        Hide
        Thiruvalluvan M. G. added a comment -

        To apply the patch, from top level source directory "patch -p0 < AVRO-700.patch" and then "sh ./apply-patch.sh"

        Show
        Thiruvalluvan M. G. added a comment - To apply the patch, from top level source directory "patch -p0 < AVRO-700 .patch" and then "sh ./apply-patch.sh"
        Hide
        Doug Cutting added a comment -

        The top-level BUILD.txt should be updated to note cmake as a dependency.

        Once I've installed cmake, running './build.sh compile' on Ubuntu 10.10 gives me:

        CMake Error at CMakeLists.txt:83 (install):
          install TARGETS given no LIBRARY DESTINATION for shared library target
          "avrocpp".
        

        Also, after running this there are a bunch of CMake* files. Should apply-patch.sh add these to svn's ignore list for the directory?

        Show
        Doug Cutting added a comment - The top-level BUILD.txt should be updated to note cmake as a dependency. Once I've installed cmake, running './build.sh compile' on Ubuntu 10.10 gives me: CMake Error at CMakeLists.txt:83 (install): install TARGETS given no LIBRARY DESTINATION for shared library target "avrocpp" . Also, after running this there are a bunch of CMake* files. Should apply-patch.sh add these to svn's ignore list for the directory?
        Hide
        Scott Banachowski added a comment -

        Regarding the use of flex and bison, since these generate C files, we could probably just check in the generated files... then on platforms without flex/bison, they can build without needing to regenerate them. Only people who need to modify the lexer code need to run flex/bison.

        The python script is a code generator to build the serialization code from a schema, and it's not strictly needed to build the library. It's used in unit tests since the output code exercises a lot of the functionality.

        Show
        Scott Banachowski added a comment - Regarding the use of flex and bison, since these generate C files, we could probably just check in the generated files... then on platforms without flex/bison, they can build without needing to regenerate them. Only people who need to modify the lexer code need to run flex/bison. The python script is a code generator to build the serialization code from a schema, and it's not strictly needed to build the library. It's used in unit tests since the output code exercises a lot of the functionality.
        Hide
        Thiruvalluvan M. G. added a comment -

        A much cleaner version all files produced during make are put into build directory leaving lang/c++ clean.

        Please download ignored_files, apply_patch.sh and AVRO-700.patch into the top level Avro source directory. From within that directory, issue:

        sh ./apply_patch.sh
        patch -p0 < AVRO-700.patch

        Show
        Thiruvalluvan M. G. added a comment - A much cleaner version all files produced during make are put into build directory leaving lang/c++ clean. Please download ignored_files, apply_patch.sh and AVRO-700 .patch into the top level Avro source directory. From within that directory, issue: sh ./apply_patch.sh patch -p0 < AVRO-700 .patch
        Hide
        Thiruvalluvan M. G. added a comment -

        The test paths were wrong in build.sh

        Show
        Thiruvalluvan M. G. added a comment - The test paths were wrong in build.sh
        Hide
        Doug Cutting added a comment -

        With this patch, C++ builds and passes tests for me on Ubuntu 10.10. However 'lang/c+/build.sh dist' does not copy the final artifact to the top-level dist/cpp directory where the release artifacts are meant to be collected, nor does it build the C+ API documentation and copy it to the top-level build/avro-doc-$VERSION/api/cpp/ as was done before.

        Show
        Doug Cutting added a comment - With this patch, C++ builds and passes tests for me on Ubuntu 10.10. However 'lang/c+ /build.sh dist' does not copy the final artifact to the top-level dist/cpp directory where the release artifacts are meant to be collected, nor does it build the C + API documentation and copy it to the top-level build/avro-doc-$VERSION/api/cpp/ as was done before.
        Hide
        Thiruvalluvan M. G. added a comment -

        This patch addresses both the issues Doug reported.

        Show
        Thiruvalluvan M. G. added a comment - This patch addresses both the issues Doug reported.
        Hide
        Doug Cutting added a comment -

        A few issues seem to remain:

        • 'cd lang/c++; ./build.sh test' fails for me, as ./build/CodecTests does not exist
        • 'cd lang/c+; ./build.sh dist' does not create a c+ tarball in build/cpp. this should create a directory like http://www.apache.org/dist//avro/stable/cpp/. the documentation does appear to be placed correctly.
        Show
        Doug Cutting added a comment - A few issues seem to remain: 'cd lang/c++; ./build.sh test' fails for me, as ./build/CodecTests does not exist 'cd lang/c+ ; ./build.sh dist' does not create a c + tarball in build/cpp. this should create a directory like http://www.apache.org/dist//avro/stable/cpp/ . the documentation does appear to be placed correctly.
        Hide
        Thiruvalluvan M. G. added a comment -

        Doug,

        I'm sorry it took so many cycles to get this trough. Hope this is the last. Thanks for your patience.

        CoecTests and StreamTests were leaked from AVRO-711. Removed them.

        Now that tgz file is created both under build and dist.

        Show
        Thiruvalluvan M. G. added a comment - Doug, I'm sorry it took so many cycles to get this trough. Hope this is the last. Thanks for your patience. CoecTests and StreamTests were leaked from AVRO-711 . Removed them. Now that tgz file is created both under build and dist.
        Hide
        Doug Cutting added a comment -

        A few minor nits:

        • s/then/else/ in the VERSION section of lang/c++/build.sh
        • update the top-level BUILD.txt, listing cmake as a requirement?
        Show
        Doug Cutting added a comment - A few minor nits: s/then/else/ in the VERSION section of lang/c++/build.sh update the top-level BUILD.txt, listing cmake as a requirement?
        Hide
        Thiruvalluvan M. G. added a comment -

        Took care both the minor problems Doug noted and committed revision 1065352.

        Show
        Thiruvalluvan M. G. added a comment - Took care both the minor problems Doug noted and committed revision 1065352.

          People

          • Assignee:
            Thiruvalluvan M. G.
            Reporter:
            Thiruvalluvan M. G.
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development