From bde733890aeea0a1bbbdcbc1ad14fe05ed58661a Mon Sep 17 00:00:00 2001 From: Teddy Wing Date: Thu, 2 Jun 2022 21:27:33 +0200 Subject: git.rs: Add ideas for new error variants These variants should make it easier to trace where in the code that a particular error occurred, and include a context-descriptive error message. --- src/git.rs | 13 +++++++++++++ 1 file changed, 13 insertions(+) (limited to 'src') diff --git a/src/git.rs b/src/git.rs index ccc0922..684cb66 100644 --- a/src/git.rs +++ b/src/git.rs @@ -25,6 +25,19 @@ use std::path::Path; #[derive(Debug, thiserror::Error)] pub enum Error { + MirrorCreateRepo(), + MirrorConfig(), + // What is remote_with_fetch? + // A: Add a remote with the provided refspec to the repository's config + MirrorFetch(), + + UpdateOpenRepo(), + UpdateGetRemotes(), + UpdateFindRemote(), + UpdateFetch(), + + GitChangeBranch(), + #[error("git error")] Git(#[from] git2::Error), -- cgit v1.2.3 From 079ca8b33657da216a8d08a9fae0fd54e488ec71 Mon Sep 17 00:00:00 2001 From: Teddy Wing Date: Thu, 2 Jun 2022 21:33:20 +0200 Subject: git.rs: Add `Error::MirrorAddRemote` variant --- src/git.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) (limited to 'src') diff --git a/src/git.rs b/src/git.rs index 684cb66..66e91fc 100644 --- a/src/git.rs +++ b/src/git.rs @@ -27,8 +27,7 @@ use std::path::Path; pub enum Error { MirrorCreateRepo(), MirrorConfig(), - // What is remote_with_fetch? - // A: Add a remote with the provided refspec to the repository's config + MirrorAddRemote(), MirrorFetch(), UpdateOpenRepo(), -- cgit v1.2.3 From 0951bb614aa1602983dacbf604c5a52e44d49bc3 Mon Sep 17 00:00:00 2001 From: Teddy Wing Date: Thu, 2 Jun 2022 23:22:39 +0200 Subject: git.rs: Ideas for error structure and context --- src/git.rs | 23 ++++++++++++++++++++--- 1 file changed, 20 insertions(+), 3 deletions(-) (limited to 'src') diff --git a/src/git.rs b/src/git.rs index 66e91fc..59a80b9 100644 --- a/src/git.rs +++ b/src/git.rs @@ -25,16 +25,32 @@ use std::path::Path; #[derive(Debug, thiserror::Error)] pub enum Error { - MirrorCreateRepo(), - MirrorConfig(), + #[error("cannot create repo '{path}'")] + MirrorCreateRepo { + #[from] + source: git2::Error, + path: String, + }, + #[error("")] MirrorAddRemote(), + #[error("")] + MirrorConfig(#[from] git2::Error), + #[error("")] + MirrorRemoteEnableMirror(), + #[error("")] MirrorFetch(), + #[error("")] UpdateOpenRepo(), + #[error("")] UpdateGetRemotes(), + #[error("")] UpdateFindRemote(), + #[error("")] UpdateFetch(), + + #[error("")] GitChangeBranch(), #[error("git error")] @@ -68,7 +84,8 @@ pub fn mirror>( // Mac OS. .external_template(false) .description(description), - )?; + ) + .map_err(|e| Error::MirrorCreateRepo{e, path})?; let remote_name = "origin"; -- cgit v1.2.3 From b8a188e19e7ec0ae381c18a5a4db86fb576bf95a Mon Sep 17 00:00:00 2001 From: Teddy Wing Date: Fri, 3 Jun 2022 00:50:17 +0200 Subject: git.rs: Add more context to errors Add full definitions for our new error variant ideas. Use a distinct variant and message in each error case in order to trace errors to the line of code where they occur. --- src/git.rs | 139 ++++++++++++++++++++++++++++++++++++++++++++----------------- 1 file changed, 101 insertions(+), 38 deletions(-) (limited to 'src') diff --git a/src/git.rs b/src/git.rs index 59a80b9..5f3c91c 100644 --- a/src/git.rs +++ b/src/git.rs @@ -25,33 +25,57 @@ use std::path::Path; #[derive(Debug, thiserror::Error)] pub enum Error { - #[error("cannot create repo '{path}'")] + #[error("mirror: cannot create repo '{path}'")] MirrorCreateRepo { - #[from] source: git2::Error, path: String, }, - #[error("")] - MirrorAddRemote(), - #[error("")] - MirrorConfig(#[from] git2::Error), - #[error("")] - MirrorRemoteEnableMirror(), - #[error("")] - MirrorFetch(), - - #[error("")] - UpdateOpenRepo(), - #[error("")] - UpdateGetRemotes(), - #[error("")] - UpdateFindRemote(), - #[error("")] - UpdateFetch(), - - - #[error("")] - GitChangeBranch(), + #[error("mirror: cannot add remote '{remote_name}:{url}'")] + MirrorAddRemote { + source: git2::Error, + remote_name: String, + url: String, + }, + #[error("mirror: cannot get repo config")] + MirrorConfigGet(#[source] git2::Error), + #[error("mirror: cannot set 'mirror' flag on remote '{remote_name}'")] + MirrorRemoteEnableMirror { + source: git2::Error, + remote_name: String, + }, + #[error("mirror: cannot fetch from remote '{remote_name}'")] + MirrorFetch { + source: git2::Error, + remote_name: String, + }, + + #[error("update: cannot open repo '{path}'")] + UpdateOpenRepo { + source: git2::Error, + path: String, + }, + #[error("update: cannot get remotes for '{path}'")] + UpdateGetRemotes { + source: git2::Error, + path: String, + }, + #[error("update: cannot find remote '{remote_name}'")] + UpdateFindRemote { + source: git2::Error, + remote_name: String, + }, + #[error("update: cannot fetch from remote '{remote_name}")] + UpdateFetch { + source: git2::Error, + remote_name: String, + }, + + #[error("{action}: cannot switch to branch '{branch}'")] + GitChangeBranch { + source: git2::Error, + action: String, + branch: String, + }, #[error("git error")] Git(#[from] git2::Error), @@ -68,7 +92,7 @@ pub enum Error { /// ```shell /// git clone --mirror URL /// ``` -pub fn mirror>( +pub fn mirror + Copy>( url: &str, path: P, description: &str, @@ -85,7 +109,10 @@ pub fn mirror>( .external_template(false) .description(description), ) - .map_err(|e| Error::MirrorCreateRepo{e, path})?; + .map_err(|e| Error::MirrorCreateRepo { + source: e, + path: format!("{}", path.as_ref().display()), + })?; let remote_name = "origin"; @@ -93,19 +120,38 @@ pub fn mirror>( remote_name, url, "+refs/*:refs/*", - )?; - - let mut config = repo.config()?; + ) + .map_err(|e| Error::MirrorAddRemote { + source: e, + remote_name: remote_name.to_owned(), + url: url.to_owned(), + })?; + + let mut config = repo.config() + .map_err(|e| Error::MirrorConfigGet(e))?; config.set_bool( &format!("remote.{}.mirror", remote_name), true, - )?; + ) + .map_err(|e| Error::MirrorRemoteEnableMirror { + source: e, + remote_name: remote_name.to_owned(), + })?; let refspecs: [&str; 0] = []; - remote.fetch(&refspecs, None, None)?; + remote.fetch(&refspecs, None, None) + .map_err(|e| Error::MirrorFetch { + source: e, + remote_name: remote_name.to_owned(), + })?; if default_branch != "master" { - repo_change_current_branch(&repo, default_branch)?; + repo_change_current_branch(&repo, default_branch) + .map_err(|e| Error::GitChangeBranch { + source: e, + action: "mirror".to_owned(), + branch: default_branch.to_owned(), + })?; } Ok(()) @@ -118,14 +164,27 @@ pub fn mirror>( /// ```shell /// git remote update /// ``` -pub fn update>( +pub fn update + Copy>( path: P, ) -> Result<(), Error> { - let repo = git2::Repository::open_bare(path)?; - - for remote_opt in &repo.remotes()? { + let repo = git2::Repository::open_bare(path) + .map_err(|e| Error::UpdateOpenRepo { + source: e, + path: format!("{}", path.as_ref().display()), + })?; + + let remotes = &repo.remotes() + .map_err(|e| Error::UpdateGetRemotes { + source: e, + path: format!("{}", path.as_ref().display()), + })?; + for remote_opt in remotes { if let Some(remote_name) = remote_opt { - let mut remote = repo.find_remote(remote_name)?; + let mut remote = repo.find_remote(remote_name) + .map_err(|e| Error::UpdateFindRemote { + source: e, + remote_name: remote_name.to_owned(), + })?; let mut fetch_options = git2::FetchOptions::new(); fetch_options @@ -133,7 +192,11 @@ pub fn update>( .download_tags(git2::AutotagOption::All); let refspecs: [&str; 0] = []; - remote.fetch(&refspecs, Some(&mut fetch_options), None)?; + remote.fetch(&refspecs, Some(&mut fetch_options), None) + .map_err(|e| Error::UpdateFetch { + source: e, + remote_name: remote_name.to_owned(), + })?; } } @@ -178,7 +241,7 @@ pub fn change_current_branch>( fn repo_change_current_branch( repo: &git2::Repository, default_branch: &str, -) -> Result<(), Error> { +) -> Result<(), git2::Error> { Ok( repo.set_head( &format!("refs/heads/{}", default_branch), -- cgit v1.2.3 From 996426344348fb72b283a2df18a981d073aecad8 Mon Sep 17 00:00:00 2001 From: Teddy Wing Date: Sat, 4 Jun 2022 05:25:48 +0200 Subject: github::Repo: Change `git_url` to `clone_url` I was getting errors mirroring and updating: failed to connect to github.com: Connection timed out; class=Os (2) and remote 'origin' already exists; class=Config (7); code=Exists(-4) It turns out that the `git_url` field, which I had been using previously to mirror and clone repositories, stopped working. My guess is that it's because Reflectub is not authorised to clone GitHub "git://" URLs, so the connection timed out. I'm not sure why this stopped being allowed, though. The URL change seems to have happened around March 2022, or at least between December 2021 and April 2022. The second error was caused by a previously-created repository existing in the filesystem, but not being in the database as it hadn't been correctly mirrored. For now, I've decided not to fix that problem and am only fixing the URL issue. The GitHub API also includes a `clone_url` field, which contains an HTTPS clone URL. Using this URL to mirror fixes the timeout problem. --- src/github.rs | 2 +- src/main.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) (limited to 'src') diff --git a/src/github.rs b/src/github.rs index a3bb74e..7af0d93 100644 --- a/src/github.rs +++ b/src/github.rs @@ -43,7 +43,7 @@ pub struct Repo { pub name: String, pub description: Option, pub fork: bool, - pub git_url: String, + pub clone_url: String, pub default_branch: String, pub size: u64, pub updated_at: String, diff --git a/src/main.rs b/src/main.rs index 6ff441b..2360f57 100644 --- a/src/main.rs +++ b/src/main.rs @@ -241,7 +241,7 @@ where P2: AsRef, { git::mirror( - &repo.git_url, + &repo.clone_url, &clone_path, repo.description(), &repo.default_branch, -- cgit v1.2.3 From 34c69bd00249d78e9277bb66c6c0bf7e5b4613a1 Mon Sep 17 00:00:00 2001 From: Teddy Wing Date: Sat, 4 Jun 2022 14:25:32 +0200 Subject: Update copyright year --- src/git.rs | 2 +- src/github.rs | 2 +- src/main.rs | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) (limited to 'src') diff --git a/src/git.rs b/src/git.rs index 5f3c91c..e5fdd75 100644 --- a/src/git.rs +++ b/src/git.rs @@ -1,4 +1,4 @@ -// Copyright (c) 2021 Teddy Wing +// Copyright (c) 2021, 2022 Teddy Wing // // This file is part of Reflectub. // diff --git a/src/github.rs b/src/github.rs index 7af0d93..c48dda6 100644 --- a/src/github.rs +++ b/src/github.rs @@ -1,4 +1,4 @@ -// Copyright (c) 2021 Teddy Wing +// Copyright (c) 2021, 2022 Teddy Wing // // This file is part of Reflectub. // diff --git a/src/main.rs b/src/main.rs index 2360f57..61996e8 100644 --- a/src/main.rs +++ b/src/main.rs @@ -1,4 +1,4 @@ -// Copyright (c) 2021 Teddy Wing +// Copyright (c) 2021, 2022 Teddy Wing // // This file is part of Reflectub. // -- cgit v1.2.3 From b324a7a96acbaf589fedca245e9e5659ee9cd8fa Mon Sep 17 00:00:00 2001 From: Teddy Wing Date: Sat, 4 Jun 2022 18:14:08 +0200 Subject: database::Repo: From<&github::Repo>: Use newest update date It turns out the GitHub `updated_at` field doesn't change when new commits are pushed to the repository, only when the repository config etc. changes. In order to update the mirrors when any update happens in the repository, we need to look at both of those date values to see if they've been updated. Take the most recent of `updated_at` or `pushed_at` and set it to `(database::Repo).updated_at`. This allows us to refresh the repo when either of those dates change, catching all GitHub repo updates. --- src/database.rs | 22 +++++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-) (limited to 'src') diff --git a/src/database.rs b/src/database.rs index ed5d327..7e7a065 100644 --- a/src/database.rs +++ b/src/database.rs @@ -44,12 +44,32 @@ impl Repo { impl From<&github::Repo> for Repo { fn from(repo: &github::Repo) -> Self { + use chrono::DateTime; + + let repo_updated_at = DateTime::parse_from_rfc3339(&repo.updated_at).ok(); + let repo_pushed_at = DateTime::parse_from_rfc3339(&repo.pushed_at).ok(); + + // Set `updated_at` to the most recent of `repo_updated_at` or + // `repo_pushed_at`. + let updated_at = + if repo_updated_at.is_none() && repo_pushed_at.is_none() { + repo.updated_at.clone() + + // `repo_updated_at` and `repo_pushed_at` are both Some. + } else if repo_pushed_at.unwrap() > repo_updated_at.unwrap() { + repo.pushed_at.clone() + + // Default to `repo.updated_at`. + } else { + repo.updated_at.clone() + }; + Self { id: repo.id, name: Some(repo.name.clone()), description: repo.description.clone(), default_branch: Some(repo.default_branch.clone()), - updated_at: Some(repo.updated_at.clone()), + updated_at: Some(updated_at), } } } -- cgit v1.2.3 From 6fe38d83ec90f5adb411706ed23d9a8b2929aed8 Mon Sep 17 00:00:00 2001 From: Teddy Wing Date: Sat, 4 Jun 2022 18:59:25 +0200 Subject: database: Update copyright year --- src/database.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'src') diff --git a/src/database.rs b/src/database.rs index 7e7a065..09fedd7 100644 --- a/src/database.rs +++ b/src/database.rs @@ -1,4 +1,4 @@ -// Copyright (c) 2021 Teddy Wing +// Copyright (c) 2021, 2022 Teddy Wing // // This file is part of Reflectub. // -- cgit v1.2.3