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

Enable multiple build output directories #575

Merged
merged 3 commits into from
Oct 10, 2021
Merged

Conversation

awvwgk
Copy link
Member

@awvwgk awvwgk commented Sep 20, 2021

This patch implements the possibility to have multiple build output directories depending on the compile flags of the respective target. This will currently only be realized for C objects, since they do not use the compile arguments of any profile.

  • remove output_directory from model and replace it with build_prefix the build prefix is generated from the toolchain (Fortran compiler currently)
  • output name, directory and file path are saved in target
  • object targets can have different module output directories depending on the compile arguments
  • all object targets can access all module output directories via include flags (could be refined by providing only access to the required directories from the module interdependencies later)
  • all output directories are created in the backend
  • executables will include libraries they link against in the build hash
  • fix installation of module files

To discuss (possibly for a follow-up patch):

  • should executable target include the (all) link arguments in the build hash? Which arguments, only the libs or also the objects?
  • should the archive include the object files in the build hash?

To do:

  • fix installation of module files

Closes #422

@LKedward
Copy link
Member

Thanks Sebastian - I'm quite busy this week, but I should be able to find some time next week to review this.

@awvwgk
Copy link
Member Author

awvwgk commented Sep 23, 2021

Carlos @brocolis, this should address the issues with relinking the executables upon changes of the link entry in the package manifest now as well.

Copy link
Member

@LKedward LKedward left a comment

Choose a reason for hiding this comment

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

Thanks Sebastian - this is a big improvement, nicely implemented.
I know Jakub had to implement something similar for his work - presumably we'll need to merge this into that?

src/fpm_targets.f90 Outdated Show resolved Hide resolved
src/fpm_targets.f90 Outdated Show resolved Hide resolved
test/fpm_test/test_module_dependencies.f90 Outdated Show resolved Hide resolved
test/fpm_test/test_module_dependencies.f90 Outdated Show resolved Hide resolved
test/fpm_test/test_module_dependencies.f90 Outdated Show resolved Hide resolved
test/fpm_test/test_module_dependencies.f90 Outdated Show resolved Hide resolved
test/fpm_test/test_module_dependencies.f90 Outdated Show resolved Hide resolved
test/fpm_test/test_module_dependencies.f90 Outdated Show resolved Hide resolved
test/fpm_test/test_module_dependencies.f90 Outdated Show resolved Hide resolved
src/fpm_backend.f90 Outdated Show resolved Hide resolved
Co-authored-by: Laurence Kedward <laurence.kedward@bristol.ac.uk>
@awvwgk
Copy link
Member Author

awvwgk commented Sep 27, 2021

I know Jakub had to implement something similar for his work - presumably we'll need to merge this into that?

Exactly, it is based on Jakub's design in #498. I think we can rebase the branch after this change is merged, which should take care of most of the current merge conflicts present.

Copy link
Contributor

@kubajj kubajj left a comment

Choose a reason for hiding this comment

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

I have tried it out and it seems to work as expected. Thank you, Sebastian.

@awvwgk
Copy link
Member Author

awvwgk commented Sep 27, 2021

Please don't merge yet, I still have to look into collecting all module files from the output directory for fpm install.

@awvwgk
Copy link
Member Author

awvwgk commented Oct 6, 2021

Will try to finish it this week. Only thing missing is a function that can return the path to all module files from the projects. This should be possible by iterating over the targets and combine the provided module file basenames with the output directory and than copy the complete list in the install command.

@awvwgk awvwgk merged commit df9b01a into fortran-lang:main Oct 10, 2021
@awvwgk awvwgk deleted the build-dirs branch October 10, 2021 16:03
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.

Changing link entry doesn't trigger relinking targets
3 participants