Auto merge of #519 - Zeegomo:detect-build-errors, r=pietroalbini

Detect build errors

This pr introduces a bunch of changes aimed to better inspect crate build failure. In particular it detects build error codes and dependencies using two new variants `FailureReason::CompilerError` and `FailureReason::DependsOn`.

Author: bors

Cargo.lock

@@ -63,11 +63,12 @@ hmac = "0.7"
 sha-1 = "0.8"
 rust_team_data = { git = "https://github.com/rust-lang/team" }
 systemstat = "0.1.4"
-rustwide = { version = "0.6.1", features = ["unstable"] }
+rustwide = { version = "0.8.0", features = ["unstable"] }
 percent-encoding = "2.1.0"
 remove_dir_all = "0.5.2"
 ctrlc = "3.1.3"
 prometheus = "0.7.0"
+cargo_metadata = "0.9.1"
 
 [dev-dependencies]
 assert_cmd = "0.10.1"

Cargo.toml

@@ -0,0 +1,9 @@
+[package]
+name = "faulty-deps"
+version = "0.1.0"
+authors = ["Giacomo Pasini <g.pasini98@gmail.com>"]
+edition = "2018"
+
+[dependencies]
+error_code = {git = "https://github.com/rust-lang/crater", rev = "c3f462bdab37a93c24b2b172b90564749e892cbc"}
+lazy_static = "=0.1.0"

local-crates/faulty-deps/Cargo.toml

@@ -0,0 +1,5 @@
+use error_code::STRING;
+
+fn main() {
+    println!("Hello, world!");
+}

local-crates/faulty-deps/src/main.rs

@@ -153,7 +153,7 @@ impl Action for EditExperiment {
             if let Some(requirement) = self.requirement {
                 let changes = t.execute(
                     "UPDATE experiments SET requirement = ?1 WHERE name = ?2;",
-                    &[&requirement.to_string(), &self.name],
+                    &[&requirement, &self.name],
                 )?;
                 assert_eq!(changes, 1);
                 ex.requirement = Some(requirement);

src/actions/experiments/edit.rs

@@ -150,7 +150,7 @@ impl AgentApi {
         krate: &Crate,
         toolchain: &Toolchain,
         log: &[u8],
-        result: TestResult,
+        result: &TestResult,
         version: Option<(&Crate, &Crate)>,
     ) -> Fallible<()> {
         self.retry(|this| {

src/agent/api.rs

@@ -86,7 +86,7 @@ impl<'a> WriteResults for ResultsUploader<'a> {
             updated.as_ref().unwrap_or(krate),
             toolchain,
             output.as_bytes(),
-            result,
+            &result,
             new_version.map(|new| (krate, new)),
         )?;
 

src/agent/results.rs

@@ -114,6 +114,7 @@ impl Config {
             Crate::Registry(ref details) => self.crates.get(&details.name),
             Crate::GitHub(ref repo) => self.github_repos.get(&repo.slug()),
             Crate::Local(ref name) => self.local_crates.get(name),
+            Crate::Git(_) | Crate::Path(_) => unimplemented!("unsupported crate"),
         }
     }
 

src/config.rs

@@ -96,6 +96,7 @@ pub(crate) fn get_crates(
                     Crate::Registry(RegistryCrate { ref name, .. }) => demo_registry.remove(name),
                     Crate::GitHub(ref repo) => demo_github.remove(&repo.slug()),
                     Crate::Local(ref name) => demo_local.remove(name),
+                    Crate::Git(_) | Crate::Path(_) => unimplemented!("unsupported crate"),
                 };
 
                 if add {

src/crates/lists.rs

@@ -3,18 +3,30 @@ mod sources;
 
 use crate::dirs::LOCAL_CRATES_DIR;
 use crate::prelude::*;
+use cargo_metadata::PackageId;
+use percent_encoding::{percent_decode_str, utf8_percent_encode, NON_ALPHANUMERIC};
 use rustwide::Crate as RustwideCrate;
+use std::convert::TryFrom;
 use std::fmt;
+use std::path::Path;
 use std::str::FromStr;
 
 pub(crate) use crate::crates::sources::github::GitHubRepo;
 pub(crate) use crate::crates::sources::registry::RegistryCrate;
 
+#[derive(Debug, Eq, PartialEq, Ord, PartialOrd, Hash, Serialize, Deserialize, Clone)]
+pub struct GitRepo {
+    pub url: String,
+    pub sha: Option<String>,
+}
+
 #[derive(Debug, Eq, PartialEq, Ord, PartialOrd, Hash, Serialize, Deserialize, Clone)]
 pub enum Crate {
     Registry(RegistryCrate),
     GitHub(GitHubRepo),
     Local(String),
+    Path(String),
+    Git(GitRepo),
 }
 
 impl Crate {
@@ -29,6 +41,20 @@ impl Crate {
                 }
             }
             Crate::Local(ref name) => format!("local/{}", name),
+            Crate::Path(ref path) => {
+                format!("path/{}", utf8_percent_encode(&path, &NON_ALPHANUMERIC))
+            }
+            Crate::Git(ref repo) => {
+                if let Some(ref sha) = repo.sha {
+                    format!(
+                        "git/{}/{}",
+                        utf8_percent_encode(&repo.url, &NON_ALPHANUMERIC),
+                        sha
+                    )
+                } else {
+                    format!("git/{}", utf8_percent_encode(&repo.url, &NON_ALPHANUMERIC),)
+                }
+            }
         }
     }
 
@@ -39,6 +65,60 @@ impl Crate {
                 RustwideCrate::git(&format!("https://github.com/{}/{}", repo.org, repo.name))
             }
             Self::Local(name) => RustwideCrate::local(&LOCAL_CRATES_DIR.join(name)),
+            Self::Path(path) => RustwideCrate::local(Path::new(&path)),
+            Self::Git(repo) => RustwideCrate::git(&repo.url),
+        }
+    }
+}
+
+impl TryFrom<&'_ PackageId> for Crate {
+    type Error = failure::Error;
+
+    fn try_from(pkgid: &PackageId) -> Fallible<Crate> {
+        let parts = &pkgid
+            .repr
+            .split_ascii_whitespace()
+            .flat_map(|s| {
+                // remove ()
+                s.trim_matches(|c: char| c.is_ascii_punctuation())
+                    // split resource and protocol
+                    .split('+')
+            })
+            .collect::<Vec<_>>();
+
+        match parts[..] {
+            [name, version, "registry", _] => Ok(Crate::Registry(RegistryCrate {
+                name: name.to_string(),
+                version: version.to_string(),
+            })),
+            [_, _, "path", path] => Ok(Crate::Path(path.to_string())),
+            [_, _, "git", repo] => {
+                if repo.starts_with("https://github.com") {
+                    Ok(Crate::GitHub(repo.replace("#", "/").parse()?))
+                } else {
+                    let mut parts = repo.split('#').rev().collect::<Vec<_>>();
+                    let url = parts.pop();
+                    let sha = parts.pop();
+
+                    match (url, sha) {
+                        (Some(url), None) => Ok(Crate::Git(GitRepo {
+                            url: url.to_string(),
+                            sha: None,
+                        })),
+                        (Some(url), Some(sha)) => Ok(Crate::Git(GitRepo {
+                            // remove additional queries if the sha is present
+                            // as the crate version is already uniquely determined
+                            url: url.split('?').next().unwrap().to_string(),
+                            sha: Some(sha.to_string()),
+                        })),
+                        _ => bail!("malformed git repo: {}", repo),
+                    }
+                }
+            }
+            _ => bail!(
+                "malformed pkgid format: {}\n maybe the representation has changed?",
+                pkgid.repr
+            ),
         }
     }
 }
@@ -50,8 +130,25 @@ impl fmt::Display for Crate {
             "{}",
             match *self {
                 Crate::Registry(ref krate) => format!("{}-{}", krate.name, krate.version),
-                Crate::GitHub(ref repo) => repo.slug(),
+                Crate::GitHub(ref repo) =>
+                    if let Some(ref sha) = repo.sha {
+                        format!("{}/{}/{}", repo.org, repo.name, sha)
+                    } else {
+                        format!("{}/{}", repo.org, repo.name)
+                    },
                 Crate::Local(ref name) => format!("{} (local)", name),
+                Crate::Path(ref path) =>
+                    format!("{}", utf8_percent_encode(path, &NON_ALPHANUMERIC)),
+                Crate::Git(ref repo) =>
+                    if let Some(ref sha) = repo.sha {
+                        format!(
+                            "{}/{}",
+                            utf8_percent_encode(&repo.url, &NON_ALPHANUMERIC),
+                            sha
+                        )
+                    } else {
+                        utf8_percent_encode(&repo.url, &NON_ALPHANUMERIC).to_string()
+                    },
             }
         )
     }
@@ -77,17 +174,94 @@ impl FromStr for Crate {
                 name: name.to_string(),
                 sha: None,
             })),
+            ["git", repo, sha] => Ok(Crate::Git(GitRepo {
+                url: percent_decode_str(repo).decode_utf8()?.to_string(),
+                sha: Some(sha.to_string()),
+            })),
+            ["git", repo] => Ok(Crate::Git(GitRepo {
+                url: percent_decode_str(repo).decode_utf8()?.to_string(),
+                sha: None,
+            })),
             ["local", name] => Ok(Crate::Local(name.to_string())),
+            ["path", path] => Ok(Crate::Path(
+                percent_decode_str(path).decode_utf8()?.to_string(),
+            )),
             _ => bail!("unexpected crate value"),
         }
     }
 }
 
 #[cfg(test)]
 mod tests {
-    use super::{Crate, GitHubRepo, RegistryCrate};
+    use super::{Crate, GitHubRepo, GitRepo, RegistryCrate};
+    use cargo_metadata::PackageId;
+    use percent_encoding::{utf8_percent_encode, NON_ALPHANUMERIC};
+    use std::convert::TryFrom;
     use std::str::FromStr;
 
+    macro_rules! test_from_pkgid {
+        ($($str:expr => $rust:expr,)*) => {
+            $(
+                let pkgid = PackageId {
+                    repr: $str.to_string(),
+                };
+
+                assert_eq!(Crate::try_from(&pkgid).unwrap(), $rust);
+            )*
+        };
+    }
+
+    #[test]
+    fn test_parse_from_pkgid() {
+        test_from_pkgid! {
+            "dummy 0.1.0 (path+file:///opt/rustwide/workdir)" => Crate::Path("file:///opt/rustwide/workdir".to_string()),
+            "dummy 0.1.0 (registry+https://github.com/rust-lang/crates.io-index)" => Crate::Registry(RegistryCrate {
+                name: "dummy".to_string(),
+                version: "0.1.0".to_string()
+            }),
+            "dummy 0.1.0 (git+https://github.com/dummy_org/dummy#9823f01cf4948a41279f6a3febcf793130cab4f6)" => Crate::GitHub(GitHubRepo {
+                org: "dummy_org".to_string(),
+                name: "dummy".to_string(),
+                sha: Some("9823f01cf4948a41279f6a3febcf793130cab4f6".to_string())
+            }),
+            "dummy 0.1.0 (git+https://github.com/dummy_org/dummy?rev=dummyrev#9823f01cf4948a41279f6a3febcf793130cab4f6)" => Crate::GitHub(GitHubRepo {
+                org: "dummy_org".to_string(),
+                name: "dummy".to_string(),
+                sha: Some("9823f01cf4948a41279f6a3febcf793130cab4f6".to_string())
+            }),
+            "dummy 0.1.0 (git+https://github.com/dummy_org/dummy)" => Crate::GitHub(GitHubRepo {
+                org: "dummy_org".to_string(),
+                name: "dummy".to_string(),
+                sha: None
+            }),
+            "dummy 0.1.0 (git+https://gitlab.com/dummy_org/dummy#9823f01cf4948a41279f6a3febcf793130cab4f6)" => Crate::Git(GitRepo {
+                url: "https://gitlab.com/dummy_org/dummy"
+                    .to_string(),
+                sha: Some("9823f01cf4948a41279f6a3febcf793130cab4f6".to_string())
+            }),
+            "dummy 0.1.0 (git+https://gitlab.com/dummy_org/dummy?branch=dummybranch#9823f01cf4948a41279f6a3febcf793130cab4f6)" => Crate::Git(GitRepo {
+                url: "https://gitlab.com/dummy_org/dummy"
+                    .to_string(),
+                sha: Some("9823f01cf4948a41279f6a3febcf793130cab4f6".to_string())
+            }),
+            "dummy 0.1.0 (git+https://gitlab.com/dummy_org/dummy)" => Crate::Git(GitRepo {
+                url: "https://gitlab.com/dummy_org/dummy"
+                    .to_string(),
+                sha: None
+            }),
+            "dummy 0.1.0 (git+https://gitlab.com/dummy_org/dummy?branch=dummybranch)" => Crate::Git(GitRepo {
+                url: "https://gitlab.com/dummy_org/dummy?branch=dummybranch"
+                    .to_string(),
+                sha: None
+            }),
+        }
+
+        assert!(Crate::try_from(&PackageId {
+            repr: "invalid".to_string()
+        })
+        .is_err());
+    }
+
     #[test]
     fn test_parse() {
         macro_rules! test_from_str {
@@ -107,8 +281,13 @@ mod tests {
 
         test_from_str! {
             "local/build-fail" => Crate::Local("build-fail".to_string()),
+            "path/pathtofile" => Crate::Path("pathtofile".to_string()),
+            &format!("path/{}", utf8_percent_encode("path/with:stange?characters", &NON_ALPHANUMERIC)) => Crate::Path("path/with:stange?characters".to_string()),
             "gh/org/user" => Crate::GitHub(GitHubRepo{org: "org".to_string(), name: "user".to_string(), sha: None}),
             "gh/org/user/sha" => Crate::GitHub(GitHubRepo{org: "org".to_string(), name: "user".to_string(), sha: Some("sha".to_string())}),
+            "git/url" => Crate::Git(GitRepo{url: "url".to_string(), sha: None}),
+            &format!("git/{}", utf8_percent_encode("url/with:stange?characters", &NON_ALPHANUMERIC)) => Crate::Git(GitRepo{url: "url/with:stange?characters".to_string(), sha: None}),
+            "git/url/sha" => Crate::Git(GitRepo{url: "url".to_string(), sha: Some("sha".to_string())}),
             "reg/name/version" => Crate::Registry(RegistryCrate{name: "name".to_string(), version: "version".to_string()}),
         }
     }