Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support shared exodus library on windows #439

Merged
merged 3 commits into from
Mar 7, 2024

Conversation

gsjaardema
Copy link
Member

Patch from external to support building shared exodus library on windows for use with exodus.py.

HI Greg,

I got an initial build with exodus3.py working on Windows.
Attached is the patch to make that work.

Many of the errors I was getting before were related to building Seacas against a shared exodus library. Once I changed TRIBITS_ADD_LIBRARY to ADD_LIBRARY, some confusion about which libraries to link with was cleared up.
I see that for building a static library, ADD_LIBRARY was already used.

In exodusII.h, I added a check for defining __declspec(dllexport) as needed by exodus3.py.

I think long term, if you needed the dllimport, and if you needed to support both shared and static builds of exodus at the same time, I think one could add a PUBLIC COMPILE_DEFINITIONS property on exodus_shared to indicate the exodusII.h will be used with a shared library. That would allow exodusII.h to be used for both static and shared builds. Putting static vs. shared information into a generated header file might prevent using the headers for both static and shared libraries. The current addition of __declspec(dllexport) is compatible with the potential long term change proposed here.

Thanks,
Clint

Reversed test just because of "feels" better
Fixed some leftover bad formatting while in the file.
@gsjaardema gsjaardema merged commit 8d50455 into master Mar 7, 2024
51 checks passed
@gsjaardema gsjaardema deleted the shared-exodus-on-windows branch March 7, 2024 17:29
@github-actions github-actions bot locked and limited conversation to collaborators Mar 7, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant