Merge pull request #9894 from Byron/fix

Byron · web-flow · commit 7a3f820cd147 · 2025-08-19T16:44:43.000+02:00
Use the deduced push-remote if there is no deduced fetch remote.
diff --git a/crates/but-core/src/ref_metadata.rs b/crates/but-core/src/ref_metadata.rs
@@ -29,6 +29,10 @@ pub struct Workspace {
     /// If there is no target name, this is a local workspace (and if no global target is set).
     /// Note that even though this is per workspace, the implementation can fill in global information at will.
     pub target_ref: Option<gix::refs::FullName>,
+    /// The symbolic name of the remote to push branches to.
+    ///
+    /// This is useful when there are no push permissions for the remote behind `target_ref`.
+    pub push_remote: Option<String>,
 }
 
 impl Workspace {
diff --git a/crates/but-graph/src/init/mod.rs b/crates/but-graph/src/init/mod.rs
@@ -291,18 +291,24 @@ impl Graph {
                 .flag_for(tip)
                 .expect("we more than one bitflags for this");
 
-        let target_symbolic_remote_names = {
+        let target_symbolic_remote_names: Vec<_> = {
             let remote_names = repo.remote_names();
             let mut v: Vec<_> = workspaces
                 .iter()
-                .filter_map(|(_, _, data)| {
-                    let target_ref = data.target_ref.as_ref()?;
-                    remotes::extract_remote_name(target_ref.as_ref(), &remote_names)
+                .flat_map(|(_, _, data)| {
+                    data.target_ref
+                        .as_ref()
+                        .and_then(|target| {
+                            remotes::extract_remote_name(target.as_ref(), &remote_names)
+                                .map(|remote| (1, remote))
+                        })
+                        .into_iter()
+                        .chain(data.push_remote.clone().map(|push_remote| (0, push_remote)))
                 })
                 .collect();
             v.sort();
             v.dedup();
-            v
+            v.into_iter().map(|(_order, remote)| remote).collect()
         };
 
         let mut next = Queue::new_with_limit(hard_limit);
diff --git a/crates/but-graph/src/ref_metadata_legacy.rs b/crates/but-graph/src/ref_metadata_legacy.rs
@@ -1,10 +1,13 @@
 use crate::virtual_branches_legacy_types::{CommitOrChangeId, Stack, StackBranch, VirtualBranches};
 use anyhow::{Context, bail};
+use bstr::ByteSlice;
 use but_core::RefMetadata;
 use but_core::ref_metadata::{
     Branch, RefInfo, StackId, ValueInfo, Workspace, WorkspaceStack, WorkspaceStackBranch,
 };
+use gitbutler_reference::RemoteRefname;
 use gix::date::SecondsSinceUnixEpoch;
+use gix::reference::Category;
 use gix::refs::{FullName, FullNameRef};
 use itertools::Itertools;
 use std::any::Any;
@@ -361,6 +364,42 @@ impl RefMetadata for VirtualBranchesTomlMetadata {
             }
             stack.in_workspace = false;
         }
+
+        // We don't support initialising this yet, for now just changes.
+        let mut changed_target = false;
+        if let Some(target) = self.data_mut().default_target.as_mut() {
+            if target.push_remote_name != value.push_remote {
+                target.push_remote_name = value.push_remote.clone();
+                changed_target = true;
+            }
+            if let Some((category, short_name)) = value
+                .target_ref
+                .as_ref()
+                .and_then(|rn| rn.category_and_short_name())
+            {
+                if category == Category::RemoteBranch {
+                    // TODO: remove this as we don't handle symbolic names with slashes correctly.
+                    // At least try to not always set this value, but this test is also ambiguous.
+                    if !short_name.ends_with_str(target.branch.branch()) {
+                        let slash_pos = short_name.find_byte(b'/').context(
+                            "remote branch didn't have '/' in the name, but should be 'origin/foo'",
+                        )?;
+                        target.branch = RemoteRefname::new(
+                            short_name[..slash_pos].to_str_lossy().as_ref(),
+                            short_name[slash_pos + 1..].to_str_lossy().as_ref(),
+                        );
+                        changed_target = true;
+                    }
+                } else {
+                    bail!(
+                        "Cannot set target branches to a branch that isn't a remote tracking branch: '{short_name}'"
+                    );
+                }
+            }
+        }
+        if changed_target {
+            self.snapshot.changed_at = Some(Instant::now());
+        }
         Ok(())
     }
 
@@ -478,15 +517,21 @@ impl RefMetadata for VirtualBranchesTomlMetadata {
 
 impl VirtualBranchesTomlMetadata {
     fn workspace_from_data(data: &VirtualBranches) -> Workspace {
-        let target_branch = data
+        let (target_branch, push_remote) = data
             .default_target
             .as_ref()
-            .and_then(|target| gix::refs::FullName::try_from(target.branch.to_string()).ok());
+            .map(|target| {
+                (
+                    gix::refs::FullName::try_from(target.branch.to_string()).ok(),
+                    target.push_remote_name.clone(),
+                )
+            })
+            .unwrap_or_default();
 
         let mut stacks: Vec<_> = data.branches.values().cloned().collect();
         stacks.sort_by_key(|s| s.order);
 
-        let workspace = but_core::ref_metadata::Workspace {
+        let workspace = Workspace {
             ref_info: managed_ref_info(),
             stacks: stacks
                 .iter()
@@ -510,6 +555,7 @@ impl VirtualBranchesTomlMetadata {
                 })
                 .collect(),
             target_ref: target_branch,
+            push_remote,
         };
         workspace
     }
diff --git a/crates/but-graph/tests/fixtures/scenarios.sh b/crates/but-graph/tests/fixtures/scenarios.sh
@@ -319,6 +319,8 @@ mkdir ws
       commit A2
     create_workspace_commit_once A
     setup_remote_tracking soon-remote A "move"
+    mkdir .git/refs/remotes/push-remote
+    cp .git/refs/remotes/origin/A .git/refs/remotes/push-remote/A
 
 cat <<EOF >>.git/config
 [remote "origin"]
diff --git a/crates/but-graph/tests/graph/init/with_workspace.rs b/crates/but-graph/tests/graph/init/with_workspace.rs
@@ -1031,7 +1031,7 @@ fn deduced_remote_ahead() -> anyhow::Result<()> {
     * 8b39ce4 (HEAD -> gitbutler/workspace) GitButler Workspace Commit
     * 9d34471 (A) A2
     * 5b89c71 A1
-    | * 3ea1a8f (origin/A) only-remote-02
+    | * 3ea1a8f (push-remote/A, origin/A) only-remote-02
     | * 9c50f71 only-remote-01
     | * 2cfbb79 merge
     |/| 
@@ -1119,6 +1119,49 @@ fn deduced_remote_ahead() -> anyhow::Result<()> {
         └── 👉:0:main
             └── ❄fafd9d0 (🏘️)
     ");
+
+    // When the push-remote is configured, it overrides the remote we use for listing, even if a fetch remote is available.
+    meta.data_mut()
+        .default_target
+        .as_mut()
+        .expect("set by default")
+        .push_remote_name = Some("push-remote".into());
+    let graph = Graph::from_head(&repo, &*meta, standard_options())?;
+    insta::assert_snapshot!(graph_tree(&graph), @r"
+    ├── 👉📕►►►:0[0]:gitbutler/workspace
+    │   └── ·8b39ce4 (⌂|🏘️|1)
+    │       └── ►:1[1]:A <> push-remote/A →:2:
+    │           ├── ·9d34471 (⌂|🏘️|11)
+    │           └── ·5b89c71 (⌂|🏘️|11)
+    │               └── ►:5[3]:anon:
+    │                   └── ·998eae6 (⌂|🏘️|11)
+    │                       └── ►:3[4]:main
+    │                           └── ·fafd9d0 (⌂|🏘️|11)
+    └── ►:2[0]:push-remote/A →:1:
+        ├── 🟣3ea1a8f
+        └── 🟣9c50f71
+            └── ►:4[1]:anon:
+                └── 🟣2cfbb79
+                    ├── →:5:
+                    └── ►:6[2]:anon:
+                        └── 🟣e898cd0
+                            └── →:5:
+    ");
+
+    insta::assert_snapshot!(graph_workspace(&graph.to_workspace()?), @r"
+    📕🏘️:0:gitbutler/workspace <> ✓!
+    └── ≡:1:A <> push-remote/A →:2:⇡2⇣4
+        ├── :1:A <> push-remote/A →:2:⇡2⇣4
+        │   ├── 🟣3ea1a8f
+        │   ├── 🟣9c50f71
+        │   ├── 🟣2cfbb79
+        │   ├── 🟣e898cd0
+        │   ├── ·9d34471 (🏘️)
+        │   ├── ·5b89c71 (🏘️)
+        │   └── ❄️998eae6 (🏘️)
+        └── :3:main
+            └── ❄fafd9d0 (🏘️)
+    ");
     Ok(())
 }
 
diff --git a/crates/but-graph/tests/graph/ref_metadata_legacy.rs b/crates/but-graph/tests/graph/ref_metadata_legacy.rs
@@ -1,8 +1,10 @@
 use but_core::RefMetadata;
 use but_core::ref_metadata::{StackId, ValueInfo, WorkspaceStack, WorkspaceStackBranch};
 use but_graph::VirtualBranchesTomlMetadata;
+use but_graph::virtual_branches_legacy_types::Target;
 use but_testsupport::gix_testtools::tempfile::{TempDir, tempdir};
 use but_testsupport::{debug_str, sanitize_uuids_and_timestamps_with_mapping};
+use gitbutler_reference::RemoteRefname;
 use std::ops::Deref;
 use std::path::PathBuf;
 
@@ -379,6 +381,8 @@ fn read_only() -> anyhow::Result<()> {
 #[test]
 fn create_workspace_and_stacks_with_branches_from_scratch() -> anyhow::Result<()> {
     let (mut store, _tmp) = empty_vb_store_rw()?;
+    store.data_mut().default_target = None;
+
     let toml_path = store.path().to_owned();
     let branch_name: gix::refs::FullName = "refs/heads/feat".try_into()?;
     let mut branch = store.branch(branch_name.as_ref())?;
@@ -745,6 +749,7 @@ fn create_workspace_and_stacks_with_branches_from_scratch() -> anyhow::Result<()
         ref_info: RefInfo { created_at: "2023-01-31 14:55:57 +0000", updated_at: None },
         stacks: [],
         target_ref: None,
+        push_remote: None,
     }
     "#);
 
@@ -753,6 +758,35 @@ fn create_workspace_and_stacks_with_branches_from_scratch() -> anyhow::Result<()
         !toml_path.exists(),
         "if everything is just the default, the file is deleted on write"
     );
+
+    let mut store = VirtualBranchesTomlMetadata::from_path(&toml_path)?;
+    store.data_mut().default_target = Some(default_target());
+
+    let toml_path = store.path().to_owned();
+    let mut ws = store.workspace(workspace_name.as_ref())?;
+
+    ws.push_remote = Some("push-remote".into());
+    ws.target_ref = Some(gix::refs::FullName::try_from(
+        "refs/remotes/new-origin/new-target",
+    )?);
+    store.set_workspace(&ws)?;
+
+    drop(store);
+    let (actual, _uuids) =
+        sanitize_uuids_and_timestamps_with_mapping(std::fs::read_to_string(&toml_path)?);
+    insta::assert_snapshot!(actual, @r#"
+    [default_target]
+    branchName = "new-target"
+    remoteName = "new-origin"
+    remoteUrl = "https://example.com/example-org/example-repo"
+    sha = "0000000000000000000000000000000000000000"
+    pushRemoteName = "push-remote"
+
+    [branch_targets]
+
+    [branches]
+    "#);
+
     Ok(())
 }
 
@@ -900,6 +934,7 @@ fn create_workspace_from_scratch_workspace_first() -> anyhow::Result<()> {
         !branch.is_default(),
         "Workspace branches are implicitly created, this isn't the case in a normal backend implementation"
     );
+
     Ok(())
 }
 
@@ -918,10 +953,20 @@ fn vb_store_rw(name: &str) -> anyhow::Result<(VirtualBranchesTomlMetadata, TempD
 
 fn empty_vb_store_rw() -> anyhow::Result<(VirtualBranchesTomlMetadata, TempDir)> {
     let tmp = tempdir()?;
-    let store = VirtualBranchesTomlMetadata::from_path(tmp.path().join("vb.toml"))?;
+    let mut store = VirtualBranchesTomlMetadata::from_path(tmp.path().join("vb.toml"))?;
+    store.data_mut().default_target = Some(default_target());
     Ok((store, tmp))
 }
 
+fn default_target() -> Target {
+    Target {
+        branch: RemoteRefname::new("origin/sub-name", "main"),
+        remote_url: "https://example.com/example-org/example-repo".to_string(),
+        sha: gix::hash::Kind::Sha1.null(),
+        push_remote_name: None,
+    }
+}
+
 /// Assure everything can round-trip and the data looks consistent, independently of the actual data,
 /// from a store that already contains data.
 fn roundtrip_journey(metadata: &mut impl RefMetadata) -> anyhow::Result<()> {
diff --git a/crates/but-graph/tests/graph/vis.rs b/crates/but-graph/tests/graph/vis.rs
@@ -23,6 +23,7 @@ fn post_graph_traversal() -> anyhow::Result<()> {
             ref_info: Default::default(),
             stacks: vec![],
             target_ref: None,
+            push_remote: None,
         })),
         ..Default::default()
     };
diff --git a/crates/but-workspace/tests/workspace/branch_details.rs b/crates/but-workspace/tests/workspace/branch_details.rs
@@ -27,7 +27,7 @@ mod with_workspace {
         |/  
         * ff045ef (main) init
         ");
-        let store = WorkspaceStore::default()
+        let store = WorkspaceRefMetadataStore::default()
             .with_target("B")
             .with_named_branch("A");
         insta::assert_debug_snapshot!(
@@ -77,7 +77,7 @@ mod with_workspace {
         * d79bba9 new file in A
         * c166d42 (origin/main, origin/HEAD, main) init-integration
         ");
-        let store = WorkspaceStore::default()
+        let store = WorkspaceRefMetadataStore::default()
             .with_target("main")
             .with_named_branch("A");
         insta::assert_debug_snapshot!(but_workspace::branch_details_v3(&repo, refname("A").as_ref(), &store).unwrap(), @r#"
@@ -129,7 +129,7 @@ mod with_workspace {
         * c166d42 (origin/main, origin/HEAD, main) init-integration
         ");
 
-        let store = WorkspaceStore::default()
+        let store = WorkspaceRefMetadataStore::default()
             .with_target("main")
             .with_named_branch("A");
         insta::assert_debug_snapshot!(but_workspace::branch_details_v3(&repo, refname("A").as_ref(), &store).unwrap(), @r#"
@@ -208,7 +208,7 @@ mod with_workspace {
         * c166d42 (origin/main, origin/HEAD, main) init-integration
         ");
 
-        let store = WorkspaceStore::default()
+        let store = WorkspaceRefMetadataStore::default()
             .with_target("main")
             .with_named_branch("A");
         insta::assert_debug_snapshot!(but_workspace::branch_details_v3(&repo, refname("A").as_ref(), &store).unwrap(), @r#"
@@ -252,17 +252,18 @@ mod with_workspace {
     }
 
     #[derive(Default)]
-    struct WorkspaceStore {
+    struct WorkspaceRefMetadataStore {
         workspace: Workspace,
         branches: Vec<(FullName, but_core::ref_metadata::Branch)>,
     }
 
-    impl WorkspaceStore {
+    impl WorkspaceRefMetadataStore {
         pub fn with_target(mut self, short_name: &str) -> Self {
             self.workspace = but_core::ref_metadata::Workspace {
                 ref_info: Default::default(),
                 stacks: vec![],
                 target_ref: Some(refname(short_name)),
+                push_remote: None,
             };
             self
         }
@@ -320,7 +321,7 @@ mod with_workspace {
         }
     }
 
-    impl RefMetadata for WorkspaceStore {
+    impl RefMetadata for WorkspaceRefMetadataStore {
         type Handle<T> = NullHandle<T>;
 
         fn iter(&self) -> impl Iterator<Item = anyhow::Result<(FullName, Box<dyn Any>)>> + '_ {
