fix(gitforge): use local repo or clone to allow git push

The `git push` command requires a git directory (bare at a minimum)
before it can work.
The git dir must also be up-to-date with the relevant references from
the remote, so
we do a `git fetch` before a `git push`.

Closes kemitix/git-next#51

src/server/actors/repo/branch.rs

@@ -1,6 +1,13 @@
+use std::time::Duration;
+
+use actix::prelude::*;
+
 use tracing::{info, warn};
 
-use crate::server::{config, gitforge};
+use crate::server::{
+    actors::repo::{RepoActor, ValidateRepo},
+    config, gitforge,
+};
 
 // advance next to the next commit towards the head of the dev branch
 #[tracing::instrument(fields(next), skip_all)]
@@ -9,6 +16,7 @@ pub async fn advance_next(
     dev_commit_history: Vec<gitforge::Commit>,
     repo_config: config::RepoConfig,
     forge: gitforge::Forge,
+    addr: Addr<super::RepoActor>,
 ) {
     let next_commit = find_next_commit_on_dev(next, dev_commit_history);
     let Some(commit) = next_commit else {
@@ -27,6 +35,8 @@ pub async fn advance_next(
     ) {
         warn!(?err, "Failed")
     }
+    tokio::time::sleep(Duration::from_secs(10)).await;
+    addr.do_send(ValidateRepo)
 }
 
 #[tracing::instrument]
@@ -67,6 +77,7 @@ pub async fn advance_main(
     next: gitforge::Commit,
     repo_config: config::RepoConfig,
     forge: gitforge::Forge,
+    addr: Addr<RepoActor>,
 ) {
     info!("Advancing main to next");
     if let Err(err) = forge.branch_reset(
@@ -76,6 +87,7 @@ pub async fn advance_main(
     ) {
         warn!(?err, "Failed")
     };
+    addr.do_send(ValidateRepo)
 }
 
 #[cfg(test)]

src/server/actors/repo/mod.rs

@@ -65,18 +65,28 @@ impl Actor for RepoActor {
         }
     }
 }
+impl std::fmt::Display for RepoActor {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        write!(
+            f,
+            "RepoActor: {}/{}",
+            self.details.forge.forge_name, self.details.repo_alias
+        )
+    }
+}
 
 #[derive(Message)]
 #[rtype(result = "()")]
 pub struct CloneRepo;
 impl Handler<CloneRepo> for RepoActor {
     type Result = ();
+    #[tracing::instrument(skip_all, fields(%self))]
     fn handle(&mut self, _msg: CloneRepo, ctx: &mut Self::Context) -> Self::Result {
         info!(%self.details, "Clone/Update Repo");
         let gitdir = self.details.gitdir.clone();
         match self.forge.repo_clone(gitdir) {
             Ok(_) => ctx.address().do_send(LoadConfigFromRepo),
-            Err(err) => warn!(?err, "Could not Clone repo"),
+            Err(err) => warn!("Could not Clone repo: {err}"),
         }
     }
 }
@@ -86,6 +96,7 @@ impl Handler<CloneRepo> for RepoActor {
 pub struct LoadConfigFromRepo;
 impl Handler<LoadConfigFromRepo> for RepoActor {
     type Result = ();
+    #[tracing::instrument(skip_all, fields(%self))]
     fn handle(&mut self, _msg: LoadConfigFromRepo, ctx: &mut Self::Context) -> Self::Result {
         info!(%self.details, "Loading .git-next.toml from repo");
         let details = self.details.clone();
@@ -102,6 +113,7 @@ impl Handler<LoadConfigFromRepo> for RepoActor {
 struct LoadedConfig(pub RepoConfig);
 impl Handler<LoadedConfig> for RepoActor {
     type Result = ();
+    #[tracing::instrument(skip_all, fields(%self))]
     fn handle(&mut self, msg: LoadedConfig, ctx: &mut Self::Context) -> Self::Result {
         let repo_config = msg.0;
         info!(%self.details, %repo_config, "Config loaded");
@@ -125,7 +137,9 @@ impl Handler<LoadedConfig> for RepoActor {
 pub struct ValidateRepo;
 impl Handler<ValidateRepo> for RepoActor {
     type Result = ();
+    #[tracing::instrument(skip_all, fields(%self))]
     fn handle(&mut self, _msg: ValidateRepo, ctx: &mut Self::Context) -> Self::Result {
+        info!("ValidateRepo");
         if let Some(repo_config) = self.details.repo_config.clone() {
             let forge = self.forge.clone();
             let addr = ctx.address();
@@ -146,7 +160,9 @@ pub struct StartMonitoring {
 }
 impl Handler<StartMonitoring> for RepoActor {
     type Result = ();
+    #[tracing::instrument(skip_all, fields(%self))]
     fn handle(&mut self, msg: StartMonitoring, ctx: &mut Self::Context) -> Self::Result {
+        info!("StartMonitoring");
         let Some(repo_config) = self.details.repo_config.clone() else {
             warn!("No config loaded");
             return;
@@ -167,7 +183,7 @@ impl Handler<StartMonitoring> for RepoActor {
                 .into_actor(self)
                 .wait(ctx);
         } else if dev_ahead_of_next {
-            branch::advance_next(msg.next, msg.dev_commit_history, repo_config, forge)
+            branch::advance_next(msg.next, msg.dev_commit_history, repo_config, forge, addr)
                 .into_actor(self)
                 .wait(ctx);
         } else if self.webhook_id.is_none() {
@@ -183,6 +199,7 @@ impl Handler<StartMonitoring> for RepoActor {
 pub struct WebhookRegistered(pub WebhookId, pub WebhookAuth);
 impl Handler<WebhookRegistered> for RepoActor {
     type Result = ();
+    #[tracing::instrument(skip_all, fields(%self))]
     fn handle(&mut self, msg: WebhookRegistered, _ctx: &mut Self::Context) -> Self::Result {
         self.webhook_id.replace(msg.0);
         self.webhook_auth.replace(msg.1);
@@ -194,13 +211,15 @@ impl Handler<WebhookRegistered> for RepoActor {
 pub struct AdvanceMainTo(pub gitforge::Commit);
 impl Handler<AdvanceMainTo> for RepoActor {
     type Result = ();
+    #[tracing::instrument(skip_all, fields(%self))]
     fn handle(&mut self, msg: AdvanceMainTo, ctx: &mut Self::Context) -> Self::Result {
         let Some(repo_config) = self.details.repo_config.clone() else {
             warn!("No config loaded");
             return;
         };
         let forge = self.forge.clone();
-        branch::advance_main(msg.0, repo_config, forge)
+        let addr = ctx.address();
+        branch::advance_main(msg.0, repo_config, forge, addr)
             .into_actor(self)
             .wait(ctx);
     }

src/server/config/mod.rs

@@ -10,6 +10,7 @@ use std::{
 use serde::Deserialize;
 
 use kxio::fs::FileSystem;
+use tracing::info;
 
 #[derive(Debug, derive_more::From, derive_more::Display)]
 pub enum Error {
@@ -203,6 +204,11 @@ impl ServerRepoConfig {
     pub fn branch(&self) -> BranchName {
         BranchName(self.branch.clone())
     }
+
+    pub fn gitdir(&self) -> Option<GitDir> {
+        self.gitdir.clone().map(GitDir::from)
+    }
+
     /// Returns a RepoConfig from the server configuration if ALL THREE branches were provided
     pub fn repo_config(&self) -> Option<RepoConfig> {
         match (&self.main, &self.next, &self.dev) {
@@ -350,22 +356,14 @@ pub struct RepoDetails {
 }
 impl RepoDetails {
     pub fn new(
-        name: &RepoAlias,
+        repo_alias: &RepoAlias,
         server_repo_config: &ServerRepoConfig,
         forge_name: &ForgeName,
         forge_config: &ForgeConfig,
-        server_storage: &ServerStorage,
+        gitdir: GitDir,
-    ) -> Result<Self> {
+    ) -> Self {
-        let path_buf = server_repo_config.gitdir.clone().unwrap_or_else(|| {
+        Self {
-            server_storage
+            repo_alias: repo_alias.clone(),
-                .path
-                .join(forge_name.to_string())
-                .join(name.to_string())
-        });
-        let path_buf = std::fs::canonicalize(path_buf)?;
-        let gitdir = GitDir(path_buf);
-        Ok(Self {
-            repo_alias: name.clone(),
             repo_path: RepoPath(server_repo_config.repo.clone()),
             repo_config: server_repo_config.repo_config(),
             branch: BranchName(server_repo_config.branch.clone()),
@@ -377,7 +375,7 @@ impl RepoDetails {
                 user: forge_config.user(),
                 token: forge_config.token(),
             },
-        })
+        }
     }
     pub fn origin(&self) -> secrecy::Secret<String> {
         let repo_details = self;
@@ -396,27 +394,6 @@ impl RepoDetails {
         self.gitdir.validate(self)
     }
 
-    pub fn find_default_push_remote(&self) -> ValidationResult<GitRemote> {
-        let repository = gix::open(self.gitdir.clone())
-            .map_err(|e| RepoValidationError::UnableToOpenRepo(e.to_string()))?;
-        let Some(Ok(remote)) = repository.find_default_remote(gix::remote::Direction::Push) else {
-            return Err(RepoValidationError::NoDefaultPushRemote);
-        };
-        let Some(url) = remote.url(gix::remote::Direction::Push) else {
-            return Err(RepoValidationError::NoUrlForDefaultPushRemote);
-        };
-        let Some(host) = url.host() else {
-            return Err(RepoValidationError::NoHostnameForDefaultPushRemote);
-        };
-        let path = url.path.to_string();
-        let path = path.strip_prefix('/').map_or(path.as_str(), |path| path);
-        let path = path.strip_suffix(".git").map_or(path, |path| path);
-        Ok(GitRemote::new(
-            Hostname(host.to_string()),
-            RepoPath(path.to_string()),
-        ))
-    }
-
     pub fn git_remote(&self) -> GitRemote {
         GitRemote::new(self.forge.hostname.clone(), self.repo_path.clone())
     }
@@ -447,7 +424,11 @@ pub enum RepoValidationError {
     Io(std::io::Error),
     MismatchDefaultPushRemote {
         found: GitRemote,
-        configured: GitRemote,
+        expected: GitRemote,
+    },
+    MismatchDefaultFetchRemote {
+        found: GitRemote,
+        expected: GitRemote,
     },
 }
 impl std::error::Error for RepoValidationError {}
@@ -461,8 +442,14 @@ impl Display for RepoValidationError {
             }
             Self::UnableToOpenRepo(err) => write!(f, "Unable to open the git dir: {err}"),
             Self::Io(err) => write!(f, "IO Error: {err:?}"),
-            Self::MismatchDefaultPushRemote{found, configured} =>
+            Self::MismatchDefaultPushRemote { found, expected } => write!(
-            write!(f, "The default push remote for the git dir doesn't match the configuration: found: {found}, expected: {configured}")
+                f,
+                "The default push remote doesn't match: {found}, expected: {expected}"
+            ),
+            Self::MismatchDefaultFetchRemote { found, expected } => write!(
+                f,
+                "The default fetch remote doesn't match: {found}, expected: {expected}"
+            ),
         }
     }
 }
@@ -490,6 +477,7 @@ pub struct GitDir(PathBuf);
 impl GitDir {
     #[cfg(test)]
     pub(crate) fn new(pathbuf: &std::path::Path) -> Self {
+        info!("GitDir::new({pathbuf:?})");
         Self(pathbuf.to_path_buf())
     }
 
@@ -498,13 +486,53 @@ impl GitDir {
     }
 
     pub fn validate(&self, repo_details: &RepoDetails) -> ValidationResult<()> {
-        let configured = repo_details.git_remote();
+        let git_remote = repo_details.git_remote();
-        let found = repo_details.find_default_push_remote()?;
+        use gix::remote::Direction;
-        if configured != found {
+        let push_remote = self.find_default_remote(Direction::Push)?;
-            return Err(RepoValidationError::MismatchDefaultPushRemote { found, configured });
+        let fetch_remote = self.find_default_remote(Direction::Fetch)?;
+        info!(gitdir = %self, ?git_remote, ?push_remote, ?fetch_remote, "Gitdir::validate");
+        if git_remote != push_remote {
+            return Err(RepoValidationError::MismatchDefaultPushRemote {
+                found: push_remote,
+                expected: git_remote,
+            });
+        }
+        if git_remote != fetch_remote {
+            return Err(RepoValidationError::MismatchDefaultFetchRemote {
+                found: fetch_remote,
+                expected: git_remote,
+            });
         }
         Ok(())
     }
+
+    #[tracing::instrument]
+    fn find_default_remote(
+        &self,
+        direction: gix::remote::Direction,
+    ) -> ValidationResult<GitRemote> {
+        let repository = gix::ThreadSafeRepository::open(self.deref())
+            .map_err(|e| RepoValidationError::UnableToOpenRepo(e.to_string()))?
+            .to_thread_local();
+        info!(?repository, from = ?self.deref(), "gix::discover");
+        let Some(Ok(remote)) = repository.find_default_remote(gix::remote::Direction::Push) else {
+            return Err(RepoValidationError::NoDefaultPushRemote);
+        };
+        let Some(url) = remote.url(direction) else {
+            return Err(RepoValidationError::NoUrlForDefaultPushRemote);
+        };
+        let Some(host) = url.host() else {
+            return Err(RepoValidationError::NoHostnameForDefaultPushRemote);
+        };
+        let path = url.path.to_string();
+        let path = path.strip_prefix('/').map_or(path.as_str(), |path| path);
+        let path = path.strip_suffix(".git").map_or(path, |path| path);
+        info!(%host, %path, "found");
+        Ok(GitRemote::new(
+            Hostname(host.to_string()),
+            RepoPath(path.to_string()),
+        ))
+    }
 }
 impl Deref for GitDir {
     type Target = PathBuf;
@@ -520,9 +548,16 @@ impl std::fmt::Display for GitDir {
 }
 impl From<&str> for GitDir {
     fn from(value: &str) -> Self {
+        info!("GitDir::from::<&str>({value:?})");
         Self(value.into())
     }
 }
+impl From<PathBuf> for GitDir {
+    fn from(value: PathBuf) -> Self {
+        info!("GitDir::from::<PathBuf>({value:?})");
+        Self(value)
+    }
+}
 impl From<GitDir> for PathBuf {
     fn from(value: GitDir) -> Self {
         value.0

src/server/config/tests.rs

@@ -1,4 +1,5 @@
 use assert2::let_assert;
+use gix::remote::Direction;
 use pretty_assertions::assert_eq;
 
 use crate::{server::gitforge::tests::common /* server::gitforge::tests::common */};
@@ -162,7 +163,8 @@ fn repo_details_find_default_push_remote_finds_correct_remote() -> Result<()> {
     );
     repo_details.forge.hostname = Hostname("git.kemitix.net".to_string());
     repo_details.repo_path = RepoPath("kemitix/git-next".to_string());
-    let found_git_remote = repo_details.find_default_push_remote()?;
+    let gitdir = &repo_details.gitdir;
+    let found_git_remote = gitdir.find_default_remote(Direction::Push)?;
     let config_git_remote = repo_details.git_remote();
 
     assert_eq!(

src/server/gitforge/forgejo/branch/fetch.rs

@@ -0,0 +1,72 @@
+use std::ops::Deref;
+
+use tracing::info;
+
+use crate::server::config::RepoDetails;
+
+#[derive(Debug, derive_more::From, derive_more::Display)]
+pub enum Error {
+    UnableToOpenRepo(Box<gix::open::Error>),
+    NoFetchRemoteFound,
+    Connect(Box<gix::remote::connect::Error>),
+    Fetch(String),
+}
+impl std::error::Error for Error {}
+
+#[tracing::instrument]
+pub fn fetch(repo_details: &RepoDetails) -> Result<(), Error> {
+    // INFO: gitdir validate tests that the default fetch remote matches the configured remote
+    let repository = gix::ThreadSafeRepository::open(repo_details.gitdir.deref())
+        .map_err(Box::new)?
+        .to_thread_local();
+    info!(?repository, "opened repo");
+    let Some(remote) = repository.find_default_remote(gix::remote::Direction::Fetch) else {
+        return Err(Error::NoFetchRemoteFound);
+    };
+    info!(?remote, "fetch remote");
+
+    remote
+        .map_err(|e| Error::Fetch(e.to_string()))?
+        .connect(gix::remote::Direction::Fetch)
+        .map_err(Box::new)?
+        .prepare_fetch(gix::progress::Discard, Default::default())
+        .map_err(|e| Error::Fetch(e.to_string()))?
+        .receive(gix::progress::Discard, &Default::default())
+        .map_err(|e| Error::Fetch(e.to_string()))?;
+
+    info!("fetched");
+
+    Ok(())
+
+    // INFO: never log the command as it contains the API token within the 'origin'
+    // let command: secrecy::Secret<String> = format!(
+    //     "/usr/bin/git push {} {to_commit}:{branch_name} {force}",
+    //     origin.expose_secret()
+    // )
+    // .into();
+    // info!("Resetting {branch_name} to {to_commit}");
+    // let ctx = gix::diff::command::Context {
+    //     git_dir: Some(repo_details.gitdir.deref().clone()),
+    //     ..Default::default()
+    // };
+    // // info!(?ctx, command = command.expose_secret(), "prepare");
+    // match gix::command::prepare(command.expose_secret())
+    //     .with_context(ctx)
+    //     .with_shell_allow_argument_splitting()
+    //     // .stdout(std::process::Stdio::null())
+    //     // .stderr(std::process::Stdio::null())
+    //     .spawn()
+    // {
+    //     Ok(mut child) => match child.wait() {
+    //         Ok(_) => Ok(()),
+    //         Err(err) => {
+    //             warn!(?err, "Failed (wait)");
+    //             Err(())
+    //         }
+    //     },
+    //     Err(err) => {
+    //         warn!(?err, "Failed (spawn)");
+    //         Err(())
+    //     }
+    // }
+}

src/server/gitforge/forgejo/branch/mod.rs

@@ -1,7 +1,9 @@
+pub mod fetch;
 mod get_all;
 mod reset;
 mod validate_positions;
 
+pub use fetch::fetch;
 pub use get_all::get_all;
 pub use reset::reset;
 pub use validate_positions::validate_positions;

src/server/gitforge/forgejo/branch/reset.rs

@@ -1,18 +1,26 @@
+use std::ops::Deref;
+
 use secrecy::ExposeSecret;
 use tracing::{info, warn};
 
 use crate::server::{
     config::{BranchName, RepoDetails},
-    gitforge::{BranchResetResult, Force},
+    gitforge::{BranchResetError, BranchResetResult, Force},
     types::GitRef,
 };
 
+// TODO: (#72) reimplement using `gix`
+#[tracing::instrument]
 pub fn reset(
     repo_details: &RepoDetails,
     branch_name: BranchName,
     to_commit: GitRef,
     force: Force,
 ) -> BranchResetResult {
+    let repository = gix::ThreadSafeRepository::open(repo_details.gitdir.deref())
+        .map_err(Box::new)?
+        .to_thread_local();
+    let gitdir = repository.git_dir();
     let origin = repo_details.origin();
     let force = match force {
         Force::No => "".to_string(),
@@ -25,22 +33,28 @@ pub fn reset(
     )
     .into();
     info!("Resetting {branch_name} to {to_commit}");
+    let ctx = gix::diff::command::Context {
+        git_dir: Some(gitdir.to_path_buf()),
+        ..Default::default()
+    };
+    // info!(?ctx, command = command.expose_secret(), "prepare");
     match gix::command::prepare(command.expose_secret())
+        .with_context(ctx)
         .with_shell_allow_argument_splitting()
-        .stdout(std::process::Stdio::null())
+        // .stdout(std::process::Stdio::null())
-        .stderr(std::process::Stdio::null())
+        // .stderr(std::process::Stdio::null())
         .spawn()
     {
         Ok(mut child) => match child.wait() {
             Ok(_) => Ok(()),
             Err(err) => {
                 warn!(?err, "Failed (wait)");
-                Err(())
+                Err(BranchResetError::Push)
             }
         },
         Err(err) => {
             warn!(?err, "Failed (spawn)");
-            Err(())
+            Err(BranchResetError::Push)
         }
     }
 }

src/server/gitforge/forgejo/mod.rs

@@ -1,4 +1,4 @@
-mod branch;
+pub mod branch;
 mod file;
 mod repo;
 
@@ -53,6 +53,7 @@ impl super::ForgeLike for ForgeJoEnv {
         to_commit: GitRef,
         force: gitforge::Force,
     ) -> gitforge::BranchResetResult {
+        branch::fetch(&self.repo_details)?;
         branch::reset(&self.repo_details, branch_name, to_commit, force)
     }
 

src/server/gitforge/types.rs

@@ -19,7 +19,15 @@ impl std::fmt::Display for Force {
     }
 }
 
-pub type BranchResetResult = Result<(), ()>;
+#[derive(Debug, derive_more::From, derive_more::Display)]
+pub enum BranchResetError {
+    Open(Box<gix::open::Error>),
+    Fetch(crate::server::gitforge::forgejo::branch::fetch::Error),
+    Push,
+}
+impl std::error::Error for BranchResetError {}
+
+pub type BranchResetResult = Result<(), BranchResetError>;
 
 #[derive(Debug)]
 pub enum CommitStatus {

src/server/mod.rs

@@ -15,7 +15,7 @@ use crate::{
     fs::FileSystem,
     server::{
         actors::webhook,
-        config::{ForgeConfig, ForgeName, RepoAlias, ServerStorage, Webhook},
+        config::{ForgeConfig, ForgeName, GitDir, RepoAlias, ServerStorage, Webhook},
     },
 };
 
@@ -32,6 +32,8 @@ pub enum Error {
     },
 
     Config(crate::server::config::Error),
+
+    Io(std::io::Error),
 }
 type Result<T> = core::result::Result<T, Error>;
 
@@ -86,23 +88,17 @@ pub async fn start(fs: FileSystem, net: Network) {
     }
 
     for (forge_name, forge_config) in server_config.forges() {
-        if let Err(err) = create_forge_repos(
+        create_forge_repos(
             forge_config,
             forge_name.clone(),
             server_storage,
             webhook,
             &net,
         )
-        .map(|repos| {
+        .into_iter()
-            repos
+        .map(start_actor)
-                .into_iter()
+        .map(|(alias, addr)| webhook::AddWebhookRecipient(alias, addr.recipient()))
-                .map(start_actor)
+        .for_each(|msg| webhook_router.do_send(msg));
-                .map(|(alias, addr)| webhook::AddWebhookRecipient(alias, addr.recipient()))
-                .for_each(|msg| webhook_router.do_send(msg));
-        }) {
-            error!(?err, ?forge_name, "Failed to create forge repo actor");
-            return;
-        }
     }
     let webhook_server = webhook::WebhookActor::new(webhook_router.recipient()).start();
     let _ = actix_rt::signal::ctrl_c().await;
@@ -136,7 +132,7 @@ fn create_forge_repos(
     server_storage: &ServerStorage,
     webhook: &Webhook,
     net: &Network,
-) -> Result<Vec<(ForgeName, RepoAlias, RepoActor)>> {
+) -> Vec<(ForgeName, RepoAlias, RepoActor)> {
     let span = tracing::info_span!("Forge", %forge_name, %forge_config);
     let _guard = span.enter();
     info!("Creating Forge");
@@ -149,9 +145,15 @@ fn create_forge_repos(
         net,
     );
     for (repo_alias, server_repo_config) in forge_config.repos() {
-        repos.push(creator((repo_alias, server_repo_config))?);
+        let forge_repo = creator((repo_alias, server_repo_config));
+        info!(
+            forge = %forge_repo.0,
+            alias = %forge_repo.1,
+            "created forge repo"
+        );
+        repos.push(forge_repo);
     }
-    Ok(repos)
+    repos
 }
 
 fn create_actor(
@@ -160,7 +162,7 @@ fn create_actor(
     server_storage: &ServerStorage,
     webhook: &Webhook,
     net: &Network,
-) -> impl Fn((RepoAlias, &ServerRepoConfig)) -> Result<(ForgeName, RepoAlias, RepoActor)> {
+) -> impl Fn((RepoAlias, &ServerRepoConfig)) -> (ForgeName, RepoAlias, RepoActor) {
     let server_storage = server_storage.clone();
     let webhook = webhook.clone();
     let net = net.clone();
@@ -168,16 +170,29 @@ fn create_actor(
         let span = tracing::info_span!("Repo", %repo_name, %server_repo_config);
         let _guard = span.enter();
         info!("Creating Repo");
+        let gitdir = server_repo_config.gitdir().map_or_else(
+            || {
+                GitDir::from(
+                    server_storage
+                        .path()
+                        .join(forge_name.to_string())
+                        .join(repo_name.to_string()),
+                )
+            },
+            |gitdir| gitdir,
+        );
+        // INFO: can't canonicalise gitdir as the path needs to exist to do that and we may not
+        //       have cloned the repo yet
         let repo_details = config::RepoDetails::new(
             &repo_name,
             server_repo_config,
             &forge_name,
             &forge_config,
-            &server_storage,
+            gitdir,
-        )?;
+        );
         let actor = actors::repo::RepoActor::new(repo_details, webhook.clone(), net.clone());
         info!("Created Repo");
-        Ok((forge_name.clone(), repo_name, actor))
+        (forge_name.clone(), repo_name, actor)
     }
 }