Merge pull request #267 from LKedward/fix-link-objects

Fix enumeration of non-library link objects

Author: LKedward

example_packages/hello_complex_2/app/say_hello/app_extra_mod.f90

@@ -0,0 +1,6 @@
+module app_extra_mod
+implicit none
+
+character(len=5) :: greet_object = "World"
+
+end module app_extra_mod

example_packages/hello_complex_2/app/say_hello/app_hello_mod.f90

@@ -1,4 +1,5 @@
 module app_hello_mod
+use app_extra_mod, only: greet_object
 implicit none
 
 integer :: hello_int = 42

example_packages/hello_complex_2/app/say_hello/say_Hello.f90

@@ -1,8 +1,8 @@
 program say_Hello
     use greet_m, only: make_greeting
-    use app_hello_mod
+    use app_hello_mod, only: greet_object
 
     implicit none
 
-    print *, make_greeting("World")
+    print *, make_greeting(greet_object)
 end program say_Hello

fpm/src/fpm_targets.f90

@@ -259,60 +259,81 @@ function find_module_dependency(targets,module_name,include_dir) result(target_p
 end function find_module_dependency
 
 
-!> For link targets, enumerate any dependency objects required for linking
+!> For libraries and executables, build a list of objects required for linking
+!>
+!> stored in `target%link_objects`
+!>
 subroutine resolve_target_linking(targets)
     type(build_target_ptr), intent(inout), target :: targets(:)
 
-    integer :: i,j,k
-    type(string_t) :: link_object
+    integer :: i
 
     do i=1,size(targets)
 
         associate(target => targets(i)%ptr)
 
             allocate(target%link_objects(0))
 
-            do j=1,size(target%dependencies)
-            
-                if (target%target_type == FPM_TARGET_ARCHIVE ) then
-            
-                    ! Construct object list for archive
-                    link_object%s = target%dependencies(j)%ptr%output_file
-                    target%link_objects = [target%link_objects, link_object]
-            
-                else if (target%target_type == FPM_TARGET_EXECUTABLE .and. &
-                        target%dependencies(j)%ptr%target_type ==  FPM_TARGET_OBJECT) then
-                    
-                    associate(exe_obj => target%dependencies(j)%ptr)
-
-                        ! Construct object list for executable
-                        link_object%s = exe_obj%output_file
-                        target%link_objects = [target%link_objects, link_object]
-                            
-                        ! Include non-library object dependencies
-                        do k=1,size(exe_obj%dependencies)
-                
-                            if (allocated(exe_obj%dependencies(k)%ptr%source)) then
-                                if (exe_obj%dependencies(k)%ptr%source%unit_scope == &
-                                     exe_obj%source%unit_scope) then
+            if (target%target_type == FPM_TARGET_ARCHIVE) then
 
-                                    link_object%s = exe_obj%dependencies(k)%ptr%output_file
-                                    target%link_objects = [target%link_objects, link_object]
+                call get_link_objects(target%link_objects,target,is_exe=.false.)
 
-                                end if
-                            end if
-                
-                        end do
+            else if (target%target_type == FPM_TARGET_EXECUTABLE) then
 
-                    end associate
-            
-                end if
+                call get_link_objects(target%link_objects,target,is_exe=.true.)
+
+            end if
 
-            end do
         end associate
 
     end do
 
+contains
+
+    !> Wrapper to build link object list
+    !>
+    !>  For libraries: just list dependency objects of lib target
+    !>
+    !>  For executables: need to recursively discover non-library
+    !>   dependency objects. (i.e. modules in same dir as program)
+    !>
+    recursive subroutine get_link_objects(link_objects,target,is_exe)
+        type(string_t), intent(inout), allocatable :: link_objects(:)
+        type(build_target_t), intent(in) :: target
+        logical, intent(in) :: is_exe
+
+        integer :: i
+        type(string_t) :: temp_str
+
+        if (.not.allocated(target%dependencies)) return
+
+        do i=1,size(target%dependencies)
+
+            associate(dep => target%dependencies(i)%ptr)
+            
+                if (.not.allocated(dep%source)) cycle
+                
+                ! Skip library dependencies for executable targets
+                !  since the library archive will always be linked 
+                if (is_exe.and.(dep%source%unit_scope == FPM_SCOPE_LIB)) cycle
+                
+                ! Skip if dependency object already listed
+                if (dep%output_file .in. link_objects) cycle
+
+                ! Add dependency object file to link object list
+                temp_str%s = dep%output_file
+                link_objects = [link_objects, temp_str]
+
+                ! For executable objects, also need to include non-library 
+                !  dependencies from dependencies (recurse)
+                if (is_exe) call get_link_objects(link_objects,dep,is_exe=.true.)
+            
+            end associate
+
+        end do
+
+    end subroutine get_link_objects
+
 end subroutine resolve_target_linking
 
 

fpm/test/fpm_test/test_module_dependencies.f90

@@ -1,14 +1,15 @@
 !> Define tests for the `fpm_sources` module (module dependency checking)
 module test_module_dependencies
     use testsuite, only : new_unittest, unittest_t, error_t, test_failed
-    use fpm_targets, only: targets_from_sources, resolve_module_dependencies
+    use fpm_targets, only: targets_from_sources, resolve_module_dependencies, &
+                            resolve_target_linking
     use fpm_model, only: fpm_model_t, srcfile_t, build_target_t, build_target_ptr, &
                 FPM_UNIT_UNKNOWN, FPM_UNIT_PROGRAM, FPM_UNIT_MODULE, &
                 FPM_UNIT_SUBMODULE, FPM_UNIT_SUBPROGRAM, FPM_UNIT_CSOURCE, &
                 FPM_UNIT_CHEADER, FPM_SCOPE_UNKNOWN, FPM_SCOPE_LIB, &
                 FPM_SCOPE_DEP, FPM_SCOPE_APP, FPM_SCOPE_TEST, &
                 FPM_TARGET_EXECUTABLE, FPM_TARGET_OBJECT, FPM_TARGET_ARCHIVE
-    use fpm_strings, only: string_t
+    use fpm_strings, only: string_t, operator(.in.)
     implicit none
     private
 
@@ -71,14 +72,16 @@ subroutine test_library_module_use(error)
         if (allocated(error)) then
             return
         end if
-
         if (size(model%targets) /= 3) then
             call test_failed(error,'Incorrect number of model%targets - expecting three')
             return
         end if
 
+        call resolve_target_linking(model%targets)
+
         call check_target(model%targets(1)%ptr,type=FPM_TARGET_ARCHIVE,n_depends=2, &
-                          deps = [model%targets(2),model%targets(3)],error=error)
+                          deps = [model%targets(2),model%targets(3)], &
+                          links = model%targets(2:3), error=error)
 
         if (allocated(error)) return
 
@@ -146,8 +149,10 @@ subroutine test_scope(exe_scope,error)
             return
         end if
 
+        call resolve_target_linking(model%targets)
+
         call check_target(model%targets(1)%ptr,type=FPM_TARGET_ARCHIVE,n_depends=1, &
-                          deps=[model%targets(2)],error=error)
+                          deps=[model%targets(2)],links=[model%targets(2)],error=error)
 
         if (allocated(error)) return
 
@@ -162,7 +167,8 @@ subroutine test_scope(exe_scope,error)
         if (allocated(error)) return
 
         call check_target(model%targets(4)%ptr,type=FPM_TARGET_EXECUTABLE,n_depends=2, &
-                            deps=[model%targets(1),model%targets(3)],error=error)
+                            deps=[model%targets(1),model%targets(3)], &
+                            links=[model%targets(3)], error=error)
 
         if (allocated(error)) return
 
@@ -202,20 +208,22 @@ subroutine test_program_with_module(error)
             return
         end if
 
+        call resolve_target_linking(model%targets)
+
         call check_target(model%targets(1)%ptr,type=FPM_TARGET_OBJECT,n_depends=0, &
                           source=sources(1),error=error)
 
         if (allocated(error)) return
 
         call check_target(model%targets(2)%ptr,type=FPM_TARGET_EXECUTABLE,n_depends=1, &
-                          deps=[model%targets(1)],error=error)
+                          deps=[model%targets(1)],links=[model%targets(1)],error=error)
 
         if (allocated(error)) return
 
     end subroutine test_program_with_module
 
 
-    !> Check program using a module in same directory
+    !> Check program using modules in same directory
     subroutine test_program_own_module_use(error)
 
         !> Error handling
@@ -233,21 +241,25 @@ subroutine test_scope(exe_scope,error)
         integer, intent(in) :: exe_scope
         type(error_t), allocatable, intent(out) :: error
 
-        type(srcfile_t) :: sources(2)
+        type(srcfile_t) :: sources(3)
         type(fpm_model_t) :: model
         character(:), allocatable :: scope_str
 
         model%output_directory = ''
 
         scope_str = merge('FPM_SCOPE_APP ','FPM_SCOPE_TEST',exe_scope==FPM_SCOPE_APP)//' - '
 
-        sources(1) = new_test_source(FPM_UNIT_MODULE,file_name="app/app_mod.f90", &
+        sources(1) = new_test_source(FPM_UNIT_MODULE,file_name="app/app_mod1.f90", &
                                     scope = exe_scope, &
-                                    provides=[string_t('app_mod')])
+                                    provides=[string_t('app_mod1')])
 
-        sources(2) = new_test_source(FPM_UNIT_PROGRAM,file_name="app/my_program.f90", &
+        sources(2) = new_test_source(FPM_UNIT_MODULE,file_name="app/app_mod2.f90", &
+                                    scope = exe_scope, &
+                                    provides=[string_t('app_mod2')],uses=[string_t('app_mod1')])
+
+        sources(3) = new_test_source(FPM_UNIT_PROGRAM,file_name="app/my_program.f90", &
                                     scope=exe_scope, &
-                                    uses=[string_t('app_mod')])
+                                    uses=[string_t('app_mod2')])
 
         call targets_from_sources(model,sources)
         call resolve_module_dependencies(model%targets,error)
@@ -256,11 +268,12 @@ subroutine test_scope(exe_scope,error)
             return
         end if
 
-        if (size(model%targets) /= 3) then
+        if (size(model%targets) /= 4) then
             call test_failed(error,scope_str//'Incorrect number of model%targets - expecting three')
             return
         end if
 
+        call resolve_target_linking(model%targets)
 
         call check_target(model%targets(1)%ptr,type=FPM_TARGET_OBJECT,n_depends=0, &
                           source=sources(1),error=error)
@@ -272,11 +285,16 @@ subroutine test_scope(exe_scope,error)
 
         if (allocated(error)) return
 
-        call check_target(model%targets(3)%ptr,type=FPM_TARGET_EXECUTABLE,n_depends=1, &
-                           deps=[model%targets(2)],error=error)
+        call check_target(model%targets(3)%ptr,type=FPM_TARGET_OBJECT,n_depends=1, &
+                          source=sources(3),deps=[model%targets(2)],error=error)
+        
+        if (allocated(error)) return
+
+        call check_target(model%targets(4)%ptr,type=FPM_TARGET_EXECUTABLE,n_depends=1, &
+                           deps=[model%targets(3)],links=model%targets(1:3), error=error)
 
         if (allocated(error)) return
-        
+
     end subroutine test_scope
     end subroutine test_program_own_module_use
 
@@ -414,12 +432,13 @@ end function new_test_source
 
 
     !> Helper to check an expected output target
-    subroutine check_target(target,type,n_depends,deps,source,error)
+    subroutine check_target(target,type,n_depends,deps,links,source,error)
         type(build_target_t), intent(in) :: target
         integer, intent(in) :: type
         integer, intent(in) :: n_depends
         type(srcfile_t), intent(in), optional :: source
         type(build_target_ptr), intent(in), optional :: deps(:)
+        type(build_target_ptr), intent(in), optional :: links(:)
         type(error_t), intent(out), allocatable :: error
 
         integer :: i
@@ -448,6 +467,34 @@ subroutine check_target(target,type,n_depends,deps,source,error)
 
         end if
 
+        if (present(links)) then
+
+            do i=1,size(links)
+
+                if (.not.(links(i)%ptr%output_file .in. target%link_objects)) then
+                    call test_failed(error,'Missing object ('//links(i)%ptr%output_file//&
+                                    ') for executable "'//target%output_file//'"')
+                    return
+                end if
+
+            end do
+
+            if (size(links) > size(target%link_objects)) then
+
+                call test_failed(error,'There are missing link objects for target "'&
+                                 //target%output_file//'"')
+                return
+                
+            elseif (size(links) < size(target%link_objects)) then
+
+                call test_failed(error,'There are more link objects than expected for target "'&
+                                 //target%output_file//'"')
+                return
+
+            end if
+
+        end if
+
         if (present(source)) then
 
             if (allocated(target%source)) then