[Fortran fpm] Internal dependencies & build backend #155

LKedward · 2020-08-26T15:14:21Z

Opening a draft PR for feedback.

Implements:

a rudimentary process for scanning sources and determining inter-module dependencies.
a simple fpm backend for building files in correct order

This is really a minimal working implementation, many places for improvement (particularly source parsing),
but it should now build any stand-alone package that only uses the src and app directories (no subdirectories).

A nice example: fortran fpm can now build itself. Run fpm run --args 'build' with Haskell fpm.

Other changes:

Moved filesystem and string routines into separate modules for now - these will eventually be substituted by stdlib routines.

Not yet implemented:

non-default layouts (requires fpm.toml)
fpm model structure
Package layout constraints
Library archiving
Incremental builds
Build output directory

Tested on Ubuntu Linux (gcc-7.5.0) and MinGW Windows (gcc-8.1.0).

Create separate modules for filesystem and string routines. These can be substituted eventually for stdlib.

Currently extract use/include dependencies and resolves these to specific source files. Also included lower and split string routines as needed.

C programs (int main) not yet allowed.

milancurcic · 2020-08-26T15:41:42Z

Thanks a lot Laurence. I only did a cursory read through for now and have a few suggestions:

For split() and lower() from ``M_strings by @urbanjost, should we import them as a dependency instead of in source? This would mean that fpm-fortran wouldn't be able to build itself (we can't download dependencies yet), but this seems to me like an unnecessary requirement for now. The advantage is that we have less code to maintain. Or would you prefer to keep it in source until we can download dependencies, and then replace them with use M_strings, only: lower, split?
FPM -> fpm everywhere (source file names and modules), as per Should we refer to this software as FPM or fpm? #77.
Shoud we rename module environment in environment.f90 to fpm_environment in fpm_environment.f90?

LKedward · 2020-08-26T17:49:03Z

Thanks for the comments @milancurcic! I agree with you on all points.

everythingfunctional · 2020-08-26T19:34:02Z

This is a great start, and a great next step. Thanks @LKedward .

I agree with @milancurcic for FPM -> fpm and environment -> fpm_environment. I'm also in favor of leaving lower and split in source until we can fetch and build dependencies. Since this seems to allow fpm to build itself, I think we should make efforts not to lose that ability again.

I will try and find time over the next few days to try and look into the CI failures a bit more closely and see what's going on.

certik · 2020-08-27T01:18:19Z

Great work! Thank you. Where is the fpm_model module? I couldn't find it (on my phone).

…

On Wed, Aug 26, 2020, at 9:14 AM, Laurence Kedward wrote: Opening a draft PR for feedback. Implements: * a rudimentary process for scanning sources and determining inter-module dependencies. * a simple fpm backend for building files in correct order This is really a minimal working implementation, many places for improvement (particularly source parsing), but it should now build any stand-alone package that only uses the `src` and `app` directories (no subdirectories). A nice example: fortran `fpm` can now build itself. Run `fpm run --args 'build'` with Haskell `fpm`. Other changes: * Moved filesystem and string routines into separate modules for now - these will eventually be substituted by stdlib routines. Not yet implemented: * non-default layouts (requires `fpm.toml`) * fpm model structure * Package layout constraints * Library archiving * Incremental builds * Build output directory Tested on Ubuntu Linux (gcc-7.5.0) and MinGW Windows (gcc-8.1.0). You can view, comment on, or merge this pull request online at: #155 Commit Summary * Restructure: move some routines out of fpm module. * Use temporary file for directory listing output. * Minor fix: to read_lines subroutine. * Add: Sourcefiles module for processing sources. * Minor fix: to count_rows in filesystem mod. * Add: initial fpm build backend. * Add: initial support for c sources. * Minor fix: add dependency pointer guard. File Changes * *A* fpm/src/FPM_Backend.f90 <https://github.com/fortran-lang/fpm/pull/155/files#diff-475cc582fa5d31afd70a27d7efa178df> (49) * *A* fpm/src/FPM_Filesystem.f90 <https://github.com/fortran-lang/fpm/pull/155/files#diff-051a34076927ef0e96a561c89801de72> (125) * *A* fpm/src/FPM_Sourcefiles.f90 <https://github.com/fortran-lang/fpm/pull/155/files#diff-7e8ed2fd5212f618126c139653c2f364> (375) * *A* fpm/src/FPM_Strings.f90 <https://github.com/fortran-lang/fpm/pull/155/files#diff-e1790889afd1b0c2e62f82231394fa23> (195) * *M* fpm/src/environment.f90 <https://github.com/fortran-lang/fpm/pull/155/files#diff-32b40b492690c8c8294e981a0fb78e70> (14) * *M* fpm/src/fpm.f90 <https://github.com/fortran-lang/fpm/pull/155/files#diff-e235e41ae1a801d1ff11bfa3aa1ea6db> (134) Patch Links: * https://github.com/fortran-lang/fpm/pull/155.patch * https://github.com/fortran-lang/fpm/pull/155.diff — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#155>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAAFAWH4WICQDIJWIZZH6DDSCURF5ANCNFSM4QL6FSFA>.

everythingfunctional · 2020-08-27T01:59:46Z

Yep, @certik is correct. It looks like the fpm_model module got missed with the commits

certik · 2020-08-27T04:35:16Z

That was the most important module I wanted to look at. ;)

…

On Wed, Aug 26, 2020, at 7:59 PM, Brad Richardson wrote: Yep, @certik <https://github.com/certik> is correct. It looks like the `fpm_model` module got missed with the commits — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#155 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAAFAWH7POTCLAX6LUPAVHDSCW4Z7ANCNFSM4QL6FSFA>.

LKedward · 2020-08-27T12:03:30Z

Yep, @certik is correct. It looks like the fpm_model module got missed with the commits

Sorry yes, I'll push an update later that works with an initial fpm_model structure (it only has the dependency structure in currently).

I don't mind use M_Strings as a dependency or keeping it self-contained for now - @certik do you have an opinion on this?

Backend now only accepts the fpm model structure. This structure currently only contains the array of sources.

certik · 2020-09-02T03:50:40Z

@LKedward do you want to finish this PR, so that others can build upon it once it is merged?

Ignores ancestor name in submodule declaration statement.

Adds output directory, compiler and compiler flags to model structure - currently hard-coded values. Adds mkdir subroutine in filesystem, implemented via command line shell.

LKedward · 2020-09-02T10:57:57Z

I've added fpm_ prefix to all modules as discussed and implemented a minimal structure for fpm_model, currently with hard-coded values - I think this should be refined in a separate PR. For now I've left the string routines in.

Currently having issues building with Haskell fpm which is causing CI to fail, but otherwise I think this is ready for review.

If necessary, I'm happy to rebase this PR after #157 is merged.

manifest and model sources files were incorrectly using Windows EOL.

…ly declare public names

milancurcic · 2020-09-03T15:19:20Z

I reviewed in more detail and I think this is good to go. I also made the following changes:

Explicit imports with use ..., only: ...
Explicitly list public names with the public statement
Remove a few unused module imports

There are no semantic changes to the code.

LKedward · 2020-09-03T15:45:44Z

Many thanks Milan! I'll be sure to follow those guidelines in future.

milancurcic · 2020-09-03T15:50:24Z

@LKedward I don't think we even had these in the style guide but I think we should.

Fortran fpm can now build 'hello_complex' example

LKedward · 2020-09-07T09:31:43Z

I've now successfully merged in the recent changes so that fortran fpm can now build slightly more complex packages like the hello_complex example package. This is enough for this PR, so I won't be contributing any more of my own changes to this branch unless requested.

Output of split (M_strings) needs trimming.

awvwgk

I just looked briefly over the code, I will try to test it more extensively soon. Here is what I got so far.

fpm/src/fpm_sources.f90

fpm/src/fpm_environment.f90

fpm/src/fpm_filesystem.f90

fpm/src/fpm_sources.f90

Adds string_array_contains helper function for determining if array of string_t contains a particular string.

LKedward · 2020-09-08T11:01:27Z

Many thanks for the helpful review comments @awvwgk, I've addressed all except the exit codes - am I misunderstanding you here?

awvwgk · 2020-09-08T11:43:13Z

@LKedward I did a bit of further testing and actually trying to break the implementation, see #167, the report is regarding bootstrap fpm but is also not working well with this PR.

certik

I mainly care about the fpm_model file, as everything else can be refactored later.

I left some comments there.

This is a great start, and we will be able to take it from here.

certik · 2020-09-10T20:59:19Z

fpm/src/fpm_model.f90

+
+contains
+
+subroutine build_model(model, settings, package)


This function should got to the "front-end". The fpm_model file should only contain the model itself, not how to construct it.

Essentially the model should be completely standalone, independent of the front end. That way, any backend that we have just takes the model as the "input", and does not have to worry about anything else. The backends should not worry about the command line, or any kind of "user settings". The front end takes user settings and populates the model.

certik · 2020-09-10T20:59:56Z

fpm/src/fpm_model.f90

+type :: fpm_model_t
+    character(:), allocatable :: package_name
+        ! Name of package
+    type(srcfile_t), allocatable :: sources(:)


The srcfile_t derived type should be defined in this file.

certik · 2020-09-10T21:01:03Z

fpm/src/fpm_model.f90

+use fpm_manifest_executable, only: executable_t
+use fpm_sources, only: resolve_module_dependencies, add_sources_from_dir, &
+                       add_executable_sources, srcfile_t
+use fpm_strings, only: string_t


None of these should be imported here, except string_t if we use it in the model derived types (we do not seem to currently).

LKedward · 2020-09-11T15:38:22Z

Thanks for the review @certik, I've implemented your suggestions for isolating the model definition from the frontend.

certik · 2020-09-11T15:42:21Z

Thanks. Looks good now. +1 to merge

…

On Fri, Sep 11, 2020, at 9:38 AM, Laurence Kedward wrote: Thanks for the review @certik <https://github.com/certik>, I've implemented your suggestions for isolating the model definition from the frontend. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#155 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAAFAWHWQ4NDOV6BSDJ35ETSFJAAFANCNFSM4QL6FSFA>.

awvwgk

Looks good to me as well.

milancurcic · 2020-09-11T20:19:39Z

@everythingfunctional Please merge when ready, in case you're still reviewing.

LKedward added 8 commits August 26, 2020 09:47

Restructure: move some routines out of fpm module.

bf86469

Create separate modules for filesystem and string routines. These can be substituted eventually for stdlib.

Use temporary file for directory listing output.

cdedb0a

Minor fix: to read_lines subroutine.

1746dd0

Add: Sourcefiles module for processing sources.

fbe3370

Currently extract use/include dependencies and resolves these to specific source files. Also included lower and split string routines as needed.

Minor fix: to count_rows in filesystem mod.

a06b644

Add: initial fpm build backend.

6d6c336

Add: initial support for c sources.

d44bb2e

C programs (int main) not yet allowed.

Minor fix: add dependency pointer guard.

434033f

LKedward added 3 commits August 28, 2020 12:51

Add: fpm_ prefix to all module names.

a6df3bb

Update: for extracting modules

43dd6e1

Add: stubs for model and manifest structures

b6000d8

Backend now only accepts the fpm model structure. This structure currently only contains the array of sources.

LKedward added 2 commits September 2, 2020 09:55

Fix: for multilevel submodules

54a5c6d

Ignores ancestor name in submodule declaration statement.

Add basic fields to model structure.

f466572

Adds output directory, compiler and compiler flags to model structure - currently hard-coded values. Adds mkdir subroutine in filesystem, implemented via command line shell.

LKedward marked this pull request as ready for review September 2, 2020 10:58

LKedward and others added 3 commits September 2, 2020 12:06

Fix: EOL to unix LF

d8afa4d

manifest and model sources files were incorrectly using Windows EOL.

Update: fortran fpm test script.

f85f291

explicit imports throughout; reorder imports alphabetically; explicit…

07c5828

…ly declare public names

milancurcic approved these changes Sep 3, 2020

View reviewed changes

LKedward added 2 commits September 7, 2020 10:10

Update: test scripts for fortran fpm

5500927

Fortran fpm can now build 'hello_complex' example

Merge remote-tracking branch 'upstream/master' into dependencies

8b1e4a6

LKedward requested review from certik, awvwgk and everythingfunctional September 7, 2020 09:32

LKedward mentioned this pull request Sep 7, 2020

Automatically discover executables & tests #164

Closed

LKedward added 3 commits September 7, 2020 15:33

Fix: allocation for default library.

f32d6c3

Fix: basename function with trim

35ae709

Output of split (M_strings) needs trimming.

Fix: more trimming of split string output.

9bd9d3f

LKedward mentioned this pull request Sep 7, 2020

Updated OS type identification #162

Merged

awvwgk reviewed Sep 7, 2020

View reviewed changes

LKedward added 3 commits September 8, 2020 10:50

Update: fpm_sources with ieee intrinsic module names

518341b

Fix: erroneous optional attribute

8c8e4e9

Updates: for improved readability

fd49a2e

Adds string_array_contains helper function for determining if array of string_t contains a particular string.

certik requested changes Sep 10, 2020

View reviewed changes

certik mentioned this pull request Sep 10, 2020

Security of execute_command_line #166

Open

Isolate model definition from model construction

eed082b

awvwgk approved these changes Sep 11, 2020

View reviewed changes

certik approved these changes Sep 11, 2020

View reviewed changes

everythingfunctional approved these changes Sep 11, 2020

View reviewed changes

everythingfunctional merged commit e02171d into fortran-lang:master Sep 11, 2020

LKedward mentioned this pull request Sep 12, 2020

Source parsing tests #170

Merged

LKedward deleted the dependencies branch September 20, 2020 16:26

awvwgk mentioned this pull request Sep 5, 2021

Special characters in paths fortran-lang/fpm-haskell#12

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Fortran fpm] Internal dependencies & build backend #155

[Fortran fpm] Internal dependencies & build backend #155

LKedward commented Aug 26, 2020

milancurcic commented Aug 26, 2020

LKedward commented Aug 26, 2020

everythingfunctional commented Aug 26, 2020

certik commented Aug 27, 2020 via email

everythingfunctional commented Aug 27, 2020

certik commented Aug 27, 2020 via email

LKedward commented Aug 27, 2020

certik commented Sep 2, 2020

LKedward commented Sep 2, 2020

milancurcic commented Sep 3, 2020

LKedward commented Sep 3, 2020

milancurcic commented Sep 3, 2020

LKedward commented Sep 7, 2020

awvwgk left a comment

LKedward commented Sep 8, 2020

awvwgk commented Sep 8, 2020

certik left a comment

certik Sep 10, 2020

certik Sep 10, 2020

certik Sep 10, 2020

certik Sep 10, 2020

LKedward commented Sep 11, 2020

certik commented Sep 11, 2020 via email

awvwgk left a comment

milancurcic commented Sep 11, 2020

[Fortran fpm] Internal dependencies & build backend #155

[Fortran fpm] Internal dependencies & build backend #155

Conversation

LKedward commented Aug 26, 2020

milancurcic commented Aug 26, 2020

LKedward commented Aug 26, 2020

everythingfunctional commented Aug 26, 2020

certik commented Aug 27, 2020 via email

everythingfunctional commented Aug 27, 2020

certik commented Aug 27, 2020 via email

LKedward commented Aug 27, 2020

certik commented Sep 2, 2020

LKedward commented Sep 2, 2020

milancurcic commented Sep 3, 2020

LKedward commented Sep 3, 2020

milancurcic commented Sep 3, 2020

LKedward commented Sep 7, 2020

awvwgk left a comment

Choose a reason for hiding this comment

LKedward commented Sep 8, 2020

awvwgk commented Sep 8, 2020

certik left a comment

Choose a reason for hiding this comment

certik Sep 10, 2020

Choose a reason for hiding this comment

certik Sep 10, 2020

Choose a reason for hiding this comment

certik Sep 10, 2020

Choose a reason for hiding this comment

certik Sep 10, 2020

Choose a reason for hiding this comment

LKedward commented Sep 11, 2020

certik commented Sep 11, 2020 via email

awvwgk left a comment

Choose a reason for hiding this comment

milancurcic commented Sep 11, 2020