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

Support workspace dependencies in the root crate of a workspace #46

Merged
merged 2 commits into from
Jan 17, 2024

Conversation

Waridley
Copy link
Contributor

The root of a workspace is itself a crate, and thus its manifest can contain both [dependencies] and [workspace.dependencies]. Without this fix I need to add a superfluous package key to my renamed dependency to get proc-macro-crate to find it, but that causes Cargo to warn about an unused manifest key.

[workspace.dependencies]
bar = { package = "foo", version = "1.2.3" }

[dependencies]
bar = { package = "foo", workspace = true }
#       ^^^^^^^^^^^^^^^ unused manifest key

Also, renaming workspace dependencies seems to only be possible within the [workspace.dependencies] table. If I try this:

[workspace.dependencies]
foo = "1.2.3"

[dependencies]
bar = { package = "foo", workspace = true }

I get:

error: failed to parse manifest at ./Cargo.toml

Caused by:
error inheriting bar from workspace root manifest's workspace.dependencies.bar

Caused by:
dependency.bar was not found in workspace.dependencies

So it should check for dep_name rather than pkg_name in the workspace dependencies.

Copy link
Owner

@bkchr bkchr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ty! Could you add some tests?

@Waridley
Copy link
Contributor Author

This actually breaks the workspace_deps_twice_renamed test, but that's because I intentionally changed it due to lack of support in Cargo. As of right now, if proc-macro-crate accepted twice-renamed dependencies in Cargo.toml, it would not actually work within the crate that used the proc macro.

I found the relevant issue in Cargo's repo:
rust-lang/cargo#12546

It doesn't look like the feature is likely to be implemented any time soon, but I could make it work in case it does happen to be implemented in the future if you want. But unless you say otherwise, I will remove the failing test and add one for workspace-root crates instead for now.

@Waridley Waridley force-pushed the root_workspace_deps branch from 2f4e243 to 0f3c27e Compare December 19, 2023 05:44
src/lib.rs Outdated Show resolved Hide resolved
@bkchr
Copy link
Owner

bkchr commented Jan 17, 2024

Ty for the work!

@bkchr bkchr merged commit dc71186 into bkchr:master Jan 17, 2024
3 checks passed
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.

2 participants