Make Intel config compiler check more robust

eirikurj commented 5 months ago

Purpose

Previous PR #30 updated the Intel config for the new compilers. However, nightly builds are failing on the u20-intel-impi-latest image with the following error. This was not detected in previous PR tests, since we do not run tests on this image.

#27 3.661 make[1]: Entering directory '/home/***/repos/pysurf'
#27 3.661 mkdir -p obj
#27 3.663 mkdir -p mod
#27 3.665 ln -sf config/config.mk config.mk
#27 3.666 ln -sf common_real.mk common.mk
#27 3.668 making discretesurf in src/common
#27 3.668 
#27 3.670 make[2]: Entering directory '/home/***/repos/pysurf/src/common'
#27 3.670 make[2]: command: Command not found
#27 3.671 make -j 4 precision.o
#27 3.673 make[3]: Entering directory '/home/***/repos/pysurf/src/common'
#27 3.673 make[3]: command: Command not found
#27 3.674 mpiifx -I../../mod -I/home/***/packages/CGNS-4.4.0/opt-intel/include -fPIC -r8 -O2 -g -std08 -c precision.F90 -o ../../obj/precision.o
#27 3.674 make[3]: mpiifx: Command not found
#27 3.674 make[3]: *** [../../rulesSources.mk:10: precision.o] Error 127
#27 3.675 make[3]: Leaving directory '/home/***/repos/pysurf/src/common'
#27 3.675 make[2]: *** [Makefile:22: all] Error 2
#27 3.675 make[2]: Leaving directory '/home/***/repos/pysurf/src/common'
#27 3.675 make[1]: *** [Makefile:46: discretesurf] Error 1
#27 3.675 make[1]: Leaving directory '/home/***/repos/pysurf'
#27 3.675 make: *** [Makefile:11: default] Error 1

The same error is also appearing in the build output of https://github.com/mdolab/docker/pull/266, for example see here (copied here for archiving), but works as the correct Intel compiler is set in the catch-all else block.

#23 0.741 Cloning into 'pyspline'...
#23 1.753 make[1]: Entering directory '/home/***/repos/pyspline'
#23 1.753 mkdir -p obj
#23 1.756 ln -sf config/config.mk config.mk
#23 1.759 making module in src/
#23 1.759 
#23 1.761 make[2]: Entering directory '/home/***/repos/pyspline/src'
#23 1.762 make[2]: command: Command not found
#23 1.764 make precision.o adtProjections.o

The issue is related to some shell optimization in make and built-in functions (see here for details) not running a proper shell.

There are a couple of approaches we can take

add a shell specific character to invoke a proper shell (done in this PR)
change command to which, but the latter is not a built-in function and thus invokes a proper shell.

I am fine with either option, but I suggest we converge here before opening PRs on all other repos

Expected time until merged

ASAP as nightly tests are failing

Type of change

[x] Bugfix (non-breaking change which fixes an issue)
[ ] New feature (non-breaking change which adds functionality)
[ ] Breaking change (non-backwards-compatible fix or feature)
[ ] Code style update (formatting, renaming)
[ ] Refactoring (no functional changes, no API changes)
[ ] Documentation update
[ ] Maintenance update
[ ] Other (please describe)

Testing

Test building (running make) with and without this bug fix

Checklist

[ ] I have run flake8 and black to make sure the Python code adheres to PEP-8 and is consistently formatted
[ ] I have formatted the Fortran code with fprettify or C/C++ code with clang-format as applicable
[ ] I have run unit and regression tests which pass locally with my changes
[ ] I have added new tests that prove my fix is effective or that my feature works
[ ] I have added necessary documentation

codecov[bot] commented 5 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 50.11%. Comparing base (bf73e00) to head (f1f8763).

Additional details and impacted files

```diff @@ Coverage Diff @@ ## main #31 +/- ## ======================================= Coverage 50.11% 50.11% ======================================= Files 5 5 Lines 1688 1688 ======================================= Hits 846 846 Misses 842 842 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

sseraj commented 5 months ago

This approach is fine with me, but I would like to add u20-intel-impi-latest to the Pipelines matrix to test the change. We can update the test matrix again after we merge https://github.com/mdolab/docker/pull/266

mdolab / pysurf