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.

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

Fix fseek test #2055

Merged
merged 3 commits into from
Oct 14, 2024
Merged

Fix fseek test #2055

merged 3 commits into from
Oct 14, 2024

Conversation

islas
Copy link
Collaborator

@islas islas commented May 31, 2024

TYPE: bug fix

KEYWORDS: fseek, compilation, cmake

SOURCE: internal

DESCRIPTION OF CHANGES:
Problem:
The fseek test lacks correct syntax causing false negative reports of feature not existing when newer compiler standards disallow this.

Solution:
Add int to the main program in the fseek test. Also to add further robustness in detecting this feature, use the -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE=1 generally *nix standard defines.

LIST OF MODIFIED FILES:
M CMakeLists.txt
M confcheck/CMakeLists.txt
M tools/fseek_test.c

RELEASE NOTE: Fix fseek test

@islas islas requested review from a team as code owners May 31, 2024 20:44
@Plantain
Copy link

Plantain commented May 31, 2024

This seems to break the build in an odd way, or at least my build:

#11 21.86 -- WRF_CONFIG           : /home/wrf/WRF/_build/wrf_config.cmake
#11 21.86 -- CMAKE_BUILD_TYPE     : Release
#11 21.86 -- WRF_CORE             : ARW
#11 21.86 -- WRF_NESTING          : BASIC
#11 21.86 -- WRF_CASE             : EM_REAL
#11 21.86 -- USE_DOUBLE           : OFF
#11 21.86 -- USE_MPI              : ON
#11 21.86 -- USE_OPENMP           : OFF
#11 21.86 -- USE_IPO              : OFF
#11 21.86 -- ENABLE_CHEM          : OFF
#11 21.86 -- ENABLE_CLM           :
#11 21.86 -- ENABLE_CMAQ          : OFF
#11 21.86 -- ENABLE_DFI_RADAR     : OFF
#11 21.86 -- ENABLE_HYDRO         : OFF
#11 21.86 -- ENABLE_KPP           : OFF
#11 21.86 -- ENABLE_MARS          : OFF
#11 21.86 -- ENABLE_TERRAIN       : OFF
#11 21.86 -- ENABLE_TITAN         : OFF
#11 21.86 -- ENABLE_VENUS         : OFF
#11 21.86 -- USE_ALLOCATABLES     : ON
#11 21.86 -- wrfmodel             : ON
#11 21.86 -- GRIB1                : ON
#11 21.86 -- INTIO                : ON
#11 21.86 -- KEEP_INT_AROUND      : ON
#11 21.86 -- LIMIT_ARGS           : ON
#11 21.86 -- FORCE_NETCDF_CLASSIC  : ON
#11 21.86 -- BUILD_RRTMG_FAST     : ON
#11 21.86 -- BUILD_RRTMK          : OFF
#11 21.86 -- BUILD_SBM_FAST       : ON
#11 21.86 -- SHOW_ALL_VARS_USED   : OFF
#11 21.86 -- WRFIO_NCD_NO_LARGE_FILE_SUPPORT      : OFF
#11 21.86 -- Performing Test Fortran_2003_IEEE
#11 22.14 -- Performing Test Fortran_2003_IEEE - Success
#11 22.14 -- Performing Test Fortran_2003_ISO_C
#11 22.42 -- Performing Test Fortran_2003_ISO_C - Success
#11 22.43 -- Performing Test Fortran_2003_FLUSH
#11 22.70 -- Performing Test Fortran_2003_FLUSH - Success
#11 22.70 -- Performing Test Fortran_2003_GAMMA
#11 22.97 -- Performing Test Fortran_2003_GAMMA - Success
#11 22.97 -- Performing Test FSEEKO64
#11 23.09 -- Performing Test FSEEKO64 - Failed
#11 23.09 -- fseeko64 not supported, checking alternate fseeko
#11 23.09 -- Performing Test FSEEKO
#11 23.34 -- Performing Test FSEEKO - Success
#11 23.35 -- Adding [io_pnetcdf] to configuration
#11 23.36 -- Setting gen_comms to RSL_LITE
#11 23.40 -- Setting module_dm to RSL_LITE
#11 23.40 -- Configuring done (5.9s)
#11 23.58 -- Generating done (0.1s)
#11 23.59 -- Build files have been written to: /home/wrf/WRF/_build
#11 23.61 Using default build directory : /home/wrf/WRF/_build
#11 23.74 [  0%] Preprocessing md_calls
#11 23.75 [  0%] Building C object external/io_grib_share/CMakeFiles/io_grib_share.dir/gridnav.c.o
#11 23.75 [  0%] Building C object external/io_grib_share/CMakeFiles/io_grib_share.dir/open_file.c.o
#11 23.75 [  0%] Building Fortran object external/io_int/CMakeFiles/io_int.dir/__/__/frame/module_internal_header_util.F.o
#11 23.75 [  0%] Building Fortran object external/io_grib_share/CMakeFiles/io_grib_share.dir/io_grib_share.F.o
#11 23.75 [  0%] Building C object external/io_grib_share/CMakeFiles/io_grib_share.dir/get_region_center.c.o
#11 23.75 [  0%] Preprocessing io_netcdf_c_preproc_wrf_io
#11 23.75 [  0%] Building Fortran object external/RSL_LITE/CMakeFiles/RSL_LITE.dir/f_pack.F90.o
#11 23.75 [  0%] Building C object tools/CMakeFiles/registry.dir/registry.c.o
#11 23.75 [  0%] Building C object external/io_grib1/MEL_grib1/CMakeFiles/MEL_grib1.dir/FTP_getfile.c.o
#11 23.75 [  0%] Preprocessing io_pnetcdf_c_preproc_wrf_io
#11 23.75 [  0%] Building C object external/io_grib1/grib1_util/CMakeFiles/grib1_util.dir/alloc_2d.c.o
#11 23.76 [  0%] Building Fortran object external/io_grib1/CMakeFiles/io_grib1.dir/io_grib1.F.o
#11 23.76 [  0%] Building C object external/io_grib1/WGRIB/CMakeFiles/WGRIB.dir/seekgrib.c.o
#11 23.76 [  0%] Building Fortran object external/esmf_time_f90/CMakeFiles/esmf_time_f90.dir/ESMF_Base.F90.o
#11 23.76 [  0%] Building Fortran object external/fftpack/fftpack5/CMakeFiles/fftpack5.dir/c1f2kb.F.o
#11 23.78 cpp: fatal error: too many input files

@islas
Copy link
Collaborator Author

islas commented May 31, 2024

@Plantain Thanks for the catch! The issue was some assumptions made in the use of cmake generator expressions. PR #2056 should fix that issue so that this will build properly.

@islas
Copy link
Collaborator Author

islas commented Aug 6, 2024

Requires #2056 and #2053

@amstokely amstokely self-requested a review September 18, 2024 22:35
amstokely
amstokely previously approved these changes Sep 18, 2024
Copy link
Collaborator

@amstokely amstokely left a comment

Choose a reason for hiding this comment

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

I’ll approve these changes, but I noticed that the use of void main() in fseek_test.c is not ISO C compliant. Is there any reason why the contents of main() can’t be moved to a new function, which can then be called from an int main()? This wouldn't alter the current code's behavior and would make it ISO C compliant.

@islas
Copy link
Collaborator Author

islas commented Sep 18, 2024

the use of void main() in fseek_test.c is not ISO C compliant

I didn't actually know that void return on main wasn't ISO C. I think we don't even need to move it to a function if instead of the exit( retval ) call we replace that with return retval

@mgduda
Copy link
Collaborator

mgduda commented Oct 11, 2024

@islas Can you update the PR description to note that main returns an int rather than void?

mgduda
mgduda previously approved these changes Oct 12, 2024
@islas
Copy link
Collaborator Author

islas commented Oct 14, 2024

Rebased to v4.6.1 to resolve conflicts from duplicate changes

@mgduda mgduda self-requested a review October 14, 2024 20:56
@amstokely amstokely self-requested a review October 14, 2024 21:07
@islas islas merged commit 2f844ad into wrf-model:release-v4.6.1 Oct 14, 2024
1 of 3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants