NOAA-EMC / NCEPLIBS

Top level repo containing submodules for NCEPLIBS and associated dependencies for superproject builds
Other
42 stars 18 forks source link

w3nco build fail on cputim in summary.c #183

Closed rgrumbine closed 3 years ago

rgrumbine commented 3 years ago

Describe the bug Make of w3nco, fails because function 'times' is being seen as implicit declaration on MacOS 10.15, gfortran, gcc. No such function seems to exist.

'times' is referenced only in cputim, in summary.c The argument to 'times' is a custom variable type.

To Reproduce cd Downloads/ git clone --recursive https://github.com/NOAA-EMC/NCEPLIBS export FC=gfortran export CC=gcc export CXX=g++ cd NCEPLIBS mkdir build cd build cmake -DCMAKE_INSTALL_PREFIX=~/Downloads/ .. make -j w3nco

or make w3nco or make -i w3nco

Expected behavior Expect successful compile

Actual behavior [ 68%] Building C object src/CMakeFiles/w3nco_8_c.dir/summary.c.o /Users/rmg3/Downloads/NCEPLIBS/download/nceplibs-w3nco/src/summary.c:259:13: error: implicit declaration of function 'times' is invalid in C99 [-Werror,-Wimplicit-function-declaration] ret = times (&Time_buffer);

Build Information How did you build or access NCEPLIBS -- above, in how to reproduce

System On what system are you running the code? -- MacOS 10.15 ('catalina')

Additional context % gfortran --version GNU Fortran (Homebrew GCC 10.2.0_2) 10.2.0 Copyright (C) 2020 Free Software Foundation, Inc.

% gcc --version Configured with: --prefix=/Applications/Xcode.app/Contents/Developer/usr --with-gxx-include-dir=/Library/Developer/CommandLineTools/SDKs/MacOSX10.15.sdk/usr/include/c++/4.2.1 Apple clang version 12.0.0 (clang-1200.0.32.28) Target: x86_64-apple-darwin19.6.0 Thread model: posix

kgerheiser commented 3 years ago

I am able to reproduce this behavior. Though, it's not using GCC, it's Clang. Apple links symlinks GCC to Clang by default on macOS.

I can't even find where times is declared (or if it is at all).

@BoiVuong-NOAA do you know about this?

BoiVuong-NOAA commented 3 years ago

No, I do not know it. But, I found "times" used in summary.c. The program developed by IBM in 1999 for AIX system. Now, It is obsolete. Do you think we should remove it from W3EMC ? Now, W3NCO has combined into W3EMC.

kgerheiser commented 3 years ago

You know more than me. If it's not used then I think it should be removed.

I see that summary is called in w3tagb.f. If that's also not being used then w3tagb.f should also be removed.

If any of these functions were used there would probably be an undefined symbol which makes me think it's not being used anywhere.

rgrumbine commented 3 years ago

I would encourage deletion or replacement with something that is language standard-compliant.

kgerheiser commented 3 years ago

I think this reared its head because of macOS 11.0 and Apple Clang 12.0. We normally test on macOS but the update made this an error instead of a warning.

BoiVuong-NOAA commented 3 years ago

Kyle, It is using in w3tagb.f to report the RESOURCE STATISTICS. we do not need this information any more in newer machine such as DELL, CRAY or ACORN or HERA. We will remove summary.c in next version of W3EMC.

rgrumbine commented 3 years ago

I'm on OS 10.15. But yes, I think as part of the preparations for 11.0, Apple made the standard compliance error instead of warning. It'd be a good test in general to compile the libs with language standard compliance flags. Alas, the flags differ between compilers.

BoiVuong-NOAA commented 3 years ago

There is a FORTRAN summary.f in W3EMC. In the mean time, You can replace (summary.c) in w3nco with Fortran version summary.f

subroutine summary end subroutine

rgrumbine commented 3 years ago

replacing summary.c with summary.f did not work as the build system expects summary.c and gave error message complaint.

but commenting out the interior code in summary.c and adding:

usr = 0; sys = 0;

allowed compilation of the library to complete.

Thanks

kgerheiser commented 3 years ago

My fix here: https://github.com/NOAA-EMC/NCEPLIBS-w3emc/pull/62

Was to just remove summary.c and w3tagb.f completely

rgrumbine commented 3 years ago

Yes, the better long term solution. But this will require updates to the build system, which expects summary.c, and I didn't want to mess with.

kgerheiser commented 3 years ago

Going to close this issue as we have fixed it in the source library.