Skip to content

G-API: fix export specifier in standalone build#15021

Merged
alalek merged 3 commits intoopencv:masterfrom
andrey-golubev:gapi_fix_standalone_exports
Jul 16, 2019
Merged

G-API: fix export specifier in standalone build#15021
alalek merged 3 commits intoopencv:masterfrom
andrey-golubev:gapi_fix_standalone_exports

Conversation

@andrey-golubev
Copy link
Copy Markdown
Member

This pullrequest

fixes G-API symbol export specifier in case of G-API standalone mode

Originally, if someone decides to use G-API standalone and link G-API static library to a user's dynamic one: the symbols from our static lib are exported and placed into the symbol table of the user's dynamic library.

build_gapi_standalone:Linux x64=ade-0.1.1d
build_gapi_standalone:Win64=ade-0.1.1d
build_gapi_standalone:Mac=ade-0.1.1d
build_gapi_standalone:Linux x64 Debug=ade-0.1.1d

Copy link
Copy Markdown
Contributor

@rgarnov rgarnov left a comment

Choose a reason for hiding this comment

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

Approved

@alalek
Copy link
Copy Markdown
Member

alalek commented Jul 11, 2019

IMHO, GAPI_STANDALONE name should not be used to switch between static/shared builds.

Alternatives:

  • allow to define GAPI_EXPORTS externally and wrap all current stuff under (#ifndef GAPI_EXPORTS. Add -DGAPI_EXPORTS for static builds
  • introduce separate flag to control statis/shared (CMake usually adds _EXPORTS variant of this flag - see DEFINE_SYMBOL CVAPI_EXPORTS property)

@andrey-golubev
Copy link
Copy Markdown
Member Author

I believe GAPI_STANDALONE is more about building G-API independently of OpenCV (and as of now the static library is produced)

I see your point about GAPI_EXPORTS but it seems like this will be an additional compile definition just for the sake of the GAPI_EXPORTS which I doubt anyone (besides us) would update anyway.

Not sure I understand the version with CVAPI_EXPORTS that you propose, from the code it looks like that the difference is only in CMake's function to be used but the actual behavior/logic is the same, am I wrong?

@alalek
Copy link
Copy Markdown
Member

alalek commented Jul 11, 2019

CMake's DEFINE_SYMBOL documentation is here.

@andrey-golubev andrey-golubev requested a review from rgarnov July 12, 2019 13:15
@andrey-golubev
Copy link
Copy Markdown
Member Author

andrey-golubev commented Jul 12, 2019

@rgarnov @alalek I simplified: looks like we don't need non-OpenCV part altogether, please look again

Copy link
Copy Markdown
Contributor

@dmatveev dmatveev left a comment

Choose a reason for hiding this comment

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

I barely understand what's going on but anyway it seems we'll move IE's g-api to a separate .so (to reuse it with plugins) so it will start exporting stuff anyway.

I wouldn't remove the code but comment it out with the above note.

@dmatveev
Copy link
Copy Markdown
Contributor

Meanwhile, if now we're making a change which is then will be 180*-reverted, y we do need to do it at all?

Maybe @andrey-golubev and @rgarnov go to IE folks and discuss moving IE's "fluid" to a separate .SO? :)

@andrey-golubev
Copy link
Copy Markdown
Member Author

@dmatveev from my discussion with @rgarnov it seems like it's easier to do it this way right now and then see what's a proper way. good point about commenting out and not deleting, will do this

@dmatveev
Copy link
Copy Markdown
Contributor

@andrey-golubev ok!

@andrey-golubev
Copy link
Copy Markdown
Member Author

@dmatveev @alalek updated

Copy link
Copy Markdown
Contributor

@dmatveev dmatveev left a comment

Choose a reason for hiding this comment

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

👍 thanks!

@alalek alalek merged commit e629785 into opencv:master Jul 16, 2019
@andrey-golubev
Copy link
Copy Markdown
Member Author

Thanks!

@andrey-golubev andrey-golubev deleted the gapi_fix_standalone_exports branch July 17, 2019 07:43
dvd42 pushed a commit to dvd42/opencv that referenced this pull request Aug 6, 2019
…ne_exports

* Fix G-API export specifier in standalone build

* Make dummy GAPI_EXPORTS in case of non-OpenCV builds

* Add old version under #if 0
a-sajjad72 pushed a commit to a-sajjad72/opencv that referenced this pull request Mar 30, 2023
…ne_exports

* Fix G-API export specifier in standalone build

* Make dummy GAPI_EXPORTS in case of non-OpenCV builds

* Add old version under #if 0
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants