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

Propagate library defs to swig #615

Merged
merged 3 commits into from Jan 13, 2021
Merged

Conversation

ferdnyc
Copy link
Contributor

@ferdnyc ferdnyc commented Jan 13, 2021

I suspect since the bindings were moved out of the src/ directory, the CMAKE_SWIG_FLAGS haven't been propagating to the swig command line that generates the wrapper script. This means that the #ifdefs in the openshot.i files are all interpreted wrong.

This PR adds generator expression logic to copy every public compile definition from the openshot library target over to the SWIG_FLAGS of the openshot.i file, which should ensure that the bindings are generated correctly.

In my version of CMake, all compile definitions are stored in the target property without the leading -D, even if it was originally set. So, the generator expression I added will prepend -D to each of the values. I'll have to check whether that holds for older CMake versions as well, otherwise we might end up with -D-DUSE_RESVG=1 or the like, which would be a problem. The CI runs should be a fair test of that, though. (And just in case, I took the -Ds out of the definitions we set, at least.)

Edit: That wasn't the issue, but older CMakes did have a problem with the generator expressions. Added a workaround using get_property(), tested back to CMake 3.5.

Also added a fix to FindResvg.cmake, for compatibility with older CMake versions.

@ferdnyc ferdnyc added build Issues related to compiling or installing libopenshot and its dependencies bindings libopenshot's Python or Ruby interface bindings labels Jan 13, 2021
@codecov
Copy link

codecov bot commented Jan 13, 2021

Codecov Report

Merging #615 (0f01cc4) into develop (8bc959b) will decrease coverage by 0.00%.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop     #615      +/-   ##
===========================================
- Coverage    52.25%   52.24%   -0.01%     
===========================================
  Files          129      129              
  Lines        10772    10774       +2     
===========================================
  Hits          5629     5629              
- Misses        5143     5145       +2     
Impacted Files Coverage Δ
src/ImageWriter.h 50.00% <0.00%> (-50.00%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8bc959b...8dfaf74. Read the comment docs.

@ferdnyc
Copy link
Contributor Author

ferdnyc commented Jan 13, 2021

Merging this, as the current state of affairs is quite broken, and it's part of the opencv branch now anyway.

@ferdnyc ferdnyc merged commit 2563c3c into OpenShot:develop Jan 13, 2021
@ferdnyc ferdnyc deleted the fix-swig-flags branch January 13, 2021 19:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bindings libopenshot's Python or Ruby interface bindings build Issues related to compiling or installing libopenshot and its dependencies
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant