Skip to content

Commit

Permalink
[x] move more code over to camino paths
Browse files Browse the repository at this point in the history
Simplifies this code quite a bit, and we're never going to support
non-UTF-8 paths anyway.
  • Loading branch information
sunshowers authored and bors-libra committed Sep 14, 2021
1 parent 153c9dc commit 4f68c85
Show file tree
Hide file tree
Showing 9 changed files with 57 additions and 71 deletions.
17 changes: 5 additions & 12 deletions devtools/x-core/src/errors.rs
Original file line number Diff line number Diff line change
@@ -1,17 +1,11 @@
// Copyright (c) The Diem Core Contributors
// SPDX-License-Identifier: Apache-2.0

use camino::{Utf8Path, Utf8PathBuf};
use guppy::TargetSpecError;
use hex::FromHexError;
use serde::{de, ser};
use std::{
borrow::Cow,
error, fmt, io,
path::{Path, PathBuf},
process::ExitStatus,
result,
str::Utf8Error,
};
use std::{borrow::Cow, error, fmt, io, process::ExitStatus, result, str::Utf8Error};

/// Type alias for the return type for `run` methods.
pub type Result<T, E = SystemError> = result::Result<T, E>;
Expand All @@ -21,8 +15,8 @@ pub type Result<T, E = SystemError> = result::Result<T, E>;
#[non_exhaustive]
pub enum SystemError {
CwdNotInProjectRoot {
current_dir: PathBuf,
project_root: &'static Path,
current_dir: Utf8PathBuf,
project_root: &'static Utf8Path,
},
Exec {
cmd: &'static str,
Expand Down Expand Up @@ -146,8 +140,7 @@ impl fmt::Display for SystemError {
} => write!(
f,
"current dir {} not in project root {}",
current_dir.display(),
project_root.display(),
current_dir, project_root,
),
SystemError::Exec { cmd, status } => match status.code() {
Some(code) => write!(f, "'{}' failed with exit code {}", cmd, code),
Expand Down
23 changes: 10 additions & 13 deletions devtools/x-core/src/git.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
// SPDX-License-Identifier: Apache-2.0

use crate::errors::*;
use camino::{Utf8Path, Utf8PathBuf};
use determinator::Utf8Paths0;
use guppy::{graph::PackageGraph, MetadataCommand};
use indoc::formatdoc;
Expand All @@ -11,7 +12,6 @@ use std::{
borrow::Cow,
ffi::{OsStr, OsString},
fmt,
path::{Path, PathBuf},
process::{Command, Stdio},
};

Expand All @@ -22,14 +22,14 @@ use std::{
/// invalidated.
#[derive(Clone, Debug)]
pub struct GitCli {
root: &'static Path,
root: &'static Utf8Path,
// Caches.
tracked_files: OnceCell<Utf8Paths0>,
}

impl GitCli {
/// Creates a new instance of the Git CLI.
pub fn new(root: &'static Path) -> Result<Self> {
pub fn new(root: &'static Utf8Path) -> Result<Self> {
let git_cli = Self {
root,
tracked_files: OnceCell::new(),
Expand Down Expand Up @@ -146,7 +146,7 @@ impl GitCli {
let msg = formatdoc!(
"unable to find a git repo at {}
(hint: did you download an archive from GitHub? x requires a git clone)",
self.root.display()
self.root
);
return Err(SystemError::git_root(msg));
}
Expand All @@ -162,11 +162,11 @@ impl GitCli {
));
}
};
if self.root != Path::new(&git_root) {
if self.root != &git_root {
let msg = formatdoc!(
"git root expected to be at {}, but actually found at {}
(hint: did you download an archive from GitHub? x requires a git clone)",
self.root.display(),
self.root,
git_root,
);
return Err(SystemError::git_root(msg));
Expand All @@ -186,15 +186,12 @@ impl GitCli {
///
/// The scratch worktree is meant to be persistent across invocations of `x`. This is done for
/// performance reasons.
fn get_or_init_scratch(&self, hash: &GitHash) -> Result<PathBuf> {
fn get_or_init_scratch(&self, hash: &GitHash) -> Result<Utf8PathBuf> {
let mut scratch_dir = self.root.join("target");
scratch_dir.extend(&["x-scratch", "tree"]);

if scratch_dir.is_dir() && self.is_git_repo(&scratch_dir)? {
debug!(
"Using existing scratch worktree at {}",
scratch_dir.display()
);
debug!("Using existing scratch worktree at {}", scratch_dir,);

// Check out the given hash in the scratch worktree.
let output = self
Expand All @@ -217,7 +214,7 @@ impl GitCli {
}

// Try creating a scratch worktree at that location.
info!("Setting up scratch worktree in {}", scratch_dir.display());
info!("Setting up scratch worktree in {}", scratch_dir);
let output = self
.git_command()
.args(&["worktree", "add"])
Expand All @@ -238,7 +235,7 @@ impl GitCli {
Ok(scratch_dir)
}

pub fn is_git_repo(&self, dir: &Path) -> Result<bool> {
pub fn is_git_repo(&self, dir: &Utf8Path) -> Result<bool> {
let output = self
.git_command()
.current_dir(dir)
Expand Down
26 changes: 13 additions & 13 deletions devtools/x-core/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,9 +1,12 @@
// Copyright (c) The Diem Core Contributors
// SPDX-License-Identifier: Apache-2.0

use crate::{core_config::XCoreConfig, git::GitCli};
use camino::{Utf8Path, Utf8PathBuf};
use graph::PackageGraphPlus;
use guppy::graph::PackageGraph;
use hakari::{HakariBuilder, TomlOptions};
use once_cell::sync::OnceCell;
use std::path::{Path, PathBuf};

pub mod core_config;
mod debug_ignore;
Expand All @@ -12,29 +15,26 @@ pub mod git;
mod graph;
mod workspace_subset;

use crate::{core_config::XCoreConfig, git::GitCli};
pub use debug_ignore::*;
pub use errors::*;
use graph::PackageGraphPlus;
use hakari::{HakariBuilder, TomlOptions};
pub use workspace_subset::*;

/// Core context shared across all of x.
#[derive(Debug)]
pub struct XCoreContext {
project_root: &'static Path,
project_root: &'static Utf8Path,
config: XCoreConfig,
current_dir: PathBuf,
current_rel_dir: PathBuf,
current_dir: Utf8PathBuf,
current_rel_dir: Utf8PathBuf,
git_cli: OnceCell<GitCli>,
package_graph_plus: DebugIgnore<OnceCell<PackageGraphPlus>>,
}

impl XCoreContext {
/// Creates a new XCoreContext.
pub fn new(
project_root: &'static Path,
current_dir: PathBuf,
project_root: &'static Utf8Path,
current_dir: Utf8PathBuf,
config: XCoreConfig,
) -> Result<Self> {
let current_rel_dir = match current_dir.strip_prefix(project_root) {
Expand All @@ -59,7 +59,7 @@ impl XCoreContext {
}

/// Returns the project root for this workspace.
pub fn project_root(&self) -> &'static Path {
pub fn project_root(&self) -> &'static Utf8Path {
self.project_root
}

Expand All @@ -69,18 +69,18 @@ impl XCoreContext {
}

/// Returns the current working directory for this process.
pub fn current_dir(&self) -> &Path {
pub fn current_dir(&self) -> &Utf8Path {
&self.current_dir
}

/// Returns the current working directory for this process, relative to the project root.
pub fn current_rel_dir(&self) -> &Path {
pub fn current_rel_dir(&self) -> &Utf8Path {
&self.current_rel_dir
}

/// Returns true if x has been run from the project root.
pub fn current_dir_is_root(&self) -> bool {
self.current_rel_dir == Path::new("")
self.current_rel_dir == ""
}

/// Returns the Git CLI for this workspace.
Expand Down
8 changes: 4 additions & 4 deletions devtools/x-core/src/workspace_subset.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
// SPDX-License-Identifier: Apache-2.0

use crate::{core_config::SubsetConfig, Result, SystemError};
use camino::Utf8PathBuf;
use camino::{Utf8Path, Utf8PathBuf};
use guppy::{
graph::{
cargo::{CargoOptions, CargoResolverVersion, CargoSet},
Expand All @@ -12,7 +12,7 @@ use guppy::{
PackageId,
};
use serde::Deserialize;
use std::{collections::BTreeMap, fs, path::Path};
use std::{collections::BTreeMap, fs};
use toml::de;

/// Contains information about all the subsets specified in this workspace.
Expand All @@ -32,7 +32,7 @@ impl<'g> WorkspaceSubsets<'g> {
/// * no dev dependencies
pub fn new(
graph: &'g PackageGraph,
project_root: &Path,
project_root: &Utf8Path,
config: &BTreeMap<String, SubsetConfig>,
) -> Result<Self> {
let mut cargo_opts = CargoOptions::new();
Expand Down Expand Up @@ -91,7 +91,7 @@ impl<'g> WorkspaceSubsets<'g> {
// Helper methods
// ---

fn read_default_members(project_root: &Path) -> Result<Vec<Utf8PathBuf>> {
fn read_default_members(project_root: &Utf8Path) -> Result<Vec<Utf8PathBuf>> {
#[derive(Deserialize)]
struct RootToml {
workspace: Workspace,
Expand Down
6 changes: 3 additions & 3 deletions devtools/x-lint/src/file_path.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@

use crate::{prelude::*, LintContext};
use camino::Utf8Path;
use std::{fs, io, path::Path};
use std::{fs, io};

/// Represents a linter that runs once per file path.
pub trait FilePathLinter: Linter {
Expand Down Expand Up @@ -54,7 +54,7 @@ impl<'l> FilePathContext<'l> {
pub(super) fn load(self) -> Result<Option<ContentContext<'l>>> {
let full_path = self.project_ctx.full_path(self.file_path);
let contents_opt = read_file(&full_path)
.map_err(|err| SystemError::io(format!("loading {}", full_path.display()), err))?;
.map_err(|err| SystemError::io(format!("loading {}", full_path), err))?;
Ok(contents_opt.map(|content| ContentContext::new(self, content)))
}
}
Expand All @@ -65,7 +65,7 @@ impl<'l> LintContext<'l> for FilePathContext<'l> {
}
}

fn read_file(full_path: &Path) -> io::Result<Option<Vec<u8>>> {
fn read_file(full_path: &Utf8Path) -> io::Result<Option<Vec<u8>>> {
match fs::read(full_path) {
Ok(bytes) => Ok(Some(bytes)),
Err(err) => {
Expand Down
6 changes: 3 additions & 3 deletions devtools/x-lint/src/project.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,9 @@
// SPDX-License-Identifier: Apache-2.0

use crate::{prelude::*, LintContext};
use camino::{Utf8Path, Utf8PathBuf};
use guppy::graph::PackageGraph;
use hakari::Hakari;
use std::path::{Path, PathBuf};
use x_core::{WorkspaceSubset, XCoreContext};

/// Represents a linter that checks some property for the overall project.
Expand Down Expand Up @@ -39,7 +39,7 @@ impl<'l> ProjectContext<'l> {
}

/// Returns the project root.
pub fn project_root(&self) -> &'l Path {
pub fn project_root(&self) -> &'l Utf8Path {
self.core.project_root()
}

Expand All @@ -49,7 +49,7 @@ impl<'l> ProjectContext<'l> {
}

/// Returns the absolute path from the project root.
pub fn full_path(&self, path: impl AsRef<Path>) -> PathBuf {
pub fn full_path(&self, path: impl AsRef<Utf8Path>) -> Utf8PathBuf {
self.core.project_root().join(path.as_ref())
}

Expand Down
8 changes: 6 additions & 2 deletions devtools/x/src/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ use crate::{
Result,
};
use anyhow::Context;
use camino::Utf8PathBuf;
use std::convert::TryInto;
use x_core::XCoreContext;

/// Global context shared across x commands.
Expand All @@ -25,8 +27,10 @@ impl XContext {

/// Creates a new `GlobalContext` based on the given config.
pub fn with_config(x_config: XConfig) -> Result<Self> {
let current_dir =
std::env::current_dir().with_context(|| "error while fetching current dir")?;
let current_dir: Utf8PathBuf = std::env::current_dir()
.with_context(|| "error while fetching current dir")?
.try_into()
.with_context(|| "current dir is not valid UTF-8")?;
let XConfig { core, config } = x_config;
Ok(Self {
core: XCoreContext::new(project_root(), current_dir, core)?,
Expand Down
15 changes: 6 additions & 9 deletions devtools/x/src/generate_summaries.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,12 @@

use crate::context::XContext;
use anyhow::Context;
use camino::{Utf8Path, Utf8PathBuf};
use guppy::graph::{
cargo::CargoOptions,
feature::{FeatureSet, StandardFeatures},
};
use std::{
fs,
path::{Path, PathBuf},
};
use std::fs;
use structopt::{clap::arg_enum, StructOpt};

arg_enum! {
Expand All @@ -25,7 +23,7 @@ arg_enum! {
pub struct Args {
#[structopt(name = "OUT_DIR")]
/// Directory to output summaries to (default: target/summaries)
out_dir: Option<PathBuf>,
out_dir: Option<Utf8PathBuf>,
#[structopt(name = "OUTPUT_FORMAT", default_value = "Toml")]
/// Output in text, toml or json
output_format: OutputFormat,
Expand Down Expand Up @@ -86,7 +84,7 @@ pub fn run(args: Args, xctx: XContext) -> crate::Result<()> {

summary_count += 1;

println!("wrote {} summaries to {}", summary_count, out_dir.display());
println!("wrote {} summaries to {}", summary_count, out_dir);

Ok(())
}
Expand All @@ -95,7 +93,7 @@ fn write_summary(
name: &str,
initials: FeatureSet<'_>,
cargo_opts: &CargoOptions<'_>,
out_dir: &Path,
out_dir: &Utf8Path,
output_format: OutputFormat,
) -> crate::Result<()> {
let build_set = initials.into_cargo_set(cargo_opts)?;
Expand Down Expand Up @@ -123,6 +121,5 @@ fn write_summary(
}
};

fs::write(&path, &out)
.with_context(|| format!("error while writing summary file {}", path.display()))
fs::write(&path, &out).with_context(|| format!("error while writing summary file {}", path))
}
Loading

0 comments on commit 4f68c85

Please sign in to comment.