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

Fix example makefiles to use proper path to symbol files #122

Merged
merged 8 commits into from
Feb 24, 2024

Conversation

garyo
Copy link
Contributor

@garyo garyo commented Jun 15, 2023

The CMake builds are working now, but the examples were still using an old path to the symbol files.

@garyo garyo requested review from john-paulsmith and barretpj June 15, 2023 17:05
@garyo garyo self-assigned this Jun 15, 2023
@garyo garyo force-pushed the fix/example-makefiles branch from afc2e52 to 742bd41 Compare October 2, 2023 21:04
@garyo garyo force-pushed the fix/example-makefiles branch from feec862 to eceb067 Compare October 2, 2023 21:32
Copy link
Contributor

@john-paulsmith john-paulsmith left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The changes now point to the correct path but the argument syntax is incorrect on macOS. Linux is fine.

Examples/Makefile.master Outdated Show resolved Hide resolved
@john-paulsmith
Copy link
Contributor

In my testing, I found that the symbol files are unnecessary on macOS and Linux - probably on Windows too but I'm not set up to test that easily. We set -fvisibility=hidden then in the code of our examples we include __attribute__((visibility("default"))) on OfxGetPlugin and OFXGetNumberOfPlugins, which overrides the command-line arg and exports these symbols.

Not specifically related to this commit, but the CMake project has the same error in the macOS arguments. I will open a new PR to set visibility=hidden in a cross-platform way there.

Per @john-paulsmith's comments, symbol files are not needed when using
`-fvisibility=hidden` and the `EXPORT` macro. I deleted the symbol
files too.

We're using `--std=gnu++14` elsewhere, and some of the examples use
initializer syntax so this fixes the build.

Also added building Examples in the CI build so we'll know if they
break. Didn't do Windows yet, and the Support examples' Makefile also
needs some work, which is a separate issue.

Fixes #122.

Signed-off-by: Gary Oberbrunner <[email protected]>
Note: the Support (C++) examples still have many internal C++ symbols
exported, at least on MacOS. But they did before as well, so this
commit only improves the situation.

Signed-off-by: Gary Oberbrunner <[email protected]>
@garyo
Copy link
Contributor Author

garyo commented Feb 24, 2024

I have this working, but I see you've also got something similar in #141, @john-paulsmith . I'll merge your changes there into this or vice versa because this also has some only tangentially-related build fixes.

@garyo garyo merged commit c189e81 into main Feb 24, 2024
8 checks passed
@garyo garyo deleted the fix/example-makefiles branch February 24, 2024 22:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants