Skip to content

Commit

Permalink
fix the actual bug
Browse files Browse the repository at this point in the history
- only skip `--target` for proc-macros
- don't try build non-host targets for proc-macros, since they're not
  supported anyway.
- change rustwide_builder to look in the platform-specific target
  directory for the default target
- stores all source docs in the target/{target_name} directory, rather
  than trying to special case anything
  • Loading branch information
jyn514 committed Nov 27, 2021
1 parent 8772e1a commit 9672ae5
Show file tree
Hide file tree
Showing 2 changed files with 111 additions and 17 deletions.
13 changes: 12 additions & 1 deletion crates/metadata/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -197,15 +197,26 @@ 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()
// Use the first element of `targets` if `default_target` is unset and `targets` is non-empty
.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);

Expand Down
115 changes: 99 additions & 16 deletions src/docbuilder/rustwide_builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -210,7 +210,8 @@ impl RustwideBuilder {
}

info!("copying essential files for {}", self.rustc_version);
let source = build.host_target_dir().join("doc");
assert!(!metadata.proc_macro);
let source = build.host_target_dir().join(HOST_TARGET).join("doc");
let dest = tempfile::Builder::new()
.prefix("essential-files")
.tempdir()?;
Expand Down Expand Up @@ -350,14 +351,23 @@ impl RustwideBuilder {
if res.result.successful {
if let Some(name) = res.cargo_metadata.root().library_name() {
let host_target = build.host_target_dir();
has_docs = host_target.join("doc").join(name).is_dir();
has_docs = host_target
.join(default_target)
.join("doc")
.join(name)
.is_dir();
}
}

let mut algs = HashSet::new();
if has_docs {
debug!("adding documentation for the default target to the database");
self.copy_docs(&build.host_target_dir(), local_storage.path(), "", true)?;
self.copy_docs(
&build.host_target_dir(),
local_storage.path(),
default_target,
true,
)?;

successful_targets.push(res.target.clone());

Expand Down Expand Up @@ -593,16 +603,18 @@ impl RustwideBuilder {
.is_ok()
});

// If we're passed a default_target which requires a cross-compile,
// cargo will put the output in `target/<target>/doc`.
// However, if this is the default build, we don't want it there,
// we want it in `target/doc`.
// NOTE: don't rename this if the build failed, because `target/<target>/doc` won't exist.
if successful && target != HOST_TARGET && is_default_target {
// mv target/$target/doc target/doc
// For proc-macros, cargo will put the output in `target/doc`.
// Move it to the target-specific directory for consistency with other builds.
// NOTE: don't rename this if the build failed, because `target/doc` won't exist.
if successful && metadata.proc_macro {
assert!(
is_default_target && target == HOST_TARGET,
"can't handle cross-compiling macros"
);
// mv target/doc target/$target/doc
let target_dir = build.host_target_dir();
let old_dir = target_dir.join(target).join("doc");
let new_dir = target_dir.join("doc");
let old_dir = target_dir.join("doc");
let new_dir = target_dir.join(target).join("doc");
debug!("rename {} to {}", old_dir.display(), new_dir.display());
std::fs::rename(old_dir, new_dir)?;
}
Expand Down Expand Up @@ -656,7 +668,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());
};
Expand Down Expand Up @@ -702,7 +723,7 @@ impl RustwideBuilder {
dest = dest.join(target);
}

info!("{} {}", source.display(), dest.display());
info!("copy {} to {}", source.display(), dest.display());
copy_dir_all(source, dest).map_err(Into::into)
}

Expand Down Expand Up @@ -835,11 +856,11 @@ mod tests {

// doc archive exists
let doc_archive = rustdoc_archive_path(crate_, version);
assert!(storage.exists(&doc_archive)?);
assert!(storage.exists(&doc_archive)?, "{}", doc_archive);

// source archive exists
let source_archive = source_archive_path(crate_, version);
assert!(storage.exists(&source_archive)?);
assert!(storage.exists(&source_archive)?, "{}", source_archive);

// default target was built and is accessible
assert!(storage.exists_in_archive(&doc_archive, &format!("{}/index.html", crate_path))?);
Expand Down Expand Up @@ -935,4 +956,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)?, "{}", doc_archive);

// source archive exists
let source_archive = source_archive_path(crate_, version);
assert!(storage.exists(&source_archive)?, "{}", 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(())
});
}
}

0 comments on commit 9672ae5

Please sign in to comment.