Skip to content

[native_assets_builder] How to deal with NativeCodeAssets with conflicting install names and conflicting file names #1425

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

Closed
3 tasks done
dcharkes opened this issue Aug 13, 2024 · 2 comments
Assignees

Comments

@dcharkes
Copy link
Collaborator

dcharkes commented Aug 13, 2024

It seems that renaming dylibs is non-trivial on some OSes. So, we might want to make it an error to create dylibs with the same names. Either within one hook or as a combination of multiple hooks.

  • Checks in native_assets_builder
  • Conflicts between build and link output in dartdev.
  • Conflicts between build and link output in Flutter.

TL;DR: So far we've been treating native code assets as being in one namespace, similar to how all native build systems work. We could consider name spacing native code assets per package (per hook), but that requires tracking dynamic linking between dylibs explicitly in the protocol.

So far, we've avoided dealing with name conflicts between native libraries. The rationale being that most native build systems do not have a way to deal with that.

In flutter/flutter#153054, we are rewriting install names and dynamic linker import paths. We could apply similar logic to rewrite library names on all OSes (not just MacOS/iOS), to prevent name clashes between NativeCodeAssets from various packages.

Some observations:

  • File names from within one hook cannot clash. So file name collisions should only come from native libraries reported from different hooks.
  • Library install names (and correspondingly the #include statements when referring to these libraries) need to align with the file paths. So library install name collisions should also only come from combining the results of multiple hooks.
  • It's perfectly fine for a dylib in package:foo to have an #import "bar.h" from the hook from package:bar. So completely namespacing native code assets per hook might not be desirable.
    • However, if a libbar.so is produced by both package:bar and package:baz hooks, then it is ambiguous which one is imported. (Unless we add a List<NativeCodeAsset> dynamicallyLinkedTo to NativeCodeAsset to explicitly communicate in the protocol which dylibs dynamically link which dylibs.)

I am not sure whether it's worth it to start name spacing.

Thanks @mkustermann for bringing this up!

FYI @blaugold (Somewhat related to the PRs you're currently working on. But just FYI, no reason to change anything there for now. It might invalidate my assertion in #190, that we don't need to track the dynamic linking between dylibs in the BuildOutput.)

@dcharkes
Copy link
Collaborator Author

It seems that renaming on Linux might be much harder than renaming on MacOS/iOS.

See the discussion on #190 (comment).

@dcharkes dcharkes added this to the Native Assets v1.0 milestone Aug 30, 2024
@dcharkes dcharkes changed the title How to deal with NativeCodeAssets with conflicting install names and conflicting file names [native_assets_builder] How to deal with NativeCodeAssets with conflicting install names and conflicting file names Aug 30, 2024
@dcharkes dcharkes moved this to In Progress in Native Assets Sep 5, 2024
@dcharkes dcharkes self-assigned this Sep 5, 2024
@dcharkes
Copy link
Collaborator Author

flutter/flutter#158214 added duplicate code asset checks to flutter_tools. Thanks @mkustermann! 🙏

@github-project-automation github-project-automation bot moved this from In Progress to Done in Native Assets Nov 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

No branches or pull requests

1 participant