From bde53327f1b873e34078e3692e443aca9a361d17 Mon Sep 17 00:00:00 2001 From: Federico Perini Date: Mon, 24 Apr 2023 22:10:24 +0200 Subject: [PATCH 1/3] return package name on invalid build key --- src/fpm/manifest/build.f90 | 15 +++++++++++---- src/fpm/manifest/package.f90 | 6 +++--- 2 files changed, 14 insertions(+), 7 deletions(-) diff --git a/src/fpm/manifest/build.f90 b/src/fpm/manifest/build.f90 index 8047dd045d..b3af26e517 100644 --- a/src/fpm/manifest/build.f90 +++ b/src/fpm/manifest/build.f90 @@ -52,7 +52,7 @@ 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 @@ -60,12 +60,15 @@ subroutine new_build_config(self, table, 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 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) @@ -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 @@ -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 diff --git a/src/fpm/manifest/package.f90 b/src/fpm/manifest/package.f90 index ddad144d75..4769101e74 100644 --- a/src/fpm/manifest/package.f90 +++ b/src/fpm/manifest/package.f90 @@ -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) @@ -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) @@ -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) From 5919057b4b7d93e1003b12fddf9f4d8999fc664c Mon Sep 17 00:00:00 2001 From: Federico Perini Date: Mon, 24 Apr 2023 22:10:29 +0200 Subject: [PATCH 2/3] write test --- test/fpm_test/test_manifest.f90 | 54 +++++++++++++++++++++++++++++++-- 1 file changed, 51 insertions(+), 3 deletions(-) diff --git a/test/fpm_test/test_manifest.f90 b/test/fpm_test/test_manifest.f90 index ccb401b7c6..26cb310e23 100644 --- a/test/fpm_test/test_manifest.f90 +++ b/test/fpm_test/test_manifest.f90 @@ -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 @@ -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), & @@ -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) @@ -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 @@ -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 @@ -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 From dfafcf3cdaa02dc6758f6f2226b8057129287e40 Mon Sep 17 00:00:00 2001 From: Federico Perini Date: Thu, 27 Apr 2023 08:15:57 -0500 Subject: [PATCH 3/3] improve output message --- src/fpm/manifest/build.f90 | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/fpm/manifest/build.f90 b/src/fpm/manifest/build.f90 index b3af26e517..ac8dad8473 100644 --- a/src/fpm/manifest/build.f90 +++ b/src/fpm/manifest/build.f90 @@ -160,8 +160,9 @@ subroutine check(table, package_name, error) continue case default - call syntax_error(error, "Key "//list(ikey)%key//" is not allowed in [build]"//& - " building package "//package_name) + + call syntax_error(error, 'Manifest file syntax error: key "'//list(ikey)%key//'" found in the [build] '//& + 'section of package/dependency "'//package_name//'" fpm.toml is not allowed') exit end select