From 3fb8c4968288105668cd21f4768bba82b14fceab Mon Sep 17 00:00:00 2001 From: Joshua Nelson Date: Wed, 24 Nov 2021 12:12:07 -0500 Subject: [PATCH] fix the actual bug - only skip `--target` for proc-macros - don't try build non-host targets for proc-macros, since they're not supported anyway. --- crates/metadata/lib.rs | 13 +++++- src/docbuilder/rustwide_builder.rs | 73 +++++++++++++++++++++++++++++- 2 files changed, 84 insertions(+), 2 deletions(-) diff --git a/crates/metadata/lib.rs b/crates/metadata/lib.rs index fb33e48e8..b837dfc3b 100644 --- a/crates/metadata/lib.rs +++ b/crates/metadata/lib.rs @@ -197,7 +197,18 @@ impl Metadata { /// If `include_default_targets` is `true` and `targets` is unset, this also includes /// [`DEFAULT_TARGETS`]. Otherwise, if `include_default_targets` is `false` and `targets` /// is unset, `other_targets` will be empty. + /// + /// All of the above is ignored for proc-macros, which are always only compiled for the host. pub fn targets(&self, include_default_targets: bool) -> BuildTargets<'_> { + // Proc macros can only be compiled for the host, so just completely ignore any configured targets. + // It would be nice to warn about this somehow ... + if self.proc_macro { + return BuildTargets { + default_target: HOST_TARGET, + other_targets: HashSet::default(), + }; + } + let default_target = self .default_target .as_deref() @@ -205,7 +216,7 @@ impl Metadata { .or_else(|| { self.targets .as_ref() - .and_then(|targets| targets.iter().next().map(String::as_str)) + .and_then(|targets| targets.first().map(String::as_str)) }) .unwrap_or(HOST_TARGET); diff --git a/src/docbuilder/rustwide_builder.rs b/src/docbuilder/rustwide_builder.rs index caadd4380..4cae81509 100644 --- a/src/docbuilder/rustwide_builder.rs +++ b/src/docbuilder/rustwide_builder.rs @@ -656,7 +656,16 @@ impl RustwideBuilder { if let Some(cpu_limit) = self.config.build_cpu_limit { cargo_args.push(format!("-j{}", cpu_limit)); } - if target != HOST_TARGET { + // Cargo has a series of frightening bugs around cross-compiling proc-macros: + // - Passing `--target` causes RUSTDOCFLAGS to fail to be passed 🤦 + // - Passing `--target` will *create* `target/{target-name}/doc` but will put the docs in `target/doc` anyway + // As a result, it's not possible for us to support cross-compiling proc-macros. + // However, all these caveats unfortunately still apply when `{target-name}` is the host. + // So, only pass `--target` for crates that aren't proc-macros. + // + // Originally, this had a simpler check `target != HOST_TARGET`, but *that* was buggy when `HOST_TARGET` wasn't the same as the default target. + // Rather than trying to keep track of it all, only special case proc-macros, which are what we actually care about. + if !metadata.proc_macro { cargo_args.push("--target".into()); cargo_args.push(target.into()); }; @@ -935,4 +944,66 @@ mod tests { Ok(()) }) } + + #[test] + #[ignore] + fn test_proc_macro() { + wrapper(|env| { + let crate_ = "thiserror-impl"; + let version = "1.0.26"; + let mut builder = RustwideBuilder::init(env).unwrap(); + assert!(builder.build_package(crate_, version, PackageKind::CratesIo)?); + + let storage = env.storage(); + + // doc archive exists + let doc_archive = rustdoc_archive_path(crate_, version); + assert!(storage.exists(&doc_archive)?); + + // source archive exists + let source_archive = source_archive_path(crate_, version); + assert!(storage.exists(&source_archive)?); + + Ok(()) + }); + } + + #[test] + #[ignore] + fn test_cross_compile_non_host_default() { + wrapper(|env| { + let crate_ = "xingapi"; + let version = "0.3.3"; + let mut builder = RustwideBuilder::init(env).unwrap(); + assert!(builder.build_package(crate_, version, PackageKind::CratesIo)?); + + let storage = env.storage(); + + // doc archive exists + let doc_archive = rustdoc_archive_path(crate_, version); + assert!(storage.exists(&doc_archive)?); + + // source archive exists + let source_archive = source_archive_path(crate_, version); + assert!(storage.exists(&source_archive)?); + + let target = "x86_64-unknown-linux-gnu"; + let crate_path = crate_.replace("-", "_"); + let target_docs_present = storage.exists_in_archive( + &doc_archive, + &format!("{}/{}/index.html", target, crate_path), + )?; + + let web = env.frontend(); + let target_url = format!( + "/{}/{}/{}/{}/index.html", + crate_, version, target, crate_path + ); + + assert!(target_docs_present); + assert_success(&target_url, web)?; + + Ok(()) + }); + } }