-
-
Notifications
You must be signed in to change notification settings - Fork 6
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 proc-macro lib type #466
base: main
Are you sure you want to change the base?
Support proc-macro lib type #466
Conversation
I'm not sure I understand the need for this change. Procedural macros are, afaik, just some special sort of library, and should be covered under library targets. Further, a package/crate/whateveryoucallit may only contain one library, so there is no need to distinguish between proc-macro libraries and "normal" libraries. I ran the test case from this PR against the main branch and the test was successful. Can you please elaborate why this change is necessary? What is the problem with the way proc-macro libraries are currently handled? |
Actually, "proc-macro" is a separate kind. The check Also, the test doesn't pass on my machine. git checkout feature/target-proc-macro
cargo metadata --format-version 1 --manifest-path tests/pass/proc-macro/Cargo.toml | jq -r '.packages[0].targets[0] | {name, kind}'
# {
# "name": "proc_macro",
# "kind": [
# "proc-macro"
# ]
# }
cargo doc2readme --out - --manifest-path tests/pass/proc-macro/Cargo.toml
# Error: Failed to find a library or binary target |
Oh I see, you marked the test case nightly-only, and I didn't run it with a nightly compiler 😅 |
I'm still not sure I support the approach you took here. From my perspective, there can be at most 1 library (proc-macro or not) per package, and I consider proc-macro to be a special case of libraries. So I would be extremely surprised if proc-macro was a separate type. Do you think it would make sense to keep the lib/bin separation but attempt to find a proc-macro target if a normal library target was not found? |
For comparison, here's
So the Because of that, the check for |
Hi, thank you for the crate!
I've implemented support for
proc-macro
lib type (seetests/pass/proc-macro
). Currently, this is done in a backwards-compatible way: there is a new option--target-name
that allows to specify the target name explicitly. The target name can be specified for a target of any type (includingproc-macro
).Would you like to merge this PR?