Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Overhaul CMake LFS support #4122

Merged
merged 1 commit into from
Mar 12, 2024
Merged

Overhaul CMake LFS support #4122

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
113 changes: 37 additions & 76 deletions config/cmake/ConfigureChecks.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unclear which Windows systems have a system name that matches "Linux" that we'd care about

# 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)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In line with the comment above, any idea when and why we switched to the 2008 POSIX standard?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's commented above. We moved it to 2008 to support pread/write back in 2019.

f833001


# 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)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These symbols will be redundant but harmless on 64-bit systems, but might be needed on 32-bit systems


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)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't use off64_t anywhere

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
Copy link
Member Author

@derobins derobins Mar 12, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I looked this up since I was curious, but I don't have a system on which to test either of these. Our Power system is Linux and the Solaris machine is dead.


#-----------------------------------------------------------------------------
# Check the size in bytes of all the int and float types
Expand Down Expand Up @@ -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)

#-----------------------------------------------------------------------------
Expand Down
3 changes: 0 additions & 3 deletions config/cmake/H5pubconf.h.in
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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@
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Though no one's likely to be using this, it still seems like it's a breaking change when we remove things from H5pubconf for a release. Not sure?

Copy link
Member Author

@derobins derobins Mar 12, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's not a documented API thing. We don't guarantee build system variables stay stable, so not breaking. We change this stuff all the time.

It does drive me bananas, though. I've been trying to find time to separate H5pubconf.h into h5config.h (with build-time stuff) and H5features.h (with things like H5_HAVE_PARALLEL) for years. Deploying config.h is considered bad practice, even when doctored up with prefixes.


/* The size of `off_t', as computed by sizeof. */
#cmakedefine H5_SIZEOF_OFF_T @H5_SIZEOF_OFF_T@

Expand Down
48 changes: 0 additions & 48 deletions config/cmake/HDFTests.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down Expand Up @@ -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>
Expand Down
34 changes: 34 additions & 0 deletions release_docs/RELEASE.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 0 additions & 2 deletions src/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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}>"
Expand Down Expand Up @@ -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
Expand Down
Loading