Skip to content

Commit

Permalink
Auto merge of #7575 - orium:fix-unused-warnings, r=alexcrichton
Browse files Browse the repository at this point in the history
Fix unused configuration key warning for a few keys under `build`.

Recently cargo started to warn about configuration keys that he doesn't know about.  However, there are a few keys under `build` that were used in a dynamic way (`rustc`, `rustdoc`, and `rustc_wrapper`) by `Config::maybe_get_tool()`.

Since these keys are not known to exist when `Config` is deserialized, cargo was emitting unused warnings.

This commit makes those config keys explicit.  Note that by doing so there is a small breaking change: before it was possible to have `build.rustc_wrapper` in the configuration file (even though the documented key uses kebak-case), and now that key will be ignored.  (Good thing we have warnings for unrecognized keys!)
  • Loading branch information
bors committed Nov 11, 2019
2 parents f612284 + e509851 commit 1292915
Show file tree
Hide file tree
Showing 2 changed files with 28 additions and 38 deletions.
2 changes: 1 addition & 1 deletion src/cargo/ops/fix.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
//! to print at the same time).
//!
//! Cargo begins a normal `cargo check` operation with itself set as a proxy
//! for rustc by setting `rustc_wrapper` in the build config. When
//! for rustc by setting `primary_unit_rustc` in the build config. When
//! cargo launches rustc to check a crate, it is actually launching itself.
//! The `FIX_ENV` environment variable is set so that cargo knows it is in
//! fix-proxy-mode.
Expand Down
64 changes: 27 additions & 37 deletions src/cargo/util/config/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -216,7 +216,7 @@ impl Config {
/// Gets the path to the `rustdoc` executable.
pub fn rustdoc(&self) -> CargoResult<&Path> {
self.rustdoc
.try_borrow_with(|| self.get_tool("rustdoc"))
.try_borrow_with(|| Ok(self.get_tool("rustdoc", &self.build_config()?.rustdoc)))
.map(AsRef::as_ref)
}

Expand All @@ -227,9 +227,9 @@ impl Config {
.join(".rustc_info.json")
.into_path_unlocked()
});
let wrapper = self.maybe_get_tool("rustc_wrapper")?;
let wrapper = self.maybe_get_tool("rustc_wrapper", &self.build_config()?.rustc_wrapper);
Rustc::new(
self.get_tool("rustc")?,
self.get_tool("rustc", &self.build_config()?.rustc),
wrapper,
&self
.home()
Expand Down Expand Up @@ -853,47 +853,34 @@ impl Config {
Ok(())
}

/// Looks for a path for `tool` in an environment variable or config path, and returns `None`
/// if it's not present.
fn maybe_get_tool(&self, tool: &str) -> CargoResult<Option<PathBuf>> {
let var = tool
.chars()
.flat_map(|c| c.to_uppercase())
.collect::<String>();
if let Some(tool_path) = env::var_os(&var) {
let maybe_relative = match tool_path.to_str() {
Some(s) => s.contains('/') || s.contains('\\'),
None => false,
};
let path = if maybe_relative {
self.cwd.join(tool_path)
} else {
PathBuf::from(tool_path)
};
return Ok(Some(path));
}

// For backwards compatibility we allow both snake_case config paths as well as the
// idiomatic kebab-case paths.
let config_paths = [
format!("build.{}", tool),
format!("build.{}", tool.replace('_', "-")),
];
/// Looks for a path for `tool` in an environment variable or the given config, and returns
/// `None` if it's not present.
fn maybe_get_tool(&self, tool: &str, from_config: &Option<PathBuf>) -> Option<PathBuf> {
let var = tool.to_uppercase();

for config_path in &config_paths {
if let Some(tool_path) = self.get_path(&config_path)? {
return Ok(Some(tool_path.val));
match env::var_os(&var) {
Some(tool_path) => {
let maybe_relative = match tool_path.to_str() {
Some(s) => s.contains('/') || s.contains('\\'),
None => false,
};
let path = if maybe_relative {
self.cwd.join(tool_path)
} else {
PathBuf::from(tool_path)
};
Some(path)
}
}

Ok(None)
None => from_config.clone(),
}
}

/// Looks for a path for `tool` in an environment variable or config path, defaulting to `tool`
/// as a path.
pub fn get_tool(&self, tool: &str) -> CargoResult<PathBuf> {
self.maybe_get_tool(tool)
.map(|t| t.unwrap_or_else(|| PathBuf::from(tool)))
fn get_tool(&self, tool: &str, from_config: &Option<PathBuf>) -> PathBuf {
self.maybe_get_tool(tool, from_config)
.unwrap_or_else(|| PathBuf::from(tool))
}

pub fn jobserver_from_env(&self) -> Option<&jobserver::Client> {
Expand Down Expand Up @@ -1473,6 +1460,9 @@ pub struct CargoBuildConfig {
pub jobs: Option<u32>,
pub rustflags: Option<StringList>,
pub rustdocflags: Option<StringList>,
pub rustc_wrapper: Option<PathBuf>,
pub rustc: Option<PathBuf>,
pub rustdoc: Option<PathBuf>,
}

/// A type to deserialize a list of strings from a toml file.
Expand Down

0 comments on commit 1292915

Please sign in to comment.