Skip to content

Commit

Permalink
correctly handle cargo +toolchain public-api
Browse files Browse the repository at this point in the history
 also deprecate `--toolchain` specifier and `BuildOptions::toolchain`
  • Loading branch information
Emilgardis committed Aug 25, 2022
1 parent a2027c7 commit 3d31ee5
Show file tree
Hide file tree
Showing 7 changed files with 31 additions and 15 deletions.
3 changes: 3 additions & 0 deletions .github/workflows/CI.yml
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ jobs:
with:
toolchain: nightly
profile: minimal
override: true
- run: cargo test --locked

check-public-apis:
Expand All @@ -59,6 +60,7 @@ jobs:
with:
toolchain: nightly
profile: minimal
override: true
- run: |
scripts/check-public-apis.sh
Expand All @@ -71,4 +73,5 @@ jobs:
with:
toolchain: nightly
profile: minimal
override: true
- run: scripts/test-invocation-variants.sh
17 changes: 12 additions & 5 deletions cargo-public-api/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -108,11 +108,16 @@ pub struct Args {
#[clap(long, hide = true)]
verbose: bool,

/// [DEPRECATED]
///
/// Use `cargo +toolchain` instead.
///
/// Allows you to build rustdoc JSON with a toolchain other than `+nightly`.
/// Useful if you have built a toolchain from source for example, or if you
/// want to use a fixed toolchain in CI.
#[clap(long, default_value = "+nightly")]
toolchain: String,
#[clap(long)]
#[deprecated(note = "use `RUSTUP_TOOLCHAIN` or `cargo +toolchain` instead")]
toolchain: Option<String>,

/// Build for the target triple
#[clap(long)]
Expand Down Expand Up @@ -295,10 +300,12 @@ fn collect_public_api_from_commit(commit: Option<&str>) -> Result<(PublicApi, Op
} else {
None
};
let mut build_options = BuildOptions::default().manifest_path(&args.manifest_path);

let mut build_options = BuildOptions::default()
.toolchain(&args.toolchain)
.manifest_path(&args.manifest_path);
#[allow(deprecated)]
if let Some(toolchain) = &args.toolchain {
build_options = build_options.toolchain(toolchain);
}
if let Some(target) = &args.target {
build_options = build_options.target(target.clone());
}
Expand Down
1 change: 0 additions & 1 deletion public-api/tests/utils/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ pub fn rustdoc_json_path_for_crate(test_crate: &str) -> PathBuf {
// so build quietly to make running tests much less noisy
rustdoc_json::build(
BuildOptions::default()
.toolchain("+nightly")
.manifest_path(&format!("{}/Cargo.toml", test_crate))
.quiet(true),
)
Expand Down
4 changes: 4 additions & 0 deletions rustdoc-json/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
## Unreleased

* Deprecate `BuildOptions::toolchain` in favour of using `RUSTUP_TOOLCHAIN`

## v0.3.0
* Change `fn build()` to take `BuildOptions`

Expand Down
4 changes: 1 addition & 3 deletions rustdoc-json/examples/build-rustdoc-json.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,7 @@ fn main() -> Result<(), Box<dyn std::error::Error>> {

// Build it
let json_path = rustdoc_json::build(
BuildOptions::default()
.toolchain("+nightly")
.manifest_path(&std::env::args().nth(1).unwrap()),
BuildOptions::default().manifest_path(&std::env::args().nth(1).unwrap()),
)?;
println!("Built and wrote rustdoc JSON to {:?}", &json_path);

Expand Down
13 changes: 8 additions & 5 deletions rustdoc-json/src/build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ const OVERRIDDEN_TOOLCHAIN: Option<&str> = option_env!("RUSTDOC_JSON_OVERRIDDEN_
/// file.
pub(crate) fn run_cargo_rustdoc(options: BuildOptions) -> Result<PathBuf, BuildError> {
let status = cargo_rustdoc_command(
#[allow(deprecated)]
options.toolchain.as_deref(),
&options.manifest_path,
options.target.as_deref(),
Expand All @@ -26,7 +27,7 @@ pub(crate) fn run_cargo_rustdoc(options: BuildOptions) -> Result<PathBuf, BuildE
rustdoc_json_path_for_manifest_path(options.manifest_path, options.target.as_deref())
} else {
let manifest = cargo_toml::Manifest::from_path(&options.manifest_path)?;
if manifest.workspace.is_some() {
if manifest.package.is_none() && manifest.workspace.is_some() {
Err(BuildError::VirtualManifest(options.manifest_path))
} else {
Err(BuildError::General(String::from("See above")))
Expand All @@ -47,13 +48,12 @@ fn cargo_rustdoc_command(
) -> Command {
let mut command = Command::new("cargo");

// These can override our `+nightly` with `+stable` unless we clear them
command.env_remove("RUSTDOC");
command.env_remove("RUSTC");

let overridden_toolchain = OVERRIDDEN_TOOLCHAIN.map(OsStr::new);
if let Some(toolchain) = overridden_toolchain.or(requested_toolchain) {
command.arg(toolchain);
// These can override our `+nightly` with `+stable` unless we clear them
command.env_remove("RUSTC");
command.env_remove("RUSTDOC");
}

command.arg("rustdoc");
Expand Down Expand Up @@ -115,6 +115,7 @@ fn package_name(manifest_path: impl AsRef<Path>) -> Result<String, BuildError> {

impl Default for BuildOptions {
fn default() -> Self {
#[allow(deprecated)]
Self {
toolchain: None,
manifest_path: PathBuf::from("Cargo.toml"),
Expand All @@ -129,6 +130,8 @@ impl BuildOptions {
/// Until rustdoc JSON has stabilized, you will want to set this to
/// `"+nightly"` or similar.
#[must_use]
#[deprecated(note = "use `RUSTUP_TOOLCHAIN` or `cargo +toolchain` instead")]
#[allow(deprecated)]
pub fn toolchain(mut self, toolchain: impl AsRef<OsStr>) -> Self {
self.toolchain = Some(toolchain.as_ref().to_owned());
self
Expand Down
4 changes: 3 additions & 1 deletion rustdoc-json/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,10 @@
//! ```
//! use rustdoc_json::BuildOptions;
//!
//! std::env::set_var("RUSTUP_TOOLCHAIN", "nightly");
//!
//! let json_path = rustdoc_json::build(
//! BuildOptions::default()
//! .toolchain("+nightly")
//! .manifest_path("Cargo.toml"),
//! ).unwrap();
//!
Expand Down Expand Up @@ -52,6 +53,7 @@ pub enum BuildError {
/// See [crate] for an example on how to use it.
#[derive(Debug)]
pub struct BuildOptions {
#[deprecated(note = "use `RUSTUP_TOOLCHAIN` or `cargo +toolchain` instead")]
toolchain: Option<OsString>,
manifest_path: std::path::PathBuf,
target: Option<String>,
Expand Down

0 comments on commit 3d31ee5

Please sign in to comment.