Kontinuation commented on code in PR #681: URL: https://github.com/apache/sedona-db/pull/681#discussion_r2894245478
########## c/sedona-gdal/src/errors.rs: ########## @@ -0,0 +1,77 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +//! Ported (and contains copied code) from georust/gdal: +//! <https://github.com/georust/gdal/blob/v0.19.0/src/errors.rs>. +//! Original code is licensed under MIT. + +use std::ffi::NulError; +use std::num::TryFromIntError; + +use thiserror::Error; + +/// Error type for the sedona-gdal crate initialization and library loading. +#[derive(Error, Debug)] +pub enum GdalInitLibraryError { + #[error("{0}")] + Invalid(String), + #[error("{0}")] + LibraryError(String), +} + +/// Error type compatible with the georust/gdal error variants used in this codebase. +#[derive(Clone, Debug, Error)] +pub enum GdalError { + #[error("CPL error class: '{class:?}', error number: '{number}', error msg: '{msg}'")] + CplError { + class: u32, + number: i32, + msg: String, + }, + + #[error("GDAL method '{method_name}' returned a NULL pointer. Error msg: '{msg}'")] + NullPointer { + method_name: &'static str, + msg: String, + }, + + #[error("Bad argument: {0}")] + BadArgument(String), + + #[error("OGR method '{method_name}' returned error: '{err:?}'")] + OgrError { err: i32, method_name: &'static str }, + + #[error("Unable to unlink mem file: {file_name}")] + UnlinkMemFile { file_name: String }, + + #[error("FFI NUL error: {0}")] + FfiNulError(#[from] NulError), + + #[error("FfiIntoStringError")] + FfiIntoStringError(#[from] std::ffi::IntoStringError), + + #[error("StrUtf8Error")] + StrUtf8Error(#[from] std::str::Utf8Error), + + #[error(transparent)] + IntConversionError(#[from] TryFromIntError), + + #[error("Buffer length {0} does not match raster size {1:?}")] + BufferSizeMismatch(usize, (usize, usize)), +} + +pub type Result<T> = std::result::Result<T, GdalError>; Review Comment: Removed ########## c/sedona-gdal/src/lib.rs: ########## @@ -0,0 +1,35 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +// --- FFI layer --- +pub(crate) mod dyn_load; +pub mod gdal_dyn_bindgen; + +// --- Error types --- +pub mod errors; + +// --- Core API --- +pub mod gdal_api; +pub mod register; + +// --- Re-exports for convenient use --- +pub use errors::GdalError; +pub use gdal_api::GdalApi; +pub use register::{ + configure_global_gdal_api, get_global_gdal_api, is_gdal_api_configured, with_global_gdal_api, + GdalApiBuilder, +}; Review Comment: Removed these reexports ########## c/sedona-gdal/src/register.rs: ########## @@ -0,0 +1,314 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +use crate::errors::GdalInitLibraryError; +use crate::gdal_api::GdalApi; +use std::path::PathBuf; +use std::sync::{Mutex, OnceLock}; + +/// Minimum GDAL version required by sedona-gdal. +#[cfg(feature = "gdal-sys")] +const MIN_GDAL_VERSION_MAJOR: i32 = 3; +#[cfg(feature = "gdal-sys")] +const MIN_GDAL_VERSION_MINOR: i32 = 4; + +/// Builder for the global [`GdalApi`]. +/// +/// Provides a way to configure how GDAL is loaded before the first use. +/// Use [`configure_global_gdal_api`] to install a builder, then the actual +/// loading happens lazily on the first call to [`get_global_gdal_api`]. +/// +/// # Examples +/// +/// ```no_run +/// use sedona_gdal::register::{GdalApiBuilder, configure_global_gdal_api}; +/// +/// // Configure with a specific shared library path +/// let builder = GdalApiBuilder::default() +/// .with_shared_library("/usr/lib/libgdal.so".into()); +/// configure_global_gdal_api(builder).unwrap(); +/// ``` +#[derive(Default)] +pub struct GdalApiBuilder { + shared_library: Option<PathBuf>, +} + +impl GdalApiBuilder { + /// Set the path to the GDAL shared library. + /// + /// If unset, GDAL symbols will be resolved from the current process image + /// (equivalent to `dlopen(NULL)`), which requires GDAL to already be linked + /// into the process (e.g. via `gdal-sys`). + /// + /// Note that the path is passed directly to `dlopen()`/`LoadLibrary()`, + /// which takes into account the working directory. As a security measure, + /// applications may wish to verify that the path is absolute. This should + /// not be specified from untrusted input. + pub fn with_shared_library(self, path: PathBuf) -> Self { + Self { + shared_library: Some(path), + } + } + + /// Build a [`GdalApi`] with the configured options. + /// + /// When `shared_library` is set, loads GDAL from that path. Otherwise, + /// resolves symbols from the current process (with an optional + /// compile-time version check when the `gdal-sys` feature is enabled). + pub fn build(&self) -> Result<GdalApi, GdalInitLibraryError> { + if let Some(shared_library) = &self.shared_library { + GdalApi::try_from_shared_library(shared_library.clone()) + } else { + #[cfg(feature = "gdal-sys")] + check_gdal_version()?; + GdalApi::try_from_current_process() + } + } +} + +/// Global builder configuration, protected by a [`Mutex`]. +/// +/// Set via [`configure_global_gdal_api`] before the first call to +/// [`get_global_gdal_api`]. Multiple calls to `configure_global_gdal_api` +/// are allowed as long as the API has not been initialized yet. +/// The same mutex also serves as the initialization guard for +/// [`get_global_gdal_api`], eliminating the need for a separate lock. +static GDAL_API_BUILDER: Mutex<Option<GdalApiBuilder>> = Mutex::new(None); + +static GDAL_API: OnceLock<GdalApi> = OnceLock::new(); + +/// Get a reference to the global GDAL API, initializing it if not already done. +/// +/// On first call, reads the builder set by [`configure_global_gdal_api`] (or uses +/// [`GdalApiBuilder::default()`] if none was configured) and calls its `build()` +/// method to create the [`GdalApi`]. The result is stored in a process-global +/// `OnceLock` and reused for all subsequent calls. +pub fn get_global_gdal_api() -> Result<&'static GdalApi, GdalInitLibraryError> { Review Comment: I thought that we can expose both. `with_global_gdal_api` has the style as the sedona-proj API so I'll prefer `with_global_gdal_api` and mark `get_global_gdal_api` as non-public. ########## c/sedona-gdal/src/register.rs: ########## @@ -0,0 +1,314 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +use crate::errors::GdalInitLibraryError; +use crate::gdal_api::GdalApi; +use std::path::PathBuf; +use std::sync::{Mutex, OnceLock}; + +/// Minimum GDAL version required by sedona-gdal. +#[cfg(feature = "gdal-sys")] +const MIN_GDAL_VERSION_MAJOR: i32 = 3; +#[cfg(feature = "gdal-sys")] +const MIN_GDAL_VERSION_MINOR: i32 = 4; + +/// Builder for the global [`GdalApi`]. +/// +/// Provides a way to configure how GDAL is loaded before the first use. +/// Use [`configure_global_gdal_api`] to install a builder, then the actual +/// loading happens lazily on the first call to [`get_global_gdal_api`]. +/// +/// # Examples +/// +/// ```no_run +/// use sedona_gdal::register::{GdalApiBuilder, configure_global_gdal_api}; +/// +/// // Configure with a specific shared library path +/// let builder = GdalApiBuilder::default() +/// .with_shared_library("/usr/lib/libgdal.so".into()); +/// configure_global_gdal_api(builder).unwrap(); +/// ``` +#[derive(Default)] +pub struct GdalApiBuilder { + shared_library: Option<PathBuf>, +} + +impl GdalApiBuilder { + /// Set the path to the GDAL shared library. + /// + /// If unset, GDAL symbols will be resolved from the current process image + /// (equivalent to `dlopen(NULL)`), which requires GDAL to already be linked + /// into the process (e.g. via `gdal-sys`). + /// + /// Note that the path is passed directly to `dlopen()`/`LoadLibrary()`, + /// which takes into account the working directory. As a security measure, + /// applications may wish to verify that the path is absolute. This should + /// not be specified from untrusted input. + pub fn with_shared_library(self, path: PathBuf) -> Self { + Self { + shared_library: Some(path), + } + } + + /// Build a [`GdalApi`] with the configured options. + /// + /// When `shared_library` is set, loads GDAL from that path. Otherwise, + /// resolves symbols from the current process (with an optional + /// compile-time version check when the `gdal-sys` feature is enabled). + pub fn build(&self) -> Result<GdalApi, GdalInitLibraryError> { + if let Some(shared_library) = &self.shared_library { + GdalApi::try_from_shared_library(shared_library.clone()) + } else { + #[cfg(feature = "gdal-sys")] + check_gdal_version()?; + GdalApi::try_from_current_process() + } + } +} + +/// Global builder configuration, protected by a [`Mutex`]. +/// +/// Set via [`configure_global_gdal_api`] before the first call to +/// [`get_global_gdal_api`]. Multiple calls to `configure_global_gdal_api` +/// are allowed as long as the API has not been initialized yet. +/// The same mutex also serves as the initialization guard for +/// [`get_global_gdal_api`], eliminating the need for a separate lock. +static GDAL_API_BUILDER: Mutex<Option<GdalApiBuilder>> = Mutex::new(None); + +static GDAL_API: OnceLock<GdalApi> = OnceLock::new(); + +/// Get a reference to the global GDAL API, initializing it if not already done. +/// +/// On first call, reads the builder set by [`configure_global_gdal_api`] (or uses +/// [`GdalApiBuilder::default()`] if none was configured) and calls its `build()` +/// method to create the [`GdalApi`]. The result is stored in a process-global +/// `OnceLock` and reused for all subsequent calls. +pub fn get_global_gdal_api() -> Result<&'static GdalApi, GdalInitLibraryError> { + if let Some(api) = GDAL_API.get() { + return Ok(api); + } + + let guard = GDAL_API_BUILDER + .lock() + .map_err(|_| GdalInitLibraryError::Invalid("GDAL API builder lock poisoned".to_string()))?; + + if let Some(api) = GDAL_API.get() { + return Ok(api); + } + + let api = guard + .as_ref() + .unwrap_or(&GdalApiBuilder::default()) + .build()?; + + // Register all GDAL drivers once, immediately after loading symbols. + // This mirrors georust/gdal's `_register_drivers()` pattern where + // `GDALAllRegister` is called via `std::sync::Once` before any driver + // lookup or dataset open. Here the `OnceLock` + `Mutex` already + // guarantees this runs exactly once. + unsafe { + let Some(gdal_all_register) = api.inner.GDALAllRegister else { + return Err(GdalInitLibraryError::LibraryError( + "GDALAllRegister symbol not loaded".to_string(), + )); + }; + gdal_all_register(); + } + + let _ = GDAL_API.set(api); + Ok(GDAL_API.get().expect("GDAL API should be set")) +} + +/// Configure the global GDAL API. +/// +/// Stores the given [`GdalApiBuilder`] for use when the global [`GdalApi`] is +/// first initialized (lazily, on the first call to [`get_global_gdal_api`]). +/// +/// This can be called multiple times before the API is initialized — each call +/// replaces the previous builder. However, once [`get_global_gdal_api`] has been +/// called and the API has been successfully initialized, subsequent configurations +/// are accepted but will have no effect (the `OnceLock` ensures the API is created +/// only once). +/// +/// # Typical usage +/// +/// 1. The application (e.g. sedona-db) calls `configure_global_gdal_api` with its +/// default builder early in startup. +/// 2. User code may call `configure_global_gdal_api` again to override the +/// configuration before the first query that uses GDAL. +/// 3. On the first actual GDAL operation, [`get_global_gdal_api`] reads the +/// builder and initializes the API. +pub fn configure_global_gdal_api(builder: GdalApiBuilder) -> Result<(), GdalInitLibraryError> { + let mut global_builder = GDAL_API_BUILDER.lock().map_err(|_| { + GdalInitLibraryError::Invalid( + "Failed to acquire lock for global GDAL configuration".to_string(), + ) + })?; + global_builder.replace(builder); + Ok(()) +} + +pub fn is_gdal_api_configured() -> bool { + GDAL_API.get().is_some() +} + +pub fn with_global_gdal_api<F, R>(func: F) -> Result<R, GdalInitLibraryError> +where Review Comment: Documentation added -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
