-
Notifications
You must be signed in to change notification settings - Fork 100
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
feat: init shared library support. #1050
base: main
Are you sure you want to change the base?
Conversation
arteevraina
commented
Jun 16, 2024
- Added shared-library field in fpm.toml which tells fpm whether to generate the shared library or not.
- Added the logic to generate shared library.
src/fpm_compiler.F90
Outdated
!> Output directory of object files. | ||
character(len=*), intent(in) :: output_dir | ||
|
||
call run(self%fc // " --shared " // output_dir// "/" // package_name // "/*.o" // " -o " // "lib" // & |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe this could use the archive we are already generating. A wildcard for all object files would not be acceptable in any case, let's be explicit about them.
src/fpm.f90
Outdated
@@ -467,6 +469,13 @@ subroutine cmd_build(settings) | |||
call build_package(targets,model,verbose=settings%verbose) | |||
endif | |||
|
|||
do i=1, size(targets) | |||
if (targets(i)%ptr%target_type == FPM_TARGET_OBJECT .and. package%build%shared_library) then |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a direct way to get the access to build directory of FPM_TARGET_OBJECT
? @awvwgk
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Best check how the static library is built
Thanks for the review @awvwgk I have updated this pull request with the required changes. |
!> Output file of library archive. | ||
character(len=*), intent(in) :: output_file | ||
|
||
call run(self%fc // " --shared " // " -o " // "lib" // package_name // ".so" // " " & |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would this command work for all compilers in the fpm
suite, or there are compiler specific flags that should be used?
Also the .so
output looks a bit odd to me: isn't a dynamic library supposed to be named .dll
on Windows and .dylib
on macOS?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the review @perazz . .so
works well with Unix based operating systems so it works for macOS
as well.
But, for windows still need to make .dll
format for it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This thread on SO seems to indicate .dylib
as the canonical "dynamic library", whereas ".so" is used for plugins. In practice the formats are (mostly) the same. The Mac Dynamic Library Usage Guidelines refer to .dylib
in many places, but don't mention .so
.
So I think .dylib
and the flag -dynamiclib
instead of -shared
would be the preferred option for MacOS.