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

document the interaction between stdout and dir #47

Open
oli-obk opened this issue May 15, 2017 · 2 comments
Open

document the interaction between stdout and dir #47

oli-obk opened this issue May 15, 2017 · 2 comments

Comments

@oli-obk
Copy link

oli-obk commented May 15, 2017

cmd!("echo", "foo").dir("bar").stdout("blub.txt").run().unwrap() creates a file blub.txt in the current directory instead of inside the bar directory.

Is this expected? I thought dir behaved like a std::env::set_current_directory before and after the call.

Either way, I think it should be at least documented.

@oconnor663
Copy link
Owner

That's expected. As much as possible, I've tried to avoid letting dir interact with any of the other paths that get passed into an expression. The dir docs (and the spec) call out one particular instance of that behavior, where we interpret relative exe paths from the parent's cwd. You're absolutely right that I should document the other instances too. (In the stdout case, the implementation doesn't have to go out of its way to get the behavior we want. We just open files in the parent before the child is spawned.)

One of the big reasons I wanted to make things work this way, is that I imagine a function like this:

fn redirect_some_expression_to_foo(e: Expression) -> Expression {
    e.stdout("./foo.txt")
}

I think this function should be valid. But note that if dir interacted with stdout, this function would be wrong. You'd need to know exactly what the caller was passing you, or maybe what the caller intended to do with the expression you returned, if you wanted to write something like this.

However, no matter how hard we try, there's going to be some inconsistency. For example, if you say cmd!("cat", "./foo.txt").dir("bar/"), cat is going to interpret the path ./foo.txt relative to bar/. That's true even if you use Path::new("./foo.txt") instead. We could try to magically interpret Path types when you use them as arguments, but a behavior difference between paths and regular strings here would be super confusing. (Above I mentioned a behavior difference that we do have in the exe name, but that one's much less drastic, just a question of prepending a ./.) Because of this inconsistency, I'm not totally convinced that the current design is right.

@oconnor663
Copy link
Owner

oconnor663 commented May 15, 2017

I thought dir behaved like a std::env::set_current_directory before and after the call.

We've also avoided setting the global cwd, because there's no way to lock it, and so it's not thread safe. (I'm not sure if that's actually what you were suggesting, but other subprocess libraries totally do this, and I'm not a fan of it.)

I should document this (non) behavior too.

@oconnor663 oconnor663 changed the title stdout method ignores preceding dir method document the interaction between stdout and dir May 23, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants