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

It does not find when it is called in crate workspace #4

Closed
zzau13 opened this issue Feb 28, 2019 · 14 comments
Closed

It does not find when it is called in crate workspace #4

zzau13 opened this issue Feb 28, 2019 · 14 comments

Comments

@zzau13
Copy link

zzau13 commented Feb 28, 2019

Basically it is a structure of crate type workspace that is,

example
lib
lib_some
Cargo.toml

Example is in own workspace and the rest share the main workspace.
There is a problem looking for the path.

@gsquire
Copy link
Owner

gsquire commented Feb 28, 2019

Hey, @botika. This is most likely a mistake on my part of not properly documenting this crate. It was intended to solve this issue so that a cargo plugin could work more seamlessly.

I could be misunderstanding your question but the goal of this crate is to find components installed via rustup. So if a user had rustfmt installed in their stable toolchain but not their nightly toolchain, the cargo plugin could still use the stable version. It shouldn't matter if the library is used in a workspace or not. Hope this helps and if not I'd be happy to discuss it further.

@zzau13
Copy link
Author

zzau13 commented Feb 28, 2019

No problem with the rust version. The case is main Cargo.toml

[workspace]
members = [ "lib", "lib_some"]

Cargo.toml lib and lib_some:

workspace = ".."

Cargo.toml example crate, where is the problem

[workspace]
members = ["."]

In the case of using lib that uses toolchain_find in the example, crate and directory, always returns None.

@gsquire
Copy link
Owner

gsquire commented Feb 28, 2019

I'm not able to reproduce this. I created a new crate to test this with a workspace. Here is my layout:

.
├── Cargo.lock
├── Cargo.toml
├── bin
│   ├── Cargo.toml
│   └── src
│       └── main.rs
├── lib
│   ├── Cargo.toml
│   └── src
│       └── lib.rs
└── src

5 directories, 6 files

Cargo.toml file:

[workspace]
members = ["lib", "bin"]

bin/Cargo.toml file:

[package]
name = "bin"
version = "0.1.0"
authors = ["Garrett Squire <redacted>"]
edition = "2018"

[dependencies]
lib = { path = "../lib" }

bin/src/main.rs:

use lib::works;

fn main() {
    works();
}

lib/Cargo.toml file:

[package]
name = "lib"
version = "0.1.0"
authors = ["Garrett Squire <redacted>"]
edition = "2018"

[dependencies]
toolchain_find = "0.1"

lib/src/lib.rs:

use toolchain_find::find_installed_component;

pub fn works() {
    println!("{:?}", find_installed_component("cargo-clippy"));
}

Commands I ran:

# Build entire package.
cargo build

# Run the binary.
cargo run -p bin

Output:

Some("/Users/garrett.squire/.rustup/toolchains/stable-x86_64-apple-darwin/bin/cargo-clippy")

@zzau13
Copy link
Author

zzau13 commented Mar 1, 2019

In that structure, add another example crate, as I describe above and you should be able to reproduce it.
But give me a touch and I raise the case in the new repository. Thank you

@gsquire
Copy link
Owner

gsquire commented Mar 1, 2019

I added another library crate to my example and still wasn't able to reproduce this. It would be helpful to see a gist or repository if you have one handy. Thanks!

@zzau13
Copy link
Author

zzau13 commented Mar 3, 2019

https://github.com/rust-iendo/debug_find_toolchain
In my machine also happens in rust stable. But it works correctly if the workspace property is

workspace = ".."

, in all cases. It's a bit strange.

Thank you.

@gsquire
Copy link
Owner

gsquire commented Mar 4, 2019

I pulled down that repository locally and was able to run the example crate without any problems. Per the documentation, the workspace properties in the any and any_lib crates are redundant. By default they will search for the first parent directory containing a workspace crate. Regardless, it seems like this still isn't a problem with the crate itself. At this point all I can do is recommend checking which toolchains contain the tools you hope this crate would find.

For reference here are the steps I took:

  • Clone the repository
  • cargo +stable build
  • cd example and cargo +stable run
  • Remove the workspace keys
  • cargo +stable run

Output:

    Finished dev [unoptimized + debuginfo] target(s) in 0.12s
     Running `target/debug/example`
Some("/Users/garrett.squire/.rustup/toolchains/stable-x86_64-apple-darwin/bin/rustfmt")

I'm sorry if I am misunderstanding you completely but I can't reproduce the bug you claim to have.

@zzau13
Copy link
Author

zzau13 commented Mar 7, 2019

I'm seeing you work on a Mac, I on a Linux, could this be?
I have already removed the workspace property and it works correctly.

I want to use it in my derives, will not I have any problem? Is there coverage for all this?

Thanks for your time.

@gsquire
Copy link
Owner

gsquire commented Mar 7, 2019

I have Docker on my system so I gave this a spin with your repository. Below are the steps I took to try and reproduce it:

Dockerfile:

FROM rust:1.33.0-slim

RUN rustup component add rustfmt

VOLUME ["/build"]

Commands run:

docker build . -t toolchain:latest

docker run --rm -it -v $PWD/debug_find_toolchain:/build -w /build/example toolchain cargo run

Output:

Finished dev [unoptimized + debuginfo] target(s) in 1m 19s
     Running `target/debug/example`
Some("/usr/local/rustup/toolchains/1.33.0-x86_64-unknown-linux-gnu/bin/rustfmt")

I added the rustfmt component via rustup and the crate was able to find it. So with this in mind, I don't think it's a matter of OS. I'm really sorry but I just can't seem to run into the same issue that you are.

@zzau13
Copy link
Author

zzau13 commented Mar 8, 2019

Looks like it's ok.

Could you add some coverage in these cases? It's not blocking for me, I just want to make sure.

@gsquire
Copy link
Owner

gsquire commented Mar 8, 2019

I'll have to think through how to add a test case. It seems like more of an integration test than a unit test to me.

Thanks for your patience.

@zzau13
Copy link
Author

zzau13 commented Mar 10, 2019

Yes, a couple of e2e tests should be enough.

@gsquire
Copy link
Owner

gsquire commented Mar 19, 2019

Looks like you can run docker images in Travis CI. I'll look into automating the test I mentioned above.

@gsquire
Copy link
Owner

gsquire commented Apr 2, 2019

After doing tome more research it seems like this would introduce some complexity into the CI pipeline. I'm going to put this on the backlog for now and circle back around later.

If you run into any more problems, feel free to make a new issue.

@gsquire gsquire closed this as completed Apr 2, 2019
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