Skip to content

Commit 879bebd

Browse files
Nemo157Nemo157
committed
wip: Change timeout to apply to the overall build process, not per-target
1 parent c088458 commit 879bebd

File tree

1 file changed

+131
-40
lines changed

1 file changed

+131
-40
lines changed

‎src/docbuilder/rustwide_builder.rs

+131-40
Original file line numberDiff line numberDiff line change
@@ -23,9 +23,12 @@ use rustwide::cmd::{Command, CommandError, SandboxBuilder, SandboxImage};
2323
use rustwide::logging::{self, LogStorage};
2424
use rustwide::toolchain::ToolchainError;
2525
use rustwide::{AlternativeRegistry, Build, Crate, Toolchain, Workspace, WorkspaceBuilder};
26-
use std::collections::{HashMap, HashSet};
27-
use std::path::Path;
28-
use std::sync::Arc;
26+
use std::{
27+
collections::{HashMap, HashSet},
28+
path::Path,
29+
sync::Arc,
30+
time::Instant,
31+
};
2932
use tracing::{debug, info, warn};
3033

3134
const USER_AGENT: &str = "docs.rs builder (https://github.com/rust-lang/docs.rs)";
@@ -244,9 +247,19 @@ impl RustwideBuilder {
244247
.run(|build| {
245248
(|| -> Result<()> {
246249
let metadata = Metadata::from_crate_root(build.host_source_dir())?;
250+
let deadline = Instant::now()
251+
.checked_add(limits.timeout())
252+
.context("deadline is not representable")?;
247253

248-
let res =
249-
self.execute_build(HOST_TARGET, true, build, &limits, &metadata, true)?;
254+
let res = self.execute_build(
255+
HOST_TARGET,
256+
true,
257+
build,
258+
&limits,
259+
&metadata,
260+
true,
261+
deadline,
262+
)?;
250263
if !res.result.successful {
251264
bail!("failed to build dummy crate for {}", self.rustc_version);
252265
}
@@ -333,15 +346,15 @@ impl RustwideBuilder {
333346
return Ok(false);
334347
}
335348

336-
self.update_toolchain()?;
337-
338-
info!("building package {} {}", name, version);
339-
340349
if is_blacklisted(&mut conn, name)? {
341350
info!("skipping build of {}, crate has been blacklisted", name);
342351
return Ok(false);
343352
}
344353

354+
self.update_toolchain()?;
355+
356+
info!("building package {} {}", name, version);
357+
345358
let limits = Limits::for_crate(&mut conn, name)?;
346359
#[cfg(target_os = "linux")]
347360
if !self.config.disable_memory_limit {
@@ -388,18 +401,56 @@ impl RustwideBuilder {
388401
default_target,
389402
other_targets,
390403
} = metadata.targets(self.config.include_default_targets);
391-
let mut targets = vec![default_target];
392-
targets.extend(&other_targets);
404+
405+
let cargo_args = metadata.cargo_args(&[], &[]);
406+
let has_build_std = cargo_args.iter().any(|arg| arg.starts_with("-Zbuild-std"))
407+
|| cargo_args
408+
.windows(2)
409+
.any(|args| args[0] == "-Z" && args[1].starts_with("build-std"));
410+
411+
let other_targets: Vec<_> = other_targets
412+
.into_iter()
413+
.filter_map(|target| {
414+
// If the explicit target is not a tier one target, we need to install it.
415+
if !docsrs_metadata::DEFAULT_TARGETS.contains(&target) && !has_build_std {
416+
// This is a no-op if the target is already installed.
417+
if let Err(e) = self.toolchain.add_target(&self.workspace, &target) {
418+
info!("Skipping target {target} since it failed to install: {e}");
419+
return None;
420+
}
421+
}
422+
Some(target)
423+
})
424+
// Limit the number of targets so that no one can try to build all 200000 possible targets
425+
.take(limits.targets())
426+
.collect();
427+
428+
let targets: Vec<_> = [&default_target]
429+
.into_iter()
430+
.chain(&other_targets)
431+
.copied()
432+
.collect();
393433
// Fetch this before we enter the sandbox, so networking isn't blocked.
394434
build.fetch_build_std_dependencies(&targets)?;
395435

396436
(|| -> Result<bool> {
437+
let deadline = Instant::now()
438+
.checked_add(limits.timeout())
439+
.context("deadline is not representable")?;
440+
397441
let mut has_docs = false;
398442
let mut successful_targets = Vec::new();
399443

400444
// Perform an initial build
401-
let mut res =
402-
self.execute_build(default_target, true, build, &limits, &metadata, false)?;
445+
let mut res = self.execute_build(
446+
default_target,
447+
true,
448+
build,
449+
&limits,
450+
&metadata,
451+
false,
452+
deadline,
453+
)?;
403454

404455
// If the build fails with the lockfile given, try using only the dependencies listed in Cargo.toml.
405456
let cargo_lock = build.host_source_dir().join("Cargo.lock");
@@ -421,6 +472,7 @@ impl RustwideBuilder {
421472
&limits,
422473
&metadata,
423474
false,
475+
deadline,
424476
)?;
425477
}
426478

@@ -448,8 +500,7 @@ impl RustwideBuilder {
448500
successful_targets.push(res.target.clone());
449501

450502
// Then build the documentation for all the targets
451-
// Limit the number of targets so that no one can try to build all 200000 possible targets
452-
for target in other_targets.into_iter().take(limits.targets()) {
503+
for target in other_targets {
453504
debug!("building package {} {} for {}", name, version, target);
454505
self.build_target(
455506
target,
@@ -458,6 +509,7 @@ impl RustwideBuilder {
458509
local_storage.path(),
459510
&mut successful_targets,
460511
&metadata,
512+
deadline,
461513
)?;
462514
}
463515
let (_, new_alg) = add_path_into_remote_archive(
@@ -580,8 +632,10 @@ impl RustwideBuilder {
580632
local_storage: &Path,
581633
successful_targets: &mut Vec<String>,
582634
metadata: &Metadata,
635+
deadline: Instant,
583636
) -> Result<()> {
584-
let target_res = self.execute_build(target, false, build, limits, metadata, false)?;
637+
let target_res =
638+
self.execute_build(target, false, build, limits, metadata, false, deadline)?;
585639
if target_res.result.successful {
586640
// Cargo is not giving any error and not generating documentation of some crates
587641
// when we use a target compile options. Check documentation exists before
@@ -600,7 +654,7 @@ impl RustwideBuilder {
600654
target: &str,
601655
build: &Build,
602656
metadata: &Metadata,
603-
limits: &Limits,
657+
deadline: Instant,
604658
) -> Result<Option<DocCoverage>> {
605659
let rustdoc_flags = vec![
606660
"--output-format".to_string(),
@@ -623,7 +677,7 @@ impl RustwideBuilder {
623677
items_with_examples: 0,
624678
};
625679

626-
self.prepare_command(build, target, metadata, limits, rustdoc_flags)?
680+
self.prepare_command(build, target, metadata, rustdoc_flags, deadline)?
627681
.process_lines(&mut |line, _| {
628682
if line.starts_with('{') && line.ends_with('}') {
629683
let parsed = match serde_json::from_str::<HashMap<String, FileCoverage>>(line) {
@@ -658,6 +712,7 @@ impl RustwideBuilder {
658712
limits: &Limits,
659713
metadata: &Metadata,
660714
create_essential_files: bool,
715+
deadline: Instant,
661716
) -> Result<FullBuildResult> {
662717
let cargo_metadata = CargoMetadata::load_from_rustwide(
663718
&self.workspace,
@@ -682,7 +737,7 @@ impl RustwideBuilder {
682737
// we have to run coverage before the doc-build because currently it
683738
// deletes the doc-target folder.
684739
// https://github.com/rust-lang/cargo/issues/9447
685-
let doc_coverage = match self.get_coverage(target, build, metadata, limits) {
740+
let doc_coverage = match self.get_coverage(target, build, metadata, deadline) {
686741
Ok(cov) => cov,
687742
Err(err) => {
688743
info!("error when trying to get coverage: {}", err);
@@ -692,10 +747,11 @@ impl RustwideBuilder {
692747
};
693748

694749
let successful = logging::capture(&storage, || {
695-
self.prepare_command(build, target, metadata, limits, rustdoc_flags)
750+
self.prepare_command(build, target, metadata, rustdoc_flags, deadline)
696751
.and_then(|command| command.run().map_err(Error::from))
697-
.is_ok()
698-
});
752+
})
753+
.map_err(|e| info!("failed build: {e:?}"))
754+
.is_ok();
699755

700756
// For proc-macros, cargo will put the output in `target/doc`.
701757
// Move it to the target-specific directory for consistency with other builds.
@@ -732,9 +788,13 @@ impl RustwideBuilder {
732788
build: &'ws Build,
733789
target: &str,
734790
metadata: &Metadata,
735-
limits: &Limits,
736791
mut rustdoc_flags_extras: Vec<String>,
792+
deadline: Instant,
737793
) -> Result<Command<'ws, 'pl>> {
794+
let timeout = deadline
795+
.checked_duration_since(Instant::now())
796+
.context("exceeded deadline")?;
797+
738798
// Add docs.rs specific arguments
739799
let mut cargo_args = vec![
740800
"--offline".into(),
@@ -783,22 +843,7 @@ impl RustwideBuilder {
783843
rustdoc_flags_extras.extend(UNCONDITIONAL_ARGS.iter().map(|&s| s.to_owned()));
784844
let cargo_args = metadata.cargo_args(&cargo_args, &rustdoc_flags_extras);
785845

786-
// If the explicit target is not a tier one target, we need to install it.
787-
let has_build_std = cargo_args.windows(2).any(|args| {
788-
args[0].starts_with("-Zbuild-std")
789-
|| (args[0] == "-Z" && args[1].starts_with("build-std"))
790-
}) || cargo_args.last().unwrap().starts_with("-Zbuild-std");
791-
if !docsrs_metadata::DEFAULT_TARGETS.contains(&target) && !has_build_std {
792-
// This is a no-op if the target is already installed.
793-
self.toolchain
794-
.add_target(&self.workspace, target)
795-
.map_err(FailureError::compat)?;
796-
}
797-
798-
let mut command = build
799-
.cargo()
800-
.timeout(Some(limits.timeout()))
801-
.no_output_timeout(None);
846+
let mut command = build.cargo().timeout(Some(timeout)).no_output_timeout(None);
802847

803848
for (key, val) in metadata.environment_variables() {
804849
command = command.env(key, val);
@@ -885,7 +930,10 @@ pub(crate) struct BuildResult {
885930
#[cfg(test)]
886931
mod tests {
887932
use super::*;
888-
use crate::test::{assert_redirect, assert_success, wrapper, TestEnvironment};
933+
use crate::{
934+
db::Overrides,
935+
test::{assert_redirect, assert_success, wrapper, TestEnvironment},
936+
};
889937
use serde_json::Value;
890938

891939
fn remove_cache_files(env: &TestEnvironment, crate_: &str, version: &str) -> Result<()> {
@@ -1218,6 +1266,49 @@ mod tests {
12181266
});
12191267
}
12201268

1269+
#[test]
1270+
#[ignore]
1271+
fn test_timeout_skips_some_targets() {
1272+
wrapper(|env| {
1273+
let crate_ = "bs58";
1274+
let version = "0.5.0";
1275+
let mut builder = RustwideBuilder::init(env).unwrap();
1276+
let get_targets = || -> i32 {
1277+
env.db()
1278+
.conn()
1279+
.query_one(
1280+
"SELECT json_array_length(releases.doc_targets) FROM releases;",
1281+
&[],
1282+
)
1283+
.unwrap()
1284+
.get(0)
1285+
};
1286+
1287+
// Build once to time it and count how many targets are built
1288+
let start = Instant::now();
1289+
assert!(builder.build_package(crate_, version, PackageKind::CratesIo)?);
1290+
let timeout = start.elapsed() / 2;
1291+
let original_targets = get_targets();
1292+
1293+
// Build again with half the time and count how many targets are built
1294+
Overrides::save(
1295+
&mut env.db().conn(),
1296+
crate_,
1297+
Overrides {
1298+
memory: None,
1299+
targets: Some(original_targets as usize),
1300+
timeout: Some(timeout),
1301+
},
1302+
)?;
1303+
assert!(builder.build_package(crate_, version, PackageKind::CratesIo)?);
1304+
let new_targets = get_targets();
1305+
1306+
assert!(new_targets < original_targets);
1307+
1308+
Ok(())
1309+
});
1310+
}
1311+
12211312
#[test]
12221313
#[ignore]
12231314
fn test_implicit_features_for_optional_dependencies() {

0 commit comments

Comments
 (0)