-
Notifications
You must be signed in to change notification settings - Fork 52
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
[native_assets_cli] Code Assets depending on other code assets #190
Comments
I need support for this with a use case where I have a library, but for some features of the library I need some native glue code so that I can use them with Dart. The library is only available as a prebuilt binary, so I can't compile the library and the glue code into one binary. The library with the glue code needs to be linked to the primary library. Loading the primary library from the glue code library works for most configurations (only Flutter + macOS has been a problem), but that's probably coincidental. @dcharkes I'm happy to work on this if you could give me some pointers. |
One of the challenges here is that the embedder renames/repackages dylibs. So relying on the C/C++ dynamic loader with a fixed And Flutter does something else than Dart. So you'd need a different file path when called from Flutter than when called from Dart. Some options:
Both of these assume that you use "dynamic loading" e.g. nothing happens at the link step in the native compiler.
I believe this is not needed. The way we have designed the link hooks should allow for simply reporting both the main and support dylib (based on the eventual tree-shaking info / resources.json) @blaugold Can you elaborate on your use case? Are you doing dynamic loading? e.g. doing |
That's precisely the issue.
I have two libraries: Here is the import 'package:cbl_native_assets/src/support/edition.dart';
import 'package:cbl_native_assets/src/version.dart';
import 'package:logging/logging.dart';
import 'package:native_assets_cli/native_assets_cli.dart';
import 'package:native_toolchain_c/native_toolchain_c.dart';
import 'cblite_builder.dart';
const _edition = Edition.enterprise;
final _logger = Logger('')
..level = Level.ALL
// ignore: avoid_print
..onRecord.listen((record) => print(record.message));
void main(List<String> arguments) async {
await build(arguments, (config, output) async {
output.addDependencies([
config.packageRoot.resolve('hook/build.dart'),
config.packageRoot.resolve('lib/src/version.dart'),
]);
const cbliteBuilder = CbliteBuilder(
version: cbliteVersion,
edition: _edition,
);
await cbliteBuilder.run(
buildConfig: config,
buildOutput: output,
logger: _logger,
);
final cbliteLibraryUri = output.assets
.whereType<NativeCodeAsset>()
.map((asset) => asset.file)
.singleOrNull;
final cblitedartBuilder = CBuilder.library(
name: 'cblitedart',
assetName: 'src/bindings/cblitedart.dart',
language: Language.cpp,
std: 'c++17',
cppLinkStdLib: config.targetOS == OS.android ? 'c++_static' : null,
defines: {
if (_edition == Edition.enterprise) 'COUCHBASE_ENTERPRISE': '1',
},
flags: [
if (cbliteLibraryUri != null)
...switch (config.targetOS) {
OS.iOS => [
'-F${cbliteLibraryUri.resolve('../').toFilePath()}',
'-framework',
'CouchbaseLite',
],
_ => [
'-L${cbliteLibraryUri.resolve('./').toFilePath()}',
'-lcblite',
]
},
if (config.targetOS == OS.iOS) '-miphoneos-version-min=12.0',
if (config.targetOS == OS.android) ...['-lc++abi']
],
includes: [
'src/vendor/cblite/include',
'src/vendor/dart/include',
'src/cblitedart/include',
],
sources: [
'src/cblitedart/src/AsyncCallback.cpp',
'src/cblitedart/src/CBL+Dart.cpp',
'src/cblitedart/src/Fleece+Dart.cpp',
'src/cblitedart/src/Sentry.cpp',
'src/cblitedart/src/Utils.cpp',
'src/vendor/dart/include/dart/dart_api_dl.c',
],
);
await cblitedartBuilder.run(
config: config,
output: output,
logger: _logger,
);
});
} The full package is also available publicly on GitHub. On macOS + Flutter I get the following exception:
|
Thanks @blaugold! Where on https://github.com/cbl-dart/cbl-dart/ is As a workaround, can you force loading of |
Sorry, I thought that was implied. 🙈
Ah, yes, I'm already doing that. If I remember correctly, that was a workaround that helped on iOS. |
I think conceptually what we have to do is: not only update the install_path with the For that we either need to (a) come up with an algorithm/heuristic to decide which dynamic linking paths we need to update or (b) explicitly specify these paths in the @blaugold Can you explore this with
Hopefully that should not be needed anymore afterwards. 🙂 |
@dcharkes I've created a PR: flutter/flutter#153054. Let me know what you think. |
Awesome! While reviewing I had some more thoughts on how to have dynamic dependencies more usable, documented and tested:
(Feel free to leave these TODOs open.) |
👍 I'll try and find some time to work on those. While updating the integration tests in flutter/flutter#153054 I found that it's currently not possible to use
|
I don't believe we have looked into supporting anything else than Linux yet w.r.t. linking. What compiler flags need adding? Please feel free to submit PRs for the missing OSes! 😄
That makes sense. (What does
Do you envision having some helper code producing
Currently, the (If there's a need to change the design of builders to allow information flowing from one to the other, I'd be open to that. But I'd like a good reason to deviate from the current design where the constructor is static configuration as object literal, and the output only streams to the hook output.) Edit: It looks like I'm reviewing out of order. Based on #1413 I think the current design with Builders being configured in the constructor should still work. Let me know if you hit any issues. 👍 |
I think I have to take a step back and consider all the scenarios in which dynamic linking of libraries built by a build hook needs to work:
This is my understanding of what embedders currently do with the libraries built by a build hook:
Dynamic linking in build hooksCurrently a build hook can get dynamic linking right so that loading of the linked library works in most scenarios. As an aside w.r.t. #1425, in Dart tests and scripts, libraries built by other hooks, would be difficult to use because libraries from different packages are not copied to the same directory. macOS and iOSRequirements:
When the linked library needs to be loaded, the dynamic linker will substitute The app executables that are created for Flutter macOS and iOS apps, the flutter-tester executable and the standalone Dart executable all have rpaths for Linux and AndroidRequirements:
The WindowsRequirements:
One of the default locations the dynamic linker will look for Directories that should be searched can be added to the PATH environment variable. Alternatively, Dynamic-link library search order Possible next steps:
|
Thanks for this very detailed write-up @blaugold! ❤️
I had the same realization. If we want to support that, we need to either copy all libs into the same dir, or have all directories as library paths.
Ah right, I might have played around with that years back.
This would entail probably also entail addressing: (Or adding a big fat disclaimer somewhere.) It would indeed be good to just always copy for all running modes. That way we ensure that naming conflicts (or our way of resolving them) already show up for
If we choose to not copy things around, that could be fine for Dart standalone (and
We should consider if this logic can be part of the native_assets_builder package so that we don't have to duplicate it across flutter_tools and dartdev. One challenge there is that all these different ways of running code package things differently. E.g. in a framework, not in a framework, etc. I am open to suggestions here.
This also depends on the directory layout of how an app is packaged. If we standardize on everything being in one directory, we can standardize the rpath. The question is again whose responsibility this would be. It would be nice if the native assets builder can do things, but in the end it is the embedder (flutter_tools, dartdev) that decides the directory layout of where the dylibs are compared to the exe and the other dylibs. We can't bake the logic into I'm a bit hesitant to mandate a directory layout. For example, we can imagine embedding dylibs inside a zip file and So, I think for now this would be best implemented in flutter_tools and dartdev. And then we need to think if we can share the implementation across those two by moving some helper functions in a shared location (
This is a function that should be called inside I don't see any call to that function in the Flutter code base though. I guess that's because we don't have a precompiled |
It's actually surprisingly difficult to modify the
Yes, |
The same bundling that is used for `dart build` is now also used for `dart test` and `dart run`, except that the output directory is `.dart_tool/native_assets`. This way all native code assets are placed next to each other in the `lib` directory, and loaded from there instead of loading them in place from where the build/link hooks placed them. By standardizing on this layout the different modes of running dart code that support native assets can use the same mechanisms to support dynamic linking between native code assets. Also, on macOS install names of dylibs are rewritten to support dynamic linking, similar to the changes in flutter/flutter#153054. Tests are added to verify that dynamic linking works as expected. Related: dart-lang/native#190 Fixes: dart-lang#56459
Interesting. Thanks for the research!
If a user does this in their (If that doesn't work, we might need to resort to having some configuration in the build config. I'd like to avoid that. Then a next question would be: How do other build systems that wrap existing build systems deal with linked dependencies on Linux.)
Yeah, that doesn't sound like a portable solution.
To know the directory (or directories), we should probably add that info to |
With these link options, the layout in build hook output directory does not matter if we standardize on copying all native code assets to a single directory.
Yeah, probably both, so that someone implementing a builder that wraps another build system is aware. |
Standardize on copying all into a single directory, and standardize on not renaming on naming conflicts! |
…153054) Native libraries that are contributed by native asset builders can depend on each other. For macOS and iOS, native libraries are repackaged into Frameworks, which renders install names that have been written into dependent libraries invalid. With this change, a mapping between old and new install names is maintained, and install names in dependent libraries are rewritten as a final step. Related to dart-lang/native#190
…lutter#153054) Native libraries that are contributed by native asset builders can depend on each other. For macOS and iOS, native libraries are repackaged into Frameworks, which renders install names that have been written into dependent libraries invalid. With this change, a mapping between old and new install names is maintained, and install names in dependent libraries are rewritten as a final step. Related to dart-lang/native#190
The same bundling that is used for `dart build` is now also used for `dart test` and `dart run`, except that the output directory is `.dart_tool/native_assets`. This way all native code assets are placed next to each other in the `lib` directory, and loaded from there instead of loading them in place from where the build/link hooks placed them. By standardizing on this layout the different modes of running dart code that support native assets can use the same mechanisms to support dynamic linking between libraries. On macOS, install names of dylibs are rewritten to support dynamic linking, similar to the changes in flutter/flutter#153054. On Windows, loading of DLLs is altered so that the directory of the DLL that is being loaded is considered when loading dependent DLLs. Tests are added to verify that dynamic linking works as expected. TEST=pkg/dartdev/test/native_assets/{build,run,test}_test.dart [email protected] Related: dart-lang/native#190 Fixes: #56459 Change-Id: Ie4a41e5b7382ab1cea39e93d29d085bf9986828b Cq-Include-Trybots: luci.dart.try:pkg-linux-debug-try,pkg-linux-release-arm64-try,pkg-linux-release-try,pkg-mac-release-arm64-try,pkg-mac-release-try,pkg-win-release-arm64-try,pkg-win-release-try Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/381580 Reviewed-by: Moritz Sümmermann <[email protected]> Commit-Queue: Daco Harkes <[email protected]> Reviewed-by: Daco Harkes <[email protected]>
Thanks a ton @blaugold! ❤️
I think this is the only thing left to do for this now. And we have some WIP in: |
Dynamic libraries can try to load other dynamic libraries when they are dlopened.
For this case, the
build.dart
needs to add the transitive closure of dynamic libraries as assets.When we want to start doing tree shaking of whole dylibs (not static linking), we need to take into account the dependencies between dylibs.
This means we need to extend the CLI protocol to be able to communicate the dependencies between assets.
Also, if the dylib paths inside the dylib need to be rewritten in Flutter:
install_name_tool -change <abs-deps-path> @executable_path/../Frameworks/lib…dylib dependent.dylib
(We already do something like this in Flutter for the main dylib name itself.)[ ] Add dependencies to protocolCBuilder
where one depends on the dynamic linker to load the other as dependency.Concrete use case:
libexif
depends onlibintl
.Thanks for reporting @mkustermann
The text was updated successfully, but these errors were encountered: