Implement reading of fpm.toml

[awvwgk]

Uses my TOML library to read the fpm.toml file and creates a Fortran type representing the TOML document.

This PR adds toml-f as dependency to fpm.

Closes #149

[milancurcic]

This is super exciting, thanks a lot @awvwgk! I look forward to reviewing it.

[awvwgk]

Not sure on the scope of this PR, currently it allows to translate and verify any TOML document as fpm package file, but does not do much with this information.

The next step would be to pull in a dependency and translate and verify its package file, which would require to create new fpm “instances” to recursively build the complete dependency tree. Giving the git_target_t a fetch method is easy, but working with the created target is a bit more involved, so this might be somewhat out-of-scope for this PR.

Regarding coding style, any conventions I should apply (mainly w.r.t. indentation)?

[certik]

@awvwgk thanks a lot for this PR! We will review it soon. I think the scope is ok as it is, and we can build upon it with further PRs once it is merged.

[awvwgk]

Guess it is ready for review than.

The current style guide I'm mostly following is this one: https://github.com/dftbplus/develguide/blob/master/docs/fortranstyle.rst

[milancurcic]

We follow the stdlib style guide. I skimmed through the contributions and didn't see anything that jumps out except the indentation width (we use 4).

[awvwgk]

Thanks, this is helpful. I've amended the commit accordingly to keep the history clean.

[everythingfunctional]

I didn't dive super deep into it, but from the high level, it looks reasonably similar to the Haskell version, so I think it's probably at least headed in the right direction.

I do have one question though. Would it be possible to write some unit tests for this? Feed it a toml string and make sure the package object is correct, feed it bad toml and make sure it reports an error, etc.

[awvwgk]

Sure can do.

[everythingfunctional]

That looks good for now. I was hoping it wouldn't require writing to the file, but it's ok for now. It's a slick little minimalist testing framework you put together too.

[awvwgk]

toml_parse can read a string too, but it would require to overload or bypass the read_package_file procedure just for testing. Having a dedicated version for testing toml-f in fpm but not the actual version used for fpm somewhat defeats the purpose of the test for me, therefore writing the file it is.

I need to work on a better constructor for toml_table types upstream so one can easily build a TOML document without having to read it from a file. But this is something for a future PR once I figured it out in toml-f.

[LKedward]

Great work @awvwgk! This all looks good to me. Good to see schema checks with useful error messages and a verbose mode.

My only (minor) complaint is naming with config where I would have used manifest (e.g. fpm_manifest) to better describe, and distinguish from the existing settings types used for command line. What do others think?

Due to the size of this PR I would recommend this be merged prior to mine and other PRs; I will rebase.

[awvwgk]

Good idea, manifest is probably a better name than config, I will adjust this.

Feel free to squash the PR into one commit to avoid the noise of renaming files.

[milancurcic]

@awvwgk @everythingfunctional @LKedward Should this PR go before or after #155? Is either way easier than the other? I can't tell.

[LKedward]

I think this PR should go first because it's bigger. I'll then merge the changes into my branch. (Only conflict between the two is fpm.f90 I think.)

[awvwgk]

They are solving different problems, the order does not matter too much, but a conflict in fpm.f90 is inevitable, due to the current status of the project. I do not mind rebasing my branch either.

Since #155 is attempting to add the functionality of fpm building itself, the question arises if we can retain this functionality with both PRs merged. In this regard my PR is working exactly against #155, due to adding an external dependency.

[LKedward]

Since #155 is attempting to add the functionality of fpm building itself, the question arises if we can retain this functionality with both PRs merged. In this regard my PR is working exactly against #155, due to adding an external dependency.

This isn't a problem; parsing the manifest is more important than Fortran fpm building itself.
I already know what changes I need to make in #155 once this PR is merged, hence why I recommend that this be merged first.

[everythingfunctional]

If @LKedward is happy to rebase his branch and fix the conflicts, I'd go with this one first. As much as I'd like to have fpm be able to build itself, it's not strictly necessary. We'll get there.

[milancurcic]

I agree. Let's wait to see if @certik has any objections, and if not we can move forward.

[awvwgk]

I did a quick coverage run on this PR (coverage.txt) and spotted a few issues which were simple oversights. The coverage is currently around 60%, so definitely something to improve.

[certik]

I am fine as long as @LKedward is fine, and it seems he his. So we can go ahead and merge this one.

[awvwgk]

Also ready from my side now. I decided to pin the revision of toml-f to a specific commit to ensure that I won't break fpm by changing something in upstream toml-f.

[milancurcic]

Thank you all and especially @awvwgk. Merging.