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 build-details view for pre-build errors, when another build set the default target #2683

Merged
merged 1 commit into from
Dec 9, 2024
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

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

98 changes: 76 additions & 22 deletions src/web/build_details.rs
Original file line number Diff line number Diff line change
Expand Up @@ -93,37 +93,53 @@ pub(crate) async fn build_details_handler(
.ok_or(AxumNope::BuildNotFound)?;

let (output, all_log_filenames, current_filename) = if let Some(output) = row.output {
// legacy case, for old builds the build log was stored in the database.
(output, Vec::new(), None)
} else {
// for newer builds we have the build logs stored in S3.
// For a long time only for one target, then we started storing the logs for other targets
// toFor a long time only for one target, then we started storing the logs for other
// targets. In any case, all the logfiles are put into a folder we can just query.
let prefix = format!("build-logs/{}/", id);
let all_log_filenames: Vec<_> = storage
.list_prefix(&prefix) // the result from S3 is ordered by key
.await
.map_ok(|path| {
path.strip_prefix(&prefix)
.expect("since we query for the prefix, it has to be always there")
.to_owned()
})
.try_collect()
.await?;

if let Some(current_filename) = params
.filename
.or(row.default_target.map(|target| format!("{}.txt", target)))
{
let path = format!("{prefix}{current_filename}");
let file = File::from_path(&storage, &path, &config).await?;
(
String::from_utf8(file.0.content).context("non utf8")?,
storage
.list_prefix(&prefix) // the result from S3 is ordered by key
.await
.map_ok(|path| {
path.strip_prefix(&prefix)
.expect("since we query for the prefix, it has to be always there")
.to_owned()
})
.try_collect()
.await?,
Some(current_filename),
)
let current_filename = if let Some(filename) = params.filename {
// if we have a given filename in the URL, we use that one.
Some(filename)
} else if let Some(default_target) = row.default_target {
// without a filename in the URL, we try to show the build log
// for the default target, if we have one.
let wanted_filename = format!("{default_target}.txt");
if all_log_filenames.contains(&wanted_filename) {
Some(wanted_filename)
} else {
None
}
} else {
// this can only happen when `releases.default_target` is NULL,
// which is the case for in-progress builds or builds which errored
// before we could determine the target.
// For the "error" case we show `row.errors`, which should contain what we need to see.
("".into(), Vec::new(), None)
}
None
};

let file_content = if let Some(ref filename) = current_filename {
let file = File::from_path(&storage, &format!("{prefix}{filename}"), &config).await?;
String::from_utf8(file.0.content).context("non utf8")?
} else {
"".to_string()
};

(file_content, all_log_filenames, current_filename)
};

Ok(BuildDetailsPage {
Expand Down Expand Up @@ -197,6 +213,44 @@ mod tests {
});
}

#[test]
fn test_partial_build_result_plus_default_target_from_previous_build() {
async_wrapper(|env| async move {
let mut conn = env.async_db().await.async_conn().await;
let (release_id, build_id) = fake_release_that_failed_before_build(
&mut conn,
"foo",
"0.1.0",
"some random error",
)
.await?;

sqlx::query!(
"UPDATE releases SET default_target = 'x86_64-unknown-linux-gnu' WHERE id = $1",
release_id.0
)
.execute(&mut *conn)
.await?;

let page = kuchikiki::parse_html().one(
env.web_app()
.await
.get(&format!("/crate/foo/0.1.0/builds/{build_id}"))
.await?
.error_for_status()?
.text()
.await?,
);

let info_text = page.select("pre").unwrap().next().unwrap().text_contents();

assert!(info_text.contains("# pre-build errors"), "{}", info_text);
assert!(info_text.contains("some random error"), "{}", info_text);

Ok(())
});
}

#[test]
fn db_build_logs() {
async_wrapper(|env| async move {
Expand Down
Loading