Skip to content

Commit

Permalink
[x] exclude from workspace-hack dependencies
Browse files Browse the repository at this point in the history
It appears that there is no caching advantage to having the x packages
depend on the workspace-hack (because the outer and inner builds have
different environments), and there's an initial performance
disadvantage.
  • Loading branch information
sunshowers authored and bors-libra committed Feb 4, 2021
1 parent ec2dbe0 commit 54388b8
Show file tree
Hide file tree
Showing 9 changed files with 101 additions and 72 deletions.
6 changes: 0 additions & 6 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

20 changes: 4 additions & 16 deletions common/workspace-hack/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ edition = "2018"

### BEGIN HAKARI SECTION
[target.x86_64-unknown-linux-gnu.dependencies]
bstr = { version = "0.2.14", features = ["default", "lazy_static", "regex-automata", "serde", "serde1", "serde1-nostd", "std", "unicode"] }
byteorder = { version = "1.4.2", features = ["default", "i128", "std"] }
bytes = { version = "1.0.1", features = ["default", "serde", "std"] }
chrono = { version = "0.4.19", features = ["clock", "default", "libc", "oldtime", "serde", "std", "time", "winapi"] }
Expand Down Expand Up @@ -39,12 +38,10 @@ once_cell = { version = "1.5.2", features = ["alloc", "default", "std"] }
petgraph = { version = "0.5.1", features = ["default", "graphmap", "matrix_graph", "stable_graph"] }
prost = { version = "0.7.0", features = ["default", "prost-derive", "std"] }
rand = { version = "0.7.3", features = ["alloc", "default", "getrandom", "getrandom_package", "libc", "rand_pcg", "small_rng", "std"] }
regex = { version = "1.4.3", features = ["aho-corasick", "default", "memchr", "perf", "perf-cache", "perf-dfa", "perf-inline", "perf-literal", "std", "thread_local", "unicode", "unicode-age", "unicode-bool", "unicode-case", "unicode-gencat", "unicode-perl", "unicode-script", "unicode-segment"] }
regex-syntax = { version = "0.6.22", features = ["default", "unicode", "unicode-age", "unicode-bool", "unicode-case", "unicode-gencat", "unicode-perl", "unicode-script", "unicode-segment"] }
reqwest = { version = "0.11.0", features = ["__tls", "async-compression", "blocking", "default", "default-tls", "gzip", "hyper-tls", "json", "native-tls", "native-tls-crate", "serde_json", "stream", "tokio-native-tls", "tokio-util"] }
rusty-fork = { version = "0.3.0", features = ["default", "timeout", "wait-timeout"] }
serde = { version = "1.0.123", features = ["alloc", "default", "derive", "rc", "serde_derive", "std"] }
serde_json = { version = "1.0.61", features = ["default", "indexmap", "preserve_order", "std", "unbounded_depth"] }
serde_json = { version = "1.0.61", features = ["default", "indexmap", "preserve_order", "std"] }
standback = { version = "0.2.14", default-features = false, features = ["std"] }
subtle = { version = "2.4.0", default-features = false, features = ["std"] }
tokio = { version = "1.1.1", features = ["bytes", "default", "fs", "full", "io-std", "io-util", "libc", "macros", "memchr", "mio", "net", "num_cpus", "once_cell", "parking_lot", "process", "rt", "rt-multi-thread", "signal", "signal-hook-registry", "sync", "test-util", "time", "tokio-macros", "winapi"] }
Expand All @@ -54,7 +51,6 @@ warp = { version = "0.3.0", features = ["default", "multipart", "tls", "tokio-ru
zeroize = { version = "1.2.0", features = ["alloc", "default", "zeroize_derive"] }

[target.x86_64-unknown-linux-gnu.build-dependencies]
bstr = { version = "0.2.14", features = ["default", "lazy_static", "regex-automata", "serde", "serde1", "serde1-nostd", "std", "unicode"] }
byteorder = { version = "1.4.2", features = ["default", "i128", "std"] }
bytes = { version = "1.0.1", features = ["default", "serde", "std"] }
cc = { version = "1.0.66", default-features = false, features = ["jobserver", "parallel"] }
Expand Down Expand Up @@ -85,12 +81,10 @@ proc-macro2 = { version = "0.4.30", features = ["default", "proc-macro"] }
prost = { version = "0.7.0", features = ["default", "prost-derive", "std"] }
quote = { version = "0.6.13", features = ["default", "proc-macro"] }
rand = { version = "0.7.3", features = ["alloc", "default", "getrandom", "getrandom_package", "libc", "rand_pcg", "small_rng", "std"] }
regex = { version = "1.4.3", features = ["aho-corasick", "default", "memchr", "perf", "perf-cache", "perf-dfa", "perf-inline", "perf-literal", "std", "thread_local", "unicode", "unicode-age", "unicode-bool", "unicode-case", "unicode-gencat", "unicode-perl", "unicode-script", "unicode-segment"] }
regex-syntax = { version = "0.6.22", features = ["default", "unicode", "unicode-age", "unicode-bool", "unicode-case", "unicode-gencat", "unicode-perl", "unicode-script", "unicode-segment"] }
reqwest = { version = "0.11.0", features = ["__tls", "async-compression", "blocking", "default", "default-tls", "gzip", "hyper-tls", "json", "native-tls", "native-tls-crate", "serde_json", "stream", "tokio-native-tls", "tokio-util"] }
rusty-fork = { version = "0.3.0", features = ["default", "timeout", "wait-timeout"] }
serde = { version = "1.0.123", features = ["alloc", "default", "derive", "rc", "serde_derive", "std"] }
serde_json = { version = "1.0.61", features = ["default", "indexmap", "preserve_order", "std", "unbounded_depth"] }
serde_json = { version = "1.0.61", features = ["default", "indexmap", "preserve_order", "std"] }
standback = { version = "0.2.14", default-features = false, features = ["std"] }
subtle = { version = "2.4.0", default-features = false, features = ["std"] }
syn-3575ec1268b04181 = { package = "syn", version = "0.15.44", features = ["clone-impls", "default", "derive", "extra-traits", "full", "parsing", "printing", "proc-macro", "quote", "visit"] }
Expand All @@ -102,7 +96,6 @@ warp = { version = "0.3.0", features = ["default", "multipart", "tls", "tokio-ru
zeroize = { version = "1.2.0", features = ["alloc", "default", "zeroize_derive"] }

[target.x86_64-apple-darwin.dependencies]
bstr = { version = "0.2.14", features = ["default", "lazy_static", "regex-automata", "serde", "serde1", "serde1-nostd", "std", "unicode"] }
byteorder = { version = "1.4.2", features = ["default", "i128", "std"] }
bytes = { version = "1.0.1", features = ["default", "serde", "std"] }
chrono = { version = "0.4.19", features = ["clock", "default", "libc", "oldtime", "serde", "std", "time", "winapi"] }
Expand Down Expand Up @@ -130,12 +123,10 @@ once_cell = { version = "1.5.2", features = ["alloc", "default", "std"] }
petgraph = { version = "0.5.1", features = ["default", "graphmap", "matrix_graph", "stable_graph"] }
prost = { version = "0.7.0", features = ["default", "prost-derive", "std"] }
rand = { version = "0.7.3", features = ["alloc", "default", "getrandom", "getrandom_package", "libc", "rand_pcg", "small_rng", "std"] }
regex = { version = "1.4.3", features = ["aho-corasick", "default", "memchr", "perf", "perf-cache", "perf-dfa", "perf-inline", "perf-literal", "std", "thread_local", "unicode", "unicode-age", "unicode-bool", "unicode-case", "unicode-gencat", "unicode-perl", "unicode-script", "unicode-segment"] }
regex-syntax = { version = "0.6.22", features = ["default", "unicode", "unicode-age", "unicode-bool", "unicode-case", "unicode-gencat", "unicode-perl", "unicode-script", "unicode-segment"] }
reqwest = { version = "0.11.0", features = ["__tls", "async-compression", "blocking", "default", "default-tls", "gzip", "hyper-tls", "json", "native-tls", "native-tls-crate", "serde_json", "stream", "tokio-native-tls", "tokio-util"] }
rusty-fork = { version = "0.3.0", features = ["default", "timeout", "wait-timeout"] }
serde = { version = "1.0.123", features = ["alloc", "default", "derive", "rc", "serde_derive", "std"] }
serde_json = { version = "1.0.61", features = ["default", "indexmap", "preserve_order", "std", "unbounded_depth"] }
serde_json = { version = "1.0.61", features = ["default", "indexmap", "preserve_order", "std"] }
standback = { version = "0.2.14", default-features = false, features = ["std"] }
subtle = { version = "2.4.0", default-features = false, features = ["std"] }
tokio = { version = "1.1.1", features = ["bytes", "default", "fs", "full", "io-std", "io-util", "libc", "macros", "memchr", "mio", "net", "num_cpus", "once_cell", "parking_lot", "process", "rt", "rt-multi-thread", "signal", "signal-hook-registry", "sync", "test-util", "time", "tokio-macros", "winapi"] }
Expand All @@ -145,7 +136,6 @@ warp = { version = "0.3.0", features = ["default", "multipart", "tls", "tokio-ru
zeroize = { version = "1.2.0", features = ["alloc", "default", "zeroize_derive"] }

[target.x86_64-apple-darwin.build-dependencies]
bstr = { version = "0.2.14", features = ["default", "lazy_static", "regex-automata", "serde", "serde1", "serde1-nostd", "std", "unicode"] }
byteorder = { version = "1.4.2", features = ["default", "i128", "std"] }
bytes = { version = "1.0.1", features = ["default", "serde", "std"] }
cc = { version = "1.0.66", default-features = false, features = ["jobserver", "parallel"] }
Expand Down Expand Up @@ -176,12 +166,10 @@ proc-macro2 = { version = "0.4.30", features = ["default", "proc-macro"] }
prost = { version = "0.7.0", features = ["default", "prost-derive", "std"] }
quote = { version = "0.6.13", features = ["default", "proc-macro"] }
rand = { version = "0.7.3", features = ["alloc", "default", "getrandom", "getrandom_package", "libc", "rand_pcg", "small_rng", "std"] }
regex = { version = "1.4.3", features = ["aho-corasick", "default", "memchr", "perf", "perf-cache", "perf-dfa", "perf-inline", "perf-literal", "std", "thread_local", "unicode", "unicode-age", "unicode-bool", "unicode-case", "unicode-gencat", "unicode-perl", "unicode-script", "unicode-segment"] }
regex-syntax = { version = "0.6.22", features = ["default", "unicode", "unicode-age", "unicode-bool", "unicode-case", "unicode-gencat", "unicode-perl", "unicode-script", "unicode-segment"] }
reqwest = { version = "0.11.0", features = ["__tls", "async-compression", "blocking", "default", "default-tls", "gzip", "hyper-tls", "json", "native-tls", "native-tls-crate", "serde_json", "stream", "tokio-native-tls", "tokio-util"] }
rusty-fork = { version = "0.3.0", features = ["default", "timeout", "wait-timeout"] }
serde = { version = "1.0.123", features = ["alloc", "default", "derive", "rc", "serde_derive", "std"] }
serde_json = { version = "1.0.61", features = ["default", "indexmap", "preserve_order", "std", "unbounded_depth"] }
serde_json = { version = "1.0.61", features = ["default", "indexmap", "preserve_order", "std"] }
standback = { version = "0.2.14", default-features = false, features = ["std"] }
subtle = { version = "2.4.0", default-features = false, features = ["std"] }
syn-3575ec1268b04181 = { package = "syn", version = "0.15.44", features = ["clone-impls", "default", "derive", "extra-traits", "full", "parsing", "printing", "proc-macro", "quote", "visit"] }
Expand Down
1 change: 0 additions & 1 deletion devtools/x-core/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ guppy = "0.7.0"
indoc = "1.0.3"
hakari = { version = "0.1.0", features = ["summaries"] }
hex = "0.4.2"
diem-workspace-hack = { path = "../../common/workspace-hack", version = "0.1.0" }
log = "0.4.14"
toml = "0.5.8"
once_cell = "1.4.1"
Expand Down
1 change: 0 additions & 1 deletion devtools/x-lint/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ license = "Apache-2.0"
[dependencies]
guppy = "0.7.0"
hakari = "0.1.0"
diem-workspace-hack = { path = "../../common/workspace-hack", version = "0.1.0" }
once_cell = "1.4.1"
toml = "0.5.8"
serde = { version = "1.0.123", features = ["derive"] }
Expand Down
1 change: 0 additions & 1 deletion devtools/x/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -27,4 +27,3 @@ rayon = "1.5.0"
indexmap = "1.6.1"
x-core = { version = "0.1.0", path = "../x-core" }
x-lint = { version = "0.1.0", path = "../x-lint" }
diem-workspace-hack = { path = "../../common/workspace-hack", version = "0.1.0" }
63 changes: 18 additions & 45 deletions devtools/x/src/lint/guppy.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,9 @@
//! Project and package linters that run queries on guppy.
use crate::config::{BannedDepsConfig, EnforcedAttributesConfig, OverlayConfig};
use anyhow::anyhow;
use guppy::{graph::feature::FeatureFilterFn, Version};
use hakari::summaries::HakariBuilderSummary;
use std::{
collections::{BTreeMap, HashMap},
ffi::OsStr,
Expand Down Expand Up @@ -188,47 +190,6 @@ impl PackageLinter for CrateNamesPaths {
}
}

/// Ensure diem-workspace-hack is a dependency
#[derive(Debug)]
pub struct WorkspaceHack;

impl Linter for WorkspaceHack {
fn name(&self) -> &'static str {
"workspace-hack"
}
}

impl PackageLinter for WorkspaceHack {
fn run<'l>(
&self,
ctx: &PackageContext<'l>,
out: &mut LintFormatter<'l, '_>,
) -> Result<RunStatus<'l>> {
let package = ctx.metadata();
let pkg_graph = ctx.package_graph();
let workspace_hack_id = pkg_graph
.workspace()
.member_by_name("diem-workspace-hack")
.expect("can't find diem-workspace-hack package")
.id();

// diem-workspace-hack does not need to depend on itself
if package.id() == workspace_hack_id {
return Ok(RunStatus::Executed);
}

let has_links = package.direct_links().next().is_some();
let has_hack_dep = pkg_graph
.directly_depends_on(package.id(), workspace_hack_id)
.expect("valid package ID");
if has_links && !has_hack_dep {
out.write(LintLevel::Error, "missing diem-workspace-hack dependency");
}

Ok(RunStatus::Executed)
}
}

/// Ensure that any workspace packages with build dependencies also have a build script.
#[derive(Debug)]
pub struct IrrelevantBuildDeps;
Expand Down Expand Up @@ -261,15 +222,27 @@ impl PackageLinter for IrrelevantBuildDeps {

/// Ensure that packages within the workspace only depend on one version of a third-party crate.
#[derive(Debug)]
pub struct DirectDepDups;
pub struct DirectDepDups<'cfg> {
hakari_package: &'cfg str,
}

impl<'cfg> DirectDepDups<'cfg> {
pub fn new(hakari_config: &'cfg HakariBuilderSummary) -> crate::Result<Self> {
let hakari_package = hakari_config
.hakari_package
.as_deref()
.ok_or_else(|| anyhow!("hakari.hakari-package not defined in x.toml"))?;
Ok(Self { hakari_package })
}
}

impl Linter for DirectDepDups {
impl<'cfg> Linter for DirectDepDups<'cfg> {
fn name(&self) -> &'static str {
"direct-dep-dups"
}
}

impl ProjectLinter for DirectDepDups {
impl<'cfg> ProjectLinter for DirectDepDups<'cfg> {
fn run<'l>(
&self,
ctx: &ProjectContext<'l>,
Expand All @@ -282,7 +255,7 @@ impl ProjectLinter for DirectDepDups {
package_graph.query_workspace().resolve_with_fn(|_, link| {
// Collect direct dependencies of workspace packages.
let (from, to) = link.endpoints();
if ctx.core().config().hakari.hakari_package.as_deref() == Some(from.name()) {
if from.name() == self.hakari_package {
// Skip the workspace hack package.
return false;
}
Expand Down
5 changes: 3 additions & 2 deletions devtools/x/src/lint/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,11 +22,12 @@ pub struct Args {
}

pub fn run(args: Args, xctx: XContext) -> crate::Result<()> {
let core_config = xctx.core().config();
let workspace_config = xctx.config().workspace_config();

let project_linters: &[&dyn ProjectLinter] = &[
&guppy::BannedDeps::new(&workspace_config.banned_deps),
&guppy::DirectDepDups,
&guppy::DirectDepDups::new(&core_config.hakari)?,
&workspace_hack::GenerateWorkspaceHack,
];

Expand All @@ -35,11 +36,11 @@ pub fn run(args: Args, xctx: XContext) -> crate::Result<()> {
&guppy::CrateNamesPaths,
&guppy::IrrelevantBuildDeps,
&guppy::OverlayFeatures::new(&workspace_config.overlay),
&guppy::WorkspaceHack,
&workspace_classify::DefaultOrTestOnly::new(
xctx.core().package_graph()?,
&workspace_config.test_only,
)?,
&workspace_hack::WorkspaceHackDep::new(xctx.core())?,
];

let file_path_linters: &[&dyn FilePathLinter] = &[
Expand Down
60 changes: 60 additions & 0 deletions devtools/x/src/lint/workspace_hack.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
// Copyright (c) The Diem Core Contributors
// SPDX-License-Identifier: Apache-2.0

use anyhow::anyhow;
use guppy::PackageId;
use hakari::HakariBuilder;
use x_core::XCoreContext;
use x_lint::prelude::*;

/// Check that the workspace hack is up-to-date.
Expand Down Expand Up @@ -60,3 +64,59 @@ impl ProjectLinter for GenerateWorkspaceHack {
Ok(RunStatus::Executed)
}
}

/// Ensure the workspace-hack package is a dependency
#[derive(Debug)]
pub struct WorkspaceHackDep<'cfg> {
hakari_builder: HakariBuilder<'cfg, 'static>,
hakari_id: &'cfg PackageId,
}

impl<'cfg> WorkspaceHackDep<'cfg> {
pub fn new(ctx: &'cfg XCoreContext) -> crate::Result<Self> {
let hakari_builder = ctx.hakari_builder()?;
let hakari_id = hakari_builder
.hakari_package()
.map(|package| package.id())
.ok_or_else(|| anyhow!("hakari.hakari-package not specified in x.toml"))?;
Ok(Self {
hakari_builder,
hakari_id,
})
}
}
impl<'cfg> Linter for WorkspaceHackDep<'cfg> {
fn name(&self) -> &'static str {
"workspace-hack-dep"
}
}

impl<'cfg> PackageLinter for WorkspaceHackDep<'cfg> {
fn run<'l>(
&self,
ctx: &PackageContext<'l>,
out: &mut LintFormatter<'l, '_>,
) -> Result<RunStatus<'l>> {
let package = ctx.metadata();
let pkg_graph = ctx.package_graph();

// Exclude omitted packages (including the workspace-hack package itself) from consideration.
if self
.hakari_builder
.omits_package(package.id())
.expect("valid package ID")
{
return Ok(RunStatus::Executed);
}

let has_links = package.direct_links().next().is_some();
let has_hack_dep = pkg_graph
.directly_depends_on(package.id(), self.hakari_id)
.expect("valid package ID");
if has_links && !has_hack_dep {
out.write(LintLevel::Error, "missing diem-workspace-hack dependency");
}

Ok(RunStatus::Executed)
}
}
16 changes: 16 additions & 0 deletions x.toml
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,22 @@ name = "diem-crypto-derive"
version = "0.1.0"
workspace-path = "crypto/crypto-derive"

# Also exclude the devtools packages since they get compiled with a different set of options.
[[hakari.omitted-packages]]
name = "x"
version = "0.1.0"
workspace-path = "devtools/x"

[[hakari.omitted-packages]]
name = "x-core"
version = "0.1.0"
workspace-path = "devtools/x-core"

[[hakari.omitted-packages]]
name = "x-lint"
version = "0.1.0"
workspace-path = "devtools/x-lint"

# This follows the same syntax as CargoOptionsSummary in guppy.
[summaries.default]
version = "v2"
Expand Down

0 comments on commit 54388b8

Please sign in to comment.