From 2d20d5a3c593b24e9a1745383dbcde3f5747a188 Mon Sep 17 00:00:00 2001 From: Michael Reichert Date: Thu, 7 Sep 2023 17:16:35 +0200 Subject: [PATCH 1/7] Add support for open options in OGR input plugin --- plugins/input/ogr/build.py | 1 + plugins/input/ogr/ogr_datasource.cpp | 9 ++- plugins/input/ogr/ogr_datasource.hpp | 1 + plugins/input/ogr/ogr_utils.cpp | 85 ++++++++++++++++++++++++++++ plugins/input/ogr/ogr_utils.hpp | 43 ++++++++++++++ test/build.py | 1 + test/unit/datasource/ogr.cpp | 69 ++++++++++++++++++++++ 7 files changed, 208 insertions(+), 1 deletion(-) create mode 100644 plugins/input/ogr/ogr_utils.cpp create mode 100644 plugins/input/ogr/ogr_utils.hpp diff --git a/plugins/input/ogr/build.py b/plugins/input/ogr/build.py index acf487311..06d9fbe6f 100644 --- a/plugins/input/ogr/build.py +++ b/plugins/input/ogr/build.py @@ -31,6 +31,7 @@ plugin_sources = Split( """ %(PLUGIN_NAME)s_converter.cpp %(PLUGIN_NAME)s_datasource.cpp + %(PLUGIN_NAME)s_utils.cpp %(PLUGIN_NAME)s_featureset.cpp %(PLUGIN_NAME)s_index_featureset.cpp """ % locals() diff --git a/plugins/input/ogr/ogr_datasource.cpp b/plugins/input/ogr/ogr_datasource.cpp index 9663fb096..7cfa0b8de 100644 --- a/plugins/input/ogr/ogr_datasource.cpp +++ b/plugins/input/ogr/ogr_datasource.cpp @@ -127,16 +127,23 @@ void ogr_datasource::init(mapnik::parameters const& params) } std::string driver = *params.get("driver", ""); + std::vector open_options_map = ogr_utils::split_open_options(*params.get("open_options", "")); + char** open_options = ogr_utils::open_options_for_ogr(open_options_map); if (!driver.empty()) { unsigned int nOpenFlags = GDAL_OF_READONLY | GDAL_OF_VECTOR; const char* papszAllowedDrivers[] = {driver.c_str(), nullptr}; + dataset_ = reinterpret_cast( - GDALOpenEx(dataset_name_.c_str(), nOpenFlags, papszAllowedDrivers, nullptr, nullptr)); + GDALOpenEx(dataset_name_.c_str(), nOpenFlags, papszAllowedDrivers, open_options, nullptr)); } else { + if (open_options[0] != nullptr) + { + throw datasource_exception(" parameter provided but is missing"); + } // open ogr driver dataset_ = reinterpret_cast(OGROpen(dataset_name_.c_str(), false, nullptr)); } diff --git a/plugins/input/ogr/ogr_datasource.hpp b/plugins/input/ogr/ogr_datasource.hpp index bde5829b0..819353421 100644 --- a/plugins/input/ogr/ogr_datasource.hpp +++ b/plugins/input/ogr/ogr_datasource.hpp @@ -47,6 +47,7 @@ MAPNIK_DISABLE_WARNING_PUSH #include MAPNIK_DISABLE_WARNING_POP #include "ogr_layer_ptr.hpp" +#include "ogr_utils.hpp" DATASOURCE_PLUGIN_DEF(ogr_datasource_plugin, ogr); diff --git a/plugins/input/ogr/ogr_utils.cpp b/plugins/input/ogr/ogr_utils.cpp new file mode 100644 index 000000000..04a3ec0f9 --- /dev/null +++ b/plugins/input/ogr/ogr_utils.cpp @@ -0,0 +1,85 @@ +/***************************************************************************** + * + * This file is part of Mapnik (c++ mapping toolkit) + * + * Copyright (C) 2023 Artem Pavlenko + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA + * + *****************************************************************************/ + +#include "ogr_utils.hpp" +#include + +std::vector ogr_utils::split_open_options(const std::string& options) +{ + size_t i; + bool escaped = false; // if previous character was a backslash to escape a backslash or space + std::vector opts; + std::string unescaped_str; // copy of input string but unescaped + for (i = 0; i < options.size(); ++i) + { + char current = options.at(i); + if (current == '\\') + { + if (escaped) + { + unescaped_str.push_back(current); + } + escaped = !escaped; + } + else if (current != ' ') { + unescaped_str.push_back(current); + } + if (current == ' ' || i + 1 == options.size()) + { + if (!escaped) { + size_t count = unescaped_str.size(); + if (count > 0) + { + option_ptr opt (new char[count + 1], [](char* arr) { delete[] arr; }); + unescaped_str.copy(opt.get(), count); + opt[count] = '\0'; + opts.push_back(std::move(opt)); + } + unescaped_str = ""; + } + else + { + escaped = false; + unescaped_str.push_back(current); + } + } + } + if (escaped) + { + throw mapnik::datasource_exception(" parameter ends with single backslash"); + } + opts.emplace_back(nullptr); + return opts; +} + + +char** ogr_utils::open_options_for_ogr(std::vector& options) +{ + char** for_ogr = new char*[options.size() + 1]; + for (size_t i = 0; i < options.size(); ++i) + { + for_ogr[i] = options.at(i).get(); + } + for_ogr[options.size()] = nullptr; + return for_ogr; +} + diff --git a/plugins/input/ogr/ogr_utils.hpp b/plugins/input/ogr/ogr_utils.hpp new file mode 100644 index 000000000..af306925d --- /dev/null +++ b/plugins/input/ogr/ogr_utils.hpp @@ -0,0 +1,43 @@ +/***************************************************************************** + * + * This file is part of Mapnik (c++ mapping toolkit) + * + * Copyright (C) 2023 Artem Pavlenko + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA + * + *****************************************************************************/ + +#ifndef PLUGINS_INPUT_OGR_OGR_UTILS_HPP_ +#define PLUGINS_INPUT_OGR_OGR_UTILS_HPP_ + +#include +#include +#include +#include + +namespace ogr_utils { + +using option_ptr = std::unique_ptr>; + +std::vector split_open_options(const std::string& options); + +char** open_options_for_ogr(std::vector& options); + +} // namespace ogr_utils + + + +#endif /* PLUGINS_INPUT_OGR_OGR_UTILS_HPP_ */ diff --git a/test/build.py b/test/build.py index 6771bae1f..1a784569a 100644 --- a/test/build.py +++ b/test/build.py @@ -35,6 +35,7 @@ else: # unit tests sources = glob.glob('./unit/*/*.cpp') sources.extend(glob.glob('./unit/*.cpp')) + sources.append('../plugins/input/ogr/ogr_utils.cpp') test_program = test_env_local.Program("./unit/run", source=sources) Depends(test_program, env.subst('../src/%s' % env['MAPNIK_LIB_NAME'])) Depends(test_program, env.subst('../src/json/libmapnik-json${LIBSUFFIX}')) diff --git a/test/unit/datasource/ogr.cpp b/test/unit/datasource/ogr.cpp index 8441ecc55..a25548b55 100644 --- a/test/unit/datasource/ogr.cpp +++ b/test/unit/datasource/ogr.cpp @@ -29,6 +29,19 @@ #include #include #include +#include "../../../plugins/input/ogr/ogr_utils.hpp" + +inline std::vector split_options(std::string const& options) +{ + return ogr_utils::split_open_options(options); +} + +void assert_option(std::vector const& options, size_t const& index, std::string const expected) +{ + auto const* opt = options.at(index).get(); + REQUIRE(opt != nullptr); + REQUIRE_FALSE(strcmp(opt, expected.c_str())); +} TEST_CASE("ogr") { @@ -57,3 +70,59 @@ TEST_CASE("ogr") } } } + +TEST_CASE("ogr open_options") +{ + const bool have_ogr_plugin = mapnik::datasource_cache::instance().plugin_registered("ogr"); + if (have_ogr_plugin) + { + SECTION("splitting open_options string") + { + std::string opts = "ZOOM=5"; + std::vector v = split_options(opts); + assert_option(v, 0, opts); + } + SECTION("splitting open_options string with multiple components") + { + std::string opts = "ZOOM=8 USE_BOUNDS=YES"; + std::vector v = split_options(opts); + assert_option(v, 0, "ZOOM=8"); + assert_option(v, 1, "USE_BOUNDS=YES"); + } + SECTION("open_options string with escaped character") + { + std::string opts = "MY_KEY=THIS\\ VALUE OTHER_KEY=SOMETHING"; + std::vector v = split_options(opts); + assert_option(v, 0, "MY_KEY=THIS VALUE"); + assert_option(v, 1, "OTHER_KEY=SOMETHING"); + } + SECTION("splitting open_options string with escaping") + { + std::string opts = "ZOOM=14 FANCY_OPTION=WITH\\ SPACE_VALUE"; + std::vector v = split_options(opts); + assert_option(v, 0, "ZOOM=14"); + assert_option(v, 1, "FANCY_OPTION=WITH SPACE_VALUE"); + } + SECTION("splitting open_options string, starts with space") + { + std::string opts = " ZOOM=14 K23=V45"; + std::vector v = split_options(opts); + assert_option(v, 0, "ZOOM=14"); + assert_option(v, 1, "K23=V45"); + } + SECTION("splitting open_options string, ends with space") + { + std::string opts = "ZOOM=14 K23=V46 "; + std::vector v = split_options(opts); + assert_option(v, 0, "ZOOM=14"); + assert_option(v, 1, "K23=V46"); + } + SECTION("splitting open_options string, doubled space") + { + std::string opts = "ZOOM=14 K23=V47"; + std::vector v = split_options(opts); + assert_option(v, 0, "ZOOM=14"); + assert_option(v, 1, "K23=V47"); + } + } +} From d745e43eb1c6c2753f4578f1cacff74cbbfd85f2 Mon Sep 17 00:00:00 2001 From: Artem Pavlenko Date: Fri, 17 Nov 2023 16:33:01 +0000 Subject: [PATCH 2/7] pre-commit run --show-diff-on-failure --color=always --all-files --- plugins/input/ogr/ogr_datasource.cpp | 3 ++- plugins/input/ogr/ogr_utils.cpp | 10 +++++----- plugins/input/ogr/ogr_utils.hpp | 4 +--- 3 files changed, 8 insertions(+), 9 deletions(-) diff --git a/plugins/input/ogr/ogr_datasource.cpp b/plugins/input/ogr/ogr_datasource.cpp index 7cfa0b8de..cdb898b2a 100644 --- a/plugins/input/ogr/ogr_datasource.cpp +++ b/plugins/input/ogr/ogr_datasource.cpp @@ -127,7 +127,8 @@ void ogr_datasource::init(mapnik::parameters const& params) } std::string driver = *params.get("driver", ""); - std::vector open_options_map = ogr_utils::split_open_options(*params.get("open_options", "")); + std::vector open_options_map = + ogr_utils::split_open_options(*params.get("open_options", "")); char** open_options = ogr_utils::open_options_for_ogr(open_options_map); if (!driver.empty()) diff --git a/plugins/input/ogr/ogr_utils.cpp b/plugins/input/ogr/ogr_utils.cpp index 04a3ec0f9..f052a2af2 100644 --- a/plugins/input/ogr/ogr_utils.cpp +++ b/plugins/input/ogr/ogr_utils.cpp @@ -40,16 +40,18 @@ std::vector ogr_utils::split_open_options(const std::stri } escaped = !escaped; } - else if (current != ' ') { + else if (current != ' ') + { unescaped_str.push_back(current); } if (current == ' ' || i + 1 == options.size()) { - if (!escaped) { + if (!escaped) + { size_t count = unescaped_str.size(); if (count > 0) { - option_ptr opt (new char[count + 1], [](char* arr) { delete[] arr; }); + option_ptr opt(new char[count + 1], [](char* arr) { delete[] arr; }); unescaped_str.copy(opt.get(), count); opt[count] = '\0'; opts.push_back(std::move(opt)); @@ -71,7 +73,6 @@ std::vector ogr_utils::split_open_options(const std::stri return opts; } - char** ogr_utils::open_options_for_ogr(std::vector& options) { char** for_ogr = new char*[options.size() + 1]; @@ -82,4 +83,3 @@ char** ogr_utils::open_options_for_ogr(std::vector& optio for_ogr[options.size()] = nullptr; return for_ogr; } - diff --git a/plugins/input/ogr/ogr_utils.hpp b/plugins/input/ogr/ogr_utils.hpp index af306925d..fe0961c80 100644 --- a/plugins/input/ogr/ogr_utils.hpp +++ b/plugins/input/ogr/ogr_utils.hpp @@ -36,8 +36,6 @@ std::vector split_open_options(const std::string& options); char** open_options_for_ogr(std::vector& options); -} // namespace ogr_utils - - +} // namespace ogr_utils #endif /* PLUGINS_INPUT_OGR_OGR_UTILS_HPP_ */ From 8cdca5f5be97b9fb4de95e0214197f879e6fcfb7 Mon Sep 17 00:00:00 2001 From: David Hummel <6109326+hummeltech@users.noreply.github.com> Date: Mon, 20 Nov 2023 10:21:12 -0700 Subject: [PATCH 3/7] Fix broken builds with libxml2 >= v2.12.0 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit **I.E.** ``` src/libxml2_loader.cpp:91:50: error: invalid conversion from ‘const xmlError*’ {aka ‘const _xmlError*’} to ‘xmlError*’ {aka ‘_xmlError*’} [-fpermissive] src/libxml2_loader.cpp:131:50: error: invalid conversion from ‘const xmlError*’ {aka ‘const _xmlError*’} to ‘xmlError*’ {aka ‘_xmlError*’} [-fpermissive] ``` --- src/libxml2_loader.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/libxml2_loader.cpp b/src/libxml2_loader.cpp index 223d8df44..78dc8aa97 100644 --- a/src/libxml2_loader.cpp +++ b/src/libxml2_loader.cpp @@ -88,7 +88,7 @@ class libxml2_loader : util::noncopyable if (!doc) { - xmlError* error = xmlCtxtGetLastError(ctx_); + const xmlError* error = xmlCtxtGetLastError(ctx_); if (error) { std::string msg("XML document not well formed:\n"); @@ -128,7 +128,7 @@ class libxml2_loader : util::noncopyable if (!doc) { std::string msg("XML document not well formed"); - xmlError* error = xmlCtxtGetLastError(ctx_); + const xmlError* error = xmlCtxtGetLastError(ctx_); if (error) { msg += ":\n"; From c8fd548133dd0565ea38d00b90f67361d8363af6 Mon Sep 17 00:00:00 2001 From: David Hummel <6109326+hummeltech@users.noreply.github.com> Date: Mon, 20 Nov 2023 10:13:29 -0700 Subject: [PATCH 4/7] Fix build issues after addition of OGR open_options support --- plugins/input/ogr/CMakeLists.txt | 1 + test/CMakeLists.txt | 1 + 2 files changed, 2 insertions(+) diff --git a/plugins/input/ogr/CMakeLists.txt b/plugins/input/ogr/CMakeLists.txt index 8f1fe9aff..3f3d11f9e 100644 --- a/plugins/input/ogr/CMakeLists.txt +++ b/plugins/input/ogr/CMakeLists.txt @@ -4,6 +4,7 @@ add_plugin_target(input-ogr "ogr") target_sources(input-ogr ${_plugin_visibility} ogr_converter.cpp ogr_datasource.cpp + ogr_utils.cpp ogr_featureset.cpp ogr_index_featureset.cpp ) diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index f275df53f..d15edecda 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -34,6 +34,7 @@ add_executable(mapnik-test-unit unit/datasource/geobuf.cpp unit/datasource/geojson.cpp unit/datasource/memory.cpp + ../plugins/input/ogr/ogr_utils.cpp unit/datasource/ogr.cpp unit/datasource/postgis.cpp unit/datasource/shapeindex.cpp From a5a96aacade824429490a58fca0430190ff3c168 Mon Sep 17 00:00:00 2001 From: Michael Reichert Date: Thu, 23 Nov 2023 22:03:11 +0100 Subject: [PATCH 5/7] Fix configure script for variables read from mapnik-settings.env Custom build variables (e.g. CXX_STD or CXX) read from mapnik-settings.env have to be provided as command line arguments because SCons does not respect the environment variables. --- configure | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/configure b/configure index b69b959ca..c2cabf48a 100755 --- a/configure +++ b/configure @@ -15,4 +15,6 @@ if [ -f mapnik-settings.env ]; then . ./mapnik-settings.env fi -$PYTHON scons/scons.py --implicit-deps-changed configure "$@" +VARS=( $(cat mapnik-settings.env) ) + +$PYTHON scons/scons.py --implicit-deps-changed configure ${VARS[*]} "$@" From ee76817d14dedd7918eb2b9123ab4431a177b9a1 Mon Sep 17 00:00:00 2001 From: Michael Reichert Date: Fri, 24 Nov 2023 13:46:29 +0100 Subject: [PATCH 6/7] Fix bugs in configure script The script expected that the shell supports arrays but only Z shell and Bash do so. Other shells return syntax errors. In addition, the script expected the mapnik-settings.env file to exist and crashed otherwise. --- configure | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/configure b/configure index c2cabf48a..39fd48c7d 100755 --- a/configure +++ b/configure @@ -1,20 +1,32 @@ -#! /bin/sh +#! /usr/bin/env bash set -eu : ${PYTHON:=python} +# Only some shells (Bash and Z shell) support arrays. Therefore, +# the following code provides an alternative for users calling the script +# with shells other than Bash or Z shell (e.g. Debian users using Dash). +THE_SHELL=$(basename $SHELL) +if [ "$THE_SHELL" != "bash" ] && [ "$THE_SHELL" != "zsh" ]; then + if [ -f mapnik-settings.env ]; then + echo "WARNING: Reading from mapnik-settings.env is supported with Bash or Z shell only." + fi + $PYTHON scons/scons.py --implicit-deps-changed configure "$@" + exit 0 +fi + # mapnik-settings.env is an optional file to store # environment variables that should be used before # running tests like PROJ_LIB, GDAL_DATA, and ICU_DATA # These do not normally need to be set except when # building against binary versions of dependencies like # done via bootstrap.sh +VARS=() if [ -f mapnik-settings.env ]; then echo "Inheriting from mapnik-settings.env" . ./mapnik-settings.env + VARS=( $(cat mapnik-settings.env) ) fi -VARS=( $(cat mapnik-settings.env) ) - $PYTHON scons/scons.py --implicit-deps-changed configure ${VARS[*]} "$@" From 4f6ab6a4a29186d21f0e82b72738289a1eed13f7 Mon Sep 17 00:00:00 2001 From: Artem Pavlenko Date: Mon, 5 Feb 2024 09:36:56 +0000 Subject: [PATCH 7/7] Fix spurious ',' in operator== --- src/gradient.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/gradient.cpp b/src/gradient.cpp index 18edf2f6e..6984d8b60 100644 --- a/src/gradient.cpp +++ b/src/gradient.cpp @@ -70,8 +70,8 @@ gradient& gradient::operator=(gradient rhs) bool gradient::operator==(gradient const& other) const { return transform_ == other.transform_ && x1_ == other.x1_ && y1_ == other.y1_ && x2_ == other.x2_ && - y2_ == other.y2_ && r_ == other.r_ && std::equal(stops_.begin(), stops_.end(), other.stops_.begin()), - units_ == other.units_ && gradient_type_ == other.gradient_type_; + y2_ == other.y2_ && r_ == other.r_ && std::equal(stops_.begin(), stops_.end(), other.stops_.begin()) && + units_ == other.units_ && gradient_type_ == other.gradient_type_; } void gradient::set_gradient_type(gradient_e grad)