3.8.0: issues wih header files and pkgconfig inteface
I made simple test on gdal header files installed by install target:
[tkloczko@pers-jacek include]$ grep -r \#include -h | sort |uniq
#include "cpl_atomic_ops.h"
#include "cpl_config-i386.h"
#include "cpl_config-ppc.h"
#include "cpl_config-ppc64.h"
#include "cpl_config-s390.h"
#include "cpl_config-s390x.h"
#include "cpl_config-sparc.h"
#include "cpl_config-sparc64.h"
#include "cpl_config-x86_64.h"
#include "cpl_config.h"
#include "cpl_conv.h"
#include "cpl_error.h"
#include "cpl_hash_set.h"
#include "cpl_json.h"
#include "cpl_minixml.h"
#include "cpl_minizip_ioapi.h"
#include "cpl_multiproc.h"
#include "cpl_port.h"
#include "cpl_progress.h"
#include "cpl_quad_tree.h"
#include "cpl_string.h"
#include "cpl_virtualmem.h"
#include "cpl_vsi.h"
#include "cpl_vsi_error.h"
#include "cpl_worker_thread_pool.h"
#include "cpl_zlib_header.h" // to avoid warnings when including zlib.h
#include "gdal.h"
#include "gdal_alg.h"
#include "gdal_frmts.h"
#include "gdal_pam.h"
#include "gdal_priv.h"
#include "gdal_rat.h"
#include "gdal_version.h"
#include "gdal_vrt.h"
#include "gdalgeorefpamdataset.h"
#include "gdalsubdatasetinfo.h"
#include "gnm.h"
#include "gnmgraph.h"
#include "ogr_api.h"
#include "ogr_core.h"
#include "ogr_feature.h"
#include "ogr_featurestyle.h"
#include "ogr_geometry.h"
#include "ogr_spatialref.h"
#include "ogr_srs_api.h"
#include "ogrsf_frmts.h"
#include "zlib.h"
#include <algorithm>
#include <array>
#include <atomic>
#include <cmath>
#include <cstddef>
#include <cstdint>
#include <ctype.h>
#include <deque>
#include <direct.h>
#include <errno.h>
#include <exception>
#include <float.h>
#include <functional>
#include <ieeefp.h>
#include <iterator>
#include <limits.h>
#include <limits>
#include <list>
#include <locale.h>
#include <map>
#include <math.h>
#include <memory>
#include <memory> // for std::unique_ptr
#include <odbcinst.h>
#include <queue>
#include <sal.h>
#include <set>
#include <sql.h>
#include <sqlext.h>
#include <stdarg.h>
#include <stdbool.h>
#include <stddef.h>
#include <stddef.h> // ptrdiff_t
#include <stdint.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <string>
#include <strings.h>
#include <sys/stat.h>
#include <sys/time.h>
#include <sys/types.h>
#include <time.h>
#include <type_traits>
#include <type_traits> // for std::is_base_of
#include <unistd.h>
#include <utility>
#include <vector>
#include <windows.h>
/* #include "cpl_config_extras.h" */
Issues:
- all above
#include "foo.h"should be replaced by#include <foo.h> - on above list is included
zlib.hhowever in cmake/template/gdal.pc.in there is noRequires: zlibso this field should be added or include ofzlib.hshould be removed.
- all above
#include "foo.h"should be replaced by#include <foo.h>
why ? Looking on my system with other packages, both styles are used. For example https://github.com/madler/zlib/blob/643e17b7498d12ab8d15565662880579692f769d/zlib.h#L34 uses #include "zconf.h"
- on above list is included zlib.h
Yes I noticed that recently. Those header files should have never been installed. They will no longer be installed in GDAL 3.9.0 per https://github.com/OSGeo/gdal/pull/8817
- all above
#include "foo.h"should be replaced by#include <foo.h>why ? Looking on my system with other packages, both styles are used. For example https://github.com/madler/zlib/blob/643e17b7498d12ab8d15565662880579692f769d/zlib.h#L34 uses
#include "zconf.h"
configs are kind of exceptions. Nevertheless `#include "foo.h" causes that such file first is checked in current directory.
https://github.com/OSGeo/gdal/issues/5778#issuecomment-1136523551
Perhaps the reason for the #include "foo.h" is because this project is trying to "solve famine problem on Earth using build framework"?
Just FTR: just checked 3.8.1 and #include "zlib.h" still is used in public API interface.
just checked 3.8.1 and
#include "zlib.h"still is used in public API interface.
yes, I mentioned I removed those files in master / 3.9.0dev. They will remain in the 3.8.x series as it is not super appropriate to do that in point releases.
closing, as I don't believe there's any further action needed