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

CMake build #28

Merged
merged 3 commits into from
Feb 15, 2023
Merged

CMake build #28

merged 3 commits into from
Feb 15, 2023

Conversation

jchristopherson
Copy link
Contributor

Hello,

I've added CMake support for the build of this library. I've included support for testing (CTest), installation, and building the example file. Hopefully this is helpful!

Regards,
Jason

Copy link
Contributor

@zoziha zoziha left a comment

Choose a reason for hiding this comment

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

@jchristopherson, thank you for adding CMake support for fftpack, looks good to me!
Do we need to add CI support for CMake?

@jchristopherson
Copy link
Contributor Author

Good point! I will add it!

@jchristopherson
Copy link
Contributor Author

The CMake process seems to be working but I am unsure why the fpm builds or doc-deployment builds are failing.

@certik
Copy link
Member

certik commented Jan 4, 2023

@jchristopherson thanks for this. I agree having a nice cmake build system is needed if you cannot or do not want to use fpm.

The only downside is that we now have to maintain two buildsystems, fpm and cmake. At the CI as well as manually, and debug bugs and when we add or change files we need to now test both.

I still think the best way forward is to teach fpm to just generate this cmake build system for you, see fortran-lang/fpm#69.

@jchristopherson would you have time to collaborate on this? The only involved part is dependencies handling, which we do not have for fftpack, and so we simply give an error message in the cmake backend in fpm if there is a dependency (for now).

Copy link
Member

@certik certik left a comment

Choose a reason for hiding this comment

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

Let's add this cmake backend to fpm, see my comment.

@jchristopherson
Copy link
Contributor Author

Hi @certik , that completely makes sense, and I do agree with your comments. I'd also be willing to help collaborate on the cmake backend for fpm; however, I am not familiar with the code base at this point - it'll take me a bit of time to get up to speed. Regardless, I can assist - just not in a timely manner.

@zoziha
Copy link
Contributor

zoziha commented Jan 5, 2023

The CMake process seems to be working but I am unsure why the fpm builds or doc-deployment builds are failing.

@jchristopherson, it seems that the CI environment has changed, causing ford and fpm to fail. Don't worry, this has nothing to do with this PR, we can fix the CI environment in a PR thereafter.

Everything looks good, thanks.

@milancurcic
Copy link
Member

While I agree that a CMake backend for fpm is a good long-term solution, it may be a long time before that's implemented. I don't think the maintenance burden here would be high--source files have not changed in almost a year, and as I understand it, fftpack is stable. @jchristopherson, would you be interested in helping maintain the CMake build for fftpack? If so, I think this PR should be merged, and we can remove the CMake build when fpm is ready.

@jchristopherson
Copy link
Contributor Author

@milancurcic I sure can maintain the CMake build for fftpack.

@certik
Copy link
Member

certik commented Jan 5, 2023

@jchristopherson if you are willing to maintain the backend to ensure everything works at the CI and can help update it if things move, then I think we can merge this, until fpm itself can "maintain such cmake backends".

@milancurcic regarding fpm, this is becoming an urgent feature, that will save time overall. The longer we wait, the more work it becomes for the whole community to create and maintain these multiple build systems.

Copy link
Member

@certik certik left a comment

Choose a reason for hiding this comment

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

Approving conditionally that @jchristopherson will maintain this build system in fftpack.

@milancurcic
Copy link
Member

Great, thanks, I think we all agree. I'll let a maintainer merge when ready.

@zoziha
Copy link
Contributor

zoziha commented Feb 15, 2023

Thanks for supporting CMake for fftpack @jchristopherson , I will merge this PR.

@jchristopherson
Copy link
Contributor Author

Sounds good.

@zoziha zoziha merged commit 14549b1 into fortran-lang:main Feb 15, 2023
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.

4 participants