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

fpm deployment enhancement #802

Merged
merged 19 commits into from
May 6, 2024
Merged

fpm deployment enhancement #802

merged 19 commits into from
May 6, 2024

Conversation

jalvesz
Copy link
Contributor

@jalvesz jalvesz commented Apr 21, 2024

#791 #790 #787

Enable building stdlib with fpm directly from the root folder

python config/fypp_deployment.py
fpm build

Deploy the stdlib-fpm from the preprocessing created in the root folder. This deployment could be rendered unnecessary by building directly stdlib from the root.

@jalvesz
Copy link
Contributor Author

jalvesz commented Apr 22, 2024

@perazz maybe you know if there is a way to avoid launching the tests test_always_fail and test_always_skip when doing fpm test ? The other solution I have in mind is to explicitly add the tests to the manifest and deactivate the auto-tests option?

@perazz
Copy link
Contributor

perazz commented Apr 23, 2024

Yes I fear that would be the only option, though it's clearly sub-optimal (for each new test, it will have to be added to fpm.toml). I think the question is why those tests are there in the first place for? In other words, is there a valid reason those dummy tests should be kept into stdlib, or they could be safely removed? At the end of the day they are testing test-drive rather than stdlib.

@jalvesz
Copy link
Contributor Author

jalvesz commented Apr 23, 2024

I think the question is why those tests are there in the first place for? In other words, is there a valid reason those dummy tests should be kept into stdlib, or they could be safely removed?

I was wondering the same thing!

@jvdp1
Copy link
Member

jvdp1 commented Apr 25, 2024

I think the question is why those tests are there in the first place for? In other words, is there a valid reason those dummy tests should be kept into stdlib, or they could be safely removed?

I was wondering the same thing!

These tests were introduced for testing the procedure check in stdlib_error. Later we started to use test-drive.
If check is not used anymore, I guess these tests could be removed.

@jalvesz
Copy link
Contributor Author

jalvesz commented Apr 27, 2024

Thanks @jvdp1, removing those tests enables indeed testing from the root build using fpm!

I'll propose a modification to the README.md before moving from draft to ready for review.

@jalvesz jalvesz marked this pull request as ready for review April 27, 2024 19:13
Copy link
Member

@jvdp1 jvdp1 left a comment

Choose a reason for hiding this comment

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

LGTM. Thank you @jalvesz . I just have a few last comments.
A nice improvement is the support of the cpp directives in the branch stdlib-fpm.

README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
config/requirements.txt Show resolved Hide resolved
jalvesz and others added 2 commits April 28, 2024 11:36
Co-authored-by: Jeremie Vandenplas <jeremie.vandenplas@gmail.com>
Copy link
Member

@jvdp1 jvdp1 left a comment

Choose a reason for hiding this comment

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

I played a bit with the script, and I think it adds nice features (especially that cpp directives are still kept in stdlib-fpm).

Thank you @jalvesz!

@jvdp1 jvdp1 requested review from perazz and a team May 4, 2024 06:02
Copy link
Contributor

@perazz perazz left a comment

Choose a reason for hiding this comment

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

Looks great to me in its current form.
Down the road, I think we may want this script to be able to pass whatever defined preprocessor macros to the fpm.toml:

[preprocess]
[preprocess.cpp]
macros = ["added","by","deployment","script"]

So, let's keep that in mind for now.

@jalvesz
Copy link
Contributor Author

jalvesz commented May 4, 2024

Down the road, I think we may want this script to be able to pass whatever defined preprocessor macros to the fpm.toml:

Totally! While in the current PR I intentionally striped out that function to make things concise, in the script I posted in #791 that function is still available and can be easily added.

@jvdp1
Copy link
Member

jvdp1 commented May 4, 2024

@jalvesz @perazz It seems that this PR is ready. I will merge it in a couple of days.

@jvdp1 jvdp1 merged commit 3a2d503 into fortran-lang:master May 6, 2024
17 checks passed
@jalvesz jalvesz deleted the deployment branch May 6, 2024 06:34
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.

None yet

3 participants