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

Use native_toolchain_rust to build ICU4X binary #857

Open
mosuem opened this issue Jul 16, 2024 · 9 comments · May be fixed by #872
Open

Use native_toolchain_rust to build ICU4X binary #857

mosuem opened this issue Jul 16, 2024 · 9 comments · May be fixed by #872
Labels
package:intl4x type-code-quality type-enhancement A request for a change that isn't a bug

Comments

@mosuem
Copy link
Member

mosuem commented Jul 16, 2024

Instead of a manual Process.run('cargo'..., use package:native_toolchain_rust.

fyi @knopp @dcharkes

@mosuem mosuem added type-enhancement A request for a change that isn't a bug type-code-quality package:intl4x labels Jul 16, 2024
@knopp
Copy link

knopp commented Jul 17, 2024

This would certainly need some changes in native_toolchain_rust. I'll do my best to accommodate those. Missing pieces seem to be custom cargo arguments and static library. I'm not sure what the current the state of supporting static libraries in native assets? @dcharkes?

@mosuem
Copy link
Member Author

mosuem commented Jul 18, 2024

Static linking will not be supported in the foreseeable future, but we are planning on producing a static library to remove unused symbols with the linker in the link hook, which produces the actual dylib being used at runtime.

@dcharkes
Copy link

This would certainly need some changes in native_toolchain_rust. I'll do my best to accommodate those. Missing pieces seem to be custom cargo arguments and static library. I'm not sure what the current the state of supporting static libraries in native assets? @dcharkes?

Yes, the idea is to build static libs in the build hooks and then use the native linker to build a dynamic lib in the link hook. (At some point far in the future we might want to actually have the static lib linked to the AOT snapshot of Dart, but that requires a lot more changes.)

@mosuem
Copy link
Member Author

mosuem commented Aug 14, 2024

@knopp - just curious, any changes in native_toolchain_rust so far to accomodate this use case? #866 adds even more rust stuff... :)

@knopp
Copy link

knopp commented Aug 14, 2024

@mosuem, I published 0.1.0-dev.7 which supports extra cargo arguments. No support for static libraries, but I think I'd wait with that until there is actual support in native assets.

Is there something else required?

@mosuem
Copy link
Member Author

mosuem commented Aug 16, 2024

Thanks - let me take a look.

No support for static libraries, but I think I'd wait with that until there is actual support in native assets.

#866 will add static libraries. Dart doesn't support static linking, but we will build static libraries in the build hook, and then link them to get a dynamic library in the link hook. This is how we intend to treeshake symbols from the library.

@knopp
Copy link

knopp commented Aug 16, 2024

Thanks - let me take a look.

No support for static libraries, but I think I'd wait with that until there is actual support in native assets.

#866 will add static libraries. Dart doesn't support static linking, but we will build static libraries in the build hook, and then link them to get a dynamic library in the link hook. This is how we intend to treeshake symbols from the library.

I don't think native_toolchain_rust can currently be used to do that, without further work. It always adds the asset (dynamic library) to the build output, what you're doing seems to be bit more exotic. Probably not a general use case.

@mosuem
Copy link
Member Author

mosuem commented Aug 16, 2024

It always adds the asset (dynamic library) to the build output

native_toolchain_c solves that with an optional parameter in the run method. So I think it should be adaptable. But let me take a look, and I might file a PR. I agree that our use case is not super standard.

@knopp
Copy link

knopp commented Aug 16, 2024

I see. That should be fairly easy to add. The remaining thing is the assumption that builder is always building a dylib.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
package:intl4x type-code-quality type-enhancement A request for a change that isn't a bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants