Skip to content

Conversation

@LKedward
Copy link
Member

@LKedward LKedward commented Mar 7, 2021

A minimal set of changes towards improved support for include/#include statements (related: #358).

  • Proposes a new manifest key include-dir in the [library] table which allows specifying a directory to pass to the compiler as an include directory (i.e. -Iinclude).
    • This allows included .f90 files to be stored separately to compilable sources so that fpm won't try to compile them.
  • The default value of include-dir is "include" which is ignored if the directory doesn't exist.
  • A package contains a library if it contains either the source-dir or the include-dir or both
    • This caters for 'header-only' libraries (useful for defining interfaces, preprocesser macros and inlinable routines)

Not addressed in this PR (#358):

  • Tracking module dependencies in include files
  • Detecting changes in include files for incremental compilation

LKedward added 6 commits March 7, 2021 10:52
A library package must consist of at least a source directory, an include directory or both. Default values of "src" and "include" are ignored if non-existent.
Header-only libraries may result in no compile targets.
Copy link
Member

@awvwgk awvwgk left a comment

Choose a reason for hiding this comment

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

Nice, this allows to use fpm as C/C++ package manager by exporting headers!

I suggest to allow include-dir to take a string or an array of strings (similar to build.link) in case multiple include directories are present.

@LKedward LKedward marked this pull request as ready for review March 15, 2021 15:25
@milancurcic
Copy link
Member

milancurcic commented Mar 18, 2021

Is this PR expected to also work for including pre-built module files?

Specifically, I'm trying to use this to build a program with NetCDF as a dependency. A system-provided netcdf.mod is in /usr/include. Here's my fpm.toml:

name = "umwm"

[library]
source-dir = "staging"
include-dir = "/usr/include"

[build]
link = "netcdff"

But I get the error during fpm build:

...
Unable to find source for module dependency: "netcdf" used by "././staging/umwm_io.f90"
ERROR STOP 1
...

Is the problem perhaps that fpm tries to look for the source file that provides the netcdf module?

Compiler options are:

 <INFO>COMPILER OPTIONS:   -Wall -Wextra -Wimplicit-interface -fPIC -fmax-errors=1 -g -fbounds-check -fcheck-array-temporaries -fbacktrace -fcoarray=single  -J build/gfortran_debug/umwm -I build/gfortran_debug/umwm

So it doesn't look like the path I provided in include-dir was passed to compiler options.

Or am I not setting it up correctly?

@LKedward
Copy link
Member Author

Is this PR expected to also work for including pre-built module files?

No, this PR isn't targeting this use-case specifically, but you are right that this is only limited by the fact that fpm tries to identify a source file all modules used. We could loosen this check to only print a warning if the module source file isn't contained within the project and then your example will work. I think your use case should be discussed separately to this PR; using external mod files isn't ideal for fpm IMO, but I guess it can't be avoided for packages like netcdf.

Looks like I also just need to update the compiler options output to run after processing the manifest.

@milancurcic
Copy link
Member

Sounds good, in that case, let's just add a note to the manifest reference that this for the time being won't work for pre-built modules.

Copy link
Member

@awvwgk awvwgk left a comment

Choose a reason for hiding this comment

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

A very useful addition, only one minor whitespace nitpick in the manifest reference from my side.

@LKedward
Copy link
Member Author

Thanks for reviewing, I will now merge.

@LKedward LKedward merged commit cb3337b into fortran-lang:master Mar 24, 2021
@LKedward LKedward deleted the include-dir branch March 24, 2021 10:08
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.

3 participants