Opened 11 years ago

Closed 5 years ago

#5116 closed defect (wontfix)

[PATCH] implib and export file erronously created when linking with GDAL on MSVC

Reported by: yakov Owned by: tamas
Priority: normal Milestone: closed_because_of_github_migration
Component: default Version: 1.9.1
Severity: normal Keywords:
Cc: tamas

Description (last modified by yakov)

When building an EXE project (not a DLL project) that links in the GDAL libraries on Visual Studio, an import library and export file are created. This is time-consuming - especially for large EXE projects, and happens due to wrong declarations inside GDAL header files.

Details about why it happens and how to fix it follow.

GDAL marks its classes for __declspec(dllexport) even in the executable that links against it. It manifests in that all its inline functions, including those of std::string, get compiled in and exported from our executables. This causes import libraries being created for our executables, a bloat of their *export* tables, binary sizes and compilation times.

Simply defining CPL_DLL to __declspec(dllimport) does not work because then it clashes with std::string members being "already deined in" both, GDAL import libs and our executables.

What should be done:

CPL_DLL shall be specified per each function, defined as dllexport when building GDAL and as dllimport when linking against it. The corresponding warnings resulting from using std::string members, arguments and return values in GDAL interface should be disabled, or the std::string shall be removed from the interface completely.

Attachments (1)

gdalexports.patch (2.8 KB ) - added by yakov 11 years ago.

Download all attachments as: .zip

Change History (10)

comment:1 by yakov, 11 years ago

Description: modified (diff)

comment:2 by Even Rouault, 11 years ago

Seems to be a duplicate of #4099

comment:3 by yakov, 11 years ago

#4099 is related but not a duplicate. It talks about exports from GDAL DLL, whether in our case GDAL headers cause compiling and exporting GDAL functions from our executables.

There are a few hundreds of such exports, like int OGREnvelope3D::IsInit(void), even though they aren't referenced from our code. All of them happen to be defined in the headers with __declspec(dllexport) specification.

comment:4 by yakov, 11 years ago

Summary: implip and export file erronously created when linking with GDAL on MSVCimplib and export file erronously created when linking with GDAL on MSVC

Guys, Any news on this? Please, just do as described in the "what should be done" section, this is important for compilation time and size of MSVC outputs.

comment:5 by Even Rouault, 11 years ago

Can you propose a patch ?

by yakov, 11 years ago

Attachment: gdalexports.patch added

comment:6 by yakov, 11 years ago

I submitted a patch.

Explanation of the changes:

  1. CPLString is made non-CPL_DLL, this prevents from exporting std::string from both, GDAL dll and our executables, thus solving #4099 on the way.
  2. Instead CPL_DLL is added to the non-inline functions, thus code that worked before will still work after the patch.
  3. When compiling with MSVC, CPL_DLL means dllexport when compiling GDAL, thus exporting the symbols from GDAL, and means dllimport otherwise. This solves the problem we had, essentially, as now our executables aren't bloated with GDAL inlines.

The reason I had to solve #4099 on the way is because otherwise defining dllimport when linking against GDAL clashed with multiply defined symbols.

There still could be some warnings that some dllexported/dllimported class or function depends on the now non-exported CPLString. Such warnings can be safely disabled.

comment:7 by Even Rouault, 11 years ago

Cc: tamas added
Summary: implib and export file erronously created when linking with GDAL on MSVC[PATCH] implib and export file erronously created when linking with GDAL on MSVC

Tamas,

are you interested in taking over on this ? The patch looks reasonable to me, but my Windows skills are a bit limited. I'm not sure about the consequence of no longer exporting the CPLString class. My fear would be that it could break compilation of plugins. Wondering also if it would cause issues to external code to GDAL that would use CPLString, but we have always said that the stability of the C++ API is not guaranteed. So if it doesn't break plugins, that should be OK.

Anyway, that's certainly material for trunk only.

comment:8 by tamas, 11 years ago

Owner: changed from warmerdam to tamas

The concept looks good, however I need to do some tests with that.

comment:9 by Even Rouault, 5 years ago

Milestone: closed_because_of_github_migration
Resolution: wontfix
Status: newclosed

This ticket has been automatically closed because Trac is no longer used for GDAL bug tracking, since the project has migrated to GitHub. If you believe this ticket is still valid, you may file it to https://github.com/OSGeo/gdal/issues if it is not already reported there.

Note: See TracTickets for help on using tickets.