Skip to content

Commit ca5c671

Browse files
committed
Use the *deduced* push-remote if there is no deduced fetch remote.
That way, the typical fork workflow would work naturally as one will push branches to the push remote and they will be shown.
1 parent a1030bf commit ca5c671

File tree

8 files changed

+165
-17
lines changed

8 files changed

+165
-17
lines changed

crates/but-core/src/ref_metadata.rs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,10 @@ pub struct Workspace {
2929
/// If there is no target name, this is a local workspace (and if no global target is set).
3030
/// Note that even though this is per workspace, the implementation can fill in global information at will.
3131
pub target_ref: Option<gix::refs::FullName>,
32+
/// The symbolic name of the remote to push branches to.
33+
///
34+
/// This is useful when there are no push permissions for the remote behind `target_ref`.
35+
pub push_remote: Option<String>,
3236
}
3337

3438
impl Workspace {

crates/but-graph/src/init/mod.rs

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -291,18 +291,24 @@ impl Graph {
291291
.flag_for(tip)
292292
.expect("we more than one bitflags for this");
293293

294-
let target_symbolic_remote_names = {
294+
let target_symbolic_remote_names: Vec<_> = {
295295
let remote_names = repo.remote_names();
296296
let mut v: Vec<_> = workspaces
297297
.iter()
298-
.filter_map(|(_, _, data)| {
299-
let target_ref = data.target_ref.as_ref()?;
300-
remotes::extract_remote_name(target_ref.as_ref(), &remote_names)
298+
.flat_map(|(_, _, data)| {
299+
data.target_ref
300+
.as_ref()
301+
.and_then(|target| {
302+
remotes::extract_remote_name(target.as_ref(), &remote_names)
303+
.map(|remote| (1, remote))
304+
})
305+
.into_iter()
306+
.chain(data.push_remote.clone().map(|push_remote| (0, push_remote)))
301307
})
302308
.collect();
303309
v.sort();
304310
v.dedup();
305-
v
311+
v.into_iter().map(|(_order, remote)| remote).collect()
306312
};
307313

308314
let mut next = Queue::new_with_limit(hard_limit);

crates/but-graph/src/ref_metadata_legacy.rs

Lines changed: 49 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,13 @@
11
use crate::virtual_branches_legacy_types::{CommitOrChangeId, Stack, StackBranch, VirtualBranches};
22
use anyhow::{Context, bail};
3+
use bstr::ByteSlice;
34
use but_core::RefMetadata;
45
use but_core::ref_metadata::{
56
Branch, RefInfo, StackId, ValueInfo, Workspace, WorkspaceStack, WorkspaceStackBranch,
67
};
8+
use gitbutler_reference::RemoteRefname;
79
use gix::date::SecondsSinceUnixEpoch;
10+
use gix::reference::Category;
811
use gix::refs::{FullName, FullNameRef};
912
use itertools::Itertools;
1013
use std::any::Any;
@@ -361,6 +364,42 @@ impl RefMetadata for VirtualBranchesTomlMetadata {
361364
}
362365
stack.in_workspace = false;
363366
}
367+
368+
// We don't support initialising this yet, for now just changes.
369+
let mut changed_target = false;
370+
if let Some(target) = self.data_mut().default_target.as_mut() {
371+
if target.push_remote_name != value.push_remote {
372+
target.push_remote_name = value.push_remote.clone();
373+
changed_target = true;
374+
}
375+
if let Some((category, short_name)) = value
376+
.target_ref
377+
.as_ref()
378+
.and_then(|rn| rn.category_and_short_name())
379+
{
380+
if category == Category::RemoteBranch {
381+
// TODO: remove this as we don't handle symbolic names with slashes correctly.
382+
// At least try to not always set this value, but this test is also ambiguous.
383+
if !short_name.ends_with_str(target.branch.branch()) {
384+
let slash_pos = short_name.find_byte(b'/').context(
385+
"remote branch didn't have '/' in the name, but should be 'origin/foo'",
386+
)?;
387+
target.branch = RemoteRefname::new(
388+
short_name[..slash_pos].to_str_lossy().as_ref(),
389+
short_name[slash_pos + 1..].to_str_lossy().as_ref(),
390+
);
391+
changed_target = true;
392+
}
393+
} else {
394+
bail!(
395+
"Cannot set target branches to a branch that isn't a remote tracking branch: '{short_name}'"
396+
);
397+
}
398+
}
399+
}
400+
if changed_target {
401+
self.snapshot.changed_at = Some(Instant::now());
402+
}
364403
Ok(())
365404
}
366405

@@ -478,15 +517,21 @@ impl RefMetadata for VirtualBranchesTomlMetadata {
478517

479518
impl VirtualBranchesTomlMetadata {
480519
fn workspace_from_data(data: &VirtualBranches) -> Workspace {
481-
let target_branch = data
520+
let (target_branch, push_remote) = data
482521
.default_target
483522
.as_ref()
484-
.and_then(|target| gix::refs::FullName::try_from(target.branch.to_string()).ok());
523+
.map(|target| {
524+
(
525+
gix::refs::FullName::try_from(target.branch.to_string()).ok(),
526+
target.push_remote_name.clone(),
527+
)
528+
})
529+
.unwrap_or_default();
485530

486531
let mut stacks: Vec<_> = data.branches.values().cloned().collect();
487532
stacks.sort_by_key(|s| s.order);
488533

489-
let workspace = but_core::ref_metadata::Workspace {
534+
let workspace = Workspace {
490535
ref_info: managed_ref_info(),
491536
stacks: stacks
492537
.iter()
@@ -510,6 +555,7 @@ impl VirtualBranchesTomlMetadata {
510555
})
511556
.collect(),
512557
target_ref: target_branch,
558+
push_remote,
513559
};
514560
workspace
515561
}

crates/but-graph/tests/fixtures/scenarios.sh

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -319,6 +319,8 @@ mkdir ws
319319
commit A2
320320
create_workspace_commit_once A
321321
setup_remote_tracking soon-remote A "move"
322+
mkdir .git/refs/remotes/push-remote
323+
cp .git/refs/remotes/origin/A .git/refs/remotes/push-remote/A
322324

323325
cat <<EOF >>.git/config
324326
[remote "origin"]

crates/but-graph/tests/graph/init/with_workspace.rs

Lines changed: 44 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1031,7 +1031,7 @@ fn deduced_remote_ahead() -> anyhow::Result<()> {
10311031
* 8b39ce4 (HEAD -> gitbutler/workspace) GitButler Workspace Commit
10321032
* 9d34471 (A) A2
10331033
* 5b89c71 A1
1034-
| * 3ea1a8f (origin/A) only-remote-02
1034+
| * 3ea1a8f (push-remote/A, origin/A) only-remote-02
10351035
| * 9c50f71 only-remote-01
10361036
| * 2cfbb79 merge
10371037
|/|
@@ -1119,6 +1119,49 @@ fn deduced_remote_ahead() -> anyhow::Result<()> {
11191119
└── 👉:0:main
11201120
└── ❄fafd9d0 (🏘️)
11211121
");
1122+
1123+
// When the push-remote is configured, it overrides the remote we use for listing, even if a fetch remote is available.
1124+
meta.data_mut()
1125+
.default_target
1126+
.as_mut()
1127+
.expect("set by default")
1128+
.push_remote_name = Some("push-remote".into());
1129+
let graph = Graph::from_head(&repo, &*meta, standard_options())?;
1130+
insta::assert_snapshot!(graph_tree(&graph), @r"
1131+
├── 👉📕►►►:0[0]:gitbutler/workspace
1132+
│ └── ·8b39ce4 (⌂|🏘️|1)
1133+
│ └── ►:1[1]:A <> push-remote/A →:2:
1134+
│ ├── ·9d34471 (⌂|🏘️|11)
1135+
│ └── ·5b89c71 (⌂|🏘️|11)
1136+
│ └── ►:5[3]:anon:
1137+
│ └── ·998eae6 (⌂|🏘️|11)
1138+
│ └── ►:3[4]:main
1139+
│ └── ·fafd9d0 (⌂|🏘️|11)
1140+
└── ►:2[0]:push-remote/A →:1:
1141+
├── 🟣3ea1a8f
1142+
└── 🟣9c50f71
1143+
└── ►:4[1]:anon:
1144+
└── 🟣2cfbb79
1145+
├── →:5:
1146+
└── ►:6[2]:anon:
1147+
└── 🟣e898cd0
1148+
└── →:5:
1149+
");
1150+
1151+
insta::assert_snapshot!(graph_workspace(&graph.to_workspace()?), @r"
1152+
📕🏘️:0:gitbutler/workspace <> ✓!
1153+
└── ≡:1:A <> push-remote/A →:2:⇡2⇣4
1154+
├── :1:A <> push-remote/A →:2:⇡2⇣4
1155+
│ ├── 🟣3ea1a8f
1156+
│ ├── 🟣9c50f71
1157+
│ ├── 🟣2cfbb79
1158+
│ ├── 🟣e898cd0
1159+
│ ├── ·9d34471 (🏘️)
1160+
│ ├── ·5b89c71 (🏘️)
1161+
│ └── ❄️998eae6 (🏘️)
1162+
└── :3:main
1163+
└── ❄fafd9d0 (🏘️)
1164+
");
11221165
Ok(())
11231166
}
11241167

crates/but-graph/tests/graph/ref_metadata_legacy.rs

Lines changed: 46 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,10 @@
11
use but_core::RefMetadata;
22
use but_core::ref_metadata::{StackId, ValueInfo, WorkspaceStack, WorkspaceStackBranch};
33
use but_graph::VirtualBranchesTomlMetadata;
4+
use but_graph::virtual_branches_legacy_types::Target;
45
use but_testsupport::gix_testtools::tempfile::{TempDir, tempdir};
56
use but_testsupport::{debug_str, sanitize_uuids_and_timestamps_with_mapping};
7+
use gitbutler_reference::RemoteRefname;
68
use std::ops::Deref;
79
use std::path::PathBuf;
810

@@ -379,6 +381,8 @@ fn read_only() -> anyhow::Result<()> {
379381
#[test]
380382
fn create_workspace_and_stacks_with_branches_from_scratch() -> anyhow::Result<()> {
381383
let (mut store, _tmp) = empty_vb_store_rw()?;
384+
store.data_mut().default_target = None;
385+
382386
let toml_path = store.path().to_owned();
383387
let branch_name: gix::refs::FullName = "refs/heads/feat".try_into()?;
384388
let mut branch = store.branch(branch_name.as_ref())?;
@@ -745,6 +749,7 @@ fn create_workspace_and_stacks_with_branches_from_scratch() -> anyhow::Result<()
745749
ref_info: RefInfo { created_at: "2023-01-31 14:55:57 +0000", updated_at: None },
746750
stacks: [],
747751
target_ref: None,
752+
push_remote: None,
748753
}
749754
"#);
750755

@@ -753,6 +758,35 @@ fn create_workspace_and_stacks_with_branches_from_scratch() -> anyhow::Result<()
753758
!toml_path.exists(),
754759
"if everything is just the default, the file is deleted on write"
755760
);
761+
762+
let mut store = VirtualBranchesTomlMetadata::from_path(&toml_path)?;
763+
store.data_mut().default_target = Some(default_target());
764+
765+
let toml_path = store.path().to_owned();
766+
let mut ws = store.workspace(workspace_name.as_ref())?;
767+
768+
ws.push_remote = Some("push-remote".into());
769+
ws.target_ref = Some(gix::refs::FullName::try_from(
770+
"refs/remotes/new-origin/new-target",
771+
)?);
772+
store.set_workspace(&ws)?;
773+
774+
drop(store);
775+
let (actual, _uuids) =
776+
sanitize_uuids_and_timestamps_with_mapping(std::fs::read_to_string(&toml_path)?);
777+
insta::assert_snapshot!(actual, @r#"
778+
[default_target]
779+
branchName = "new-target"
780+
remoteName = "new-origin"
781+
remoteUrl = "https://example.com/example-org/example-repo"
782+
sha = "0000000000000000000000000000000000000000"
783+
pushRemoteName = "push-remote"
784+
785+
[branch_targets]
786+
787+
[branches]
788+
"#);
789+
756790
Ok(())
757791
}
758792

@@ -900,6 +934,7 @@ fn create_workspace_from_scratch_workspace_first() -> anyhow::Result<()> {
900934
!branch.is_default(),
901935
"Workspace branches are implicitly created, this isn't the case in a normal backend implementation"
902936
);
937+
903938
Ok(())
904939
}
905940

@@ -918,10 +953,20 @@ fn vb_store_rw(name: &str) -> anyhow::Result<(VirtualBranchesTomlMetadata, TempD
918953

919954
fn empty_vb_store_rw() -> anyhow::Result<(VirtualBranchesTomlMetadata, TempDir)> {
920955
let tmp = tempdir()?;
921-
let store = VirtualBranchesTomlMetadata::from_path(tmp.path().join("vb.toml"))?;
956+
let mut store = VirtualBranchesTomlMetadata::from_path(tmp.path().join("vb.toml"))?;
957+
store.data_mut().default_target = Some(default_target());
922958
Ok((store, tmp))
923959
}
924960

961+
fn default_target() -> Target {
962+
Target {
963+
branch: RemoteRefname::new("origin/sub-name", "main"),
964+
remote_url: "https://example.com/example-org/example-repo".to_string(),
965+
sha: gix::hash::Kind::Sha1.null(),
966+
push_remote_name: None,
967+
}
968+
}
969+
925970
/// Assure everything can round-trip and the data looks consistent, independently of the actual data,
926971
/// from a store that already contains data.
927972
fn roundtrip_journey(metadata: &mut impl RefMetadata) -> anyhow::Result<()> {

crates/but-graph/tests/graph/vis.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ fn post_graph_traversal() -> anyhow::Result<()> {
2323
ref_info: Default::default(),
2424
stacks: vec![],
2525
target_ref: None,
26+
push_remote: None,
2627
})),
2728
..Default::default()
2829
};

crates/but-workspace/tests/workspace/branch_details.rs

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ mod with_workspace {
2727
|/
2828
* ff045ef (main) init
2929
");
30-
let store = WorkspaceStore::default()
30+
let store = WorkspaceRefMetadataStore::default()
3131
.with_target("B")
3232
.with_named_branch("A");
3333
insta::assert_debug_snapshot!(
@@ -77,7 +77,7 @@ mod with_workspace {
7777
* d79bba9 new file in A
7878
* c166d42 (origin/main, origin/HEAD, main) init-integration
7979
");
80-
let store = WorkspaceStore::default()
80+
let store = WorkspaceRefMetadataStore::default()
8181
.with_target("main")
8282
.with_named_branch("A");
8383
insta::assert_debug_snapshot!(but_workspace::branch_details_v3(&repo, refname("A").as_ref(), &store).unwrap(), @r#"
@@ -129,7 +129,7 @@ mod with_workspace {
129129
* c166d42 (origin/main, origin/HEAD, main) init-integration
130130
");
131131

132-
let store = WorkspaceStore::default()
132+
let store = WorkspaceRefMetadataStore::default()
133133
.with_target("main")
134134
.with_named_branch("A");
135135
insta::assert_debug_snapshot!(but_workspace::branch_details_v3(&repo, refname("A").as_ref(), &store).unwrap(), @r#"
@@ -208,7 +208,7 @@ mod with_workspace {
208208
* c166d42 (origin/main, origin/HEAD, main) init-integration
209209
");
210210

211-
let store = WorkspaceStore::default()
211+
let store = WorkspaceRefMetadataStore::default()
212212
.with_target("main")
213213
.with_named_branch("A");
214214
insta::assert_debug_snapshot!(but_workspace::branch_details_v3(&repo, refname("A").as_ref(), &store).unwrap(), @r#"
@@ -252,17 +252,18 @@ mod with_workspace {
252252
}
253253

254254
#[derive(Default)]
255-
struct WorkspaceStore {
255+
struct WorkspaceRefMetadataStore {
256256
workspace: Workspace,
257257
branches: Vec<(FullName, but_core::ref_metadata::Branch)>,
258258
}
259259

260-
impl WorkspaceStore {
260+
impl WorkspaceRefMetadataStore {
261261
pub fn with_target(mut self, short_name: &str) -> Self {
262262
self.workspace = but_core::ref_metadata::Workspace {
263263
ref_info: Default::default(),
264264
stacks: vec![],
265265
target_ref: Some(refname(short_name)),
266+
push_remote: None,
266267
};
267268
self
268269
}
@@ -320,7 +321,7 @@ mod with_workspace {
320321
}
321322
}
322323

323-
impl RefMetadata for WorkspaceStore {
324+
impl RefMetadata for WorkspaceRefMetadataStore {
324325
type Handle<T> = NullHandle<T>;
325326

326327
fn iter(&self) -> impl Iterator<Item = anyhow::Result<(FullName, Box<dyn Any>)>> + '_ {

0 commit comments

Comments
 (0)