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

Use cargo metadata to find cargo manifest #60

Merged
merged 2 commits into from Nov 16, 2021

Conversation

jDomantas
Copy link
Contributor

Use cargo metadata itself to find Cargo.toml instead of manually walking up the directory tree.

There is a small difference in behavior: now you can't run bacon path/to/Cargo.toml (and instead you have to do bacon path/to). It seems that previous behavior was accidentally allowed as it would treat the path like a directory and first check path/to/Cargo.toml/Cargo.toml for existence, followed by path/to/Cargo.toml.

The couple of expect()s should not fail unless I misunderstood cargo metadata docs.

Fixes #59.

@Canop
Copy link
Owner

Canop commented Nov 16, 2021

I'm OK for not allowing a path to the Cargo.toml file but there's a regression IMO.

Old and new behaviors when launched without path as argument and not in a Rust project:

image

Can you please restore the old error message ? The new one doesn't hint at the possibility to give a path as argument.

@jDomantas
Copy link
Contributor Author

Restored the old message. Sadly cargo metadata does not emit structured error data, so I hope that string matching on stderr is good enough.

@Canop
Copy link
Owner

Canop commented Nov 16, 2021

This is probably the right approach.
I'll have a lot of checks and tests to do before I can merge that, though. People with strangely structured workspaces are encouraged to test and tell me how it is.

Regarding the errors in metadata, a PR there would probably be a good idea too (it's still 0.x so this kind of breaking change should be OK).

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

Successfully merging this pull request may close these issues.

Bacon fails on a workspace with inter-member dependencies on Windows
2 participants