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.

Sign up for GitHub

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

Improve error message for [build] structure errors #890

Merged
merged 3 commits into from
Apr 28, 2023
Merged

Improve error message for [build] structure errors #890

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
bde5332
return package name on invalid build key
perazz Apr 24, 2023
5919057
write test
perazz Apr 24, 2023
dfafcf3
improve output message
perazz Apr 27, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 11 additions & 4 deletions src/fpm/manifest/build.f90
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,20 +52,23 @@ module fpm_manifest_build


!> Construct a new build configuration from a TOML data structure
subroutine new_build_config(self, table, error)
subroutine new_build_config(self, table, package_name, error)

!> Instance of the build configuration
type(build_config_t), intent(out) :: self

!> Instance of the TOML data structure
type(toml_table), intent(inout) :: table

!> Package name
character(len=*), intent(in) :: package_name
perazz marked this conversation as resolved.
Show resolved Hide resolved

!> Error handling
type(error_t), allocatable, intent(out) :: error

integer :: stat

call check(table, error)
call check(table, package_name, error)
if (allocated(error)) return

call get_value(table, "auto-executables", self%auto_executables, .true., stat=stat)
Expand Down Expand Up @@ -128,11 +131,14 @@ subroutine new_build_config(self, table, error)
end subroutine new_build_config

!> Check local schema for allowed entries
subroutine check(table, error)
subroutine check(table, package_name, error)

!> Instance of the TOML data structure
type(toml_table), intent(inout) :: table

!> Package name
character(len=*), intent(in) :: package_name

!> Error handling
type(error_t), allocatable, intent(out) :: error

Expand All @@ -154,7 +160,8 @@ subroutine check(table, error)
continue

case default
call syntax_error(error, "Key "//list(ikey)%key//" is not allowed in [build]")
call syntax_error(error, "Key "//list(ikey)%key//" is not allowed in [build]"//&
" building package "//package_name)
exit

end select
Expand Down
6 changes: 3 additions & 3 deletions src/fpm/manifest/package.f90
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -172,7 +172,7 @@ subroutine new_package(self, table, root, error)
call fatal_error(error, "Type mismatch for build entry, must be a table")
return
end if
call new_build_config(self%build, child, error)
call new_build_config(self%build, child, self%name, error)
if (allocated(error)) return

call get_value(table, "install", child, requested=.true., stat=stat)
Expand Down Expand Up @@ -232,7 +232,7 @@ subroutine new_package(self, table, root, error)
call new_library(self%library, child, error)
if (allocated(error)) return
end if

call get_value(table, "profiles", child, requested=.false.)
if (associated(child)) then
call new_profiles(self%profiles, child, error)
Expand Down Expand Up @@ -442,7 +442,7 @@ subroutine info(self, unit, verbosity)
call self%dev_dependency(ii)%info(unit, pr - 1)
end do
end if

if (allocated(self%profiles)) then
if (size(self%profiles) > 1 .or. pr > 2) then
write(unit, fmti) "- profiles", size(self%profiles)
Expand Down
54 changes: 51 additions & 3 deletions test/fpm_test/test_manifest.f90
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ module test_manifest
use fpm_manifest
use fpm_manifest_profile, only: profile_config_t, find_profile
use fpm_strings, only: operator(.in.)
use fpm_error, only: fatal_error, error_t
implicit none
private
public :: collect_manifest
Expand Down Expand Up @@ -42,6 +43,7 @@ subroutine collect_manifest(tests)
& new_unittest("build-config-valid", test_build_valid), &
& new_unittest("build-config-empty", test_build_empty), &
& new_unittest("build-config-invalid-values", test_build_invalid_values, should_fail=.true.), &
& new_unittest("build-key-invalid", test_build_invalid_key), &
& new_unittest("library-empty", test_library_empty), &
& new_unittest("library-wrongkey", test_library_wrongkey, should_fail=.true.), &
& new_unittest("package-simple", test_package_simple), &
Expand Down Expand Up @@ -693,6 +695,52 @@ subroutine test_build_valid(error)
end subroutine test_build_valid


!> Try to read values from the [build] table
subroutine test_build_invalid_key(error)

!> Error handling
type(error_t), allocatable, intent(out) :: error

type(package_config_t) :: package
character(:), allocatable :: temp_file
integer :: unit
type(error_t), allocatable :: build_error

allocate(temp_file, source=get_temp_filename())

open(file=temp_file, newunit=unit)
write(unit, '(a)') &
& 'name = "example"', &
& '[build]', &
& 'auto-executables = false', &
& 'auto-tests = false ', &
& 'module-naming = true ', &
& 'this-will-fail = true '
close(unit)

call get_package_data(package, temp_file, build_error)

! Error message should contain both package name and key name
if (allocated(build_error)) then

if (.not.index(build_error%message,'this-will-fail')>0) then
call fatal_error(error, 'no invalid key name is printed to output')
return
end if

if (.not.index(build_error%message,'example')>0) then
call fatal_error(error, 'no package name is printed to output')
return
end if

else
call fatal_error(error, 'no error allocated on invalid [build] section key ')
return
end if

end subroutine test_build_invalid_key


!> Try to read values from an empty [build] table
subroutine test_build_empty(error)

Expand Down Expand Up @@ -1156,7 +1204,7 @@ subroutine test_link_string(error)
table = toml_table()
call set_value(table, "link", "z", stat=stat)

call new_build_config(build, table, error)
call new_build_config(build, table, 'test_link_string', error)

end subroutine test_link_string

Expand All @@ -1179,7 +1227,7 @@ subroutine test_link_array(error)
call set_value(children, 1, "blas", stat=stat)
call set_value(children, 2, "lapack", stat=stat)

call new_build_config(build, table, error)
call new_build_config(build, table, 'test_link_array', error)

end subroutine test_link_array

Expand All @@ -1200,7 +1248,7 @@ subroutine test_invalid_link(error)
table = toml_table()
call add_table(table, "link", child, stat=stat)

call new_build_config(build, table, error)
call new_build_config(build, table, 'test_invalid_link', error)

end subroutine test_invalid_link

Expand Down