Merge pull request #784 from Skgland/follow-up-to-770

Follow up to 770 and minor fixes

Author: Mark-Simulacrum

Cargo.lock

@@ -68,7 +68,7 @@ tera = "1.19.1"
 thiserror = "1.0.38"
 tokio = "1.24"
 toml = "0.8.6"
-url = "2"
+url = { version = "2", features = ["serde"] }
 walkdir = "2"
 warp = "0.3"
 zstd = "0.13.0"

Cargo.toml

@@ -107,15 +107,17 @@ available:
 * `top-{n}`: run the experiment on the `n` most downloaded crates on
   [crates.io](crates.io) (e.g. `top-100`).
 * `random-{n}`: run the experiment on `n` randomly selected crates (e.g. `random-20`).
-* `list:{...}`: run the experiment on the specified crates.
+* `list:{...}`: run the experiment on the comma-seperated list of crates.
+* `{url}`: run the experiment on the crates specified in the file specified by `{url}`,
+  The url must start with `http[s]://` and must point to a list in the format of a crater runs [`retry-regressed-list.txt`][retry-list].
 
-For `list:`, the value after the colon can either be a comma-separated list of
-crates to run or a link to a newline-separated list of crates ([example][list]).
-For example, `list:lazy_static,brson/hello-rs` and `list:https://git.io/Jes7o`
-will both run an experiment on the `lazy_static` crate and the git repo at
-`github.com/brson/hello-rs`. A link must begin with `http[s]://`.
+### Examples
 
-[list]: https://gist.githubusercontent.com/ecstatic-morse/837c558b63fc73ab469bfbf4ad419a1f/raw/example-crate-list
+Both of the following will run an experiment on the `lazy_static` crate and the git repo at `github.com/brson/hello-rs`
+- `crates=list:lazy_static,brson/hello-rs` 
+- `crates=https://git.io/Jes7o`
+
+[retry-list]: https://crater-reports.s3.amazonaws.com/pr-133502-3/retry-regressed-list.txt
 
 [Go back to the TOC][h-toc]
 

docs/bot-usage.md

@@ -130,10 +130,6 @@ pub(crate) fn get_crates(
                     crates.push(krate);
                 }
             }
-
-            if !desired.is_empty() {
-                bail!("missing desired crates: {:?}", desired);
-            }
         }
 
         CrateSelect::Random(n) => {

src/crates/lists.rs

@@ -517,10 +517,10 @@ fn compare(
                 }
             }
 
-            (PrepareFail(_), _) | (_, PrepareFail(_)) => Comparison::PrepareFail,
-            (Error, _) | (_, Error) => Comparison::Error,
             (Skipped, _) | (_, Skipped) => Comparison::Skipped,
             (BrokenCrate(_), _) | (_, BrokenCrate(_)) => Comparison::Broken,
+            (PrepareFail(_), _) | (_, PrepareFail(_)) => Comparison::PrepareFail,
+            (Error, _) | (_, Error) => Comparison::Error,
             (TestFail(_) | TestPass, TestSkipped) | (TestSkipped, TestFail(_) | TestPass) => {
                 panic!("can't compare {res1} and {res2}");
             }
@@ -838,6 +838,8 @@ mod tests {
                 // PrepareFail
                 PrepareFail(Unknown), BuildFail(Unknown) => PrepareFail;
                 BuildFail(Unknown), PrepareFail(Unknown) => PrepareFail;
+                PrepareFail(Unknown), Error => PrepareFail;
+                Error, PrepareFail(Unknown) => PrepareFail;
 
                 // Errors
                 Error, TestPass => Error;
@@ -870,6 +872,11 @@ mod tests {
                 TestSkipped, BrokenCrate(BrokenReason::Unknown) => Broken;
                 TestFail(Unknown), BrokenCrate(BrokenReason::Unknown) => Broken;
                 BuildFail(Unknown), BrokenCrate(BrokenReason::Unknown) => Broken;
+
+                PrepareFail(Unknown), BrokenCrate(BrokenReason::Unknown) => Broken;
+                BrokenCrate(BrokenReason::Unknown), PrepareFail(Unknown) => Broken;
+                BrokenCrate(BrokenReason::Unknown), Error => Broken;
+                Error, BrokenCrate(BrokenReason::Unknown) => Broken;
             ]
         );
 

src/report/mod.rs

@@ -253,7 +253,7 @@ pub(super) fn run_test(
         let mut build = build_dir.build(ctx.toolchain, krate, sandbox);
 
         for patch in ctx.toolchain.patches.iter() {
-            build = build.patch_with_git(&patch.name, &patch.repo, &patch.branch);
+            build = build.patch_with_git(&patch.name, patch.repo.as_str(), &patch.branch);
         }
 
         detect_broken(build.run(|build| {

src/runner/test.rs

@@ -144,16 +144,14 @@ impl<'a> Worker<'a> {
         error!("task {task:?} failed");
         utils::report_failure(&e);
 
-        let mut result = if self.config.is_broken(&task.krate) {
+        let result = if let Some(OverrideResult(res)) = e.downcast_ref() {
+            res.clone()
+        } else if self.config.is_broken(&task.krate) {
             TestResult::BrokenCrate(BrokenReason::Unknown)
         } else {
             TestResult::Error
         };
 
-        if let Some(OverrideResult(res)) = e.downcast_ref() {
-            result = res.clone();
-        }
-
         Err((e, result))
     }
 

src/runner/worker.rs

@@ -124,7 +124,7 @@ fn reports_thread(data: &Data, github_data: Option<&GithubData>) -> Fallible<()>
                             )
                             .line(
                                 "newspaper",
-                                format!("[Open the full report]({report_url})."),
+                                format!("[Open the summary report]({report_url})."),
                             )
                             .note(
                                 "warning",

src/server/reports.rs

@@ -105,6 +105,8 @@ pub enum ToolchainParseError {
     InvalidFlag(String),
     #[error("invalid toolchain SHA: {0} is missing a `try#` or `master#` prefix")]
     PrefixMissing(String),
+    #[error("invalid url {0:?}: {1}")]
+    InvalidUrl(String, url::ParseError),
 }
 
 lazy_static! {
@@ -187,7 +189,9 @@ impl FromStr for Toolchain {
 #[derive(Serialize, Deserialize, PartialEq, Eq, Hash, Debug, Clone)]
 pub struct CratePatch {
     pub name: String,
-    pub repo: String,
+    // cargo currently doesn't accept scp-style "URLs" rust-lang/crates#1851
+    // so ensure its a proper URL
+    pub repo: url::Url,
     pub branch: String,
 }
 
@@ -202,7 +206,8 @@ impl FromStr for CratePatch {
         } else {
             Ok(CratePatch {
                 name: params[0].into(),
-                repo: params[1].into(),
+                repo: url::Url::parse(params[1])
+                    .map_err(|err| ToolchainParseError::InvalidUrl(params[1].into(), err))?,
                 branch: params[2].into(),
             })
         }
@@ -291,7 +296,7 @@ mod tests {
                         ci_try: $ci_try,
                         patches: vec![CratePatch {
                             name: "example".to_string(),
-                            repo: "https://git.example.com/some/repo".to_string(),
+                            repo: url::Url::parse("https://git.example.com/some/repo").unwrap(),
                             branch: "master".to_string()
                         }]
                     });
@@ -306,7 +311,7 @@ mod tests {
                         ci_try: $ci_try,
                         patches: vec![CratePatch {
                             name: "example".to_string(),
-                            repo: "https://git.example.com/some/repo".to_string(),
+                            repo: url::Url::parse("https://git.example.com/some/repo").unwrap(),
                             branch: "master".to_string()
                         }]
                     });
@@ -358,6 +363,13 @@ mod tests {
         assert!(Toolchain::from_str("stable+rustdocflags=").is_err());
         assert!(Toolchain::from_str("stable+donotusethisflag=ever").is_err());
         assert!(Toolchain::from_str("stable+patch=").is_err());
+        assert!(matches!(
+            Toolchain::from_str(
+                "stable+patch=getrandom=git@github.com:rust-random/getrandom=backports/v0.2"
+            )
+            .unwrap_err(),
+            super::ToolchainParseError::InvalidUrl(..)
+        ));
         assert!(Toolchain::from_str("try#1234+target=").is_err());
         assert!(Toolchain::from_str("0000000000000000000000000000000000000000").is_err());
     }

src/toolchain.rs

@@ -10,7 +10,7 @@ <h1>Experiment <b>{{ experiment.name }}</b></h1>
             <div class="toolbar">
                 {% if experiment.report_url %}
                 <a rel="noopener" target="_blank" class="button" href="{{ experiment.report_url }}">
-                    Open full report
+                    Open summary report
                 </a>
                 {% endif %}
                 {% if experiment.github_url %}