Avro
  1. Avro
  2. AVRO-440

config.h output not correctly used

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 1.3.0
    • Fix Version/s: 1.3.1
    • Component/s: c
    • Labels:
      None

      Description

      While config.h is generated, it is only included from within st.c to make some things work correctly within st.h.

      I would suggest changing things a little:

      • Put an include of config.h into src/avro_private.h
      • Include avro_private.h into all .c files.
      • Not sure if the values from config.h are needed in any of the tests or examples ... I would hope not though and that this is fully insulated from being visible within anything exposed by avro.h.

      Given some feedback, I can readily prepare a patch.

        Activity

        Hide
        Matt Massie added a comment -

        Ignore that last comment... wrong Jira. Sigh. Doing too many things at once.

        Show
        Matt Massie added a comment - Ignore that last comment... wrong Jira. Sigh. Doing too many things at once.
        Hide
        Matt Massie added a comment -

        This patch doesn't work, as is, on CentOS 5.4 using cmake 2.4.

        I was able to get it to build using the following changes:

        diff --git a/lang/c/CMakeLists.txt b/lang/c/CMakeLists.txt
        index e675154..df596c6 100644
        --- a/lang/c/CMakeLists.txt
        +++ b/lang/c/CMakeLists.txt
        @@ -16,7 +16,7 @@
         # specific language governing permissions and limitations
         # under the License.
         #
        -cmake_minimum_required(VERSION 2.6)
        +cmake_minimum_required(VERSION 2.4)
         project(AvroC)
         enable_testing()
         file(READ "${CMAKE_CURRENT_SOURCE_DIR}/../../share/VERSION.txt" AVRO_VERSION)
        @@ -25,9 +25,9 @@ if(APPLE)
             set(CMAKE_OSX_ARCHITECTURES "ppc;i386;x86_64" CACHE STRING "Build architectures for Mac OS X" FORCE) 
         endif(APPLE)
         
        -if(CMAKE_COMPILER_IS_GNUCC)
        -    set(WARNING_FLAGS "-W -Wall")
        -endif(CMAKE_COMPILER_IS_GNUCC)
        +#if(CMAKE_COMPILER_IS_GNUCC)
        +#    set(WARNING_FLAGS "-W -Wall")
        +#endif(CMAKE_COMPILER_IS_GNUCC)
         set(CMAKE_C_FLAGS ${CMAKE_C_FLAGS} ${WARNING_FLAGS})
         
         include_directories(${AvroC_SOURCE_DIR}/src)
        

        Since I know very little about CMake it would be nice to know the Right way to do this.

        Show
        Matt Massie added a comment - This patch doesn't work, as is, on CentOS 5.4 using cmake 2.4. I was able to get it to build using the following changes: diff --git a/lang/c/CMakeLists.txt b/lang/c/CMakeLists.txt index e675154..df596c6 100644 --- a/lang/c/CMakeLists.txt +++ b/lang/c/CMakeLists.txt @@ -16,7 +16,7 @@ # specific language governing permissions and limitations # under the License. # -cmake_minimum_required(VERSION 2.6) +cmake_minimum_required(VERSION 2.4) project(AvroC) enable_testing() file(READ "${CMAKE_CURRENT_SOURCE_DIR}/../../share/VERSION.txt" AVRO_VERSION) @@ -25,9 +25,9 @@ if (APPLE) set(CMAKE_OSX_ARCHITECTURES "ppc;i386;x86_64" CACHE STRING "Build architectures for Mac OS X" FORCE) endif(APPLE) - if (CMAKE_COMPILER_IS_GNUCC) - set(WARNING_FLAGS "-W -Wall" ) -endif(CMAKE_COMPILER_IS_GNUCC) +# if (CMAKE_COMPILER_IS_GNUCC) +# set(WARNING_FLAGS "-W -Wall" ) +#endif(CMAKE_COMPILER_IS_GNUCC) set(CMAKE_C_FLAGS ${CMAKE_C_FLAGS} ${WARNING_FLAGS}) include_directories(${AvroC_SOURCE_DIR}/src) Since I know very little about CMake it would be nice to know the Right way to do this.
        Hide
        Matt Massie added a comment -

        Committed to trunk

        Show
        Matt Massie added a comment - Committed to trunk
        Hide
        Bruce Mitchener added a comment -

        Apply AVRO-452's patch and then this one ... it will allow the use of stuff via config.h in an autotools build without messing up a CMake build. That said, nothing actually uses config.h at this point.

        Show
        Bruce Mitchener added a comment - Apply AVRO-452 's patch and then this one ... it will allow the use of stuff via config.h in an autotools build without messing up a CMake build. That said, nothing actually uses config.h at this point.

          People

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

            Dates

            • Created:
              Updated:
              Resolved:

              Development