Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

git: Use merge-tree in patchsets #187

Open
wants to merge 1 commit into
base: jerry/revup/main/gitconfig
Choose a base branch
from

Conversation

jerry-skydio
Copy link
Collaborator

@jerry-skydio jerry-skydio commented Jul 3, 2024

Previously patchsets used a hacky invocation of update-index
to overlay changed files from old tree to new tree. In addition
to being slower than necessary, this has the downside of showing
upstream changes in the diff, even those that were not changed
in the given review upload.

Using git merge-tree for this instead is cleaner, faster, and better.
We do this by "cherry-picking" the old branch with git merge-tree, but providing -X ours as
the strategy. This provides a tree that looks like the new base with the old changes applied.
For cases where the old changes don't apply cleanly, the ours strategy prevents conflicts by
taking the version in the old tree. Although this can be a bit confusing when looking at the
file as a whole, it provides a succinct way to view a difference between the old version and
new version.

Fixes: #183

@jerry-skydio
Copy link
Collaborator Author

jerry-skydio commented Jul 3, 2024

Reviews in this chain:
#188 git: Add option for no_config in git shell
 └#187 git: Use merge-tree in patchsets

@jerry-skydio
Copy link
Collaborator Author

jerry-skydio commented Jul 3, 2024

# head base diff date summary
0 b427e643 740c5cdb diff Jul 3 12:15 PM 1 file changed, 21 insertions(+), 36 deletions(-)
1 672d68bd 740c5cdb diff Jul 3 12:53 PM 1 file changed, 1 insertion(+), 2 deletions(-)
2 6f599e77 740c5cdb diff Jul 3 13:03 PM 1 file changed, 7 insertions(+), 17 deletions(-)
3 61afb41d 740c5cdb diff Jul 3 13:04 PM 0 files changed
4 b4fe6239 740c5cdb diff Jul 3 13:25 PM 1 file changed, 1 insertion(+), 1 deletion(-)
5 775056df 740c5cdb diff Jul 3 13:58 PM 1 file changed, 14 deletions(-)
6 21f00572 1d315c52 diff Jul 3 14:06 PM 0 files changed
7 cbf755de cb8fa697 diff Jul 3 14:08 PM 1 file changed, 3 insertions(+), 4 deletions(-)
8 62ebf0b9 64fe866f diff Jul 8 16:07 PM 0 files changed

@jerry-skydio jerry-skydio force-pushed the jerry/revup/main/mergetreepatch branch 6 times, most recently from 775056d to 21f0057 Compare July 3, 2024 21:06
@jerry-skydio jerry-skydio changed the base branch from main to jerry/revup/main/gitconfig July 3, 2024 21:06
@jerry-skydio jerry-skydio force-pushed the jerry/revup/main/mergetreepatch branch from 21f0057 to cbf755d Compare July 3, 2024 21:08
Previously patchsets used a hacky invocation of update-index
to overlay changed files from old tree to new tree. In addition
to being slower than necessary, this has the downside of showing
upstream changes in the diff, even those that were not changed
in the given review upload.

Using git merge-tree for this instead is cleaner, faster, and better.
We do this by "cherry-picking" the old branch with git merge-tree, but providing -X ours as
the strategy. This provides a tree that looks like the new base with the old changes applied.
For cases where the old changes don't apply cleanly, the ours strategy prevents conflicts by
taking the version in the old tree. Although this can be a bit confusing when looking at the
file as a whole, it provides a succinct way to view a difference between the old version and
new version.

Fixes: #183
Copy link
Contributor

@aaron-skydio aaron-skydio left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cool

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Use merge-tree in patchsets instead of update-index
3 participants