[jira] [Commented] (HADOOP-11997) CMake CMAKE_C_FLAGS are non-portable

["Colin Patrick McCabe (JIRA)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/HADOOP-11997?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14550691#comment-14550691 ] 

Colin Patrick McCabe commented on HADOOP-11997:
-----------------------------------------------

There is no harm in setting {{_GNU_SOURCE}}.  It only makes Linux-only functions available, but doesn't force you to use them.  And many Linux-only things like the non-POSIX strerror_r are defined even without {{_GNU_SOURCE}}.  It is very frustrating to work without {{_GNU_SOURCE}} since important definitions are often missing or provided in an incorrect form.

"-g -Wall -O2" is set because it was felt to be a balance between optimizing the code and keeping it debuggable.  I agree that setting "-g" is not traditional, but it is difficult enough to debug JNI with it, let alone without it.  If you want to support a separate native build without -g, please file a JIRA with that and provide the improved performance numbers and other motivation.

"D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64": failing to set these flags on Linux can be disastrous because it can result in some files running with the 32-bit definitions of off_t and so forth, and other files running with the 64-bit definitions.  If two files compiled with different settings pass the structures back and forth, you will get silent data corruption or crashes.  By your own admission, these flags have no relevance to Solaris, so I don't see why we are even considering changing it.

I think we should close this JIRA, since I don't see any changes here that would truly be helpful.  Perhaps the only change that might make sense is to avoid setting _GNU_SOURCE on Solaris.  But _GNU_SOURCE has no effect on Solaris, so this would be a no-op.  However it would be conceptually cleaner.

> CMake CMAKE_C_FLAGS are non-portable
> ------------------------------------
>
>                 Key: HADOOP-11997
>                 URL: https://issues.apache.org/jira/browse/HADOOP-11997
>             Project: Hadoop Common
>          Issue Type: Sub-task
>          Components: build
>    Affects Versions: 2.7.0
>         Environment: All
>            Reporter: Alan Burlison
>            Assignee: Alan Burlison
>            Priority: Critical
>
> hadoop-common-project/hadoop-common/src/CMakeLists.txt (https://github.com/apache/hadoop/blob/trunk/hadoop-common-project/hadoop-common/src/CMakeLists.txt#L110) contains the following unconditional assignments to CMAKE_C_FLAGS:
> set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -g -Wall -O2")
> set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -D_REENTRANT -D_GNU_SOURCE")
> set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64")
> There are several issues here:
> 1. "-D_GNU_SOURCE" globally enables the use of all Linux-only extensions in hadoop-common native source. This is probably a major contributor to the poor cross-platform portability of Hadoop native code to non-Linux platforms as it makes it easy for developers to use non-portable Linux features without realising. Use of Linux-specific features should be correctly bracketed with conditional macro blocks that provide an alternative for non-Linux platforms.
> 2. "-g -Wall -O2" turns on debugging for all builds, I believe the correct mechanism is to set the CMAKE_BUILD_TYPE CMake variable. If it is still necessary to override CFLAGS it should probably be done conditionally dependent on the value of CMAKE_BUILD_TYPE.
> 3. "-D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64" On Solaris these flags are only needed for largefile support in ILP32 applications, LP64 applications are largefile by default. I believe the same is true on Linux, so these flags are harmless but redundant for 64-bit compilation.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)