Skip to content

Add fortran tests and clean cmake #226

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 6 commits into from
Jan 29, 2023
Merged

Add fortran tests and clean cmake #226

merged 6 commits into from
Jan 29, 2023

Conversation

LecrisUT
Copy link
Collaborator

@LecrisUT LecrisUT commented Jan 28, 2023 •
edited
Loading

@atztogo @lan496

Here is the skeleton for the Fortran tests. Simply print to stdout/stderr and return non-zero for any failed test, and make sure the file name and function are identical.

I have also made a minor fix in the find_package(Python), switching it to Python3 for easier packaging in RPM (waiting on scikit-build/scikit-build#834 to finish that).

You should have write access to my branch, so feel free to just push commits on top of it.

Edit: Previous tests had some issues with sanitizer flags, which I had cleaned up in a separate branch, so I decided to just add these ones in as well. So all together this PR does:

  • Adds a ctest skeleton for fortran tests
  • Removes the separate targets for shared and static libraries. Instead use spglib_SHARED_LIBS to switch between either of those.
    • Reasoning: static libraries would only be useful if spglib is bundled in a project. Otherwise, for local development or production it should be using shared libraries
    • I've added a default value to static libraries if spglib is bundled, otherwise build shared (similar to spglib_WITH_TESTS)
  • Adds namespaced alias targets and namespaced options
    • This is similarly to the one above, if the project is a bundled project, it should not be injected by the options of the parent project
    • This is still in rc, and there has been some previous changes to these flags, when I made this project importable, so it shouldn't affect much
  • Use FindPython3 for better compatibility with RPM packaging

Closes #225, #122

@codecov-commenter
Copy link

codecov-commenter commented Jan 28, 2023 •
edited
Loading

Codecov Report

Base: 93.34% // Head: 93.34% // No change to project coverage 👍

Coverage data is based on head (33c0929) compared to base (aee574d).
Patch has no changes to coverable lines.

📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

Additional details and impacted files 
@@           Coverage Diff            @@
##           develop     #226   +/-   ##
========================================
  Coverage    93.34%   93.34%           
========================================
  Files           15       15           
  Lines          902      902           
========================================
  Hits           842      842           
  Misses          60       60

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@LecrisUT
Correct cmake FindPython and use Python3
c1b5d65 
Signed-off-by: Cristian Le <cristian.le@mpsd.mpg.de>
@LecrisUT
Add Fortran test skeleton
545e2fc 
Signed-off-by: Cristian Le <cristian.le@mpsd.mpg.de>
@LecrisUT
Cleanup cmake
057664d 
- Move to single shared/static targets defined by `BUILD_SHARED_LIBS=ON/OFF`
- Cleanup cmake to use modern standards, e.g. using `target_compile_definitions`
- Moved pkg-config template file to cmake folder

Signed-off-by: Cristian Le <cristian.le@mpsd.mpg.de>
@LecrisUT
Fix fortran mod install directory
7fc0647 
Signed-off-by: Cristian Le <cristian.le@mpsd.mpg.de>
@LecrisUT
Add namespace alias and namespace options
125f3bf 
This will make it possible for external projects to include this library via `FetchContent` and equivalent.

Signed-off-by: Cristian Le <cristian.le@mpsd.mpg.de>
@LecrisUT
Add spglib_SHARED_LIBS
33c0929 
Signed-off-by: Cristian Le <cristian.le@mpsd.mpg.de>
@LecrisUT LecrisUT changed the title Add fortran tests Add fortran tests and clean cmake Jan 28, 2023
@LecrisUT
Copy link
Collaborator Author

LecrisUT commented Jan 28, 2023 •
edited
Loading

Ok, since the last tests failed, I think I will just add the other fixes I had lying around. I'll edit the top comment for more details.

@LecrisUT LecrisUT mentioned this pull request Jan 28, 2023
Closed
lan496
lan496 approved these changes Jan 29, 2023
View reviewed changes
Copy link
Member

@lan496 lan496 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@LecrisUT, many thanks! I see some configurations in CMakeLists.txt are moved to src/CMakeLists.txt.

@atztogo I am not a Fortran person. So, can you check and merge this PR?

@atztogo atztogo merged commit 4b72ed9 into spglib:develop Jan 29, 2023
@atztogo
Copy link
Collaborator

atztogo commented Jan 29, 2023

Merged. Thanks @LecrisUT. This is really great.
I just started to learn how to write tests in C (thanks @lan496 for the google test and @LecrisUT for ctest). It was good for me to know that fortran tests can also be handled by ctest.

@LecrisUT LecrisUT deleted the ctest-fortran branch January 29, 2023 12:05
@atztogo
Copy link
Collaborator

atztogo commented Jan 30, 2023

@LecrisUT, do you know the license of c_interface_module.F90? If it is from Fortran wiki or it is unclear, it is better not to use it. The license of full Fortran wiki is GPL, https://fortranwiki.org/fortran/show/Fortran+Wiki, though not sure for a part of it.

@atztogo
Copy link
Collaborator

atztogo commented Jan 30, 2023

@LecrisUT. Sorry, it seems public domain about source code on Fortran wiki, https://fortranwiki.org/fortran/show/Copyrights.

@LecrisUT
Copy link
Collaborator Author

Yeah, that's what I've found as well. As far as I understand there are no copyright or license restrictions with those.

@atztogo atztogo mentioned this pull request Feb 3, 2023
Closed
@LecrisUT LecrisUT added this to the 2.1 milestone Feb 21, 2023
@LecrisUT LecrisUT added the enhancement Significant ehancements label Mar 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lan496 lan496 lan496 approved these changes

Assignees
No one assigned
Labels
enhancement Significant ehancements
Projects
None yet
Milestone
2.1
Development

Successfully merging this pull request may close these issues.

Fortran interface test using ctest
4 participants
@LecrisUT @codecov-commenter @atztogo @lan496