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

[native_toolchain_c] Export tools in package #856

Open
miDeb opened this issue Dec 12, 2023 · 6 comments
Open

[native_toolchain_c] Export tools in package #856

miDeb opened this issue Dec 12, 2023 · 6 comments
Labels
P3 A lower priority bug or feature request package:native_toolchain_c

Comments

@miDeb
Copy link
Contributor

miDeb commented Dec 12, 2023

I was playing around with native assets (at https://github.com/miDeb/turbo_jpeg_ffi/tree/v3_all-platforms for reference), and I found I needed the location of the Android NDK and of vcvars64 on Windows. These can be imported

import 'package:native_toolchain_c/src/native_toolchain/android_ndk.dart';
import 'package:native_toolchain_c/src/native_toolchain/msvc.dart';

but they are not exported in native_toolchain_c.dart so you can't import 'package:native_toolchain_c/native_toolchain_c.dart'; and the analyzer is unable to suggest the necessary import.

I think it would be nice if the exports in native_toolchain_c.dart could be expanded to also include those platform-specific tools. As a sidenote, I don't really understand why resolve() returns a List, I would expect only a single result? Or is it possible that a tool is installed multiple times on a machine?

@dcharkes
Copy link
Collaborator

I think it would be nice if the exports in native_toolchain_c.dart could be expanded to also include those platform-specific tools.

Yes we'd totally want to do that.

This is kind of blocked on us designing a nice API. Currently these are implementation details.

Some decisions to tackle before making a public API:

As a sidenote, I don't really understand why resolve() returns a List, I would expect only a single result? Or is it possible that a tool is installed multiple times on a machine?

Yes, the idea is that the tool discovery logic can discover multiple tools on a system. It should not simply return the first one found.

I was playing around with native assets (at https://github.com/miDeb/turbo_jpeg_ffi/tree/v3_all-platforms for reference), and I found I needed the location of the Android NDK and of vcvars64 on Windows.

Can you tell me a bit more about your setup?

Are you using Flutter? If so, the BuildConfig should contain the paths to the Android NDK when building for Android and to the vcvars when building for Windows. Moreover, it should provide the paths to the MSVC and NDK installation that Flutter is using to build your app. Not just the one that native_toolchain_c finds on the host system. (Flutter does not use native_toolchain_c for discovering tools, it uses it's own logic that mostly relies on PATH.)

If build.dart is run from Dart, the BuildConfig does not provide any tool paths. But native_toolchain_c's compilation step uses indeed the logic in this package for finding the MSVC and NDK.

@miDeb
Copy link
Contributor Author

miDeb commented Dec 12, 2023

Are you using Flutter? If so, the BuildConfig should contain the paths to the Android NDK when building for Android and to the vcvars when building for Windows.

Thanks for pointing this out, I didn't see that. Yes, I was planning to eventually use this in a flutter app.

The use case is to use libjpeg-turbo for compression of images, and I was trying to follow the library's build instructions at https://github.com/libjpeg-turbo/libjpeg-turbo/blob/main/BUILDING.md.

Feel free to close this issue if it is alredy covered in other issues

@dcharkes
Copy link
Collaborator

Thanks for pointing this out, I didn't see that. Yes, I was planning to eventually use this in a flutter app.

The use case is to use libjpeg-turbo for compression of images, and I was trying to follow the library's build instructions at https://github.com/libjpeg-turbo/libjpeg-turbo/blob/main/BUILDING.md.

Let me know if you succeed in wrapping the existing build from the build.dart script or if you are missing some capabilities in build.dart.

Feel free to close this issue if it is already covered in other issues

Well, we didn't have a tracking issue yet for making the tools API public, so we can leave this one open.

@dcharkes dcharkes added the P3 A lower priority bug or feature request label Dec 12, 2023
@miDeb
Copy link
Contributor Author

miDeb commented Dec 15, 2023

@dcharkes I was in the end able to wrap the existing build in build.dart, some of the problems I ran into:

  • I only get the path to the compiler from the build config, but for the android build I need the path to the android.toolchain.cmake, which requires going from e.g. ~/Android/Sdk/ndk/26.1.10909125/toolchains/llvm/prebuilt/linux-x86_64/bin/clang to ~/Android/Sdk/ndk/26.1.10909125/build/cmake/android.toolchain.cmake
  • For extracting the environment from vcvars.bat I used the envFromBat helper from native_toolchain_c (which could also be a candidate for exporting from the package)

@dcharkes
Copy link
Collaborator

  • I only get the path to the compiler from the build config, but for the android build I need the path to the android.toolchain.cmake, which requires going from e.g. ~/Android/Sdk/ndk/26.1.10909125/toolchains/llvm/prebuilt/linux-x86_64/bin/clang to ~/Android/Sdk/ndk/26.1.10909125/build/cmake/android.toolchain.cmake

This feels like another instance of #857, because in some sense the cmake is in a tool group with clang because it's the same NDK directory. I don't have a good idea of how to express all those kinds of relations yet though.

  • For extracting the environment from vcvars.bat I used the envFromBat helper from native_toolchain_c (which could also be a candidate for exporting from the package)

I don't see any issue exporting that. 👍

@dcharkes
Copy link
Collaborator

For using the c compiler toolchain discovery in the Dart SDK, for example for dart-lang/sdk#54254, we also need to export the Tool abstraction from this package.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P3 A lower priority bug or feature request package:native_toolchain_c
Projects
None yet
Development

No branches or pull requests

2 participants