dora icon indicating copy to clipboard operation
dora copied to clipboard

Split CLI into library and binary part

Open phil-opp opened this issue 1 year ago • 10 comments

Enables other executables to invoke dora CLI commands directly without going through the argument parsing.

phil-opp avatar Jun 12 '24 15:06 phil-opp

To move forward with this PR, we need to remove std::env::current_exe(). : https://github.com/dora-rs/dora/blob/edf22da8f16272d16847a29b98ea21d91440e86c/binaries/cli/src/up.rs#L86 from our cli as it will call the executable in which the cli library has been linked to

haixuanTao avatar Jul 26 '24 09:07 haixuanTao

To move forward with this PR, we need to remove std::env::current_exe(). :

https://github.com/dora-rs/dora/blob/edf22da8f16272d16847a29b98ea21d91440e86c/binaries/cli/src/up.rs#L86

I replaced the current_exe calls with a dora_cli_path parameter in ce18beb. We still need a CLI executable that is capable of parsing the arguments and starting the coordinator, daemon, and runtime. So I'm not sure how useful this PR is in practice, since you still need to build some sort of dora binary even if you want to call into the dora library directly.

phil-opp avatar Jul 29 '24 13:07 phil-opp

To move forward with this PR, we need to remove std::env::current_exe(). : https://github.com/dora-rs/dora/blob/edf22da8f16272d16847a29b98ea21d91440e86c/binaries/cli/src/up.rs#L86

I replaced the current_exe calls with a dora_cli_path parameter in ce18beb. We still need a CLI executable that is capable of parsing the arguments and starting the coordinator, daemon, and runtime. So I'm not sure how useful this PR is in practice, since you still need to build some sort of dora binary even if you want to call into the dora library directly.

To be honest, I fear that passing the pass to dora path might not necessarily be easy, might create confusion, and might make it hard for people to share code in machine-agnostic manner.

I was thinking about using which::which that would make the application behave with the same fashion of a terminal.

haixuanTao avatar Jul 31 '24 08:07 haixuanTao

To move forward with this PR, we need to remove std::env::current_exe(). : https://github.com/dora-rs/dora/blob/edf22da8f16272d16847a29b98ea21d91440e86c/binaries/cli/src/up.rs#L86

I replaced the current_exe calls with a dora_cli_path parameter in ce18beb. We still need a CLI executable that is capable of parsing the arguments and starting the coordinator, daemon, and runtime. So I'm not sure how useful this PR is in practice, since you still need to build some sort of dora binary even if you want to call into the dora library directly.

I think that people like the idea of being able to spawn a dora application within python or Rust, so that they can wrap dora within their own application ( UI, library, ...)

I think that making it a library makes distributing dora application a lot easier.

haixuanTao avatar Jul 31 '24 09:07 haixuanTao

I have pushed a new branch with the changes to use which::which : https://github.com/dora-rs/dora/compare/split-cli...split-cli-with-which?expand=1

haixuanTao avatar Jul 31 '24 09:07 haixuanTao

I'm not sure if which is good idea. There is no guarantee that dora is installed, that it is in the PATH, and that it's the same version as the binary.

Maybe it's possible to spawn the deamon/coordinator/runtime as threads instead of subprocesses? Or we could use fork on Unix systems. Or we could require some basic form of argument parsing when the dora library is used, then we could continue using current_exe.

phil-opp avatar Jul 31 '24 09:07 phil-opp

deamon/coordinator/runtime as threads.

That could be a good idea and it could scope the up environment to the duration of the initial process.

It seems that fork has some issue: https://internals.rust-lang.org/t/why-no-fork-in-std-process/13770

If at the end of the day. If this is too hard, let's just wrap start and stop and leave up for another PR.

haixuanTao avatar Jul 31 '24 09:07 haixuanTao

Yeah, I'm not a fan of using fork either.

deamon/coordinator/runtime as threads.

That could be a good idea and it could scope the up environment to the duration of the initial process.

Ok, let me try to set it up. The challenge of this approach is that all background threads are stopped immediately when the process exits, without doing any cleanup. So we need some kind of handle that leads to a proper stop on drop.

If at the end of the day. If this is too hard, let's just wrap start and stop and leave up for another PR

That's also fine with me.

phil-opp avatar Jul 31 '24 11:07 phil-opp

Ok, let me try to set it up. The challenge of this approach is that all background threads are stopped immediately when the process exits, without doing any cleanup. So we need some kind of handle that leads to a proper stop on drop.

That's true about the cleanup

haixuanTao avatar Aug 02 '24 01:08 haixuanTao

Ok, let me try to set it up. The challenge of this approach is that all background threads are stopped immediately when the process exits, without doing any cleanup. So we need some kind of handle that leads to a proper stop on drop.

That's true about the cleanup

I guess we should ask users to use the destroy command before closing the script they are using.

haixuanTao avatar Aug 02 '24 02:08 haixuanTao