Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: single query for cycle and depth checks #443

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
53 changes: 52 additions & 1 deletion packages/cli/tests/build.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use indoc::indoc;
use insta::assert_snapshot;
use std::future::Future;
use tangram_cli::{assert_failure, assert_success, test::test};
use tangram_cli::{assert_failure, assert_success, test::test, Config};
use tangram_temp::{self as temp, Temp};

const TG: &str = env!("CARGO_BIN_EXE_tangram");
Expand Down Expand Up @@ -694,6 +694,57 @@ async fn value_cycle_detection_array() {
test_build(directory, path, target, args, assertions).await;
}

#[tokio::test]
async fn build_max_depth_detection() {
let directory = temp::directory! {
"foo" => temp::directory! {
"tangram.ts" => indoc!("
export let x = tg.target((depth) => {
depth = depth ?? 0;
return x(depth + 1);
})
"
)
},
};
let path = "foo";
let target = "x";
let args: Vec<String> = vec![];
let assertions = |output: std::process::Output| async move {
assert_failure!(output);
};

let artifact = directory;

test(TG, move |context| async move {
let mut context = context.lock().await;
let mut config = Config::default();
let mut build = tangram_cli::config::Build::default();
build.max_depth = Some(100);
config.build = Some(Some(build));
let server = context.spawn_server_with_config(config).await.unwrap();

let artifact: temp::Artifact = artifact.into();
let temp = Temp::new();
artifact.to_path(temp.as_ref()).await.unwrap();

let path = temp.path().join(path);
let target = format!("{path}#{target}", path = path.display());

// Build the module.
let mut command = server.tg();
command.arg("build").arg(target);
for arg in args {
command.arg("--arg");
command.arg(arg);
}
let output = command.output().await.unwrap();

assertions(output).await;
})
.await;
}

#[tokio::test]
async fn builtin_download_unsafe_checksum() {
let directory = temp::directory! {
Expand Down
100 changes: 36 additions & 64 deletions packages/server/src/target/build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -278,41 +278,27 @@ impl Server {
.await
.map_err(|source| tg::error!(!source, "failed to get a connection"))?;

// First check for a self-cycle.
let p = connection.p();
let statement = formatdoc!(
"
select exists (
select 1 from builds
where id = {p}1 and target = {p}2
);
"
);
with recursive ancestors as (
select builds.id, builds.target
from builds
where id = {p}1 and target= {p}2

let params = db::params![parent, target];
let cycle = connection
.query_one_value_into(statement, params)
.await
.map_err(|source| tg::error!(!source, "failed to execute the statement"))?;
if cycle {
return Ok(true);
}
union all

// Otherwise, recurse.
let statement = formatdoc!(
"
with recursive ancestors as (
select b.id, b.target
from builds b
join build_children c on b.id = c.child
where c.child = {p}1
select builds.id, builds.target
from builds
join build_children on builds.id = build_children.child
where build_children.child = {p}1

union all

select b.id, b.target
from ancestors a
join build_children c on a.id = c.child
join builds b on c.build = b.id
select builds.id, builds.target
from ancestors
join build_children on ancestors.id = build_children.child
join builds on build_children.build = builds.id
)
select exists (
select 1
Expand All @@ -338,54 +324,40 @@ impl Server {
.map_err(|source| tg::error!(!source, "failed to get a connection"))?;
let p = connection.p();

#[derive(serde::Deserialize, serde::Serialize)]
struct Row {
id: tg::build::Id,
depth: u64,
}
// Get the max build depth.
let statement = formatdoc!(
"
with recursive ancestors as (
select b.id, b.depth
from builds b
join build_children c on b.id = c.child
where c.child = {p}1
select builds.id, builds.depth
from builds
join build_children on builds.id = build_children.child
where build_children.child = {p}1

union all

select b.id, b.depth
from ancestors a
join build_children c on a.id = c.child
join builds b on c.build = b.id
select builds.id, builds.depth
from ancestors
join build_children on ancestors.id = build_children.child
join builds on build_children.build = builds.id
), max_depth as (
select coalesce(max(depth), 0) as max_depth from ancestors
)
select id, depth from ancestors;
update builds
set depth = builds.depth + 1
from ancestors
where builds.id = ancestors.id
returning (select max_depth from max_depth);
"
);

let params = db::params![parent];
let ancestors = connection
.query_all_into::<Row>(statement, params)
let max_depth: u64 = connection
.query_optional_value_into(statement, params)
.await
.map_err(|source| tg::error!(!source, "failed to execute the statement"))?;

let max_depth = ancestors.iter().map(|row| row.depth).max();
let ancestors = ancestors.iter().map(|row| row.id.clone()).collect_vec();
let ancestors = serde_json::to_string(&ancestors).unwrap();
if let Some(max_depth) = max_depth {
if max_depth >= self.config.build.as_ref().unwrap().max_depth {
return Ok(true);
}
let statement = formatdoc!(
"
update builds
set depth = depth + 1
where id in (select value from json_each({p}1));
"
);
let params = db::params![ancestors];
connection
.execute(statement, params)
.await
.map_err(|source| tg::error!(!source, "failed to execute the statement"))?;
.map_err(|source| tg::error!(!source, "failed to execute the statement"))?
.unwrap_or(0);
if max_depth >= self.config.build.as_ref().unwrap().max_depth {
return Ok(true);
}

// Drop the connection.
Expand Down
4 changes: 2 additions & 2 deletions tangram.lock
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,11 @@
"entries": {
"tangram.ts": 1
},
"id": "dir_01q7nskkpmvmcz0zjac5m65gesk4q1qc81s4ts27fzbm5ne6yfrc9g"
"id": "dir_01w0hcs6d17j0jq6rhb5n2gjf5bf0v4hjw5x0ntkfwpxtdjba5z6cg"
},
{
"kind": "file",
"id": "fil_01a325dbcae5bnxv5cwpr3vbws16fwfzrb5hwe58rpknwbj4tzezzg"
"id": "fil_01xkv9790k2a0pf84e9p4y6w9j003g2mx70q84dqabh0abecvkxy40"
}
]
}