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

[maybe a bug] common dependencies are not unified between main crate and proc-macro crate, if features differ #14740

Closed
0x53A opened this issue Oct 29, 2024 · 4 comments
Labels
A-features Area: features — conditional compilation C-bug Category: bug S-needs-info Status: Needs more info, such as a reproduction or more background for a feature request.

Comments

@0x53A
Copy link

0x53A commented Oct 29, 2024

Note

I found a simpler repro

If there are three crates, A depends on B and C, B, depends on C, and B is a proc-macro crate and A uses more/different features than B, then crate C is not unified, but instead compiled twice.

Please see the following three repositories:

https://github.com/rustlang-issue-14740/repo-A
https://github.com/rustlang-issue-14740/repo-B
https://github.com/rustlang-issue-14740/repo-C


Clone the last one, run cargo tree -d.

Expected: empty result
Actual:

> cargo tree -d
repo-A v0.1.0 (https://github.com/rustlang-issue-14740/repo-A?branch=main#521e12a9)
└── repo-B v0.1.0 (proc-macro) (https://github.com/rustlang-issue-14740/repo-B?branch=main#d35f10c5)
    └── repo-C v0.1.0 (D:\repos\rustlang-issue-14740\repo-C)

repo-A v0.1.0 (https://github.com/rustlang-issue-14740/repo-A?branch=main#521e12a9)
└── repo-C v0.1.0 (D:\repos\rustlang-issue-14740\repo-C)

IF you edit cargo.toml (in crate C), remove the , features = ["sub"] and rerun cargo tree -d, then the result is empty, as expected.

Note: there is no patching, there are no different branches. Every crate references only branch "main" of dependencies.

I believe I also understand why this is happening. The proc-macro is compiled and executed. After the macro is expanded, C does not use any exports from the crate B (it can't, since a proc macro crate can't export anything except the proc macros). Therefore I'd guess that from the perspective of cargo, they are two independent compilation targets?

BUT! In my original issue, the dependency chain is deeper, with the proc-macro crate referencing a different proc-macro crate, and using its macros inside the macro. Because the proc-macro and the main app have different features, these macro expansions are not compatible.

I'll try to add that to my example, but at this point I'm not even sure if it's actually a bug anymore. I guess it is, because whether it's a proc-macro crate or not shouldn't influence dependency resolution?


old report

Problem

I'm using a patch section to redirect an indirect dependency to my own fork. The main crate directly references this dependency.

my main cargo.toml

[dependencies]
gdext_coroutines    = { git = "https://github.com/0x53A/gdext_coroutines", branch = "dev" }
godot               = { git = "https://github.com/0x53A/gdext", branch = "dev-multiple-impl-blocks", features = ["experimental-wasm", "lazy-function-tables"] }
fe-o-godot-macros   = { git = "https://github.com/0x53A/fe-o-godot-macros", branch = "dev" }

[patch."https://github.com/godot-rust/gdext"]
godot               = { package = "godot", git = "https://github.com/0x53A/gdext", branch = "dev-multiple-impl-blocks" }

gdext_coroutines references godot from the original repository:

godot = { package = "godot", git = "https://github.com/godot-rust/gdext" }

https://github.com/0x53A/gdext_coroutines/blob/41fafbd58fb6694f42b67b1c2794461fed52aee3/rust/Cargo.toml#L19-L20


fe-o-godot-macros references godot from my fork, but without the additional features

godot           = { git = "https://github.com/0x53A/gdext", branch = "dev-multiple-impl-blocks" }

https://github.com/0x53A/fe-o-godot-macros/blob/4ef212df813bd166782213bef6fbcf6ffff6db47/Cargo.toml#L11-L12


When I run cargo build in my main project, I get compile errors related to the feature "lazy-function-tables", as one example:

D:\repos\RustedArmor\rust>cargo build
   Compiling godot-ffi v0.1.3 (https://github.com/0x53A/gdext?branch=dev-multiple-impl-blocks#c8ce78d1)
   Compiling gdext_coroutines v0.6.0 (https://github.com/0x53A/gdext_coroutines?branch=dev#ea23c8ca)
error[E0433]: failed to resolve: could not find `lazy_keys` in the crate root
 --> D:\repos\RustedArmor\rust\target\debug\build\godot-ffi-acf9a0802143616d\out\table_builtins.rs:5:80
  |
5 |     string_cache: StringCache < 'static >, function_pointers: HashMap < crate::lazy_keys::BuiltinMethodKey, crate::BuiltinMethodBind >,
  |                                                                                ^^^^^^^^^ could not find `lazy_keys` in the crate root

It seems like cargo has a duplicated reference to 0x53A/gdext and loses the features ("experimental-wasm", "lazy-function-tables") at some point.

here is the output of cargo tree -d, which I would expect to be empty

D:\repos\RustedArmor\rust>cargo tree -d
godot v0.1.3 (https://github.com/0x53A/gdext?branch=dev-multiple-impl-blocks#c8ce78d1)
└── fe-o-godot-macros v0.0.1 (proc-macro) (https://github.com/0x53A/fe-o-godot-macros?branch=dev#09563587)
    └── rustato_lib v0.1.0 (D:\repos\RustedArmor\rust)

godot v0.1.3 (https://github.com/0x53A/gdext?branch=dev-multiple-impl-blocks#c8ce78d1)
├── gdext_coroutines v0.6.0 (https://github.com/0x53A/gdext_coroutines?branch=dev#ea23c8ca)
│   └── rustato_lib v0.1.0 (D:\repos\RustedArmor\rust)
└── rustato_lib v0.1.0 (D:\repos\RustedArmor\rust)

godot-core v0.1.3 (https://github.com/0x53A/gdext?branch=dev-multiple-impl-blocks#c8ce78d1)
└── godot v0.1.3 (https://github.com/0x53A/gdext?branch=dev-multiple-impl-blocks#c8ce78d1) (*)

godot-core v0.1.3 (https://github.com/0x53A/gdext?branch=dev-multiple-impl-blocks#c8ce78d1)
└── godot v0.1.3 (https://github.com/0x53A/gdext?branch=dev-multiple-impl-blocks#c8ce78d1) (*)

godot-ffi v0.1.3 (https://github.com/0x53A/gdext?branch=dev-multiple-impl-blocks#c8ce78d1)
└── godot-core v0.1.3 (https://github.com/0x53A/gdext?branch=dev-multiple-impl-blocks#c8ce78d1) (*)

godot-ffi v0.1.3 (https://github.com/0x53A/gdext?branch=dev-multiple-impl-blocks#c8ce78d1)
└── godot-core v0.1.3 (https://github.com/0x53A/gdext?branch=dev-multiple-impl-blocks#c8ce78d1) (*)

Steps

  1. clone https://github.com/0x53A/rustlang-cargo-issue-14740
  2. run cargo build and observe that it fails, cargo tree -d prints a list of duplicated dependencies
  3. in cargo.toml, disable the # doesn't work block and enable the # works block
  4. run cargo build and observe that it works, cargo tree -d is empty

Possible Solution(s)

It works when I set the correct repository and features in all packages (inside gdext_coroutines and fe-o-godot-macros.

godot = { git = "https://github.com/0x53A/gdext", branch = "dev-multiple-impl-blocks", features = ["experimental-wasm", "lazy-function-tables"] }

Then compilation succeeds and cargo tree -d returns an empty result as expected.

Notes

There are a lot of git related issues already open, but most of them are about unifying slightly different urls (like tag vs rev). In my case, the url is exactly the same and they are still not unified.

Version

cargo 1.84.0-nightly (e75214ea4 2024-10-25)
release: 1.84.0-nightly
commit-hash: e75214ea4936d2f2c909a71a1237042cc0e14b07
commit-date: 2024-10-25
host: x86_64-pc-windows-msvc
libgit2: 1.8.1 (sys:0.19.0 vendored)
libcurl: 8.9.0-DEV (sys:0.4.74+curl-8.9.0 vendored ssl:Schannel)
os: Windows 10.0.22621 (Windows 11 Professional) [64-bit]
@0x53A 0x53A added C-bug Category: bug S-triage Status: This issue is waiting on initial triage. labels Oct 29, 2024
@0x53A
Copy link
Author

0x53A commented Oct 29, 2024

I think it's actually not related to the patch and purely caused by fe-o-godot-macros. I'll try to create a smaller repro when I have some more time ...

@epage epage added A-git Area: anything dealing with git A-patch Area: [patch] table override S-needs-info Status: Needs more info, such as a reproduction or more background for a feature request. and removed S-triage Status: This issue is waiting on initial triage. labels Oct 29, 2024
@0x53A
Copy link
Author

0x53A commented Nov 6, 2024

Got it! please see the edit at the top

@0x53A 0x53A changed the title when patching dependencies, common dependencies are not deduplicated and features are missing [maybe a bug] common dependencies are not unified between main crate and proc-macro crate, if features differ Nov 6, 2024
@epage epage added A-features Area: features — conditional compilation and removed A-git Area: anything dealing with git A-patch Area: [patch] table override labels Nov 6, 2024
@epage
Copy link
Contributor

epage commented Nov 6, 2024

resolver = "2" (default with 2021 edition) resolves features for the host (proc macros, etc) separate from the target (normal dependencies). That the code generated by the proc macro can depend on the target having the same features is being tracked in #14415.

Closing in favor of #14415. If there is a reason for us to keep this open separately, let us know!

@epage epage closed this as not planned Won't fix, can't repro, duplicate, stale Nov 6, 2024
@0x53A
Copy link
Author

0x53A commented Nov 6, 2024

TIL. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-features Area: features — conditional compilation C-bug Category: bug S-needs-info Status: Needs more info, such as a reproduction or more background for a feature request.
Projects
None yet
Development

No branches or pull requests

2 participants