taskwarrior icon indicating copy to clipboard operation
taskwarrior copied to clipboard

src/tc/rust doesn't correctly depend on taskchampion/*

Open djmitche opened this issue 3 years ago • 7 comments

The src/tc/rust CMake target (tc_rust) doesn't automatically get rebuilt if you change something in taskchampion, despite depending on the crates in that directory. This is because the dependency is represented in Cargo.toml, and CMake is unaware of it.

However, I don't know how to fix this with CMake.

Any change in taskchampion/ should trigger a rebuild of tc_rust.

djmitche avatar Jul 17 '22 23:07 djmitche

As a note-to-self you can work around this by running make -C src/tc/rust clean to force-rebuild taskchampion.

djmitche avatar Dec 23 '22 16:12 djmitche

Did a search on this, found this pdf, which links to this CMake module.

Shadow53 avatar Mar 13 '23 04:03 Shadow53

I think this might be resolved already?

$ make
[ 15%] Built target task
[ 29%] Built target libshared
[ 52%] Built target columns
[ 92%] Built target commands
[ 96%] Built target tc
[ 96%] Built target tc-rust_target
[ 98%] Built target task_executable
[ 98%] Built target calc_executable
[100%] Built target lex_executable
$ echo "pub fn foo(){}" >> src/tc/rust/src/lib.rs
$ make
[ 15%] Built target task
[ 29%] Built target libshared
[ 52%] Built target columns
[ 92%] Built target commands
[ 96%] Built target tc
[ 96%] running cargo
   Compiling tc-rust v0.1.0 (/Users/dathanb/dev/dathanb/taskwarrior/src/tc/rust)
    Finished dev [unoptimized + debuginfo] target(s) in 0.24s
[ 96%] Built target tc-rust_target
[ 98%] Linking CXX executable task
[ 98%] Built target task_executable
[ 98%] Linking CXX executable calc
[ 98%] Built target calc_executable
[ 98%] Linking CXX executable lex
[100%] Built target lex_executable

dathanb avatar Mar 13 '23 05:03 dathanb

We are using CMakeRust, and that does work. However, @dathanb's reproduction doesn't quite hit the issue. Instead, try modifying taskchampion/taskchampion/src/lib.rs, and noting that CMake doesn't rebuild anything.

Basically, the issue is that CMake doesn't understand crate dependencies within the repository.

I'm not sure if there's a quick fix for this. If not, it might make sense to move the Taskchampion crates back out to another repository, and require them by version in Taskwarrior. One thing I do not want to do is replace the Cargo infrastructure with something that would make these crates difficult for a "regular" Rust programmer to hack on.

djmitche avatar Mar 13 '23 23:03 djmitche

I spoke with some Rust pros about this. Given that we eventually want to expose Taskchampion itself as a dependency usable by other things, in other languages, it probably makes the most sense to use two different build processes here, rather than trying to shoehorn Taskchampion compilation into Taskwarrior's CMake automation.

Basically, Taskchampion should build into libtaskchampion.so and taskchampion.hs, in a relatively straightforward way that could for example be packaged into a .deb/-devel.deb pair. Then, Taskwarrior should link against that like it would against any other external library.

djmitche avatar Sep 02 '23 02:09 djmitche

In Chromium, which builds everything from source rather than linking external binary blobs, there's quite a bit of tooling in place to "translate" Cargo.toml's into the Chromium build system, which then runs rustc directly. If we wanted to further integrate TC and TW, that would probably be the way to go -- either keeping Cargo.toml and translating it, or just building CMake rules to call rustc directly and not using Cargo at all. Neither of those are very attractive options, since (a) we would like Taskchampion to be something contributors can work on using "normal" Rust tools and (b) we would like Taskchampion to be usable outside of the context of Taskwarrior (and thus uploaded to crates.io, documented on docs.rs, etc.).

I filed #3209 to choose the alternative I described in the previous comment.

djmitche avatar Nov 23 '23 14:11 djmitche

I suppose this remains an open issue for now! If someone knows a good way to solve this, let's do it.

djmitche avatar Jan 21 '24 23:01 djmitche