Stream: brlcad

Topic: HAVE_CONFIG_H

starseeker (Jun 05 2020 at 17:37):

@Sean Does it make sense these days to have the HAVE_CONFIG_H define in common.h? As far as I know we're not doing anything to make sure it works without defining that variable.

starseeker (Jun 05 2020 at 17:40):

Anything defining the *_DLL_EXPORTS and *_DLL_IMPORTS defines will need it for sure at the moment, since the COMPILER_DLLEXPORT and COMPILER_DLLIMPORT definitions are coming from CMake. I could check that COMPILER_DLLEXPORT is defined and define out the *_EXPORT if it's not, but that would lead to working, silent skipping of the hidden visibility feature as well as complicating all the *_EXPORT blocks...

starseeker (Jun 05 2020 at 17:55):

After thinking it through that's probably the best option though - it will keep default behavior for codes that don't use brlcad_config.h

starseeker (Jun 05 2020 at 18:24):

Well, best for Linux anyway - Windows is still going to require HAVE_CONFIG_H, because without it they won't have the declspec definitions they need in the headers

starseeker (Jun 05 2020 at 18:29):

We were more less accidentally stand-alone previously with every definition having the platform specific logic...

Looks like the options are require HAVE_CONFIG_H in client codes or go back to having the ifdef PLATFORM piece in common.h that sets COMPILER_DLL* values without the configure test.

starseeker (Jun 05 2020 at 18:38):

Actually, they'll need both BRLCADBUILD and HAVE_CONFIG_H. Ugh.

starseeker (Jun 05 2020 at 18:39):

Should we make a second generated header that's not specific to BRLCADBUILD?

starseeker (Jun 05 2020 at 18:45):

@Sean Ugh. I know you don't like platform symbol tests Sean, but darned if I'm seeing a better solution to this one so far...

Sean (Jun 05 2020 at 18:45):

Conceptually, the config header is supposed to be a report out of the compiling-computer's configuration

Sean (Jun 05 2020 at 18:46):

So it shouldn't have the import/export logic in it as to an installed brl-cad, the config header shouldn't exist to them.

Sean (Jun 05 2020 at 18:47):

the typical way this is dealt with is compile-time definitions passed as flags

starseeker (Jun 05 2020 at 18:48):

But how would the client code know it needs to define them in the first place?

Sean (Jun 05 2020 at 18:51):

API instructions for each lib typically if import isn't the default, but usually import is simply the default and then the local build has to define exports accordingly.

Sean (Jun 05 2020 at 18:52):

Did a problem crop up?

starseeker (Jun 05 2020 at 18:53):

yes. client code attempted to build, but COMPILE_DLLIMPORT wasn't defined

Sean (Jun 05 2020 at 18:54):

I'm not seeing a reference to COMPILE_DLLIMPORT on trunk -- where is it?

Sean (Jun 05 2020 at 18:54):

I found it, common.h

Sean (Jun 05 2020 at 18:55):

typo

starseeker (Jun 05 2020 at 18:55):

Sorry, COMPILER_DLLIMPORT.

starseeker (Jun 05 2020 at 18:55):

Set in toplevel CMakeLists.txt line 1980, written to the brlcad_config.h header

starseeker (Jun 05 2020 at 18:56):

The __declspec(dllimport) used to be hardcoded everywhere, so client codes didn't have to worry about it.

Sean (Jun 05 2020 at 18:57):

right, so yeah, should take it out of config. that belongs in common.h or down in each lib

Sean (Jun 05 2020 at 19:03):

I’d put it in common with some offed logic, like ifdef attribute

starseeker (Jun 05 2020 at 19:08):

So really, brlcad_config.h shouldn't be installed at all? Or do we include it as a reference so we know how the libs were built?

Sean (Jun 06 2020 at 06:29):

it's for the latter. granted someone can set BRLCADBUILD and HAVE_CONFIG_H to try and use it, but that's on them and why they have to take extra steps.

Last updated: Oct 09 2024 at 00:44 UTC