Merge pull request #2120 from Urgau/gha_logs-paths

Add anchor around repository paths in our GHA logs viewer

Author: Urgau

src/gha_logs.rs

@@ -1,8 +1,9 @@
-use crate::github;
+use crate::github::{self, WorkflowRunJob};
 use crate::handlers::Context;
 use anyhow::Context as _;
 use hyper::header::{CACHE_CONTROL, CONTENT_SECURITY_POLICY, CONTENT_TYPE};
 use hyper::{Body, Response, StatusCode};
+use itertools::Itertools;
 use std::collections::VecDeque;
 use std::str::FromStr;
 use std::sync::Arc;
@@ -14,11 +15,17 @@ const MAX_CACHE_CAPACITY_BYTES: u64 = 50 * 1024 * 1024; // 50 Mb
 #[derive(Default)]
 pub struct GitHubActionLogsCache {
     capacity: u64,
-    entries: VecDeque<(String, Arc<String>)>,
+    entries: VecDeque<(String, Arc<CachedLog>)>,
+}
+
+pub struct CachedLog {
+    job: WorkflowRunJob,
+    tree_roots: String,
+    logs: String,
 }
 
 impl GitHubActionLogsCache {
-    pub fn get(&mut self, key: &String) -> Option<Arc<String>> {
+    pub fn get(&mut self, key: &String) -> Option<Arc<CachedLog>> {
         if let Some(pos) = self.entries.iter().position(|(k, _)| k == key) {
             // Move previously cached entry to the front
             let entry = self.entries.remove(pos).unwrap();
@@ -29,26 +36,26 @@ impl GitHubActionLogsCache {
         }
     }
 
-    pub fn put(&mut self, key: String, value: Arc<String>) -> Arc<String> {
-        if value.len() as u64 > MAX_CACHE_CAPACITY_BYTES {
+    pub fn put(&mut self, key: String, value: Arc<CachedLog>) -> Arc<CachedLog> {
+        if value.logs.len() as u64 > MAX_CACHE_CAPACITY_BYTES {
             // Entry is too large, don't cache, return as is
             return value;
         }
 
         // Remove duplicate or last entry when necessary
         let removed = if let Some(pos) = self.entries.iter().position(|(k, _)| k == &key) {
             self.entries.remove(pos)
-        } else if self.capacity + value.len() as u64 >= MAX_CACHE_CAPACITY_BYTES {
+        } else if self.capacity + value.logs.len() as u64 >= MAX_CACHE_CAPACITY_BYTES {
             self.entries.pop_back()
         } else {
             None
         };
         if let Some(removed) = removed {
-            self.capacity -= removed.1.len() as u64;
+            self.capacity -= removed.1.logs.len() as u64;
         }
 
         // Add entry the front of the list ane return it
-        self.capacity += value.len() as u64;
+        self.capacity += value.logs.len() as u64;
         self.entries.push_front((key, value.clone()));
         value
     }
@@ -99,34 +106,78 @@ async fn process_logs(
 
     let log_uuid = format!("{owner}/{repo}${log_id}");
 
-    let logs = 'logs: {
+    let CachedLog {
+        job,
+        tree_roots,
+        logs,
+    } = &*'logs: {
         if let Some(logs) = ctx.gha_logs.write().await.get(&log_uuid) {
             tracing::info!("gha_logs: cache hit for {log_uuid}");
             break 'logs logs;
         }
 
         tracing::info!("gha_logs: cache miss for {log_uuid}");
-        let logs = ctx
-            .github
-            .raw_job_logs(
-                &github::IssueRepository {
-                    organization: owner.to_string(),
-                    repository: repo.to_string(),
-                },
-                log_id,
-            )
-            .await
-            .context("unable to get the raw logs")?;
-
-        let json_logs = serde_json::to_string(&*logs).context("unable to JSON-ify the raw logs")?;
-
-        ctx.gha_logs
-            .write()
-            .await
-            .put(log_uuid.clone(), json_logs.into())
+
+        let repo = github::IssueRepository {
+            organization: owner.to_string(),
+            repository: repo.to_string(),
+        };
+
+        let job_and_tree_roots = async {
+            let job = ctx
+                .github
+                .workflow_run_job(&repo, log_id)
+                .await
+                .context("unable to fetch job details")?;
+            let trees = ctx
+                .github
+                .repo_git_trees(&repo, &job.head_sha)
+                .await
+                .context("unable to fetch git tree for the repository")?;
+
+            // To minimize false positives in paths linked to the GitHub repositories,
+            // we restrict matching to only the top-level directories of the repository.
+            // We achieve this by retrieving all "tree" objects and concatenating them
+            // into a regex OR pattern (e.g., `compiler|tests|src`) which is used in the
+            // JS regex.
+            let tree_roots = trees
+                .tree
+                .iter()
+                .filter_map(|t| (t.object_type == "tree").then_some(&t.path))
+                .join("|");
+
+            anyhow::Result::<_>::Ok((job, tree_roots))
+        };
+
+        let logs = async {
+            let logs = ctx
+                .github
+                .raw_job_logs(&repo, log_id)
+                .await
+                .context("unable to get the raw logs")?;
+
+            let json_logs =
+                serde_json::to_string(&*logs).context("unable to JSON-ify the raw logs")?;
+
+            anyhow::Result::<_>::Ok(json_logs)
+        };
+
+        let (job_and_tree_roots, logs) = futures::join!(job_and_tree_roots, logs);
+        let ((job, tree_roots), logs) = (job_and_tree_roots?, logs?);
+
+        ctx.gha_logs.write().await.put(
+            log_uuid.clone(),
+            CachedLog {
+                job,
+                tree_roots,
+                logs,
+            }
+            .into(),
+        )
     };
 
     let nonce = Uuid::new_v4().to_hyphenated().to_string();
+    let sha = &*job.head_sha;
 
     let html = format!(
         r###"<!DOCTYPE html>
@@ -157,6 +208,9 @@ async fn process_logs(
         .warning-marker {{
             color: #c69026;
         }}
+        .path-marker {{
+            color: #26c6a8;
+        }}
     </style>
     <script type="module" nonce="{nonce}">
         import {{ AnsiUp }} from '{ANSI_UP_URL}'
@@ -189,11 +243,30 @@ async fn process_logs(
             `<span class="warning-marker">##[warning]</span>`
         );
 
-        // 5. Add the html to the DOM
+        // 5. Add anchors around some paths
+        //  Detailed examples of what the regex does is at https://regex101.com/r/vCnx9Y/2
+        //
+        //  But simply speaking the regex tries to find absolute (with `/checkout` prefix) and
+        //  relative paths, the path must start with one of the repository top-level directory.
+        //  We also try to retrieve the lines and cols if given (`<path>:line:col`).
+        //
+        //  Some examples of paths we want to find:
+        //   - src/tools/test-float-parse/src/traits.rs:173:11
+        //   - /checkout/compiler/rustc_macros
+        //   - /checkout/src/doc/rustdoc/src/advanced-features.md
+        //
+        //  Any other paths, in particular if prefixed by `./` or `obj/` should not taken.
+        const pathRegex = /(?<boundary>[^a-zA-Z0-9.\\/])(?<inner>(?:[\\\/]?(?:checkout[\\\/])?(?<path>(?:{tree_roots})[\\\/][a-zA-Z0-9_$\-.\\\/]+))(?::(?<line>[0-9]+):(?<col>[0-9]+))?)/g;
+        html = html.replace(pathRegex, (match, boundary, inner, path, line, col) => {{
+            const pos = (line !== undefined) ? `#L${{line}}` : "";
+            return `${{boundary}}<a href="https://github.com/{owner}/{repo}/blob/{sha}/${{path}}${{pos}}" class="path-marker">${{inner}}</a>`;
+        }});
+
+        // 6. Add the html to the DOM
         var cdiv = document.getElementById("console");
         cdiv.innerHTML = html;
         
-        // 6. If no anchor is given, scroll to the last error
+        // 7. If no anchor is given, scroll to the last error
         if (location.hash === "" && errorCounter >= 0) {{
             const hasSmallViewport = window.innerWidth <= 750;
             document.getElementById(`error-${{errorCounter}}`).scrollIntoView({{

src/github.rs

@@ -232,6 +232,28 @@ impl GithubClient {
             .context("failed to retrieve job logs")?;
         Ok(String::from_utf8_lossy(&body).to_string())
     }
+
+    pub async fn workflow_run_job(
+        &self,
+        repo: &IssueRepository,
+        job_id: u128,
+    ) -> anyhow::Result<WorkflowRunJob> {
+        let url = format!("{}/actions/jobs/{job_id}", repo.url(&self));
+        self.json(self.get(&url))
+            .await
+            .context("failed to retrive workflow job run details")
+    }
+
+    pub async fn repo_git_trees(
+        &self,
+        repo: &IssueRepository,
+        sha: &str,
+    ) -> anyhow::Result<GitTrees> {
+        let url = format!("{}/git/trees/{sha}", repo.url(&self));
+        self.json(self.get(&url))
+            .await
+            .context("failed to retrive git trees")
+    }
 }
 
 #[derive(Debug, serde::Serialize)]
@@ -1240,6 +1262,11 @@ impl IssuesEvent {
 #[derive(Debug, serde::Deserialize)]
 struct PullRequestEventFields {}
 
+#[derive(Debug, serde::Deserialize)]
+pub struct WorkflowRunJob {
+    pub head_sha: String,
+}
+
 #[derive(Clone, Debug, serde::Deserialize)]
 pub struct CommitBase {
     pub sha: String,
@@ -2829,6 +2856,12 @@ pub struct GitTreeObject {
     pub sha: String,
 }
 
+#[derive(Debug, serde::Deserialize)]
+pub struct GitTrees {
+    pub sha: String,
+    pub tree: Vec<GitTreeEntry>,
+}
+
 #[derive(Debug, serde::Serialize, serde::Deserialize)]
 pub struct GitTreeEntry {
     pub path: String,