google / cargo-raze

Generate Bazel BUILD from Cargo dependencies!
Apache License 2.0
480 stars 104 forks source link

Build failure for clap #468

Open maghoff opened 2 years ago

maghoff commented 2 years ago

I am unable to add clap as a dependency because it includes its README.md as part of its build, and this is not declared as an input to the build rules generated by raze. I expect to be able to use clap 🙂 More details:

Adding clap as a dependency by adding the following to Cargo.toml:

[dependencies]
clap = { version = "3.0.14", features = ["derive"] }

gives me the following error during build:

error: couldn't read external/raze__clap__3_0_14/src/../README.md: No such file or directory (os error 2)
 --> external/raze__clap__3_0_14/src/lib.rs:8:39
  |
8 | #![cfg_attr(feature = "derive", doc = include_str!("../README.md"))]
  |                                       ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  |
  = note: this error originates in the macro `include_str` (in Nightly builds, run with -Z macro-backtrace for more info)

The line in question on GitHub: https://github.com/clap-rs/clap/blob/v3.0.14/src/lib.rs#L8

I can work around the problem by adding README.md to the data of the generated build rules, but that is a hack. Maybe #287 is the correct solution to this issue?

(By the way, clap_derive exposes the same issue)

maghoff commented 2 years ago

Could it be a good resolution to blanket include all non-.rs-files from the package as data for all packages?

illicitonion commented 2 years ago

Could it be a good resolution to blanket include all non-.rs-files from the package as data for all packages?

That's what we did in crate_universe for exactly this reason, wanting most things to Just Work by default.

dfreese commented 2 years ago

@illicitonion Have you run into any problems with that? Curious if there are edge cases I'm not thinking of.

If not, I'd be supportive of that here.

illicitonion commented 2 years ago

We haven't run into any problems - the only down-side really is that in a remote execution environment, you may be uploading a few slightly bigger files (e.g. images that that are used as test data but haven't been excluded from the crate packaging, or similar), but in practice I've not run into any crates where this has been a noticeable effect.

We have a short list of exceptions to the default glob here with comments explaining each one.

bcmyers commented 2 years ago

Sharing my (somewhat hacky and probably not optimal) workaround for those who might be interested in using it.

Relevant parts of Cargo.toml...

[dependencies]
clap = { version = "=3.1.6", features = ["env", "derive", "unicode", "wrap_help", "yaml"] }
clap_derive = "=3.1.4"

[package.metadata.raze.crates.clap.'3.1.6']
data_attr = '[":README.md"]'

[package.metadata.raze.crates.clap_derive.'3.1.4']
patch_args = ["-p1"]
patches = [
    "//rust-shed/third-party/patches:clap_derive.patch",
]

Contents of patch file referenced (//rust-shed/third-party/patches:clap_derive.patch)...

diff --git a/src/lib.rs b/src/lib.rs
index 973b61e1..125f62a4 100644
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -13,7 +13,6 @@
 // MIT/Apache 2.0 license.

 #![doc(html_logo_url = "https://raw.githubusercontent.com/clap-rs/clap/master/assets/clap.png")]
-#![doc = include_str!("../README.md")]
 #![forbid(unsafe_code)]

 extern crate proc_macro;
dayfine commented 1 year ago

Could it be a good resolution to blanket include all non-.rs-files from the package as data for all packages?

That's what we did in crate_universe for exactly this reason, wanting most things to Just Work by default.

Do you mean crate_universe can be used to solve the issue here? Is there an example? Thanks!

For the record, for 4.3.12, I had to do:

[dependencies]
clap = {version = "=4.3.12", features = ["derive"]}

...

[package.metadata.raze.crates.clap.'4.3.12']
data_attr = '[":README.md", ":examples/demo.md"]'

[package.metadata.raze.crates.clap_builder.'4.3.12']
data_attr = '[":README.md"]'

[package.metadata.raze.crates.clap_derive.'4.3.12']
data_attr = '[":README.md"]'