Implement Clone to Shell

[elichai]

This allows you to clone the shell so you can create a "sub-shell", with different environments and reuse it
(that way you "inherit" the shell and modify it for your needs,)
push_env almost does that, except across function returns where it's hard to keep the guards around and sometimes you need to "reset" those envs because you want to execute something else in the meantime and then go back to that other "environment"

(imagine making a shell for the project, then preparing android by changing the cwd and env vars, afterward, there are other preparations for other targets, and then when it comes to building it needs to rebuild all these environments, unless it created a subshell and stored it)

Solves #79

[matklad]

🤔 makes sense to me. In 0.1, xshell was mutating global state, so cloning wouldn't make sense. But now that the state is local, it's a valid operation to do.

One might wonder if, as we are doing cloning anyway, we should retire push_ APIs in favor of something like

let sh = sh
    .with_cwd("crates/foo")
    .with_env("RUST_BACKTRACE", "full");

I think that'll also let us get rid of those RefCells, as we won't have the need for drop guards to "close over" a shell.

But that'll for the hypothetical next major version of xshell with actually nice APIs...

Could you update the patch version in Cargo.toml and add an entry to the changelog? Otherwise, looks good to me!

[elichai]

Will do,
I agree that that would be a nicer design,
I might have time to do this re-work if you'll be interested in reviewing,
The downside of removing the RefCell is full cloning of shell envs but I'm not sure it matters as this is only for build tools,
Worst case we can replace OsString with Rc to somewhat reduce cloning costs

[matklad]

I might have time to do this re-work if you'll be interested in reviewing,

Good question. I'd be delighted to review a PR which basically says "xshell has some nice ideas to it, but the current API is crappy, so here's an (almost entirely new) better API". I do feel that there are a bunch of things I don't like about the API as it is. And of course it goes without saying that the person who comes up with the better API gets to co-maintain xshell.

The problem is, I don't have a ready list of what I don't like, and what I'd want to do better. At least it is #83, then the overall semantics of inheriting & echoing streams are not right I think: #63 (comment). And there's probably more things which are just not right, which I am not recalling at the moment.

So, while I very much welcome such a PR, a big risk there is that it's going to be on the author to design mostly right API, and that's just quite a hard thing to do (as you can see observing rather haphazard existing API). This'll probably take at least a couple of weekends of your life off. But also, I do think that the Rust ecosystem deserves a good crate of roughly xshell's shape.

This is the rare case where I'd love to avoid just piling up incremental improvements, as you can't really improve the API without removing old stuff.

---

The downside of removing the RefCell is full cloning of shell envs but I'm not sure it matters as this is only for build tools,
Worst case we can replace OsString with Rc to somewhat reduce cloning costs

Yeah, what I have in mind is to basically store a parent: Option<Arc<Shell>> field, and Arc::get_mut appropriately to get semantics of persistent data structures.

[elichai]

Good question. I'd be delighted to review a PR which basically says "xshell has some nice ideas to it, but the current API is crappy, so here's an (almost entirely new) better API". I do feel that there are a bunch of things I don't like about the API as it is. And of course it goes without saying that the person who comes up with the better API gets to co-maintain xshell.

Cool,
I might just open a draft PR soon enough :)
(obviously feel free to dislike whatever I do and/or provide feedback if applicable)

I want to say that xtask+xshell is a much better experience than any Makefile/Bazel etc for a complicated multi targets project so thank you :)

Yeah, what I have in mind is to basically store a parent: Option<Arc<Shell>> field, and Arc::get_mut appropriately to get semantics of persistent data structures.

Yeah, maybe throwing Arc's on a bunch of things will reduce the overhead as much as possible while still providing a simpler API.

[elichai]

Could you update the patch version in Cargo.toml and add an entry to the changelog? Otherwise, looks good to me!

Bumped :)

[matklad]

@elichai oh, if you are doing a larger refator, consider taking a look at dax APIs: https://github.com/dsherret/dax

I believe that they put more thoughts in their API than we did for xshell so far

[matklad]

Ok, this is now live: https://crates.io/crates/xshell/0.2.6