Overhaul CMake LFS support (HDFGroup#4122)

Externally visible: * The HDF_ENABLE_LARGE_FILE option (advanced) has been removed * We no longer run a test program to determine if LFS works, which will help with cross-compiling * On Linux we now unilaterally set -D_LARGEFILE_SOURCE and -D_FILE_OFFSET_BITS=64, regardless of 32/64 bit system. CMake doesn't offer a nice equivalent to AC_SYS_LARGEFILE and since those options do nothing on 64-bit systems, this seems safe and covers all our bases. We don't set -D_LARGEFILE64_SOURCE since we don't use any of the POSIX 64-bit specific API calls like ftello64, as noted above. * We didn't test for LFS support on non-Linux platforms. We've added comments for how LFS should probably be supported on AIX and Solaris, which seem to be alive, though uncommon. PRs would be appreciated if anyone wishes to test this. Internal: * Drops off64_t size checks since this is unused (as in Autotools) * Remove HDF_EXTRA_FLAGS, which is now unused * Remove hack around deprecated LINUX_LFS Fixes HDFGroup#2395
bmribler · Mar 12, 2024 · 56f1092 · 56f1092
1 parent 27f7318
commit 56f1092
Show file tree

Hide file tree

Showing 5 changed files with 71 additions and 129 deletions.
diff --git a/config/cmake/ConfigureChecks.cmake b/config/cmake/ConfigureChecks.cmake
@@ -212,86 +212,51 @@ macro (HDF_FUNCTION_TEST OTHER_TEST)
 endmacro ()
 
 #-----------------------------------------------------------------------------
-#  Check for large file support
+#  Platform-specific flags
 #-----------------------------------------------------------------------------
 
-# The linux-lfs option is deprecated.
-set (LINUX_LFS 0)
-
 set (HDF_EXTRA_C_FLAGS)
-set (HDF_EXTRA_FLAGS)
-if (MINGW OR NOT WINDOWS)
-  if (CMAKE_SYSTEM_NAME MATCHES "Linux")
-    # Linux Specific flags
-    # This was originally defined as _POSIX_SOURCE which was updated to
-    # _POSIX_C_SOURCE=199506L to expose a greater amount of POSIX
-    # functionality so clock_gettime and CLOCK_MONOTONIC are defined
-    # correctly. This was later updated to 200112L so that
-    # posix_memalign() is visible for the direct VFD code on Linux
-    # systems.
-    # POSIX feature information can be found in the gcc manual at:
-    # http://www.gnu.org/s/libc/manual/html_node/Feature-Test-Macros.html
-    set (HDF_EXTRA_C_FLAGS -D_POSIX_C_SOURCE=200809L)
-
-    # Need to add this so that O_DIRECT is visible for the direct
-    # VFD on Linux systems.
-    set (HDF_EXTRA_C_FLAGS ${HDF_EXTRA_C_FLAGS} -D_GNU_SOURCE)
-
-    option (HDF_ENABLE_LARGE_FILE "Enable support for large (64-bit) files on Linux." ON)
-    mark_as_advanced (HDF_ENABLE_LARGE_FILE)
-    if (HDF_ENABLE_LARGE_FILE AND NOT DEFINED TEST_LFS_WORKS_RUN)
-      set (msg "Performing TEST_LFS_WORKS")
-      try_run (TEST_LFS_WORKS_RUN   TEST_LFS_WORKS_COMPILE
-          ${CMAKE_BINARY_DIR}
-          ${HDF_RESOURCES_DIR}/HDFTests.c
-          COMPILE_DEFINITIONS "-DTEST_LFS_WORKS"
-      )
 
-      # The LARGEFILE definitions were from the transition period
-      # and are probably no longer needed. The FILE_OFFSET_BITS
-      # check should be generalized for all POSIX systems as it
-      # is in the Autotools.
-      if (TEST_LFS_WORKS_COMPILE)
-        if (TEST_LFS_WORKS_RUN MATCHES 0)
-          set (TEST_LFS_WORKS 1 CACHE INTERNAL ${msg})
-          set (LARGEFILE 1)
-          set (HDF_EXTRA_FLAGS ${HDF_EXTRA_FLAGS} -D_FILE_OFFSET_BITS=64 -D_LARGEFILE64_SOURCE -D_LARGEFILE_SOURCE)
-          message (VERBOSE "${msg}... yes")
-        else ()
-          set (TEST_LFS_WORKS "" CACHE INTERNAL ${msg})
-          message (VERBOSE "${msg}... no")
-          file (APPEND ${CMAKE_BINARY_DIR}${CMAKE_FILES_DIRECTORY}/CMakeError.log
-                "Test TEST_LFS_WORKS Run failed with the following exit code:\n ${TEST_LFS_WORKS_RUN}\n"
-          )
-        endif ()
-      else ()
-        set (TEST_LFS_WORKS "" CACHE INTERNAL ${msg})
-        message (VERBOSE "${msg}... no")
-        file (APPEND ${CMAKE_BINARY_DIR}${CMAKE_FILES_DIRECTORY}/CMakeError.log
-            "Test TEST_LFS_WORKS Compile failed\n"
-        )
-      endif ()
-    endif ()
-    set (CMAKE_REQUIRED_DEFINITIONS ${CMAKE_REQUIRED_DEFINITIONS} ${HDF_EXTRA_FLAGS})
-  endif ()
+# Linux-specific flags
+if (CMAKE_SYSTEM_NAME MATCHES "Linux")
+  # This was originally defined as _POSIX_SOURCE which was updated to
+  # _POSIX_C_SOURCE=199506L to expose a greater amount of POSIX
+  # functionality so clock_gettime and CLOCK_MONOTONIC are defined
+  # correctly. This was later updated to 200112L so that
+  # posix_memalign() is visible for the direct VFD code on Linux
+  # systems.
+  # POSIX feature information can be found in the gcc manual at:
+  # http://www.gnu.org/s/libc/manual/html_node/Feature-Test-Macros.html
+  set (HDF_EXTRA_C_FLAGS -D_POSIX_C_SOURCE=200809L)
+
+  # Need to add this so that O_DIRECT is visible for the direct
+  # VFD on Linux systems.
+  set (HDF_EXTRA_C_FLAGS ${HDF_EXTRA_C_FLAGS} -D_GNU_SOURCE)
+
+  # Set up large file support. This is only necessary on 32-bit systems
+  # but is used on all Linux systems. It has no effect on 64-bit systems
+  # so it's not worth hacking up a 32/64-bit test to selectively include it.
+  #
+  # The library currently does not use any of the 64-flavored API calls
+  # or types
+  set (HDF_EXTRA_C_FLAGS ${HDF_EXTRA_C_FLAGS} -D_LARGEFILE_SOURCE)
+  set (HDF_EXTRA_C_FLAGS ${HDF_EXTRA_C_FLAGS} -D_FILE_OFFSET_BITS=64)
+
+  set (CMAKE_REQUIRED_DEFINITIONS ${CMAKE_REQUIRED_DEFINITIONS} ${HDF_EXTRA_C_FLAGS})
 endif ()
 
-#-----------------------------------------------------------------------------
-# Check for HAVE_OFF64_T functionality
-#-----------------------------------------------------------------------------
-if (MINGW OR NOT WINDOWS)
-  HDF_FUNCTION_TEST (HAVE_OFF64_T)
-  if (${HDF_PREFIX}_HAVE_OFF64_T)
-    CHECK_FUNCTION_EXISTS (lseek64            ${HDF_PREFIX}_HAVE_LSEEK64)
-  endif ()
+# As of 2024, both AIX and Solaris are uncommon, but still exist! The default
+# compiler options are also often set to -m32, which produces 32-bit binaries.
 
-  CHECK_FUNCTION_EXISTS (fseeko               ${HDF_PREFIX}_HAVE_FSEEKO)
+# 32-bit AIX compiles might require _LARGE_FILES, but we don't have a system on
+# which to test this (yet).
+#
+# https://www.ibm.com/docs/en/aix/7.1?topic=volumes-writing-programs-that-access-large-files
 
-  CHECK_STRUCT_HAS_MEMBER("struct stat64" st_blocks "sys/types.h;sys/stat.h" HAVE_STAT64_STRUCT)
-  if (HAVE_STAT64_STRUCT)
-    CHECK_FUNCTION_EXISTS (stat64             ${HDF_PREFIX}_HAVE_STAT64)
-  endif ()
-endif ()
+# 32-bit Solaris probably needs _LARGEFILE_SOURCE and _FILE_OFFSET_BITS=64,
+# as in Linux, above.
+#
+# https://docs.oracle.com/cd/E23824_01/html/821-1474/lfcompile-5.html
 
 #-----------------------------------------------------------------------------
 #  Check the size in bytes of all the int and float types
@@ -358,10 +323,6 @@ if (MINGW OR NOT WINDOWS)
 endif ()
 
 HDF_CHECK_TYPE_SIZE (off_t          ${HDF_PREFIX}_SIZEOF_OFF_T)
-HDF_CHECK_TYPE_SIZE (off64_t        ${HDF_PREFIX}_SIZEOF_OFF64_T)
-if (NOT ${HDF_PREFIX}_SIZEOF_OFF64_T)
-  set (${HDF_PREFIX}_SIZEOF_OFF64_T 0)
-endif ()
 HDF_CHECK_TYPE_SIZE (time_t          ${HDF_PREFIX}_SIZEOF_TIME_T)
 
 #-----------------------------------------------------------------------------

diff --git a/config/cmake/H5pubconf.h.in b/config/cmake/H5pubconf.h.in
@@ -538,9 +538,6 @@
  #define H5_SIZEOF_LONG_LONG 8
 #endif
 
-/* The size of `off64_t', as computed by sizeof. */
-#cmakedefine H5_SIZEOF_OFF64_T @H5_SIZEOF_OFF64_T@
-
 /* The size of `off_t', as computed by sizeof. */
 #cmakedefine H5_SIZEOF_OFF_T @H5_SIZEOF_OFF_T@
 

diff --git a/config/cmake/HDFTests.c b/config/cmake/HDFTests.c
@@ -89,17 +89,6 @@ int main ()
 
 #endif /* DEV_T_IS_SCALAR */
 
-#ifdef HAVE_OFF64_T
-
-#include <sys/types.h>
-
-int main(void)
-{
-    off64_t n = 0;
-    return (int)n;
-}
-#endif
-
 #ifdef TEST_DIRECT_VFD_WORKS
 
 #include <sys/types.h>
@@ -138,43 +127,6 @@ main(void)
 }
 #endif
 
-#ifdef TEST_LFS_WORKS
-
-/* Return 0 when LFS is available and 1 otherwise.  */
-
-#define _LARGEFILE_SOURCE
-#define _LARGEFILE64_SOURCE
-#define _LARGE_FILES
-#define _FILE_OFFSET_BITS 64
-
-#include <sys/types.h>
-#include <sys/stat.h>
-#include <assert.h>
-#include <stdio.h>
-
-#define OFF_T_64 (((off_t) 1 << 62) - 1 + ((off_t) 1 << 62))
-
-int main(int argc, char **argv)
-{
-
-    /* Check that off_t can hold 2^63 - 1 and perform basic operations... */
-    if (OFF_T_64 % 2147483647 != 1)
-        return 1;
-
-    /* stat breaks on SCO OpenServer */
-    struct stat buf;
-    stat(argv[0], &buf);
-    if (!S_ISREG(buf.st_mode))
-        return 2;
-
-    FILE *file = fopen(argv[0], "r");
-    off_t offset = ftello(file);
-    fseek(file, offset, SEEK_CUR);
-    fclose(file);
-    return 0;
-}
-#endif
-
 #ifdef HAVE_IOEO
 
 #include <windows.h>

diff --git a/release_docs/RELEASE.txt b/release_docs/RELEASE.txt
@@ -47,6 +47,40 @@ New Features
 
     Configuration:
     -------------
+    - Overhauled LFS support checks
+
+      In 2024, we can assume that Large File Support (LFS) exists on all
+      systems we support, though it may require flags to enable it,
+      particularly when building 32-bit binaries. The HDF5 source does
+      not use any of the 64-bit specific API calls (e.g., ftello64)
+      or explicit 64-bit offsets via off64_t.
+
+      Autotools
+
+      * We now use AC_SYS_LARGEFILE to determine how to support LFS. We
+        previously used a custom m4 script for this.
+
+      CMake
+
+      * The HDF_ENABLE_LARGE_FILE option (advanced) has been removed
+      * We no longer run a test program to determine if LFS works, which
+        will help with cross-compiling
+      * On Linux we now unilaterally set -D_LARGEFILE_SOURCE and
+        -D_FILE_OFFSET_BITS=64, regardless of 32/64 bit system. CMake
+        doesn't offer a nice equivalent to AC_SYS_LARGEFILE and since
+        those options do nothing on 64-bit systems, this seems safe and
+        covers all our bases. We don't set -D_LARGEFILE64_SOURCE since
+        we don't use any of the POSIX 64-bit specific API calls like
+        ftello64, as noted above.
+      * We didn't test for LFS support on non-Linux platforms. We've added
+        comments for how LFS should probably be supported on AIX and Solaris,
+        which seem to be alive, though uncommon. PRs would be appreciated if
+        anyone wishes to test this.
+
+      This overhaul also fixes GitHub #2395, which points out that the LFS flags
+      used when building with CMake differ based on whether CMake has been
+      run before. The LFS check program that caused this problem no longer exists.
+
     - The CMake HDF5_ENABLE_DEBUG_H5B option has been removed
 
       This enabled some additional version-1 B-tree checks. These have been

diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt
@@ -1065,7 +1065,6 @@ if (BUILD_STATIC_LIBS)
   target_compile_definitions(${HDF5_LIB_TARGET}
       PUBLIC
           ${HDF_EXTRA_C_FLAGS}
-          ${HDF_EXTRA_FLAGS}
       PRIVATE
           "$<$<BOOL:${HDF5_ENABLE_TRACE}>:H5_DEBUG_API>" # Enable tracing of the API
           "$<$<BOOL:${HDF5_ENABLE_DEBUG_APIS}>:${HDF5_DEBUG_APIS}>"
@@ -1100,7 +1099,6 @@ if (BUILD_SHARED_LIBS)
       PUBLIC
           "H5_BUILT_AS_DYNAMIC_LIB"
           ${HDF_EXTRA_C_FLAGS}
-          ${HDF_EXTRA_FLAGS}
       PRIVATE
           "$<$<BOOL:${HDF5_ENABLE_THREADSAFE}>:H5_HAVE_THREADSAFE>"
           "$<$<BOOL:${HDF5_ENABLE_TRACE}>:H5_DEBUG_API>"  # Enable tracing of the API