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

Add only_exts argument #13698

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
Draft

Add only_exts argument #13698

wants to merge 1 commit into from

Conversation

eerii
Copy link

@eerii eerii commented Sep 21, 2024

This PR follows the work of @thiblahute to allow for creating static libraries that contain all of the dependencies inside. This is motivated by work on gstreamer-full. The main change is:

  • A new only_exts argument is added to partial_dependency which allows to link recursively all of the dependencies of the generated static library.

@eerii eerii marked this pull request as draft September 21, 2024 08:25
@eerii eerii force-pushed the fullpartial branch 2 times, most recently from 33f1c80 to 037b963 Compare September 23, 2024 08:50
So that one can create a dependency objects for fully statically
linked objects from all its dependencies (in a recursive way)
@eerii eerii changed the title Make the extract_all_objects recursive and add only_exts argument Add only_exts argument Sep 24, 2024
Comment on lines +313 to +319
final_compile_args = self.compile_args.copy() if compile_args and not only_exts else []
final_link_args = self.link_args.copy() if link_args and not only_exts else []
final_libraries = self.libraries.copy() if links and not only_exts else []
final_whole_libraries = self.whole_libraries.copy() if links and not only_exts else []
final_sources = self.sources.copy() if sources and not only_exts else []
final_extra_files = self.extra_files.copy() if extra_files and not only_exts else []
final_includes = self.include_directories.copy() if includes and not only_exts else []
Copy link
Member

Choose a reason for hiding this comment

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

I understand why whole_libraries and libraries would be disabled by only_exts, but I don't understand why compile_args, link_args, sources, extra_files, and includes would be.

Copy link
Member

@dcbaker dcbaker Sep 24, 2024

Choose a reason for hiding this comment

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

Given the fact that I would actually expect an error if you tried to combine only_exts with whole_libraries and libraries, I wonder whether it would be better to just add a new method at the API level, even if the implementation used the same interface, and then simply had an assert like assert(not only_exts and (libraries or whole_libraries), 'only_exts conflicts with libraries or whole_libraries, this is a programming error'). From the API level you'd just get something like dep.pack_static_lib() (or however you like to paint your bikeshed).

Comment on lines +325 to +327
final_deps += [d.get_partial_dependency(
compile_args=compile_args, link_args=link_args, links=links,
includes=includes, sources=sources, only_exts=False) for d in library.external_deps]
Copy link
Member

Choose a reason for hiding this comment

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

This gets repeated so much I think it would be worth writing a helper functions like:

def make_deps(deps: T.Iterable[Dependency]) -> T.List[Dependency]:
    return [d.get_partial_dependency(...) for d in deps]

@eerii
Copy link
Author

eerii commented Sep 25, 2024

Hi @dcbaker, thank you so much for the review! This may be missing a bit of context because intialally we were also getting objects recursively, but that broke the tests and it was not the proper solution, so we pivoted.

Our goal is to have a "fat" statically linked library that contains our project and all of the dependencies (excluding the system ones, we are aware of #9218, and we have subprojects for them anyways). Initially we were using the get all of the objects approach, but we are now trying to use as_link_whole to modify dependencies.

There are a few issues with this. Firstly, as_link_whole doesn't seem to be applied recursively to subdependencies. I have a patch for this using get_dependencies and we might add an option of having it recurse.

The other issue is generating pkg-config files for each of the dependencies that are bundled into the static library that point to the big library instead of the system. I don't know if you have any pointers in a good direction.

I also found #12638 that might be affecting things, though we do have the full library marked as install.

Sorry about the PR! It was yesterday when we started trying the new method and I didn't want to outright close it since we might end up needing it in the end. Since we are still trying to figure out how to do this I'm unsure.

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.

3 participants