Skip to content

Commit 343925d

Browse files
committed
feat(push): revert overzealous modified status
When pushing a patch, we would ideally like to show the "(modified)" status when the patch's diff changes. In StGit 2.4.11, the push behavior was changed such that "(modified)" would be displayed whenever the patch's underlying commit changed. But this meant that "(modified)" would be shown in many circumstances where the patch itself didn't change or didn't have a material change. Revert to the previous behavior where "(modified)" would only be shown if the initial patch application failed such that `git merge-recursive` had to be used to apply the patch. With this change, pushes will effectively never be shown as "(modified)". This remains a regression in StGit 2.x relative to 1.x. The root of this regression is that StGit 2.x, when pushing patches, uses `git apply --3way` whereas `git apply` withouth the `--3way` option was used in StGit 1.x. As a consequence, StGit 2.x patch application effectively never has to fallback to using `git merge-recursive` because `--3way` is correct/effective. But that also means that StGit's previous mechanism for determining whether a push modifies a patch no longer works.
1 parent 1fd0d5d commit 343925d

File tree

2 files changed

+11
-8
lines changed

2 files changed

+11
-8
lines changed

src/stack/transaction/mod.rs

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1090,6 +1090,7 @@ impl<'repo> StackTransaction<'repo> {
10901090
conflicts: false,
10911091
})?;
10921092
self.current_tree_id = tree_id;
1093+
push_status = PushStatus::Modified;
10931094
tree_id
10941095
}
10951096
Ok(false) => {
@@ -1130,12 +1131,10 @@ impl<'repo> StackTransaction<'repo> {
11301131
// execute() performs the checkout. Setting the transaction head
11311132
// here ensures that the real stack top will be checked-out.
11321133
self.updated_head = Some(commit.clone());
1133-
} else if push_status != PushStatus::AlreadyMerged {
1134-
if new_tree_id == new_parent_ref.tree() {
1135-
push_status = PushStatus::Empty;
1136-
} else {
1137-
push_status = PushStatus::Modified;
1138-
}
1134+
} else if push_status != PushStatus::AlreadyMerged
1135+
&& new_tree_id == new_parent_ref.tree()
1136+
{
1137+
push_status = PushStatus::Empty;
11391138
}
11401139

11411140
self.updated_patches

t/t1204-push-updated.sh

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -49,9 +49,13 @@ test_expect_success 'Move hunk to p0' '
4949
stg refresh
5050
'
5151

52-
test_expect_success 'Push indicates p1 was modified' '
52+
# NOTE: since StGit started using --3way when pushing patches, it is no longer
53+
# trivial to determine whether a patch was "modified" due to the push. In this
54+
# test, the status would ideally be "(modified)", but in practice StGit cannot
55+
# determine that cheaply.
56+
test_expect_success 'Push indicates p1 was not modified' '
5357
cat >expected <<-\EOF &&
54-
> p1 (modified)
58+
> p1
5559
EOF
5660
stg push p1 >out &&
5761
test_cmp expected out

0 commit comments

Comments
 (0)