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

Test/document version support, miscellanous #3

Merged
merged 4 commits into from
Nov 4, 2022

Conversation

wpbonelli
Copy link
Contributor

@wpbonelli wpbonelli commented Oct 28, 2022

Hi, thanks for the action. I have a few changes on a fork and wondered if you might be interested in pulling any upstream:

  • expand CI test matrix across runner OS and gcc versions
  • test runner compatibility daily (sample run)
  • add runner/gcc version compatibility table to README
  • add CI badge to README
  • replace soon-deprecated set-output mechanism with GITHUB_OUTPUT file
  • use Chocolatey to install mingw on Windows (considering caching this...)
  • add msys* and cygwin cases for detected platform (can be returned by uname -s on Windows)
  • test pwsh (Powershell Core), powershell (Powershell Desktop) and cmd on Windows
  • link to libgfortran-5.dll on Windows (avoid missing DLL issues)
  • link to older gcc lib paths on Mac (avoid missing *.dylib issues)
  • inline setup step, source platform-specific functions from setup-fortran.sh

Also saw the note on supporting the Intel toolchain — currently accomplishing this with modflowpy/install-intelfortran-action, perhaps something could be merged here? It would be nice to have one action for GNU or Intel fortran compilers. (It's now possible to cache files and update environment variables in composite actions.)

.github/workflows/test.yml Outdated Show resolved Hide resolved
@wpbonelli
Copy link
Contributor Author

wpbonelli commented Oct 30, 2022

Fixed CI continue-on-error config so jobs stay green even if runner/toolchain are incompatible (sample run). Also added a mechanism to detect compatibility changes — the compat job diffs the compat.csv generated by the test matrix with one stored in the repo (if any), and a PR is opened into the default branch to update the CSV and README if changes are found (example)

* expand test matrix
* add version compatibility table to README
* replace set-output with GITHUB_OUTPUT file (pending deprecation)
* add tests for pwsh, powershell and cmd on Windows
* add cases for uname on Windows: msys* and cygwin*
* use Chocolatey to install mingw on Windows
* link to libgfortran-5.dll on Windows
* link to older version lib paths on Mac
* store runner compatibility in CSV file
* autodetect runner compatiblity changes
* autocreate PR to update README on compat changes
@wpbonelli wpbonelli marked this pull request as ready for review October 30, 2022 23:04
* add table of contents
* add sections: outputs, environment variables
@wpbonelli
Copy link
Contributor Author

wpbonelli commented Nov 1, 2022

Looks like CI may have failed due to restrictions on the automatically provided GITHUB_TOKEN. I've updated the compat job with write permission on PRs and repo contents as documented here.

I'm not sure this will pass in this PR however, as the link above states the maximum access level for forks is read. To succeed, the workflow may need to be triggered by a branch or event associated with this repo rather than my fork.

@awvwgk
Copy link
Member

awvwgk commented Nov 2, 2022

You can exclude the update step from pull requests, it only makes sense to change the README on a push event.

@wpbonelli
Copy link
Contributor Author

Happy to try Intel support here or a separate PR, if you still have that in mind. Could select just C/C++ and Fortran components from the HPC kit to reduce download size — the action could minimally guarantee C/C++/Fortran compilers for either toolchain?

@awvwgk
Copy link
Member

awvwgk commented Nov 4, 2022

For the Intel installation the installation procedure provided in spack might be useful: https://github.com/spack/spack/blob/develop/var/spack/repos/builtin/packages/intel-oneapi-compilers/package.py. There seem to be offline installers for the individual components of the toolkits which can be used here.

If that PR is ready, I would go ahead and merge it. Support for Intel compilers is best done in a separate PR.

@wpbonelli
Copy link
Contributor Author

Ok, will look into Intel/spack in a separate PR.

I think this one is ready. A bot update PR is expected on merge, as the CSV I checked in is currently empty. After that it should only update if compatibility changes.

@awvwgk awvwgk merged commit 8cc9bcf into fortran-lang:main Nov 4, 2022
@wpbonelli wpbonelli deleted the develop branch November 4, 2022 17:21
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.

2 participants