oconnor663 / duct.rs

a Rust library for running child processes
MIT License
819 stars 34 forks source link

Update dependencies #32

Closed mre closed 7 years ago

mre commented 7 years ago

Using the newest duct.rs release and the newest error-chain I get a compile error when trying to link the duct's error type.

Cargo.toml:

[package]
name = "error-chain-failure"
version = "0.1.0"

[dependencies]
error-chain = "0.8.1"
duct = "0.6.0"

src/lib.rs:

#[macro_use]
extern crate duct;
#[macro_use]
extern crate error_chain;

error_chain!{
    links {
        ShellError(duct::Error, duct::ErrorKind);
    }
}

Compiler output:

   Compiling error-chain-failure v0.1.0 (file:///.../error-chain-failure)
error[E0308]: mismatched types
  --> src/lib.rs:6:1
   |
6  |   error_chain!{
   |  _^ starting here...
7  | |  links {
8  | |      ShellError(duct::Error, duct::ErrorKind);
9  | |  }
10 | | }
   | |_^ ...ending here: expected struct `error_chain::State`, found a different struct `error_chain::State`
   |
   = note: expected type `error_chain::State` (struct `error_chain::State`)
   = note:    found type `error_chain::State` (struct `error_chain::State`)
note: Perhaps two different versions of crate `error_chain` are being used?
  --> src/lib.rs:6:1
   |
6  |   error_chain!{
   |  _^ starting here...
7  | |  links {
8  | |      ShellError(duct::Error, duct::ErrorKind);
9  | |  }
10 | | }
   | |_^ ...ending here
   = note: this error originates in a macro outside of the current crate

error: aborting due to previous error

error: Could not compile `error-chain-failure`.

Updating the dependencies fixes this error.

oconnor663 commented 7 years ago

Could you tell me a little bit about what version 0.8 changes? Is it one of those things where packages requiring 0.7 and packages requiring 0.8 are totally incompatible?

mre commented 7 years ago

Ah yeah. Was kind of in a hurry when creating this issue. Updated the description. Yes, it seems like the newest error-chain version contains a breaking change. Although I have to admit that the error message is a bit convoluted.

oconnor663 commented 7 years ago

Thanks!

oconnor663 commented 7 years ago

Would it help you to have this out as a proper release, or are you building from master?

mre commented 7 years ago

Thanks for merging. A proper release woiuld be really helpful yes.

oconnor663 commented 7 years ago

Just released v0.7.0. I do have a build break in versions of rustc before 0.15.0 that I haven't figured out yet, so if you're using an old compiler you might run into this: https://github.com/oconnor663/duct.rs/issues/33

Do you know if error-chain is expected to break back-compat often? Is this the sort of thing that might prevent us from stabilizing duct?

mre commented 7 years ago

Well, I don't know. For sure this can also happen in the future. It's not 1.0 yet after all. In the long run I guess it's a safe bet but maybe @brson or @Yamakaky have more input on that.

Yamakaky commented 7 years ago

I'm don't think it's a breaking change problem. My bet is that there are two error_chain::State in two versions of the error_chain, and rustc consider them the same even if they have the same structure.

oconnor663 commented 7 years ago

@mre I want to get your thoughts: I'm strongly considering getting rid of my custom error type entirely, and just using io::Error. Would that cause any problems for you?

The main reason is that I'm tweaking my Handle API (the thing you get if you start() an expression instead of using run()), to have two different ways to wait. One will be into_output or something, which consumes the handle and works like wait does now. The other will be a shared method that either just returns the exit status, or returns a reference to the output without giving away ownership. However it ends up working, that shared method is not going to be able to return the duct::ErrorKind::Status error, because that variant owns the output.

So I could add another variant that just returns the exit status, say, and have callers just know in advance which variant to expect. Or I could stop having Status return any output at all, so that it works for both cases. Or now that .unchecked() preserves status codes, most callers should use that anyway, and maybe we can get rid of the Status error type entirely. If we do that, probably makes sense to fold in Utf8Error too, and just return io::Error.

mre commented 7 years ago

error_chain handles io::Error just fine so this will (probably) not cause any difficulties on my side. On the other side, the easier I can find out what exactly caused the error, the better. For example, while handling an io::Error might be a reason to panic, a Utf8Error could mean I want to go on with the next command. As long as we can avoid matching against a string or an integer to find out what exactly went wrong, we're fine I guess.

oconnor663 commented 7 years ago

Thanks, good to know. The plan is to use ErrorKind::InvalidData for the Utf8 case, which the docs there mention as an intended use case. Also for what it's worth, that error case is only possible in the read method, which does string conversion for you. If you expect non-UTF8 bytes, you should probably avoid read, and instead get the bytes directly from .stdout_capture().run(), to do the string conversion yourself as appropriate.

For the non-zero-exit-code case, there's not really a meaningful error kind defined, so I'll probably use ErrorKind::Other with an error string that includes the command and the status. Kind of like above, callers that expect to handle this one should probably use .unchecked() instead, rather than piecing apart the error.

Hopefully once this is done, callers will be feel comfortable short-circuiting on all errors, rather than needing to figure out exactly what error got returned.

oconnor663 commented 7 years ago

@mre just landed the changes in master.

mre commented 7 years ago

@oconnor663 just wanted to report back that this is not causing any issues on my side. Only thing I had to do was to change this:

use duct;

error_chain!{
    links {
        ShellError(duct::Error, duct::ErrorKind);
    }
}

into this:


error_chain!{
    foreign_links {
       IoError(::std::io::Error);
    }
}```