search

GothenburgBitFactory / taskwarrior

Taskwarrior - Command line Task Management
https://taskwarrior.org
MIT License
4.03k stars 277 forks source link

Replace hand-rolled FFI with cxx #3426

Open djmitche opened 4 months ago

djmitche commented 4 months ago

There's a heavy load of both Rust and C++ code interfacing between Taskchampion and Taskwarrior. It's very tricky to get that stuff right, and easy to add UB.

One of the popular alternatives is https://cxx.rs/, which generates Rust and C++ glue for you. It's a little limited in that it only supports some basic types, but most of what we need is vectors and strings, so that should be OK.

djmitche commented 4 months ago

WIP in my cxx-experiment branch

djmitche commented 4 months ago

@aktaboot suggests https://codeberg.org/pitbuster/dolphin-rom-thumbnailer/src/branch/main/src/CMakeLists.txt as an example that uses CMake, Corrosion, and the Corrosion CXX support (corrosion_add_cxxbridge).

djmitche commented 4 months ago

More updates in the linked branch, if anyone wants to follow along.

djmitche commented 4 months ago

In particular, some notes here

ryneeverett commented 4 months ago

This approach looks really promising!

some creativity is required in writing the bridge, and I think it can safely be considered TW-specific. But, that means it can also omit all of the functionality that TW doesn't use!

Am I correct in interpreting this to mean that you plan to vendor the cxx integration from corrosion?

I was initially curious about the stability of this feature given the ⚠️⚠️⚠️ EXPERIMENTAL ⚠️⚠️⚠️ warning in the docs but if we're vendoring this part I suppose it's less relevant.

djmitche commented 4 months ago

It seems to work even without vendoring it. I suspect we're not doing anything too exciting from corrosion's perspective, so maybe the EXPERIMENTAL bits are not problematic. Or, maybe it won't work on some other platform than mine :(

aktaboot commented 4 months ago

I can test if there's PR :)

djmitche commented 3 months ago

Hey @n8henrie, I saw your issue in https://github.com/dtolnay/cxx/issues/1327 -- I'm also deep in the weeds of CXX, although going the other way (supporting calling into Taskchampion from Taskwarrior). Would you be interested in joining forces?

n8henrie commented 3 months ago

Yes, would love to pitch in as much as I can -- I gave a little more background in an email reply I just sent.

My greatest interest / dream would be providing a rust library for TaskWarrior that would enable downstream work to reimplement the CLI and potentially other front-ends in rust (potentially cross-compiling to other architectures, etc.). As a disclosure, part of my motivation is that I don't know C++, which will limit the amount of help I can be!

As we've mentioned in the past, my goal for https://github.com/n8henrie/taskwarrior-rs/ was to create FFI bindings to fulfill this purpose (and potentially could act as a bridge to progressively migrate the TW codebase to rust -- though I'm not sure how rust-friendly/interested the rest of the TW team is), but obviously my limited understanding of C++ makes that an extremely slow-going task.

And as mentioned before, I'm more than happy to release the taskwarrior crate to the official team once / if there is working rust interop (or perhaps a frontend)!

jschwe commented 3 months ago

I was initially curious about the stability of this feature given the ⚠️⚠️⚠️ EXPERIMENTAL ⚠️⚠️⚠️ warning in the docs but if we're vendoring this part I suppose it's less relevant.

Corrosion Maintainer here. The experimental here means:

  1. This function does not have any SemVer guarantees, and I reserve the right to make breaking changes even outside of major releases. I would document such changes in the release notes though. The reason for this is point 2:
  2. I have received basically 0 feedback from users on how well or not well this function is working for them. I haven't really worked much with C++ / cxx, so it's hard for me to say if there is still something important missing in this function.

Implementation wise, I haven't touched this function in a while. As long as you read the changelog before upgrading corrosion, you should be fine.

djmitche commented 3 months ago

Thanks for having a look at Taskwarrior, @jschwe!

djmitche commented 2 months ago

I was mistaken in thinking this should be solved in the Taskchampion crate. That crate should be pretty much pure Rust, with other applications left to figure out how to link to it. That's how I've just set things up in #3209.