Skip to content

Fix not queueing TryParent into pull_request_builds #1066

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

Merged
Merged
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
18 changes: 10 additions & 8 deletions site/src/github.rs
Original file line number Diff line number Diff line change
Expand Up @@ -521,12 +521,14 @@ pub async fn post_finished(ctxt: &SiteCtxt) {
conn.queued_commits(),
conn.in_progress_artifacts()
);
let master_commits = if let Ok(mcs) = master_commits {
mcs.into_iter().map(|c| c.sha).collect::<HashSet<_>>()
} else {
// If we can't fetch master commits, return.
// We'll eventually try again later
return;
let master_commits = match master_commits {
Ok(mcs) => mcs.into_iter().map(|c| c.sha).collect::<HashSet<_>>(),
Err(e) => {
log::error!("posting finished did not load master commits: {:?}", e);
// If we can't fetch master commits, return.
// We'll eventually try again later
return;
}
};

for aid in in_progress_artifacts {
Expand Down Expand Up @@ -606,7 +608,7 @@ fn master_run_body(direction: Option<Direction>) -> String {

format!(
"
{next_steps}
{next_steps}

@rustbot label: {label}",
next_steps = next_steps,
Expand Down Expand Up @@ -637,7 +639,7 @@ Benchmarking this pull request likely means that it is \
perf-sensitive, so we're automatically marking it as not fit \
for rolling up. While you can manually mark this PR as fit \
for rollup, we strongly recommend not doing so since this PR led to changes in \
compiler perf.{next_steps}
compiler perf.{next_steps}

@bors rollup=never
@rustbot label: +S-waiting-on-review -S-waiting-on-perf {label}",
Expand Down
34 changes: 30 additions & 4 deletions site/src/load.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,8 @@ pub enum MissingReason {
Master {
pr: u32,
parent_sha: String,
is_try_parent: bool,
},
TryParent,
Try {
pr: u32,
include: Option<String>,
Expand Down Expand Up @@ -214,6 +214,7 @@ fn calculate_missing_from(
MissingReason::Master {
pr: c.pr.unwrap_or(0),
parent_sha: c.parent_sha,
is_try_parent: false,
},
)
})
Expand All @@ -235,11 +236,31 @@ fn calculate_missing_from(
// Enqueue the `TryParent` commit before the `TryCommit` itself, so that
// all of the `try` run's data is complete when the benchmark results
// of that commit are available.
if let Some((try_parent, _)) = master_commits
if let Some((try_parent, metadata)) = master_commits
.iter()
.find(|(m, _)| m.sha == parent_sha.as_str())
{
missing.push((try_parent.clone(), MissingReason::TryParent));
let (pr, parent_sha) = if let MissingReason::Master {
pr,
parent_sha,
is_try_parent: _,
} = &metadata
{
(*pr, parent_sha.clone())
} else {
unreachable!(
"non-master missing reason in master_commits: {:?}",
metadata
);
};
missing.push((
try_parent.clone(),
MissingReason::Master {
pr,
parent_sha,
is_try_parent: true,
},
));
}
missing.push((
Commit {
Expand Down Expand Up @@ -351,7 +372,11 @@ mod tests {
sha: "foo".into(),
date: database::Date(time),
},
MissingReason::TryParent,
MissingReason::Master {
pr: 77,
parent_sha: "bar".into(),
is_try_parent: true,
},
),
(
Commit {
Expand All @@ -373,6 +398,7 @@ mod tests {
MissingReason::Master {
pr: 11,
parent_sha: "345".into(),
is_try_parent: false,
},
),
];
Expand Down
5 changes: 4 additions & 1 deletion site/src/request_handlers/next_commit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,10 @@ pub async fn handle_next_commit(ctxt: Arc<SiteCtxt>) -> next_commit::Response {
let next_commit = if let Some((commit, missing_reason)) = next_commit {
// If we're going to run a master commit next, make sure
// it's been enqueued in the pull_request_build table
if let MissingReason::Master { pr, ref parent_sha } = missing_reason {
if let MissingReason::Master {
pr, ref parent_sha, ..
} = missing_reason
{
let conn = ctxt.conn().await;
// TODO: add capability of doing the following in one step
// to avoid possibile illegal inbetween states.
Expand Down
4 changes: 3 additions & 1 deletion site/static/status.html
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,9 @@
return `${reason_to_string(reason.InProgress)} - in progress`;
} else if (reason["Master"] != undefined && reason.Master.pr) {
return `<a href="https://github.com/rust-lang/rust/pull/${reason["Master"].pr}">
#${reason["Master"].pr}</a>`;
#${reason["Master"].pr}</a>${
reason.Master.is_try_parent ? " - Try commit parent" : ""
}`;
} else if (reason["Master"] != undefined && reason.Master.pr == 0) {
return "Master";
} else if (reason["Try"] != undefined && reason.Try.pr) {
Expand Down