diff --git a/.github/workflows/cam_sima_fortran_ci.yml b/.github/workflows/cam_sima_fortran_ci.yml index 36e1aa3b..aaf342fc 100644 --- a/.github/workflows/cam_sima_fortran_ci.yml +++ b/.github/workflows/cam_sima_fortran_ci.yml @@ -29,11 +29,13 @@ jobs: name: Check if jobs should run runs-on: ubuntu-slim outputs: - should-run: ${{ steps.conditional.outputs.result }} + # Encoded in base64 to preserve the matched paths exactly. + matched-paths: ${{ steps.conditional.outputs.matched-paths }} + should-run: ${{ steps.conditional.outputs.should-run }} timeout-minutes: 1 steps: - name: Checkout CAM-SIMA - uses: actions/checkout@v6 + uses: actions/checkout@v7 - name: Check if there are changes to Fortran source code id: conditional @@ -41,15 +43,21 @@ jobs: set -euo pipefail write_result_and_exit() { + if [ -f matched-paths.txt ]; then + echo "matched-paths=$(base64 -w 0 < matched-paths.txt)" >> "$GITHUB_OUTPUT" + else + echo "matched-paths=" >> "$GITHUB_OUTPUT" + fi + case "$1" in false) echo "Result: Skip jobs." - echo "result=$1" >> "$GITHUB_OUTPUT" + echo "should-run=$1" >> "$GITHUB_OUTPUT" exit 0 ;; true) echo "Result: Run jobs." - echo "result=$1" >> "$GITHUB_OUTPUT" + echo "should-run=$1" >> "$GITHUB_OUTPUT" exit 0 ;; error|*) @@ -114,7 +122,7 @@ jobs: echo "Finding changed paths between $BASE_COMMIT..$HEAD_COMMIT..." git diff --name-only "$BASE_COMMIT..$HEAD_COMMIT" | tee changed-paths.txt || write_result_and_exit error - if grep -E -q '(^\.github\/workflows\/cam_sima_fortran_ci\.yml$|^share$|^src\/.+$|^test\/unit\/.+$)' changed-paths.txt; then + if grep -E '^\.github/workflows/cam_sima_fortran_ci\.yml$|^share$|^src/.+$|^test/unit/.+$' changed-paths.txt > matched-paths.txt; then write_result_and_exit true else write_result_and_exit false @@ -129,6 +137,7 @@ jobs: name: Overall status (CAM-SIMA Fortran CI) needs: - conditional-check + - source-code-linting - unit-tests if: ${{ always() }} runs-on: ubuntu-slim @@ -167,6 +176,18 @@ jobs: ;; esac + case "${{ needs.source-code-linting.result }}" in + skipped|success) + : + ;; + cancelled|failure) + write_result_and_exit false + ;; + *) + write_result_and_exit error + ;; + esac + case "${{ needs.unit-tests.result }}" in skipped|success) : @@ -180,6 +201,73 @@ jobs: esac write_result_and_exit true + source-code-linting: + name: Lint Fortran source code + needs: conditional-check + if: ${{ needs.conditional-check.outputs.should-run == 'true' }} + runs-on: ubuntu-slim + env: + FORTITUDE_VERSION: 0.9.* + SOURCE_CODE_PATH: ${{ github.workspace }} + timeout-minutes: 10 + steps: + - name: Checkout CAM-SIMA + uses: actions/checkout@v7 + + - name: Setup Python + uses: actions/setup-python@v7 + with: + cache: pip + python-version: 3.12 + + - name: Install Fortitude + run: | + python3 -m venv venv-fortitude + source venv-fortitude/bin/activate + python3 -m pip install --upgrade pip + python3 -m pip install "fortitude-lint==$FORTITUDE_VERSION" + + - name: Lint Fortran source code + run: | + # Retrieve the matched paths. + echo "${{ needs.conditional-check.outputs.matched-paths }}" | base64 -d > matched-paths.txt + + # Filter Fortran files for Fortitude. + # Fortitude will choke if it encounters non-Fortran files in the command line arguments. + if grep -E '[^/]+\.(F|f|F90|f90|pf)$' matched-paths.txt > filtered-paths.txt; then + set -- + while IFS="" read -r LINE || [ -n "$LINE" ]; do + [ -f "$LINE" ] && set -- "$@" "$LINE" + done < filtered-paths.txt + + # Fortitude will still choke if there are no command line arguments. + if [ "$#" -gt 0 ]; then + source venv-fortitude/bin/activate + # First run: Annotated output for GitHub Actions. + fortitude \ + --config-file "$SOURCE_CODE_PATH/test/fortitude.toml" \ + check \ + --exit-zero \ + --output-format github \ + "$@" | sed -e "s|file=$SOURCE_CODE_PATH/|file=|g" + # Second run: Output to a log file for uploading. + fortitude \ + --config-file "$SOURCE_CODE_PATH/test/fortitude.toml" \ + check \ + --exit-zero \ + --output-file "$SOURCE_CODE_PATH/source-code-linting.log" \ + "$@" + fi + fi + + - name: Upload Fortran source code linting log + if: ${{ always() }} + uses: actions/upload-artifact@v7 + with: + if-no-files-found: ignore + name: source-code-linting-log + path: ${{ env.SOURCE_CODE_PATH }}/source-code-linting.log + retention-days: 7 unit-tests: name: Build and run unit tests (${{ matrix.compiler }}) needs: conditional-check @@ -218,14 +306,14 @@ jobs: timeout-minutes: 10 steps: - name: Checkout CAM-SIMA - uses: actions/checkout@v6 + uses: actions/checkout@v7 - name: Checkout dependencies run: | ./bin/git-fleximod update ncar-physics share - name: Checkout rrtmgp-data for PIO file reader tests - uses: actions/checkout@v6 + uses: actions/checkout@v7 with: repository: earth-system-radiation/rrtmgp-data ref: ${{ env.RRTMGP_DATA_REFERENCE }} diff --git a/.github/workflows/mpas_dynamical_core_ci.yml b/.github/workflows/mpas_dynamical_core_ci.yml index 541138f7..4128b1ba 100644 --- a/.github/workflows/mpas_dynamical_core_ci.yml +++ b/.github/workflows/mpas_dynamical_core_ci.yml @@ -29,11 +29,13 @@ jobs: name: Check if jobs should run runs-on: ubuntu-slim outputs: - should-run: ${{ steps.conditional.outputs.result }} + # Encoded in base64 to preserve the matched paths exactly. + matched-paths: ${{ steps.conditional.outputs.matched-paths }} + should-run: ${{ steps.conditional.outputs.should-run }} timeout-minutes: 1 steps: - name: Checkout CAM-SIMA - uses: actions/checkout@v6 + uses: actions/checkout@v7 - name: Check if there are changes to MPAS dynamical core id: conditional @@ -41,15 +43,21 @@ jobs: set -euo pipefail write_result_and_exit() { + if [ -f matched-paths.txt ]; then + echo "matched-paths=$(base64 -w 0 < matched-paths.txt)" >> "$GITHUB_OUTPUT" + else + echo "matched-paths=" >> "$GITHUB_OUTPUT" + fi + case "$1" in false) echo "Result: Skip jobs." - echo "result=$1" >> "$GITHUB_OUTPUT" + echo "should-run=$1" >> "$GITHUB_OUTPUT" exit 0 ;; true) echo "Result: Run jobs." - echo "result=$1" >> "$GITHUB_OUTPUT" + echo "should-run=$1" >> "$GITHUB_OUTPUT" exit 0 ;; error|*) @@ -114,7 +122,7 @@ jobs: echo "Finding changed paths between $BASE_COMMIT..$HEAD_COMMIT..." git diff --name-only "$BASE_COMMIT..$HEAD_COMMIT" | tee changed-paths.txt || write_result_and_exit error - if grep -E -q '(^\.github\/workflows\/mpas_dynamical_core_ci\.yml$|^src\/dynamics\/mpas\/.+$)' changed-paths.txt; then + if grep -E '^\.github/workflows/mpas_dynamical_core_ci\.yml$|^src/dynamics/mpas/.+$' changed-paths.txt > matched-paths.txt; then write_result_and_exit true else write_result_and_exit false @@ -199,15 +207,15 @@ jobs: if: ${{ needs.conditional-check.outputs.should-run == 'true' }} runs-on: ubuntu-slim env: - FORTITUDE_VERSION: 0.7.* + FORTITUDE_VERSION: 0.9.* SOURCE_CODE_PATH: ${{ github.workspace }}/src/dynamics/mpas timeout-minutes: 10 steps: - name: Checkout CAM-SIMA - uses: actions/checkout@v6 + uses: actions/checkout@v7 - name: Setup Python - uses: actions/setup-python@v6 + uses: actions/setup-python@v7 with: cache: pip python-version: 3.12 @@ -216,6 +224,7 @@ jobs: run: | python3 -m venv venv-fortitude source venv-fortitude/bin/activate + python3 -m pip install --upgrade pip python3 -m pip install "fortitude-lint==$FORTITUDE_VERSION" - name: Lint Fortran source code @@ -226,7 +235,6 @@ jobs: check \ --exit-zero \ --output-file "$SOURCE_CODE_PATH/source-code-linting.log" \ - --preview \ "$SOURCE_CODE_PATH" cat "$SOURCE_CODE_PATH/source-code-linting.log" @@ -267,7 +275,7 @@ jobs: timeout-minutes: 10 steps: - name: Checkout CAM-SIMA - uses: actions/checkout@v6 + uses: actions/checkout@v7 - name: Build unit tests run: | diff --git a/src/control/cam_logfile.F90 b/src/control/cam_logfile.F90 index 0316297b..4604a691 100644 --- a/src/control/cam_logfile.F90 +++ b/src/control/cam_logfile.F90 @@ -12,12 +12,13 @@ module cam_logfile !----------------------------------------------------------------------- !- use statements ------------------------------------------------------ !----------------------------------------------------------------------- + use, intrinsic :: iso_fortran_env, only: output_unit + !----------------------------------------------------------------------- !- module boilerplate -------------------------------------------------- !----------------------------------------------------------------------- implicit none private - save !----------------------------------------------------------------------- ! Public interfaces ---------------------------------------------------- @@ -35,7 +36,7 @@ module cam_logfile integer, public, protected :: debug_output = DEBUGOUT_NONE !> \section arg_table_cam_logfile Argument Table !! \htmlinclude cam_logfile.html - integer, public, protected :: iulog = 6 + integer, public, protected :: iulog = output_unit logical, public, protected :: log_output = .false. !----------------------------------------------------------------------- @@ -48,7 +49,7 @@ module cam_logfile module procedure cam_log_multiwrite_nr8 ! Multiple 8-byte reals end interface cam_log_multiwrite -CONTAINS +contains !----------------------------------------------------------------------- ! Subroutines and functions -------------------------------------------- @@ -72,6 +73,7 @@ end subroutine cam_set_log_unit subroutine cam_logfile_readnl(nlfile) use mpi, only: mpi_integer + use shr_kind_mod, only: cx => shr_kind_cx use shr_nl_mod, only: find_group_name => shr_nl_find_group_name use spmd_utils, only: mpicom, masterprocid, masterproc @@ -79,6 +81,7 @@ subroutine cam_logfile_readnl(nlfile) character(len=*), intent(in) :: nlfile ! Local variables + character(len=cx) :: cerr integer :: unitn integer :: ierr @@ -92,13 +95,14 @@ subroutine cam_logfile_readnl(nlfile) log_output = masterproc if (masterproc) then - open(newunit=unitn, file=trim(nlfile), status='old') + open(newunit=unitn, action='read', file=trim(nlfile), status='old') call find_group_name(unitn, 'cam_logfile_nl', status=ierr) if (ierr == 0) then - read(unitn, cam_logfile_nl, iostat=ierr) + read(unitn, cam_logfile_nl, iomsg=cerr, iostat=ierr) if (ierr /= 0) then ! Can't call endrun because of dependency loop - write(iulog, *) subname, ': ERROR: reading namelist' + write(iulog, *) subname, ': ERROR: reading namelist' // new_line('') // & + trim(adjustl(cerr)) end if end if close(unitn) @@ -123,16 +127,16 @@ subroutine cam_logfile_readnl(nlfile) if (ierr /= 0) then ! Can't call endrun because of dependency loop ! But MPI usually crashes in Fortran - write(iulog, *) subname, ": ERROR: mpi_bcast: debug_output" + write(iulog, *) subname, ': ERROR: mpi_bcast: debug_output' end if end subroutine cam_logfile_readnl subroutine cam_log_multiwrite_ni(subname, headers, fmt_string, values) ! Print out values from every task + use mpi, only: mpi_integer use shr_sys_mod, only: shr_sys_flush use spmd_utils, only: mpicom, masterprocid, masterproc, npes - use mpi, only: mpi_integer ! Dummy arguments character(len=*), intent(in) :: subname @@ -165,10 +169,10 @@ end subroutine cam_log_multiwrite_ni subroutine cam_log_multiwrite_nr8(subname, headers, fmt_string, values) ! Print out values from every task - use iso_fortran_env, only: r8 => REAL64 + use, intrinsic :: iso_fortran_env, only: r8 => REAL64 + use mpi, only: mpi_real8 use shr_sys_mod, only: shr_sys_flush use spmd_utils, only: mpicom, masterprocid, masterproc, npes - use mpi, only: mpi_real8 ! Dummy arguments character(len=*), intent(in) :: subname diff --git a/src/dynamics/mpas/assets/fortitude_config.toml b/src/dynamics/mpas/assets/fortitude_config.toml index 3bcf5cdd..3c4f7701 100644 --- a/src/dynamics/mpas/assets/fortitude_config.toml +++ b/src/dynamics/mpas/assets/fortitude_config.toml @@ -1,29 +1,67 @@ +include = [ + '*.F', + '*.f', + '*.F90', + '*.f90', + '*.pf' +] + [check] -exclude = [ +extend-exclude = [ + 'src/dynamics/mpas/assets', 'src/dynamics/mpas/dycore' ] -file-extensions = [ - 'F', - 'f', - 'F90', - 'f90' -] -# When ignoring a linting rule, a reason should be provided. +force-exclude = true +# Do not apply or suggest any fixes automatically. Manual intervention is required. +fix = false +fixable = [] +# When ignoring a linting rule, a reason should be provided for documentation purposes. ignore = [ - 'C003', # Temporarily ignored due to lack of support for Fortran 2018 `implicit none (external)` statement - # in the NVIDIA HPC SDK as of version 25.9. + 'C003', # Temporarily ignored due to lack of support for the Fortran 2018 `implicit none (external)` statement + # in the NVIDIA HPC SDK as of version 25.11. 'C182', # Requiring separate allocations and deallocations for each variable is too restrictive. # Related variables can be grouped together at the discretion of developers. 'S102' # Prefer only one space instead of two. ] line-length = 132 output-format = 'grouped' +preview = true select = [ 'ALL' ] +target-std = 'f2018' + +[check.complexity] +max-args = 10 +max-complexity = 20 [check.exit-unlabelled-loops] allow-unnested-loops = true +[check.inconsistent-dimensions] +prefer-attribute = 'never' + +[check.incorrect-keyword-case] +keyword-case = 'lowercase' + +[check.invalid-tab] +indent-width = 4 + +[check.keyword-whitespace] +goto-with-space = false +inout-with-space = false + +[check.line-too-long] +ignore-comments = false + +[check.portability] +allow-cray-file-units = false + [check.strings] quotes = 'single' + +[check.use-statements] +allow-bare-use = [ + 'funit', + 'pfunit' +] diff --git a/test/fortitude.toml b/test/fortitude.toml new file mode 100644 index 00000000..a0169279 --- /dev/null +++ b/test/fortitude.toml @@ -0,0 +1,68 @@ +include = [ + '*.F', + '*.f', + '*.F90', + '*.f90', + '*.pf' +] + +[check] +extend-exclude = [ + 'src/dynamics/mpas' +] +force-exclude = true +# Do not apply or suggest any fixes automatically. Manual intervention is required. +fix = false +fixable = [] +# When ignoring a linting rule, a reason should be provided for documentation purposes. +ignore = [ + 'C003', # Temporarily ignored due to lack of support for the Fortran 2018 `implicit none (external)` statement + # in the NVIDIA HPC SDK as of version 25.11. + 'C182', # Requiring separate allocations and deallocations for each variable is too restrictive. + # Related variables can be grouped together at the discretion of developers. + 'MOD201', # Switching to the `mpi_f08` module is planned post-CESM3. Re-enable this rule when the time comes. + 'S102', # Prefer only one space instead of two. + 'S271' # Allow `use` statements to be grouped by functionality at the discretion of developers. +] +line-length = 132 +output-format = 'grouped' +preview = true +select = [ + 'ALL' +] +target-std = 'f2018' + +[check.complexity] +max-args = 20 +max-complexity = 20 + +[check.exit-unlabelled-loops] +allow-unnested-loops = true + +[check.inconsistent-dimensions] +prefer-attribute = 'never' + +[check.incorrect-keyword-case] +keyword-case = 'lowercase' + +[check.invalid-tab] +indent-width = 4 + +[check.keyword-whitespace] +goto-with-space = false +inout-with-space = false + +[check.line-too-long] +ignore-comments = false + +[check.portability] +allow-cray-file-units = false + +[check.strings] +quotes = 'single' + +[check.use-statements] +allow-bare-use = [ + 'funit', + 'pfunit' +]