git-toolbelt icon indicating copy to clipboard operation
git-toolbelt copied to clipboard

Add git-merge-directly-to

Open mmatrosov opened this issue 3 years ago • 4 comments

It works just like merge command, but instead of merging the specified branch into the current, it merges the current branch into the specified.

When on branch src, the result of this:

git merge-directly-to dst

Is the same as the result of this:

git checkout dst
git merge src

But you don't actually checkout dst before the merge. Instead, merge-directly-to merges dst into src and then swaps parents of the merge commit. This is more efficient, and it won't touch the timestamps of the files you modified in src. This might be crucial for incremental builds.

The whole reason to create this command was to optimize incremental build times in huge C++ projects. It might save hours comparing to the naive merge.

mmatrosov avatar Jul 09 '20 14:07 mmatrosov

Hi @mmatrosov — thanks so much for this contribution! I finally took a chance to look at this and use it directly myself. I like the general trick on this one, but I rewrote some parts of the script to be more consistent with the style of the rest of the scripts in here, see 78f3ed2f5ffd93fb66c030c177646a5a220dbab.

Tweaks and polishes I made:

  • Rename script: git-merge-directly-to => git-merge-into
  • Quote all shell arguments
  • Use more succinct helpers tools from git-toolbelt suite directly
  • Abort when there are local changes
  • Abort when there are merge conflicts ahead
  • Improve error messaging/usage display

Can you verify this is still behaving the same for you?

Also, I was wondering whether this script should perform the final git switch $dst at the very end, or if that should be left up to the user. If they want to, they can always simply switch themselves, right after. But while playing with this script myself, I found it useful to work from a feature branch and occasionally — at stable points — invoke git merge-into master and then simply continue working on my feature branch, which seems the better default here, rather than to switch to master automatically.

nvie avatar Aug 11 '20 12:08 nvie

(I tried adding that commit to this PR, but it looks like you'll have to give me permissions to do so.)

nvie avatar Aug 11 '20 12:08 nvie

Hi, @nvie! Really glad you finally made it.

(I tried adding that commit to this PR, but it looks like you'll have to give me permissions to do so.)

Hm, that's weird. I have and I had the "Allow edits by maintainers" option enabled for this PR. Well, anyway, if you have trouble adding commits to the PR, I can cherry-pick your changes. Let me know if I should.

Tweaks and polishes I made

Cool, all of them look great except this one: "Use more succinct helpers tools from git-toolbelt suite directly".

I have to tell about my setup. In fact, I use git under Windows and 90% of the time I use it with help of SmartGit GUI client. It allows to customize itself with external commands. I created a new command which calls git bash to execute the present shell script: image

Before your changes, the script was self-contained and did not require to install anything. I'd like not to change this. Yet I want to share the script with other developers. I hope we can do this.

Also, I was wondering whether this script should perform the final git switch $dst at the very end, or if that should be left up to the user

I see your point. Indeed, the default behavior might be different. However, during my workflow I never wanted to switch back to the branch I merged from. Meaning we can have different situations here. Thus, I'd ask you to leave the current default as it is.

Rename script: git-merge-directly-to => git-merge-into

Yes, this seems like a better name, I support it. However, I would ask you to do the rename in a separate commit. Because currently, it is very hard to tell what was changed in the code. The whole file is shown as removed, and then the whole new file is shown as added. At least by github.

mmatrosov avatar Aug 11 '20 14:08 mmatrosov

Hm, that's weird. I have and I had the "Allow edits by maintainers" option enabled for this PR. Well, anyway, if you have trouble adding commits to the PR, I can cherry-pick your changes. Let me know if I should.

Yes, feel free to!

I have to tell about my setup. In fact, I use git under Windows and 90% of the time I use it with help of SmartGit GUI client. It allows to customize itself with external commands. I created a new command which calls git bash to execute the present shell script:

I see. It looks like all you have to do is put the git-toolbelt directory on the %PATH%, and you should be good. There's no need to install anything except doing that. After doing so, Git will magically find these commands. In fact, the whole point of the suite is to do just that.

The goal of this suite is not to make each script self-contained. Many of the scripts in this repo invoke other (smaller) helper scripts from this same repo as building blocks. Inlining all the logic isn't something I'd like to do, because it increases the maintenance burden. (And scripts like git-is-dirty or git-merges-cleanly aren't trivial to inline anyway.)

I see your point. Indeed, the default behavior might be different. However, during my workflow I never wanted to switch back to the branch I merged from. Meaning we can have different situations here. Thus, I'd ask you to leave the current default as it is.

Not switching would support both use cases. You could simply invoke merge-into, then switch, right? That way, the building blocks are smaller and everyone can assemble them in a way that works for them. I can be pursuaded to add this behaviour under a -s (for switch to the target branch after the merge) behaviour, but I think the default behaviour should not switch.

Yes, this seems like a better name, I support it. However, I would ask you to do the rename in a separate commit.

Renaming it in a separate commit will not display the net diff differently in Git (or GitHub for that matter), so I don't think there's anything I can do about that. The file has structurally changed too much for Git to detect it as a rename. I don't think it matters, though? Once done, I'll merge this PR in a way that you will be the main author of this commit/script/PR anyway, and I will only show up as a co-author.

nvie avatar Aug 11 '20 14:08 nvie