Cleanup and refactor for PR

Move enumeration of link target object dependencies out of backend and into fpm_targets module.
Add string_cat function for concatenating arrays of string_t.
Add comments and procedure descriptions.

Author: LKedward

fpm/src/fpm.f90

@@ -8,10 +8,11 @@ module fpm
 use fpm_model, only: fpm_model_t, srcfile_t, build_target_t, &
                     FPM_SCOPE_UNKNOWN, FPM_SCOPE_LIB, &
                     FPM_SCOPE_DEP, FPM_SCOPE_APP, FPM_SCOPE_TEST, &
-                    FPM_TARGET_EXECUTABLE
+                    FPM_TARGET_EXECUTABLE, FPM_TARGET_ARCHIVE
 
 use fpm_sources, only: add_executable_sources, add_sources_from_dir
-use fpm_targets, only: targets_from_sources, resolve_module_dependencies, FPM_TARGET_ARCHIVE
+use fpm_targets, only: targets_from_sources, resolve_module_dependencies, &
+                        resolve_target_linking
 use fpm_manifest, only : get_package_data, default_executable, &
     default_library, package_t, default_test
 use fpm_error, only : error_t, fatal_error
@@ -247,6 +248,8 @@ subroutine build_model(model, settings, package, error)
 
     call resolve_module_dependencies(model%targets,error)
 
+    call resolve_target_linking(model%targets)
+
 end subroutine build_model
 
 !> Apply package defaults

fpm/src/fpm_backend.f90

@@ -1,86 +1,87 @@
+!> Implements the native fpm build backend
 module fpm_backend
 
-! Implements the native fpm build backend
-
-use fpm_environment, only: run, get_os_type, OS_WINDOWS
-use fpm_filesystem, only: basename, dirname, join_path, exists, mkdir
-use fpm_model, only: fpm_model_t, srcfile_t, build_target_t, build_target_ptr, &
-                      FPM_UNIT_MODULE, FPM_UNIT_SUBMODULE, FPM_UNIT_SUBPROGRAM, &
-                     FPM_UNIT_CSOURCE, FPM_UNIT_PROGRAM, &
-                     FPM_SCOPE_TEST, FPM_TARGET_OBJECT, FPM_TARGET_ARCHIVE, FPM_TARGET_EXECUTABLE
+use fpm_environment, only: run
+use fpm_filesystem, only: dirname, join_path, exists, mkdir
+use fpm_model, only: fpm_model_t, build_target_t, build_target_ptr, &
+                     FPM_TARGET_OBJECT, FPM_TARGET_ARCHIVE, FPM_TARGET_EXECUTABLE
 
-use fpm_strings, only: split
+use fpm_strings, only: string_cat
 
 implicit none
 
 private
-public :: build_package
+public :: build_package, sort_target, schedule_targets
 
 contains
 
-
+!> Top-level routine to build package described by `model`
 subroutine build_package(model)
     type(fpm_model_t), intent(inout) :: model
 
     integer :: i, j
     type(build_target_ptr), allocatable :: queue(:)
-    integer, allocatable :: region_ptr(:)
+    integer, allocatable :: schedule_ptr(:)
 
-    if (.not.exists(model%output_directory)) then
-        call mkdir(model%output_directory)
-    end if
+    ! Need to make output directory for include (mod) files
     if (.not.exists(join_path(model%output_directory,model%package_name))) then
         call mkdir(join_path(model%output_directory,model%package_name))
     end if
 
+    ! Perform depth-first topological sort of targets
     do i=1,size(model%targets)
 
-        call schedule_target(model%targets(i)%ptr)
+        call sort_target(model%targets(i)%ptr)
 
     end do
 
-    call get_build_queue(queue, region_ptr, model%targets)
+    ! Construct build schedule queue
+    call schedule_targets(queue, schedule_ptr, model%targets)
 
-    do i=1,size(region_ptr)-1
+    ! Loop over parallel schedule regions
+    do i=1,size(schedule_ptr)-1
 
+        ! Build targets in schedule region i
         !$OMP PARALLEL DO DEFAULT(SHARED)
-        do j=region_ptr(i),(region_ptr(i+1)-1)
+        do j=schedule_ptr(i),(schedule_ptr(i+1)-1)
 
             call build_target(model,queue(j)%ptr)
 
         end do
         !$OMP END PARALLEL DO
 
     end do
-
 
 end subroutine build_package
 
 
-
-recursive subroutine schedule_target(target)
-    ! 
-    !  
+!> Topologically sort a target for scheduling by 
+!>  recursing over it's dependencies.
+!> 
+!> Checks disk-cached source hashes to determine if objects are
+!>  up-to-date. Up-to-date sources are tagged as skipped.
+!>
+recursive subroutine sort_target(target)
     type(build_target_t), intent(inout), target :: target
 
     integer :: i, j, fh, stat
     type(build_target_t), pointer :: exe_obj
 
-    if (target%scheduled .or. target%skip) then
+    ! Check if target has already been processed (as a dependency)
+    if (target%sorted .or. target%skip) then
         return
     end if
 
-    if (.not.exists(dirname(target%output_file))) then
-        call mkdir(dirname(target%output_file))
-    end if
-
+    ! Check for a circular dependency
+    ! (If target has been touched but not processed)
     if (target%touched) then
         write(*,*) '(!) Circular dependency found with: ',target%output_file
         stop
     else
-        target%touched = .true.
+        target%touched = .true.  ! Set touched flag
     end if
 
+    ! Load cached source file digest if present
     if (.not.allocated(target%digest_cached) .and. &
          exists(target%output_file) .and. &
          exists(target%output_file//'.digest')) then
@@ -90,120 +91,117 @@ recursive subroutine schedule_target(target)
         read(fh,*,iostat=stat) target%digest_cached
         close(fh)
 
-        if (stat /= 0) then
-            write(*,*) 'Internal error: unable to read cached source hash'
-            write(*,*) target%output_file//'.digest',' stat = ', stat
-            error stop
+        if (stat /= 0) then    ! Cached digest is not recognized
+            deallocate(target%digest_cached)
         end if
 
     end if
 
     if (allocated(target%source)) then
+
+        ! Skip if target is source-based and source file is unmodified
         if (allocated(target%digest_cached)) then
             if (target%digest_cached == target%source%digest) target%skip = .true.
         end if
+
     elseif (exists(target%output_file)) then
+
+        ! Skip if target is not source-based and already exists
         target%skip = .true.
+
     end if
 
-    target%link_objects = " "
-    target%region = 1
+    ! Loop over target dependencies
+    target%schedule = 1
     do i=1,size(target%dependencies)
 
-        call schedule_target(target%dependencies(i)%ptr)
+        ! Sort dependency
+        call sort_target(target%dependencies(i)%ptr)
 
         if (.not.target%dependencies(i)%ptr%skip) then
 
+            ! Can't skip target if any dependency is not skipped
             target%skip = .false.
-            target%region = max(target%region,target%dependencies(i)%ptr%region+1)
 
-        end if
-
-        if (target%target_type == FPM_TARGET_ARCHIVE ) then
-
-            ! Construct object list for archive
-            target%link_objects = target%link_objects//" "//target%dependencies(i)%ptr%output_file
-
-        else if (target%target_type == FPM_TARGET_EXECUTABLE .and. &
-                 target%dependencies(i)%ptr%target_type ==  FPM_TARGET_OBJECT) then
-
-            exe_obj => target%dependencies(i)%ptr
-                
-            ! Construct object list for executable
-            target%link_objects = " "//exe_obj%output_file
-                
-            ! Include non-library object dependencies
-            do j=1,size(exe_obj%dependencies)
-
-                if (allocated(exe_obj%dependencies(j)%ptr%source)) then
-                    if (exe_obj%dependencies(j)%ptr%source%unit_scope == exe_obj%source%unit_scope) then
-                        target%link_objects = target%link_objects//" "//exe_obj%dependencies(j)%ptr%output_file
-                    end if
-                end if
-
-            end do
+            ! Set target schedule after all of its dependencies
+            target%schedule = max(target%schedule,target%dependencies(i)%ptr%schedule+1)
 
         end if
 
     end do
 
-    target%scheduled = .not.target%skip
+    ! Mark flag as processed: either sorted or skipped
+    target%sorted = .not.target%skip
 
-end subroutine schedule_target
+end subroutine sort_target
 
 
-subroutine get_build_queue(queue, region_ptr, targets)
+!> Construct a build schedule from the sorted targets.
+!>
+!> The schedule is broken into regions, described by `schedule_ptr`,
+!>  where targets in each region can be compiled in parallel.
+!>
+subroutine schedule_targets(queue, schedule_ptr, targets)
     type(build_target_ptr), allocatable, intent(out) :: queue(:)
-    integer, allocatable :: region_ptr(:)
+    integer, allocatable :: schedule_ptr(:)
     type(build_target_ptr), intent(in) :: targets(:)
 
     integer :: i, j
-    integer :: nRegion, n_scheduled
+    integer :: n_schedule, n_sorted
 
-    nRegion = 0
-    n_scheduled = 0
+    n_schedule = 0   ! Number of schedule regions
+    n_sorted = 0     ! Total number of targets to build
     do i=1,size(targets)
 
-        if (targets(i)%ptr%scheduled) then
-            n_scheduled = n_scheduled + 1
+        if (targets(i)%ptr%sorted) then
+            n_sorted = n_sorted + 1
         end if
-        nRegion = max(nRegion, targets(i)%ptr%region)
+        n_schedule = max(n_schedule, targets(i)%ptr%schedule)
 
     end do
 
-    allocate(queue(n_scheduled))
-    allocate(region_ptr(nRegion+1))
+    allocate(queue(n_sorted))
+    allocate(schedule_ptr(n_schedule+1))
 
-    n_scheduled = 1
-    region_ptr(n_scheduled) = 1
-    do i=1,nRegion
+    ! Construct the target queue and schedule region pointer
+    n_sorted = 1
+    schedule_ptr(n_sorted) = 1
+    do i=1,n_schedule
 
         do j=1,size(targets)
 
-            if (targets(j)%ptr%scheduled) then
-                if (targets(j)%ptr%region == i) then
+            if (targets(j)%ptr%sorted) then
+                if (targets(j)%ptr%schedule == i) then
 
-                    queue(n_scheduled)%ptr => targets(j)%ptr
-                    n_scheduled = n_scheduled + 1
+                    queue(n_sorted)%ptr => targets(j)%ptr
+                    n_sorted = n_sorted + 1
                 end if
             end if
 
         end do
 
-        region_ptr(i+1) = n_scheduled
+        schedule_ptr(i+1) = n_sorted
 
     end do
 
-end subroutine get_build_queue
+end subroutine schedule_targets
 
 
+!> Call compile/link command for a single target.
+!>
+!> If successful, also caches the source file digest to disk.
+!>
 subroutine build_target(model,target)
     type(fpm_model_t), intent(in) :: model
     type(build_target_t), intent(in), target :: target
 
     integer :: ilib, fh
     character(:), allocatable :: link_flags
 
+    if (.not.exists(dirname(target%output_file))) then
+        call mkdir(dirname(target%output_file))
+    end if
+
     select case(target%target_type)
 
     case (FPM_TARGET_OBJECT)
@@ -218,16 +216,16 @@ subroutine build_target(model,target)
         end if
 
         if (allocated(target%link_libraries)) then
-            do ilib = 1, size(target%link_libraries)
-                link_flags = link_flags // " -l" // target%link_libraries(ilib)%s
-            end do
+            if (size(target%link_libraries) > 0) then
+                link_flags = link_flags // " -l" // string_cat(target%link_libraries," -l")
+            end if
         end if
 
-        call run("gfortran " // target%link_objects // model%fortran_compile_flags &
+        call run("gfortran " // string_cat(target%link_objects," ") // model%fortran_compile_flags &
               //link_flags// " -o " // target%output_file)
 
     case (FPM_TARGET_ARCHIVE)
-        call run("ar -rs " // target%output_file // target%link_objects)
+        call run("ar -rs " // target%output_file // " " // string_cat(target%link_objects," "))
 
     end select
 

fpm/src/fpm_model.f90

@@ -73,18 +73,18 @@ module fpm_model
     integer :: target_type = FPM_TARGET_UNKNOWN
     type(string_t), allocatable :: link_libraries(:)
         ! Native libraries to link against
-
-    character(:), allocatable :: link_objects
+    type(string_t), allocatable :: link_objects(:)
         ! Objects needed to link this target
+
     logical :: touched = .false.
         ! Flag set when first visited to check for circular dependencies
-    logical :: scheduled = .false.
-        ! Flag set if build target is scheduled for building
+    logical :: sorted = .false.
+        ! Flag set if build target is sorted for building
     logical :: skip = .false.
         ! Flag set if build target will be skipped (not built)
 
-    integer :: region
-        ! Targets in the same region are guaranteed independent
+    integer :: schedule
+        ! Targets in the same schedule group are guaranteed to be independent
     integer(int64), allocatable :: digest_cached
         ! Previous hash