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

Expose C++ 3rdparty libraries from Conan only to the targets that depend on the 3rdparty library #6178

Closed
CMLivingston opened this issue Jul 18, 2018 · 6 comments
Assignees

Comments

@CMLivingston
Copy link
Contributor

The current iteration of the ctypes-based native backend compiles and links all native_external_library targets in the target closure, and does not partition compiles or links based on which external library targets belong to which dependee targets.

The scope of this ticket is to rework the product produced by the native_external_library_fetch task to map targets to subproducts, and ensure that proper partitioning is occurring during the compile and link tasks.

@cosmicexplorer
Copy link
Contributor

See #5998 for more context on this issue. This solution should use or extend the NativeTargetDependencies product computed in native_compile.py to respect strict_deps for the 3rdparty libraries (calculating the value of strict_deps for a given native target is also done in native_compile.py). A full solution for this probably involves a few unit tests (if possible), or an integration test (if  necessary).

@cosmicexplorer cosmicexplorer changed the title Improve C++ external library compiling + linking Expose C++ 3rdparty libraries from Conan only to the targets that depend on the 3rdparty library Sep 4, 2018
@cosmicexplorer
Copy link
Contributor

cosmicexplorer commented Sep 4, 2018

Basically, NativeExternalLibraryFetch.NativeExternalLibraryFiles needs to be manipulated the way NativeTargetDependencies is now, in that instead of building up a single datatype instance and calling register_data() with all our lib names, lib dirs, and include dirs at once, we should have a ProductMapping of (3rdparty library target) -> (single instance of a datatype like NativeExternalLibraryFiles).

We would probably also want to search for all Conan targets that the current target depends on in NativeCompile.execute() and make them into a separate ThirdPartyNativeDependencies datatype used as a ProductMapping, so that we don't have to recalculate dependencies in the link phase (this is very similar to NativeTargetDependencies). This means: add another line below this one that gets all the dependent Conan 3rdparty targets, respecting strict_deps, and stick that into its own ProductMapping of (NativeLibrary target) -> [list of dependent 3rdparty targets], so that it can be retrieved by LinkSharedLibraries to get all the lib names and lib dirs for the dependees.

Finally, there would be a small change made to NativeCompile and LinkSharedLibraries to consume the now-plural 3rdparty libraries. NativeCompile._make_compile_request() should be changed to use external_libs_product as a ProductMapping (as described at the bottom of the first paragraph), as with LinkSharedLibraries._make_link_request() should be changed in the exact same way.

Note the two methods NativeCompile._add_product_at_target_base() and LinkSharedLibraries._retrieve_single_product_at_target_base(). These are used to interact with ProductMappings when there is intended to be only one value (not a list of values) at the specified target's key in the ProductMapping.

@stuhood stuhood self-assigned this Sep 5, 2018
@stuhood
Copy link
Member

stuhood commented Sep 8, 2018

Started this and hit #6473.

@stuhood
Copy link
Member

stuhood commented Sep 8, 2018

...and then was not able to actually authenticate using my bintray account.

@stuhood
Copy link
Member

stuhood commented Sep 11, 2018

I've made a bit of progress here, but have a bit more to do before I can put up a PR.

@stuhood
Copy link
Member

stuhood commented Sep 12, 2018

Reviewable: #6492.

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 a pull request may close this issue.

3 participants