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 Fortran Include File Generation #127

Merged
merged 3 commits into from
Nov 26, 2023

Conversation

ArmstrongJ
Copy link
Contributor

This request takes three major actions:

  • Removes the prewritten FORTRAN/superlu_config.fh and generates it using a new step in FORTRAN/Makefile
  • Adds a valid CMake step to generate one in the build directory rather than the source directory
  • Fix the step to generate FORTRAN/superlu_config.fh to include a valid trailing #endif

The first step simply eliminates a prebuilt header because it is hard-coded and would ignore the current build configuration. If a developer is using CMake, though, the Fortran compiler (at least GNU Fortran...) will default to reading the source directory's copy of superlu_config.fh rather than one created in the build directory. This issue is especially problematic if we make the second change because...

The second change eliminates the generation of superlu_config.fh in the source directory. I don't think anyone expects a CMake build process to modify the contents of the source directory.

Finally, the sed command to generate FORTRAN/superlu_config.fh was broken because it would delete any line from SRC/superlu_config.h that contains a C comment, including the closing #endif which contained one. The new sed command will delete any text including and following a forward slash (rather than just deleting any line with a forward slash).

…ove any include path pointing at original Fortran source directory. Add Fortran build directory as an include path.
@xiaoyeli
Copy link
Owner

xiaoyeli commented Nov 2, 2023

FORTRAN/Makefile is invoked only using the manual 'make' command. But it is not used using CMake.
So we still need to fix the sed command, in order for CMake to work.

@gruenich
Copy link
Contributor

gruenich commented Nov 21, 2023

You are right, the header should be generated in the build directory. I would not rely on sed, but use CMake's configure_file. If you want, I can prepare an according pull request.

Leave the Makefile the way it is. I still hope that one day, we can drop the Makefiles and just rely on CMake.

@xiaoyeli
Copy link
Owner

@gruenich
Please prepare a PR to use configure_file feature. Thanks.

@xiaoyeli xiaoyeli merged commit f530ef3 into xiaoyeli:master Nov 26, 2023
1 check 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.

3 participants