From 1dda3d4c73021c62ccbc64513bbbcf8a7f5f1b66 Mon Sep 17 00:00:00 2001 From: Predrag Gruevski <2348618+obi1kenobi@users.noreply.github.com> Date: Thu, 20 Apr 2023 23:30:23 -0400 Subject: [PATCH] Load lib target name from `cargo metadata`. (#433) * Load lib target name from `cargo metadata`. * Clean up test cases. * Reproduce the `perf-event-open-sys2` crash in CI. * Restore old behavior around bin targets. * Support proc macro crates too. --- .github/workflows/ci.yml | 64 ++++ src/manifest.rs | 26 -- src/query.rs | 2 +- src/rustdoc_cmd.rs | 276 ++++++++++-------- src/rustdoc_gen.rs | 146 ++------- .../crate_in_workspace/Cargo.toml | 0 .../crate_in_workspace/crate1/Cargo.toml | 0 .../crate_in_workspace/crate1/src/lib.rs | 0 .../lib-target-with-dashes}/Cargo.toml | 3 +- test_crates/lib-target-with-dashes/src/lib.rs | 0 .../proc_macro_crate}/Cargo.toml | 5 +- test_crates/proc_macro_crate/src/lib.rs | 7 + .../renamed_lib_target}/Cargo.toml | 7 +- .../renamed_lib_target}/src/lib.rs | 0 .../Cargo.toml | 2 +- .../target_specific_dependencies/src/lib.rs | 0 .../src/main.rs | 0 test_rustdoc/crate_in_workspace/Cargo.lock | 7 - test_rustdoc/implicit_bin/Cargo.lock | 7 - test_rustdoc/implicit_bin/src/main.rs | 3 - test_rustdoc/implicit_lib/Cargo.lock | 7 - test_rustdoc/renamed_bin/Cargo.lock | 7 - test_rustdoc/renamed_bin/Cargo.toml | 12 - test_rustdoc/renamed_bin/src/main.rs | 3 - test_rustdoc/renamed_lib/Cargo.lock | 7 - test_rustdoc/renamed_lib/src/lib.rs | 14 - tests/detects_target_dependencies.rs | 13 - tests/rustdoc_edge_cases.rs | 77 +++++ 28 files changed, 342 insertions(+), 353 deletions(-) rename {test_rustdoc => test_crates}/crate_in_workspace/Cargo.toml (100%) rename {test_rustdoc => test_crates}/crate_in_workspace/crate1/Cargo.toml (100%) rename {test_rustdoc => test_crates}/crate_in_workspace/crate1/src/lib.rs (100%) rename {test_rustdoc/implicit_lib => test_crates/lib-target-with-dashes}/Cargo.toml (76%) create mode 100644 test_crates/lib-target-with-dashes/src/lib.rs rename {test_rustdoc/implicit_bin => test_crates/proc_macro_crate}/Cargo.toml (75%) create mode 100644 test_crates/proc_macro_crate/src/lib.rs rename {test_rustdoc/renamed_lib => test_crates/renamed_lib_target}/Cargo.toml (80%) rename {test_rustdoc/implicit_lib => test_crates/renamed_lib_target}/src/lib.rs (100%) rename test_crates/{target-specific-dependencies => target_specific_dependencies}/Cargo.toml (78%) create mode 100644 test_crates/target_specific_dependencies/src/lib.rs rename test_crates/{target-specific-dependencies => target_specific_dependencies}/src/main.rs (100%) delete mode 100644 test_rustdoc/crate_in_workspace/Cargo.lock delete mode 100644 test_rustdoc/implicit_bin/Cargo.lock delete mode 100644 test_rustdoc/implicit_bin/src/main.rs delete mode 100644 test_rustdoc/implicit_lib/Cargo.lock delete mode 100644 test_rustdoc/renamed_bin/Cargo.lock delete mode 100644 test_rustdoc/renamed_bin/Cargo.toml delete mode 100644 test_rustdoc/renamed_bin/src/main.rs delete mode 100644 test_rustdoc/renamed_lib/Cargo.lock delete mode 100644 test_rustdoc/renamed_lib/src/lib.rs delete mode 100644 tests/detects_target_dependencies.rs create mode 100644 tests/rustdoc_edge_cases.rs diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index c7a98c2d..8b830d94 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -38,6 +38,7 @@ jobs: - run-on-ref-slice-fork-windows - run-on-tokio-explicit - run-on-tokio-implicit + - run-on-perf-event-open-sys2 if: ${{ success() || failure() }} # Run this job even if a dependency has failed. steps: - name: Job outcomes @@ -58,6 +59,7 @@ jobs: echo "run-on-ref-slice-fork-windows: ${{ needs.run-on-ref-slice-fork-windows.result }}" echo "run-on-tokio-explicit: ${{ needs.run-on-tokio-explicit.result }}" echo "run-on-tokio-implicit: ${{ needs.run-on-tokio-implicit.result }}" + echo "run-on-perf-event-open-sys2: ${{ needs.run-on-perf-event-open-sys2.result }}" # Fail this required job if any of its dependent jobs have failed. # @@ -96,6 +98,8 @@ jobs: run: exit 1 - if: ${{ needs.run-on-tokio-implicit.result != 'success' }} run: exit 1 + - if: ${{ needs.run-on-perf-event-open-sys2.result != 'success' }} + run: exit 1 lint: name: Check lint and rustfmt @@ -1319,6 +1323,66 @@ jobs: key: ${{ runner.os }}-${{ steps.toolchain.outputs.cachekey }}-${{ hashFiles('semver\**\Cargo.lock') }}-${{ github.job }}-rustdoc path: subject\target\semver-checks\cache + run-on-perf-event-open-sys2: + # cargo-semver-checks failed to find the generated rustdoc JSON + # because the crate used a lib name different from the crate name. + # https://github.com/obi1kenobi/cargo-semver-checks/issues/432 + name: 'Semver: perf-event-open-sys2 v5.0.0' + runs-on: ubuntu-latest + needs: + - build-binary + steps: + - name: Checkout cargo-semver-checks + uses: actions/checkout@v3 + with: + persist-credentials: false + path: 'semver' + + - name: Checkout bevy + uses: actions/checkout@v3 + with: + persist-credentials: false + repository: 'phantomical/perf-event' + ref: '47dc882dbfb44b86584540a6e4f2815caaf72c10' + path: 'subject' + + - name: Install rust + id: toolchain + uses: dtolnay/rust-toolchain@stable + + - name: Restore binary + id: cache-binary + uses: actions/cache/restore@v3 + with: + path: bins/ + key: bins-${{ runner.os }}-${{ github.run_id }}-${{ github.run_attempt }} + fail-on-cache-miss: true + + - name: Restore rustdoc + id: cache + uses: actions/cache/restore@v3 + with: + key: ${{ runner.os }}-${{ steps.toolchain.outputs.cachekey }}-${{ hashFiles('semver/**/Cargo.lock') }}-${{ github.job }}-rustdoc + path: subject/target/semver-checks/cache + + - name: Restore cargo index and rustdoc target dir + uses: Swatinem/rust-cache@v2 + with: + workspaces: | + subject/target/semver-checks/local-perf-event-open-sys2-5_0_0 + + - name: Run semver-checks + run: | + cd semver + ../bins/cargo-semver-checks semver-checks check-release --manifest-path="../subject/perf-event-open-sys/Cargo.toml" --baseline-version 5.0.0 --verbose + + - name: Save rustdoc + uses: actions/cache/save@v3 + if: steps.cache.outputs.cache-hit != 'true' + with: + key: ${{ runner.os }}-${{ steps.toolchain.outputs.cachekey }}-${{ hashFiles('semver/**/Cargo.lock') }}-${{ github.job }}-rustdoc + path: subject/target/semver-checks/cache + init-release: name: Run the release workflow needs: diff --git a/src/manifest.rs b/src/manifest.rs index 4871a004..46935647 100644 --- a/src/manifest.rs +++ b/src/manifest.rs @@ -44,32 +44,6 @@ pub(crate) fn get_package_version(manifest: &Manifest) -> anyhow::Result<&str> { Ok(version) } -pub(crate) fn get_lib_target_name(manifest: &Manifest) -> anyhow::Result { - // If there's a [lib] section, return the name it specifies, if any. - if let Some(product) = &manifest.parsed.lib { - if let Some(lib_name) = &product.name { - return Ok(lib_name.clone()); - } - } - - // Otherwise, assume the crate is a lib crate with the default lib target name: - // the same name as the package but with dashes replaced with underscores. - Ok(get_package_name(manifest)?.replace('-', "_")) -} - -pub(crate) fn get_first_bin_target_name(manifest: &Manifest) -> anyhow::Result { - // If there's a [[bin]] section, return the first item's name. - if let Some(product) = manifest.parsed.bin.first() { - if let Some(bin_name) = &product.name { - return Ok(bin_name.clone()); - } - } - - // Otherwise, assume the crate is a bin crate with the default bin target name: - // the same name as the package but with dashes replaced with underscores. - Ok(get_package_name(manifest)?.replace('-', "_")) -} - pub(crate) fn get_project_dir_from_manifest_path( manifest_path: &std::path::Path, ) -> anyhow::Result { diff --git a/src/query.rs b/src/query.rs index 61959404..824c7af2 100644 --- a/src/query.rs +++ b/src/query.rs @@ -246,7 +246,7 @@ mod tests { ); let mut actual_paths = actual_results[0]["path"] - .as_vec(|val| val.as_vec(FieldValue::as_str)) + .as_vec_with(|val| val.as_vec_with(FieldValue::as_str)) .expect("not a Vec>"); actual_paths.sort_unstable(); diff --git a/src/rustdoc_cmd.rs b/src/rustdoc_cmd.rs index 71f59024..f7da2acd 100644 --- a/src/rustdoc_cmd.rs +++ b/src/rustdoc_cmd.rs @@ -1,4 +1,11 @@ -use crate::GlobalConfig; +use std::path::{Path, PathBuf}; + +use anyhow::Context; + +use crate::{ + rustdoc_gen::{CrateDataForRustdoc, CrateSource}, + GlobalConfig, +}; #[derive(Clone, Debug, PartialEq, Eq)] pub struct RustdocCommand { @@ -36,26 +43,34 @@ impl RustdocCommand { self } - /// Produce a rustdoc JSON file for the specified configuration. - pub(crate) fn dump( + /// Produce a rustdoc JSON file for the specified crate and source. + pub(crate) fn generate_rustdoc( &self, config: &mut GlobalConfig, - manifest_path: &std::path::Path, - pkg_spec: Option<&str>, - all_features: bool, + build_dir: PathBuf, + crate_source: &CrateSource, + crate_data: &CrateDataForRustdoc, ) -> anyhow::Result { + // Generate an empty placeholder project with a dependency on the crate + // whose rustdoc we need. We take this indirect generation path to avoid issues like: + // https://github.com/obi1kenobi/cargo-semver-checks/issues/167#issuecomment-1382367128 + let placeholder_manifest = create_placeholder_rustdoc_manifest(crate_source, crate_data) + .context("failed to create placeholder manifest")?; + let placeholder_manifest_path = + save_placeholder_rustdoc_manifest(build_dir.as_path(), placeholder_manifest) + .context("failed to save placeholder rustdoc manifest")?; + let metadata = cargo_metadata::MetadataCommand::new() - .manifest_path(manifest_path) - .no_deps() + .manifest_path(&placeholder_manifest_path) .exec()?; - let manifest_target_directory = metadata + let placeholder_target_directory = metadata .target_directory .as_path() .as_std_path() // HACK: Avoid potential errors when mixing toolchains .join(crate::util::SCOPE) .join("target"); - let target_dir = manifest_target_directory.as_path(); + let target_dir = placeholder_target_directory.as_path(); let stderr = if self.silence { std::process::Stdio::piped() @@ -64,6 +79,18 @@ impl RustdocCommand { std::process::Stdio::inherit() }; + let crate_name = crate_source.name()?; + let version = crate_source.version()?; + let pkg_spec = format!("{crate_name}@{version}"); + + // Run the rustdoc generation command on the placeholder crate, + // specifically requesting the rustdoc of *only* the crate specified in `pkg_spec`. + // + // N.B.: Passing `--all-features` here has no effect, since that only applies to + // the top-level project i.e. the placeholder, which has no features. + // To generate rustdoc for our intended crate with features enabled, + // those features must be enabled on the dependency in the `Cargo.toml` + // of the placeholder project. let mut cmd = std::process::Command::new("cargo"); cmd.env("RUSTC_BOOTSTRAP", "1") .env( @@ -74,53 +101,54 @@ impl RustdocCommand { .stderr(stderr) .arg("doc") .arg("--manifest-path") - .arg(manifest_path) + .arg(&placeholder_manifest_path) .arg("--target-dir") - .arg(target_dir); - if let Some(pkg_spec) = pkg_spec { - cmd.arg("--package").arg(pkg_spec); - } + .arg(target_dir) + .arg("--package") + .arg(pkg_spec); if !self.deps { cmd.arg("--no-deps"); } - if all_features { - cmd.arg("--all-features"); - } if config.is_stderr_tty() { cmd.arg("--color=always"); } + let output = cmd.output()?; if !output.status.success() { if self.silence { anyhow::bail!( "Failed when running cargo-doc on {}:\n{}", - manifest_path.display(), + placeholder_manifest_path.display(), String::from_utf8_lossy(&output.stderr) ) } else { anyhow::bail!( "Failed when running cargo-doc on {}. See stderr.", - manifest_path.display(), + placeholder_manifest_path.display(), ) } } - if let Some(pkg_spec) = pkg_spec { - // N.B.: This package spec is not necessarily part of the manifest at `manifest_path`! - // For example, when getting rustdoc JSON for a crate version from the registry, - // the manifest is "synthetic" with only a dependency on that crate and version - // and therefore is not a manifest *for* that crate itself. - let crate_name = pkg_spec - .split_once('@') - .map(|s| s.0) - .unwrap_or(pkg_spec) - .to_owned(); - - // N.B.: Crates named like `foo-bar` have rustdoc JSON named like `foo_bar.json`. - let crate_name_with_underscores = crate_name.replace('-', "_"); - let json_path = target_dir.join(format!("doc/{crate_name_with_underscores}.json")); + let subject_crate = metadata + .packages + .iter() + .find(|dep| dep.name == crate_name) + .expect("we declared a dependency on a crate that doesn't exist in the metadata"); + + // Figure out the name of the JSON file where rustdoc will produce the output we want. + // The name is: + // - the name of the *library or proc-macro target* of the crate, not the crate's name + // - but with all `-` chars replaced with `_` instead. + // Related: https://github.com/obi1kenobi/cargo-semver-checks/issues/432 + if let Some(lib_target) = subject_crate.targets.iter().find(|target| { + target.is_lib() || target.kind.iter().any(|k| k.as_str() == "proc-macro") + }) { + let lib_name = lib_target.name.as_str(); + let rustdoc_json_file_name = lib_name.replace('-', "_"); + + let json_path = target_dir.join(format!("doc/{rustdoc_json_file_name}.json")); if json_path.exists() { - Ok(json_path) + return Ok(json_path); } else { anyhow::bail!( "Could not find expected rustdoc output for `{}`: {}", @@ -128,33 +156,29 @@ impl RustdocCommand { json_path.display() ); } - } else { - let manifest = crate::manifest::Manifest::parse(manifest_path.to_path_buf())?; + } + + // This crate does not have a lib target. + // For backward compatibility with older cargo-semver-checks versions, + // we currently preserve the old behavior of using the first bin target's rustdoc. + // At some future point, this is likely going to be deprecated and then become an error. + if let Some(bin_target) = subject_crate.targets.iter().find(|target| target.is_bin()) { + let bin_name = bin_target.name.as_str(); + let rustdoc_json_file_name = bin_name.replace('-', "_"); - let lib_target_name = crate::manifest::get_lib_target_name(&manifest)?; - let json_path = target_dir.join(format!("doc/{lib_target_name}.json")); + let json_path = target_dir.join(format!("doc/{rustdoc_json_file_name}.json")); if json_path.exists() { return Ok(json_path); - } - - let first_bin_target_name = crate::manifest::get_first_bin_target_name(&manifest)?; - let json_path = target_dir.join(format!("doc/{first_bin_target_name}.json")); - if !json_path.exists() { - let crate_name = if let Some(pkg_spec) = pkg_spec { - pkg_spec.split_once('@').map(|s| s.0).unwrap_or(pkg_spec) - } else { - crate::manifest::get_package_name(&manifest)? - }; - + } else { anyhow::bail!( "Could not find expected rustdoc output for `{}`: {}", crate_name, json_path.display() ); } - - Ok(json_path) } + + anyhow::bail!("No lib or bin targets so nothing to scan for crate {crate_name}") } } @@ -164,82 +188,82 @@ impl Default for RustdocCommand { } } -#[cfg(test)] -mod tests { - use crate::GlobalConfig; - use std::path::Path; - - use super::RustdocCommand; - - #[test] - fn rustdoc_for_lib_crate_without_lib_section() { - RustdocCommand::default() - .dump( - &mut GlobalConfig::new(), - Path::new("./test_rustdoc/implicit_lib/Cargo.toml"), - None, - true, - ) - .expect("no errors"); - } - - #[test] - fn rustdoc_for_lib_crate_with_lib_section() { - RustdocCommand::default() - .dump( - &mut GlobalConfig::new(), - Path::new("./test_rustdoc/renamed_lib/Cargo.toml"), - None, - true, - ) - .expect("no errors"); - } +/// To get the rustdoc of the project, we first create a placeholder project somewhere +/// with the project as a dependency, and run `cargo rustdoc` on it. +fn create_placeholder_rustdoc_manifest( + crate_source: &CrateSource, + _crate_data: &CrateDataForRustdoc, // TODO: use this to select crate features to enable +) -> anyhow::Result> { + use cargo_toml::*; - #[test] - fn rustdoc_for_bin_crate_without_bin_section() { - RustdocCommand::default() - .dump( - &mut GlobalConfig::new(), - Path::new("./test_rustdoc/implicit_bin/Cargo.toml"), - None, - true, - ) - .expect("no errors"); - } + Ok(Manifest::<()> { + package: { + let mut package = Package::new("rustdoc", "0.0.0"); + package.publish = Inheritable::Set(Publish::Flag(false)); + Some(package) + }, + workspace: Some(Workspace::<()>::default()), + lib: { + let product = Product { + path: Some("lib.rs".to_string()), + ..Product::default() + }; + Some(product) + }, + dependencies: { + let project_with_features: DependencyDetail = match crate_source { + CrateSource::Registry { crate_ } => DependencyDetail { + // We need the *exact* version as a dependency, or else cargo will + // give us the latest semver-compatible version which is not we want. + // Fixes: https://github.com/obi1kenobi/cargo-semver-checks/issues/261 + version: Some(format!("={}", crate_.version())), + features: crate_source.all_features(), + ..DependencyDetail::default() + }, + CrateSource::ManifestPath { manifest } => DependencyDetail { + path: Some({ + let dir_path = + crate::manifest::get_project_dir_from_manifest_path(&manifest.path)?; + // The manifest will be saved in some other directory, + // so for convenience, we're using absolute paths. + dir_path + .canonicalize() + .context("failed to canonicalize manifest path")? + .to_str() + .context("manifest path is not valid UTF-8")? + .to_string() + }), + features: crate_source.all_features(), + ..DependencyDetail::default() + }, + }; + let mut deps = DepsSet::new(); + deps.insert( + crate_source.name()?.to_string(), + Dependency::Detailed(project_with_features), + ); + deps + }, + ..Default::default() + }) +} - #[test] - fn rustdoc_for_bin_crate_with_bin_section() { - RustdocCommand::default() - .dump( - &mut GlobalConfig::new(), - Path::new("./test_rustdoc/renamed_bin/Cargo.toml"), - None, - true, - ) - .expect("no errors"); - } +fn save_placeholder_rustdoc_manifest( + placeholder_build_dir: &Path, + placeholder_manifest: cargo_toml::Manifest<()>, +) -> anyhow::Result { + std::fs::create_dir_all(placeholder_build_dir).context("failed to create build dir")?; + let placeholder_manifest_path = placeholder_build_dir.join("Cargo.toml"); - #[test] - fn rustdoc_for_crate_in_workspace_with_workspace_manifest() { - RustdocCommand::default() - .dump( - &mut GlobalConfig::new(), - Path::new("./test_rustdoc/crate_in_workspace/Cargo.toml"), - Some("crate_in_workspace_crate1"), - true, - ) - .expect("no errors"); - } + // Possibly fixes https://github.com/libp2p/rust-libp2p/pull/2647#issuecomment-1280221217 + let _: std::io::Result<()> = std::fs::remove_file(placeholder_build_dir.join("Cargo.lock")); - #[test] - fn rustdoc_for_crate_in_workspace_with_crate_manifest() { - RustdocCommand::default() - .dump( - &mut GlobalConfig::new(), - Path::new("./test_rustdoc/crate_in_workspace/crate1/Cargo.toml"), - None, - true, - ) - .expect("no errors"); - } + std::fs::write( + &placeholder_manifest_path, + toml::to_string(&placeholder_manifest)?, + ) + .context("failed to write placeholder manifest")?; + std::fs::write(placeholder_build_dir.join("lib.rs"), "") + .context("failed to create empty lib.rs")?; + Ok(placeholder_manifest_path) } diff --git a/src/rustdoc_gen.rs b/src/rustdoc_gen.rs index 8490e32a..2fb051f8 100644 --- a/src/rustdoc_gen.rs +++ b/src/rustdoc_gen.rs @@ -1,4 +1,4 @@ -use std::path::{Path, PathBuf}; +use std::path::PathBuf; use anyhow::Context; use crates_index::Crate; @@ -10,29 +10,42 @@ use crate::util::slugify; use crate::GlobalConfig; #[derive(Debug, Clone)] -enum CrateSource<'a> { +pub(crate) enum CrateSource<'a> { Registry { crate_: &'a crates_index::Version }, ManifestPath { manifest: &'a Manifest }, } impl<'a> CrateSource<'a> { - fn name(&self) -> anyhow::Result<&str> { + pub(crate) fn name(&self) -> anyhow::Result<&str> { Ok(match self { Self::Registry { crate_ } => crate_.name(), Self::ManifestPath { manifest } => crate::manifest::get_package_name(manifest)?, }) } - fn version(&self) -> anyhow::Result<&str> { + pub(crate) fn version(&self) -> anyhow::Result<&str> { Ok(match self { Self::Registry { crate_ } => crate_.version(), Self::ManifestPath { manifest } => crate::manifest::get_package_version(manifest)?, }) } + /// A path-safe unique identifier that includes the crate's name, version, and source. + pub(crate) fn slug(&self) -> anyhow::Result { + Ok(format!( + "{}-{}-{}", + match self { + CrateSource::Registry { .. } => "registry", + CrateSource::ManifestPath { .. } => "local", + }, + slugify(self.name()?), + slugify(self.version()?) + )) + } + /// Returns features listed in `[features]` section in the manifest /// - fn regular_features(&self) -> Vec { + pub(crate) fn regular_features(&self) -> Vec { match self { Self::Registry { crate_ } => crate_.features().keys().cloned().collect(), Self::ManifestPath { manifest } => manifest.parsed.features.keys().cloned().collect(), @@ -41,7 +54,7 @@ impl<'a> CrateSource<'a> { /// Returns features implicitly defined by optional dependencies /// - fn implicit_features(&self) -> std::collections::BTreeSet { + pub(crate) fn implicit_features(&self) -> std::collections::BTreeSet { let mut implicit_features: std::collections::BTreeSet<_> = match self { Self::Registry { crate_ } => crate_ .dependencies() @@ -90,7 +103,7 @@ impl<'a> CrateSource<'a> { /// By default, we want to generate rustdoc with `--all-features`, /// but that option isn't available outside of the current crate, /// so we have to implement it ourselves. - fn all_features(&self) -> Vec { + pub(crate) fn all_features(&self) -> Vec { // Implicit features from optional dependencies have to be added separately // from regular features: https://github.com/obi1kenobi/cargo-semver-checks/issues/265 let mut all_crate_features = self.implicit_features(); @@ -99,85 +112,6 @@ impl<'a> CrateSource<'a> { } } -/// To get the rustdoc of the project, we first create a placeholder project somewhere -/// with the project as a dependency, and run `cargo rustdoc` on it. -fn create_placeholder_rustdoc_manifest( - crate_source: &CrateSource, -) -> anyhow::Result> { - use cargo_toml::*; - - Ok(Manifest::<()> { - package: { - let mut package = Package::new("rustdoc", "0.0.0"); - package.publish = Inheritable::Set(Publish::Flag(false)); - Some(package) - }, - workspace: Some(Workspace::<()>::default()), - lib: { - let product = Product { - path: Some("lib.rs".to_string()), - ..Product::default() - }; - Some(product) - }, - dependencies: { - let project_with_features: DependencyDetail = match crate_source { - CrateSource::Registry { crate_ } => DependencyDetail { - // We need the *exact* version as a dependency, or else cargo will - // give us the latest semver-compatible version which is not we want. - // Fixes: https://github.com/obi1kenobi/cargo-semver-checks/issues/261 - version: Some(format!("={}", crate_.version())), - features: crate_source.all_features(), - ..DependencyDetail::default() - }, - CrateSource::ManifestPath { manifest } => DependencyDetail { - path: Some({ - let dir_path = - crate::manifest::get_project_dir_from_manifest_path(&manifest.path)?; - // The manifest will be saved in some other directory, - // so for convenience, we're using absolute paths. - dir_path - .canonicalize() - .context("failed to canonicalize manifest path")? - .to_str() - .context("manifest path is not valid UTF-8")? - .to_string() - }), - features: crate_source.all_features(), - ..DependencyDetail::default() - }, - }; - let mut deps = DepsSet::new(); - deps.insert( - crate_source.name()?.to_string(), - Dependency::Detailed(project_with_features), - ); - deps - }, - ..Default::default() - }) -} - -fn save_placeholder_rustdoc_manifest( - placeholder_build_dir: &Path, - placeholder_manifest: cargo_toml::Manifest<()>, -) -> anyhow::Result { - std::fs::create_dir_all(placeholder_build_dir).context("failed to create build dir")?; - let placeholder_manifest_path = placeholder_build_dir.join("Cargo.toml"); - - // Possibly fixes https://github.com/libp2p/rust-libp2p/pull/2647#issuecomment-1280221217 - let _: std::io::Result<()> = std::fs::remove_file(placeholder_build_dir.join("Cargo.lock")); - - std::fs::write( - &placeholder_manifest_path, - toml::to_string(&placeholder_manifest)?, - ) - .context("failed to write placeholder manifest")?; - std::fs::write(placeholder_build_dir.join("lib.rs"), "") - .context("failed to create empty lib.rs")?; - Ok(placeholder_manifest_path) -} - #[derive(Debug, Clone)] pub(crate) enum CrateType<'a> { Current, @@ -197,7 +131,7 @@ pub(crate) struct CrateDataForRustdoc<'a> { } impl<'a> CrateType<'a> { - fn type_name(&self) -> &'static str { + pub(crate) fn type_name(&self) -> &'static str { match self { CrateType::Current => "current", CrateType::Baseline { .. } => "baseline", @@ -214,20 +148,8 @@ fn generate_rustdoc( ) -> anyhow::Result { let name = crate_source.name()?; let version = crate_source.version()?; + let crate_identifier = crate_source.slug()?; - let crate_identifier = format!( - "{}-{}-{}", - match crate_source { - CrateSource::Registry { .. } => "registry", - // Identifiers of manifest-based crates cannot be used for caching, - // since they probably correspond to a specific (and unknown) gitrev and git state - // so cached entries cannot be checked to see if they are a match or not. - CrateSource::ManifestPath { .. } => "local", - }, - slugify(name), - slugify(version) - ); - let build_dir = target_root.join(&crate_identifier); let (cache_dir, cached_rustdoc) = match crate_source { CrateSource::Registry { .. } => { let cache_dir = target_root.join("cache"); @@ -248,26 +170,22 @@ fn generate_rustdoc( (Some(cache_dir), Some(cached_rustdoc)) } - CrateSource::ManifestPath { .. } => (None, None), + CrateSource::ManifestPath { .. } => { + // Manifest-based crates cannot be cached since they correspond + // to a specific (and unknown) gitrev and git state which is not part of their slug. + // There's no way to check whether a cached entry is a match or not. + (None, None) + } }; - let placeholder_manifest = create_placeholder_rustdoc_manifest(&crate_source) - .context("failed to create placeholder manifest")?; - let placeholder_manifest_path = - save_placeholder_rustdoc_manifest(build_dir.as_path(), placeholder_manifest) - .context("failed to save placeholder rustdoc manifest")?; - config.shell_status( "Parsing", format_args!("{name} v{version} ({})", crate_data.crate_type.type_name()), )?; - let rustdoc_path = rustdoc_cmd.dump( - config, - placeholder_manifest_path.as_path(), - Some(&format!("{name}@{version}")), - false, - )?; + let build_dir = target_root.join(&crate_identifier); + let rustdoc_path = + rustdoc_cmd.generate_rustdoc(config, build_dir.clone(), &crate_source, &crate_data)?; match crate_source { CrateSource::Registry { .. } => { @@ -286,7 +204,7 @@ fn generate_rustdoc( } CrateSource::ManifestPath { .. } => { // We don't do any caching here -- since the crate is saved locally, - // it could be modified by the user since it was cached. + // it could be modified by the user after it was cached. Ok(rustdoc_path) } } diff --git a/test_rustdoc/crate_in_workspace/Cargo.toml b/test_crates/crate_in_workspace/Cargo.toml similarity index 100% rename from test_rustdoc/crate_in_workspace/Cargo.toml rename to test_crates/crate_in_workspace/Cargo.toml diff --git a/test_rustdoc/crate_in_workspace/crate1/Cargo.toml b/test_crates/crate_in_workspace/crate1/Cargo.toml similarity index 100% rename from test_rustdoc/crate_in_workspace/crate1/Cargo.toml rename to test_crates/crate_in_workspace/crate1/Cargo.toml diff --git a/test_rustdoc/crate_in_workspace/crate1/src/lib.rs b/test_crates/crate_in_workspace/crate1/src/lib.rs similarity index 100% rename from test_rustdoc/crate_in_workspace/crate1/src/lib.rs rename to test_crates/crate_in_workspace/crate1/src/lib.rs diff --git a/test_rustdoc/implicit_lib/Cargo.toml b/test_crates/lib-target-with-dashes/Cargo.toml similarity index 76% rename from test_rustdoc/implicit_lib/Cargo.toml rename to test_crates/lib-target-with-dashes/Cargo.toml index bfa533f0..10e30a3f 100644 --- a/test_rustdoc/implicit_lib/Cargo.toml +++ b/test_crates/lib-target-with-dashes/Cargo.toml @@ -1,5 +1,6 @@ [package] -name = "implicit_lib" +publish = false +name = "lib-target-with-dashes" version = "0.1.0" edition = "2021" diff --git a/test_crates/lib-target-with-dashes/src/lib.rs b/test_crates/lib-target-with-dashes/src/lib.rs new file mode 100644 index 00000000..e69de29b diff --git a/test_rustdoc/implicit_bin/Cargo.toml b/test_crates/proc_macro_crate/Cargo.toml similarity index 75% rename from test_rustdoc/implicit_bin/Cargo.toml rename to test_crates/proc_macro_crate/Cargo.toml index cf0a86d1..f2f792df 100644 --- a/test_rustdoc/implicit_bin/Cargo.toml +++ b/test_crates/proc_macro_crate/Cargo.toml @@ -1,8 +1,11 @@ [package] -name = "implicit_bin" +name = "proc_macro_crate" version = "0.1.0" edition = "2021" # See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html +[lib] +proc-macro = true + [dependencies] diff --git a/test_crates/proc_macro_crate/src/lib.rs b/test_crates/proc_macro_crate/src/lib.rs new file mode 100644 index 00000000..6480304c --- /dev/null +++ b/test_crates/proc_macro_crate/src/lib.rs @@ -0,0 +1,7 @@ +extern crate proc_macro; +use proc_macro::TokenStream; + +#[proc_macro] +pub fn identity(item: TokenStream) -> TokenStream { + item +} diff --git a/test_rustdoc/renamed_lib/Cargo.toml b/test_crates/renamed_lib_target/Cargo.toml similarity index 80% rename from test_rustdoc/renamed_lib/Cargo.toml rename to test_crates/renamed_lib_target/Cargo.toml index 03a4bd02..a288a37d 100644 --- a/test_rustdoc/renamed_lib/Cargo.toml +++ b/test_crates/renamed_lib_target/Cargo.toml @@ -1,11 +1,12 @@ [package] -name = "renamed_lib" +publish = false +name = "renamed_lib_target" version = "0.1.0" edition = "2021" # See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html +[dependencies] + [lib] name = "librenamed" - -[dependencies] diff --git a/test_rustdoc/implicit_lib/src/lib.rs b/test_crates/renamed_lib_target/src/lib.rs similarity index 100% rename from test_rustdoc/implicit_lib/src/lib.rs rename to test_crates/renamed_lib_target/src/lib.rs diff --git a/test_crates/target-specific-dependencies/Cargo.toml b/test_crates/target_specific_dependencies/Cargo.toml similarity index 78% rename from test_crates/target-specific-dependencies/Cargo.toml rename to test_crates/target_specific_dependencies/Cargo.toml index 8b1c7dc0..56bfdf1d 100644 --- a/test_crates/target-specific-dependencies/Cargo.toml +++ b/test_crates/target_specific_dependencies/Cargo.toml @@ -1,6 +1,6 @@ [package] publish = false -name = "target-specific-dependencies" +name = "target_specific_dependencies" version = "0.1.0" edition = "2021" diff --git a/test_crates/target_specific_dependencies/src/lib.rs b/test_crates/target_specific_dependencies/src/lib.rs new file mode 100644 index 00000000..e69de29b diff --git a/test_crates/target-specific-dependencies/src/main.rs b/test_crates/target_specific_dependencies/src/main.rs similarity index 100% rename from test_crates/target-specific-dependencies/src/main.rs rename to test_crates/target_specific_dependencies/src/main.rs diff --git a/test_rustdoc/crate_in_workspace/Cargo.lock b/test_rustdoc/crate_in_workspace/Cargo.lock deleted file mode 100644 index 2318e853..00000000 --- a/test_rustdoc/crate_in_workspace/Cargo.lock +++ /dev/null @@ -1,7 +0,0 @@ -# This file is automatically @generated by Cargo. -# It is not intended for manual editing. -version = 3 - -[[package]] -name = "crate_in_workspace_crate1" -version = "0.1.0" diff --git a/test_rustdoc/implicit_bin/Cargo.lock b/test_rustdoc/implicit_bin/Cargo.lock deleted file mode 100644 index 251ee569..00000000 --- a/test_rustdoc/implicit_bin/Cargo.lock +++ /dev/null @@ -1,7 +0,0 @@ -# This file is automatically @generated by Cargo. -# It is not intended for manual editing. -version = 3 - -[[package]] -name = "implicit_bin" -version = "0.1.0" diff --git a/test_rustdoc/implicit_bin/src/main.rs b/test_rustdoc/implicit_bin/src/main.rs deleted file mode 100644 index e7a11a96..00000000 --- a/test_rustdoc/implicit_bin/src/main.rs +++ /dev/null @@ -1,3 +0,0 @@ -fn main() { - println!("Hello, world!"); -} diff --git a/test_rustdoc/implicit_lib/Cargo.lock b/test_rustdoc/implicit_lib/Cargo.lock deleted file mode 100644 index 2d7a921a..00000000 --- a/test_rustdoc/implicit_lib/Cargo.lock +++ /dev/null @@ -1,7 +0,0 @@ -# This file is automatically @generated by Cargo. -# It is not intended for manual editing. -version = 3 - -[[package]] -name = "implicit_lib" -version = "0.1.0" diff --git a/test_rustdoc/renamed_bin/Cargo.lock b/test_rustdoc/renamed_bin/Cargo.lock deleted file mode 100644 index bd049f8b..00000000 --- a/test_rustdoc/renamed_bin/Cargo.lock +++ /dev/null @@ -1,7 +0,0 @@ -# This file is automatically @generated by Cargo. -# It is not intended for manual editing. -version = 3 - -[[package]] -name = "renamed_bin" -version = "0.1.0" diff --git a/test_rustdoc/renamed_bin/Cargo.toml b/test_rustdoc/renamed_bin/Cargo.toml deleted file mode 100644 index f2960074..00000000 --- a/test_rustdoc/renamed_bin/Cargo.toml +++ /dev/null @@ -1,12 +0,0 @@ -[package] -name = "renamed_bin" -version = "0.1.0" -edition = "2021" - -# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html - -[[bin]] -name = "renamed" -path = "src/main.rs" - -[dependencies] diff --git a/test_rustdoc/renamed_bin/src/main.rs b/test_rustdoc/renamed_bin/src/main.rs deleted file mode 100644 index e7a11a96..00000000 --- a/test_rustdoc/renamed_bin/src/main.rs +++ /dev/null @@ -1,3 +0,0 @@ -fn main() { - println!("Hello, world!"); -} diff --git a/test_rustdoc/renamed_lib/Cargo.lock b/test_rustdoc/renamed_lib/Cargo.lock deleted file mode 100644 index ae072b59..00000000 --- a/test_rustdoc/renamed_lib/Cargo.lock +++ /dev/null @@ -1,7 +0,0 @@ -# This file is automatically @generated by Cargo. -# It is not intended for manual editing. -version = 3 - -[[package]] -name = "renamed_lib" -version = "0.1.0" diff --git a/test_rustdoc/renamed_lib/src/lib.rs b/test_rustdoc/renamed_lib/src/lib.rs deleted file mode 100644 index 7d12d9af..00000000 --- a/test_rustdoc/renamed_lib/src/lib.rs +++ /dev/null @@ -1,14 +0,0 @@ -pub fn add(left: usize, right: usize) -> usize { - left + right -} - -#[cfg(test)] -mod tests { - use super::*; - - #[test] - fn it_works() { - let result = add(2, 2); - assert_eq!(result, 4); - } -} diff --git a/tests/detects_target_dependencies.rs b/tests/detects_target_dependencies.rs deleted file mode 100644 index 22165a82..00000000 --- a/tests/detects_target_dependencies.rs +++ /dev/null @@ -1,13 +0,0 @@ -use assert_cmd::Command; - -#[test] -fn detects_target_dependencies() { - // This test checks whether the tool correctly detects - // implicit features defined by target-specific dependencies. - // https://github.com/obi1kenobi/cargo-semver-checks/issues/369 - let mut cmd = Command::cargo_bin("cargo-semver-checks").unwrap(); - cmd.current_dir("test_crates/target-specific-dependencies") - .args(["semver-checks", "check-release", "--baseline-root=."]) - .assert() - .success(); -} diff --git a/tests/rustdoc_edge_cases.rs b/tests/rustdoc_edge_cases.rs new file mode 100644 index 00000000..b3837157 --- /dev/null +++ b/tests/rustdoc_edge_cases.rs @@ -0,0 +1,77 @@ +use assert_cmd::Command; + +/// This test checks whether the tool correctly detects +/// implicit features defined by target-specific dependencies. +/// https://github.com/obi1kenobi/cargo-semver-checks/issues/369 +#[test] +fn detects_target_dependencies() { + let mut cmd = Command::cargo_bin("cargo-semver-checks").unwrap(); + cmd.current_dir("test_crates/target_specific_dependencies") + .args(["semver-checks", "check-release", "--baseline-root=."]) + .assert() + .success(); +} + +/// Ensure that crate and lib target names with dashes work properly. +/// Rustdoc uses the lib target name with dashes replaced with underscores as the JSON file name. +/// https://github.com/obi1kenobi/cargo-semver-checks/issues/432 +#[test] +fn lib_target_with_dashes() { + let mut cmd = Command::cargo_bin("cargo-semver-checks").unwrap(); + cmd.current_dir("test_crates/lib-target-with-dashes") + .args(["semver-checks", "check-release", "--baseline-root=."]) + .assert() + .success(); +} + +/// Ensure that proc macro crates can be semver-checked correctly. +#[test] +fn proc_macro_target() { + let mut cmd = Command::cargo_bin("cargo-semver-checks").unwrap(); + cmd.current_dir("test_crates/proc_macro_crate") + .args(["semver-checks", "check-release", "--baseline-root=."]) + .assert() + .success(); +} + +/// Ensure that crates whose lib targets have a different name can be semver-checked correctly. +/// Rustdoc uses the lib target name with dashes replaced with underscores as the JSON file name. +/// https://github.com/obi1kenobi/cargo-semver-checks/issues/432 +#[test] +fn renamed_lib_target() { + let mut cmd = Command::cargo_bin("cargo-semver-checks").unwrap(); + cmd.current_dir("test_crates/renamed_lib_target") + .args(["semver-checks", "check-release", "--baseline-root=."]) + .assert() + .success(); +} + +/// Ensure that semver-checking a crate that uses workspace-provided values works fine. +#[test] +fn crate_in_workspace() { + // Run at workspace level. + let mut cmd = Command::cargo_bin("cargo-semver-checks").unwrap(); + cmd.current_dir("test_crates/crate_in_workspace") + .args([ + "semver-checks", + "check-release", + "--manifest-path=./Cargo.toml", + "--baseline-root=.", + ]) + .assert() + .success(); + + // Run at workspace level but point out the crate explicitly. + let mut cmd = Command::cargo_bin("cargo-semver-checks").unwrap(); + cmd.current_dir("test_crates/crate_in_workspace") + .args([ + "semver-checks", + "check-release", + "--manifest-path=./Cargo.toml", + "-p", + "crate1", + "--baseline-root=.", + ]) + .assert() + .success(); +}