Merge pull request OSGeo#10068 from rouault/rfc96_adjustments

RFC96 Deferred driver loading adjustment to fix functional regression
innFactory · May 31, 2024 · 9266cab · 9266cab
2 parents 603e800 + 14351e8
commit 9266cab
Show file tree

Hide file tree

Showing 126 changed files with 708 additions and 217 deletions.
diff --git a/cmake/helpers/GdalDriverHelper.cmake b/cmake/helpers/GdalDriverHelper.cmake
@@ -12,6 +12,8 @@ GdalDriverHelper
 
   Symptoms add_gdal_driver( TARGET <target_name>
                             [SOURCES <source file> [<source file>[...]]]
+                            [CORE_SOURCES <source file> [<source file>[...]]]
+                            NO_SHARED_SYMBOL_WITH_CORE
                             BUILTIN | PLUGIN_CAPABLE | [PLUGIN_CAPABLE_IF <cond>]
                             [NO_DEPS]
                           )
@@ -31,6 +33,34 @@ GdalDriverHelper
 
   The NO_DEPS option express that the driver has no non-core external dependencies.
 
+  The CORE_SOURCES option points to a list of files that are used for deferred plugin
+  loading (cf RFC 96 https://gdal.org/development/rfc/rfc96_deferred_plugin_loading.html),
+  when the driver is built as a plugin.
+  Those files are build in libgdal so it has access to plugin metadata and the
+  identification method, and are typically exported so that the driver can also
+  access them.
+  However it has been found that this resulted in the inability of building a
+  plugin for a libgdal that hadn't been built with any form of support for it,
+  which is undesirable.
+  Starting with GDAL 3.9.1, NO_SHARED_SYMBOL_WITH_CORE must also be declared
+  when CORE_SOURCES is declared, so that the files pointed by CORE_SOURCES
+  are built twice: once in libgdal with a "GDAL_core_" prefix, and another time
+  in the plugin itself with a "GDAL_driver_" prefix, by using the
+  PLUGIN_SYMBOL_NAME() macro of gdal_priv.h
+
+  Example in ogrocidrivercore.h:
+
+        #define OGROCIDriverIdentify \
+           PLUGIN_SYMBOL_NAME(OGROCIDriverIdentify)
+        #define OGROCIDriverSetCommonMetadata \
+           PLUGIN_SYMBOL_NAME(OGROCIDriverSetCommonMetadata)
+
+        int OGROCIDriverIdentify(GDALOpenInfo *poOpenInfo);
+
+        void OGROCIDriverSetCommonMetadata(GDALDriver *poDriver);
+
+
+
   There are several examples to show how to write build cmake script.
 
  ex.1 Driver which is referenced by other drivers
@@ -91,7 +121,7 @@ function(_set_driver_core_sources _KEY _DRIVER_TARGET)
 endfunction()
 
 function(add_gdal_driver)
-    set(_options BUILTIN PLUGIN_CAPABLE NO_DEPS STRONG_CXX_WFLAGS CXX_WFLAGS_EFFCXX NO_CXX_WFLAGS)
+    set(_options BUILTIN PLUGIN_CAPABLE NO_DEPS STRONG_CXX_WFLAGS CXX_WFLAGS_EFFCXX NO_CXX_WFLAGS NO_SHARED_SYMBOL_WITH_CORE)
     set(_oneValueArgs TARGET DESCRIPTION DEF PLUGIN_CAPABLE_IF DRIVER_NAME_OPTION)
     set(_multiValueArgs SOURCES CORE_SOURCES)
     cmake_parse_arguments(_DRIVER "${_options}" "${_oneValueArgs}" "${_multiValueArgs}" ${ARGN})
@@ -254,6 +284,15 @@ function(add_gdal_driver)
         set_property(GLOBAL APPEND PROPERTY PLUGIN_MODULES ${_DRIVER_TARGET})
 
         if (_DRIVER_CORE_SOURCES)
+            if (_DRIVER_NO_SHARED_SYMBOL_WITH_CORE)
+                foreach(f IN LISTS _DRIVER_CORE_SOURCES)
+                    # Create a separate source file, to make sure we get a different object file
+                    file(WRITE "${CMAKE_CURRENT_BINARY_DIR}/for_driver_${f}" "#include \"${CMAKE_CURRENT_SOURCE_DIR}/${f}\"")
+                    target_sources(${_DRIVER_TARGET} PRIVATE "${CMAKE_CURRENT_BINARY_DIR}/for_driver_${f}")
+                endforeach()
+            else()
+                message(FATAL_ERROR "Driver ${_DRIVER_TARGET} should declare DRIVER_NO_SHARED_SYMBOL_WITH_CORE")
+            endif()
             _set_driver_core_sources(${_KEY} ${_DRIVER_TARGET} ${_DRIVER_CORE_SOURCES})
         endif ()
 

diff --git a/doc/source/development/rfc/rfc96_deferred_plugin_loading.rst b/doc/source/development/rfc/rfc96_deferred_plugin_loading.rst
@@ -494,6 +494,66 @@ Related issues and PRs
 
 - https://github.com/OSGeo/gdal/pull/8695: candidate implementation
 
+Adjustments done post GDAL 3.9.0, for GDAL 3.9.1
+------------------------------------------------
+
+After GDAL 3.9.0 release, it has been noticed that the following setup which
+used to work in prior releases no longer worked:
+
+- Step 1: building libgdal without support for driver X
+- Step 2: building driver X as a plugin, discarding the libgdal share library,
+          built at that stage
+- Step 3: using driver X built as a plugin against libgdal built at step 1. In that
+          scenario, driver X is expected to be loaded as if it was an out-of-tree drivers.
+
+Such scenario is used when delivering a fully open-source libgdal without any
+prior knowledge of which drivers could be later built as plugins, or for which
+pre-configuring libgdal to support such drivers is not practical because they
+rely on a proprietary SDK and the identification method and/or driver metadata
+depends on the availability of the SDK include files (e.g. MrSID).
+
+Starting with GDAL 3.9.1, the ``add_gdal_driver()`` function in the CMakeLists.txt
+of drivers which use the ``CORE_SOURCES`` keyword must also declare the
+``NO_SHARED_SYMBOL_WITH_CORE`` keyword, so that the files pointed by CORE_SOURCES
+are built twice: once in libgdal with a ``GDAL_core_`` prefix, and another time
+in the plugin itself with a ``GDAL_driver_`` prefix, by using the
+PLUGIN_SYMBOL_NAME() macro of :file:`gdal_priv.h`.
+
+Example in ogr/ogrsf_frmsts/oci/CMakeLists.txt:
+
+.. code-block::
+
+    add_gdal_driver(TARGET ogr_OCI
+                    SOURCES ${SOURCE}
+                    CORE_SOURCES ogrocidrivercore.cpp
+                    PLUGIN_CAPABLE
+                    NO_SHARED_SYMBOL_WITH_CORE)
+
+
+Example in ogrocidrivercore.h:
+
+.. code-block:: cpp
+
+    #define OGROCIDriverIdentify \
+       PLUGIN_SYMBOL_NAME(OGROCIDriverIdentify)
+    #define OGROCIDriverSetCommonMetadata \
+       PLUGIN_SYMBOL_NAME(OGROCIDriverSetCommonMetadata)
+
+    int OGROCIDriverIdentify(GDALOpenInfo *poOpenInfo);
+
+    void OGROCIDriverSetCommonMetadata(GDALDriver *poDriver);
+
+
+A consequence of that change is that drivers built as a plugin against GDAL 3.9.0
+will not be loadable by GDAL 3.9.1 (or later patch in the 3.9 series), because
+they relied on driver-specific functions that are no longer exported by libgdal >= 3.9.1.
+
+After that, things should work as they used to, and drivers built against libgdal 3.9.1
+should work against libgdal 3.9.2 for example.
+
+Also note that the above only affects *in-tree* plugin drivers. Out-of-tree plugin drivers
+are not affected.
+
 Voting history
 --------------
 

diff --git a/frmts/basisu_ktx2/CMakeLists.txt b/frmts/basisu_ktx2/CMakeLists.txt
@@ -8,7 +8,8 @@ add_gdal_driver(TARGET gdal_BASISU_KTX2
                     ktx2drivercore.cpp
                     commoncore.cpp
                 STRONG_CXX_WFLAGS
-                PLUGIN_CAPABLE)
+                PLUGIN_CAPABLE
+                NO_SHARED_SYMBOL_WITH_CORE)
 
 if(NOT TARGET gdal_BASISU_KTX2)
     return()

diff --git a/frmts/basisu_ktx2/basisudrivercore.h b/frmts/basisu_ktx2/basisudrivercore.h
@@ -33,8 +33,12 @@
 
 constexpr const char *BASISU_DRIVER_NAME = "BASISU";
 
-int CPL_DLL BASISUDriverIdentify(GDALOpenInfo *poOpenInfo);
+#define BASISUDriverIdentify PLUGIN_SYMBOL_NAME(BASISUDriverIdentify)
+#define BASISUDriverSetCommonMetadata                                          \
+    PLUGIN_SYMBOL_NAME(BASISUDriverSetCommonMetadata)
 
-void CPL_DLL BASISUDriverSetCommonMetadata(GDALDriver *poDriver);
+int BASISUDriverIdentify(GDALOpenInfo *poOpenInfo);
+
+void BASISUDriverSetCommonMetadata(GDALDriver *poDriver);
 
 #endif
diff --git a/frmts/basisu_ktx2/ktx2drivercore.h b/frmts/basisu_ktx2/ktx2drivercore.h
@@ -33,8 +33,12 @@
 
 constexpr const char *KTX2_DRIVER_NAME = "KTX2";
 
-int CPL_DLL KTX2DriverIdentify(GDALOpenInfo *poOpenInfo);
+#define KTX2DriverIdentify PLUGIN_SYMBOL_NAME(KTX2DriverIdentify)
+#define KTX2DriverSetCommonMetadata                                            \
+    PLUGIN_SYMBOL_NAME(KTX2DriverSetCommonMetadata)
 
-void CPL_DLL KTX2DriverSetCommonMetadata(GDALDriver *poDriver);
+int KTX2DriverIdentify(GDALOpenInfo *poOpenInfo);
+
+void KTX2DriverSetCommonMetadata(GDALDriver *poDriver);
 
 #endif
diff --git a/frmts/dds/CMakeLists.txt b/frmts/dds/CMakeLists.txt
@@ -1,7 +1,8 @@
 add_gdal_driver(TARGET gdal_DDS
                 SOURCES ddsdataset.cpp
                 CORE_SOURCES ddsdrivercore.cpp
-                PLUGIN_CAPABLE)
+                PLUGIN_CAPABLE
+                NO_SHARED_SYMBOL_WITH_CORE)
 
 if(NOT TARGET gdal_DDS)
     return()

diff --git a/frmts/dds/ddsdrivercore.h b/frmts/dds/ddsdrivercore.h
@@ -36,8 +36,12 @@ constexpr const char *DRIVER_NAME = "DDS";
 
 #define DDS_SIGNATURE "DDS "
 
-int CPL_DLL DDSDriverIdentify(GDALOpenInfo *poOpenInfo);
+#define DDSDriverIdentify PLUGIN_SYMBOL_NAME(DDSDriverIdentify)
+#define DDSDriverSetCommonMetadata                                             \
+    PLUGIN_SYMBOL_NAME(DDSDriverSetCommonMetadata)
 
-void CPL_DLL DDSDriverSetCommonMetadata(GDALDriver *poDriver);
+int DDSDriverIdentify(GDALOpenInfo *poOpenInfo);
+
+void DDSDriverSetCommonMetadata(GDALDriver *poDriver);
 
 #endif
diff --git a/frmts/ecw/CMakeLists.txt b/frmts/ecw/CMakeLists.txt
@@ -4,7 +4,8 @@ add_gdal_driver(
   SOURCES ${SOURCE}
   CORE_SOURCES ecwdrivercore.cpp
   DRIVER_NAME_OPTION ECW
-  DEF FRMT_ecw PLUGIN_CAPABLE)
+  DEF FRMT_ecw PLUGIN_CAPABLE
+  NO_SHARED_SYMBOL_WITH_CORE)
 
 if(TARGET gdal_ECW_JP2ECW_core)
     target_include_directories(gdal_ECW_JP2ECW_core PRIVATE $<TARGET_PROPERTY:ECW::ECW_ALL,INTERFACE_INCLUDE_DIRECTORIES>)

diff --git a/frmts/ecw/ecwdrivercore.h b/frmts/ecw/ecwdrivercore.h
@@ -35,12 +35,20 @@ constexpr const char *ECW_DRIVER_NAME = "ECW";
 
 constexpr const char *JP2ECW_DRIVER_NAME = "JP2ECW";
 
-int CPL_DLL ECWDatasetIdentifyECW(GDALOpenInfo *poOpenInfo);
+#define ECWDatasetIdentifyECW PLUGIN_SYMBOL_NAME(ECWDatasetIdentifyECW)
+#define ECWDatasetIdentifyJPEG2000                                             \
+    PLUGIN_SYMBOL_NAME(ECWDatasetIdentifyJPEG2000)
+#define ECWDriverSetCommonMetadata                                             \
+    PLUGIN_SYMBOL_NAME(ECWDriverSetCommonMetadata)
+#define JP2ECWDriverSetCommonMetadata                                          \
+    PLUGIN_SYMBOL_NAME(JP2ECWDriverSetCommonMetadata)
 
-int CPL_DLL ECWDatasetIdentifyJPEG2000(GDALOpenInfo *poOpenInfo);
+int ECWDatasetIdentifyECW(GDALOpenInfo *poOpenInfo);
 
-void CPL_DLL ECWDriverSetCommonMetadata(GDALDriver *poDriver);
+int ECWDatasetIdentifyJPEG2000(GDALOpenInfo *poOpenInfo);
 
-void CPL_DLL JP2ECWDriverSetCommonMetadata(GDALDriver *poDriver);
+void ECWDriverSetCommonMetadata(GDALDriver *poDriver);
+
+void JP2ECWDriverSetCommonMetadata(GDALDriver *poDriver);
 
 #endif
diff --git a/frmts/exr/CMakeLists.txt b/frmts/exr/CMakeLists.txt
@@ -1,7 +1,8 @@
 add_gdal_driver(TARGET gdal_EXR
                 SOURCES exrdataset.cpp
                 CORE_SOURCES exrdrivercore.cpp
-                PLUGIN_CAPABLE)
+                PLUGIN_CAPABLE
+                NO_SHARED_SYMBOL_WITH_CORE)
 
 if(NOT TARGET gdal_EXR)
     return()

diff --git a/frmts/exr/exrdrivercore.h b/frmts/exr/exrdrivercore.h
@@ -32,8 +32,12 @@
 
 constexpr const char *DRIVER_NAME = "EXR";
 
-int CPL_DLL EXRDriverIdentify(GDALOpenInfo *poOpenInfo);
+#define EXRDriverIdentify PLUGIN_SYMBOL_NAME(EXRDriverIdentify)
+#define EXRDriverSetCommonMetadata                                             \
+    PLUGIN_SYMBOL_NAME(EXRDriverSetCommonMetadata)
 
-void CPL_DLL EXRDriverSetCommonMetadata(GDALDriver *poDriver);
+int EXRDriverIdentify(GDALOpenInfo *poOpenInfo);
+
+void EXRDriverSetCommonMetadata(GDALDriver *poDriver);
 
 #endif
diff --git a/frmts/fits/CMakeLists.txt b/frmts/fits/CMakeLists.txt
@@ -1,7 +1,8 @@
 add_gdal_driver(TARGET gdal_FITS
                 SOURCES fitsdataset.cpp
                 CORE_SOURCES fitsdrivercore.cpp
-                PLUGIN_CAPABLE)
+                PLUGIN_CAPABLE
+                NO_SHARED_SYMBOL_WITH_CORE)
 
 if(NOT TARGET gdal_FITS)
     return()

diff --git a/frmts/fits/fitsdrivercore.h b/frmts/fits/fitsdrivercore.h
@@ -35,8 +35,12 @@
 
 constexpr const char *DRIVER_NAME = "FITS";
 
-int CPL_DLL FITSDriverIdentify(GDALOpenInfo *poOpenInfo);
+#define FITSDriverIdentify PLUGIN_SYMBOL_NAME(FITSDriverIdentify)
+#define FITSDriverSetCommonMetadata                                            \
+    PLUGIN_SYMBOL_NAME(FITSDriverSetCommonMetadata)
 
-void CPL_DLL FITSDriverSetCommonMetadata(GDALDriver *poDriver);
+int FITSDriverIdentify(GDALOpenInfo *poOpenInfo);
+
+void FITSDriverSetCommonMetadata(GDALDriver *poDriver);
 
 #endif
diff --git a/frmts/georaster/CMakeLists.txt b/frmts/georaster/CMakeLists.txt
@@ -3,7 +3,8 @@ add_gdal_driver(
   SOURCES georaster_dataset.cpp georaster_priv.h georaster_rasterband.cpp georaster_wrapper.cpp oci_wrapper.cpp
           cpl_vsil_ocilob.cpp
   CORE_SOURCES georasterdrivercore.cpp
-  DEF FRMT_georaster PLUGIN_CAPABLE_IF "NOT GDAL_USE_ZLIB_INTERNAL\\\;NOT GDAL_USE_JPEG_INTERNAL")
+  DEF FRMT_georaster PLUGIN_CAPABLE_IF "NOT GDAL_USE_ZLIB_INTERNAL\\\;NOT GDAL_USE_JPEG_INTERNAL"
+  NO_SHARED_SYMBOL_WITH_CORE)
 
 if(NOT TARGET gdal_GEOR)
     return()

diff --git a/frmts/georaster/georasterdrivercore.h b/frmts/georaster/georasterdrivercore.h
@@ -35,8 +35,12 @@
 
 constexpr const char *DRIVER_NAME = "GeoRaster";
 
-int CPL_DLL GEORDriverIdentify(GDALOpenInfo *poOpenInfo);
+#define GEORDriverIdentify PLUGIN_SYMBOL_NAME(GEORDriverIdentify)
+#define GEORDriverSetCommonMetadata                                            \
+    PLUGIN_SYMBOL_NAME(GEORDriverSetCommonMetadata)
 
-void CPL_DLL GEORDriverSetCommonMetadata(GDALDriver *poDriver);
+int GEORDriverIdentify(GDALOpenInfo *poOpenInfo);
+
+void GEORDriverSetCommonMetadata(GDALDriver *poDriver);
 
 #endif
diff --git a/frmts/gif/CMakeLists.txt b/frmts/gif/CMakeLists.txt
@@ -2,7 +2,8 @@ add_gdal_driver(
   TARGET gdal_GIF
   SOURCES gifabstractdataset.h gifabstractdataset.cpp biggifdataset.cpp gifdataset.cpp
   CORE_SOURCES gifdrivercore.cpp
-  PLUGIN_CAPABLE)
+  PLUGIN_CAPABLE
+  NO_SHARED_SYMBOL_WITH_CORE)
 
 if(NOT TARGET gdal_GIF)
     return()

diff --git a/frmts/gif/gifdrivercore.h b/frmts/gif/gifdrivercore.h
@@ -36,10 +36,16 @@ constexpr const char *GIF_DRIVER_NAME = "GIF";
 
 constexpr const char *BIGGIF_DRIVER_NAME = "BIGGIF";
 
-int CPL_DLL GIFDriverIdentify(GDALOpenInfo *poOpenInfo);
+#define GIFDriverIdentify PLUGIN_SYMBOL_NAME(GIFDriverIdentify)
+#define BIGGIFDriverSetCommonMetadata                                          \
+    PLUGIN_SYMBOL_NAME(BIGGIFDriverSetCommonMetadata)
+#define GIFDriverSetCommonMetadata                                             \
+    PLUGIN_SYMBOL_NAME(GIFDriverSetCommonMetadata)
 
-void CPL_DLL BIGGIFDriverSetCommonMetadata(GDALDriver *poDriver);
+int GIFDriverIdentify(GDALOpenInfo *poOpenInfo);
 
-void CPL_DLL GIFDriverSetCommonMetadata(GDALDriver *poDriver);
+void BIGGIFDriverSetCommonMetadata(GDALDriver *poDriver);
+
+void GIFDriverSetCommonMetadata(GDALDriver *poDriver);
 
 #endif
diff --git a/frmts/grib/CMakeLists.txt b/frmts/grib/CMakeLists.txt
@@ -50,7 +50,8 @@ add_gdal_driver(
           gribdrivercore.cpp
       PLUGIN_CAPABLE_IF
           "NOT GDAL_USE_PNG_INTERNAL\\\;NOT GDAL_USE_ZLIB_INTERNAL"
-      NO_DEPS)
+      NO_DEPS
+      NO_SHARED_SYMBOL_WITH_CORE)
 
 if(NOT TARGET gdal_GRIB)
     return()

diff --git a/frmts/grib/gribdrivercore.h b/frmts/grib/gribdrivercore.h
@@ -34,8 +34,12 @@
 
 constexpr const char *DRIVER_NAME = "GRIB";
 
-int CPL_DLL GRIBDriverIdentify(GDALOpenInfo *poOpenInfo);
+#define GRIBDriverIdentify PLUGIN_SYMBOL_NAME(GRIBDriverIdentify)
+#define GRIBDriverSetCommonMetadata                                            \
+    PLUGIN_SYMBOL_NAME(GRIBDriverSetCommonMetadata)
 
-void CPL_DLL GRIBDriverSetCommonMetadata(GDALDriver *poDriver);
+int GRIBDriverIdentify(GDALOpenInfo *poOpenInfo);
+
+void GRIBDriverSetCommonMetadata(GDALDriver *poDriver);
 
 #endif
diff --git a/frmts/gta/CMakeLists.txt b/frmts/gta/CMakeLists.txt
@@ -1,7 +1,8 @@
 add_gdal_driver(TARGET gdal_GTA
                 SOURCES gta_headers.h gtadataset.cpp
                 CORE_SOURCES gtadrivercore.cpp
-                PLUGIN_CAPABLE)
+                PLUGIN_CAPABLE
+                NO_SHARED_SYMBOL_WITH_CORE)
 
 if(NOT TARGET gdal_GTA)
     return()

diff --git a/frmts/gta/gtadrivercore.h b/frmts/gta/gtadrivercore.h
@@ -34,8 +34,13 @@
 
 constexpr const char *DRIVER_NAME = "GTA";
 
-int CPL_DLL GTADriverIdentify(GDALOpenInfo *poOpenInfo);
+#define GTADriverIdentify PLUGIN_SYMBOL_NAME(GTADriverIdentify)
 
-void CPL_DLL GTADriverSetCommonMetadata(GDALDriver *poDriver);
+#define GTADriverSetCommonMetadata                                             \
+    PLUGIN_SYMBOL_NAME(GTADriverSetCommonMetadata)
+
+int GTADriverIdentify(GDALOpenInfo *poOpenInfo);
+
+void GTADriverSetCommonMetadata(GDALDriver *poDriver);
 
 #endif