gdal icon indicating copy to clipboard operation
gdal copied to clipboard

3.8.0: issues wih header files and pkgconfig inteface

Open kloczek opened this issue 2 years ago • 5 comments

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.h however in cmake/template/gdal.pc.in there is no Requires: zlib so this field should be added or include of zlib.h should be removed.

kloczek avatar Nov 29 '23 21:11 kloczek

  • 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

rouault avatar Nov 29 '23 21:11 rouault

  • 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.

kloczek avatar Nov 30 '23 01:11 kloczek

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"?

eli-schwartz avatar Dec 01 '23 06:12 eli-schwartz

Just FTR: just checked 3.8.1 and #include "zlib.h" still is used in public API interface.

kloczek avatar Dec 01 '23 07:12 kloczek

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.

rouault avatar Dec 01 '23 12:12 rouault

closing, as I don't believe there's any further action needed

rouault avatar Apr 18 '24 15:04 rouault