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

Clarify that ff-only is git's default pull behavior #498

Merged
merged 1 commit into from
Sep 23, 2023

Conversation

kaste
Copy link
Contributor

@kaste kaste commented May 9, 2023

Fixes git-for-windows/git#4407

We stated that "Merge" would be the default git pull behavior but it is not. The default mode is in fact "ff-only" and we follow here.

image

@dscho
Copy link
Member

dscho commented May 9, 2023

I'm not sure that I am prepared for the users' backlash against changing behavior Just Like That.

In other words, I think the best we can do here is to reword the existing choices, but not change the default. Probably a good idea would be to mention that "Fast-forward or merge" used to be Git's default and remains Git for Windows' default for now, and that "only ever fast-forward" is the new Git default (but that's not quite accurate, is it?).

@kaste
Copy link
Contributor Author

kaste commented May 10, 2023

The issue I had with git-for-windows was that (I just stormed through the installer) it set a default not expected and not in line with git itself. (I know the git default, why should I know the git-for-windows default?) It is not a wording issue.

But the PR here just applies for new installs/users; it will not change a running system on the next update. Is this correct?

When we do (as suggested)

[x] Fast-forward or merge
[ ] Rebase
[ ] Only ever fast-forward: This is the standard behavior of `git pull`   

we basically emphasize the "fast-forward or merge" option because we select something that differs from the git default. That is: we suggest it. Same when we would choose "Rebase" by default; we basically say: that is the old default of git but we have a better new one. So in the end, we guide new users to the old "ff or merge" behavior, when they should actually move away and choose between "ff-only" and "rebase".

Did you consider the not-yet option

[x] Do nothing
[ ] Only ever fast-forward: This is the standard behavior of `git pull`
[ ] Rebase
[ ] Fast-forward or merge: This was the standard behavior of git <2.33.1

Aside: The breaking default change comes from git/git@031e2f7 which appears (as GitHub finds) first in 2.33.1. And it is not even mentioned in the release notes.

I think git-for-windows choose the wrong default from the beginning: You wanted to save the user from the new nag messages, but the nag messages were introduced preliminary to change the default (to ff-only).

@rimrul
Copy link
Member

rimrul commented May 10, 2023

I think git-for-windows choose the wrong default from the beginning: You wanted to save the user from the new nag messages, but the nag messages were introduced preliminary to change the default (to ff-only).

It's simpler than that. We added a way to choose in the setup, way before the messages existed. And we chose our default based on the default at the time. When the nag messages came we saved the users from them, as a side effect, because they had already explicitly set a default.

@kaste
Copy link
Contributor Author

kaste commented May 10, 2023

Ah okay, I thought git introduced it git/git@d18c950 (Mar 10, 2020) and 6ab14d9 (#287) (May 20, 2020) here would be in reply of that. Didn't look further.

@dscho
Copy link
Member

dscho commented Aug 25, 2023

So we're at an impasse.

@kaste wants to unleash a new default to all Git for Windows users, based on the rationale that upstream Git forces users to make their choice explicit when trying to pull a divergent branch.

Git for Windows, on the other hand, asks users up front what Git's default pull behavior should be. And with millions of users, Git for Windows has to be careful when unleashing such a change. Disgruntled users will not be mad at @kaste, after all, they will be mad at the Git for Windows maintainers when their workflows and scripts need to be adapted.

So I am mildly negative about this PR, in particular because there was no room for a compromise to first reword the explanations (I'd be happy with a "(RECOMMENDED!) ... This is Git's default behavior.", for example) and then see where we're at in a year or so and maybe flip the default then.

You can convince me with a gentler deprecation path, but not with a firm no-compromises-possible stance.

@kaste
Copy link
Contributor Author

kaste commented Sep 14, 2023

I don't see the "no-compromise" Haltung that much as I go for the lengthy discussion about wordings after the initial "It is not a wording issue."

Now, three years later, I feel it might be just a wording issue more than before though, as we currently state "Default (fast-forward or merge) This is the standard behavior of git pull..." and that might be just wrong and irritating.

How about

[x] Fast-forward or merge:  Fast-forward the current branch to the fetched branch when possible, otherwise create a merge commit.
[ ] Rebase:  Rebase the current branch onto the fetched branch. If there are no local commits to rebase, this is equivalent to a fast-forward.
[ ] Only ever fast-forward:  Fast-forward to the fetched branch. Fail if that is not possible. This is the standard behavior of `git pull`.   

I usually like a "Do nothing - decide later" option because it is the best option for new users (you can't decide the question if you don't know git) but you did not comment on that (and it is not part of the PR anyway).

@dscho
Copy link
Member

dscho commented Sep 18, 2023

How about

  • Fast-forward or merge: Fast-forward the current branch to the fetched branch when possible, otherwise create a merge commit.
  • Rebase: Rebase the current branch onto the fetched branch. If there are no local commits to rebase, this is equivalent to a fast-forward.
  • Only ever fast-forward: Fast-forward to the fetched branch. Fail if that is not possible. This is the standard behavior of git pull.

I like it.

I usually like a "Do nothing - decide later" option because it is the best option for new users (you can't decide the question if you don't know git) but you did not comment on that (and it is not part of the PR anyway).

In my experience, this "do nothing, force the user to decide later, right when they just need to Get Stuff Done and Don't Want To Be Bothered With Configuring Git Again" stance of Git is a big, gigantic source of (totally avoidable) friction.

Fixes git-for-windows/git#4407

We stated that "Merge" would be the default `git pull` behavior but it
is not.  The default mode is in fact "ff-only".

Changing also what we select by default was discussed in
git-for-windows#498 but rejected as
it would be too disruptive for the end-users (for now).

Signed-off-by: herr kaste <herr.kaste@gmail.com>
@kaste kaste force-pushed the flip-pull-behavior-default branch from 41add90 to eb153b9 Compare September 22, 2023 12:42
@kaste kaste changed the title Make ff-only the default pull behavior Clarify that ff-only is git's default pull behavior Sep 22, 2023
@kaste
Copy link
Contributor Author

kaste commented Sep 22, 2023

Okay, had some time to do this today. It now looks like

image

right when they just need to Get Stuff Done

Well yeah, that's a bit the problem here again. "Get Stuff Done" in an installer is to just install something quickly. For new users to git, this pull-behavior question is impossible to answer. Unlikely they know what "rebase" or "ff-only" is. For them, it makes the install more stressful and slower. Should I google this question? How can I change that later? And honestly, you can live in git for years and only ever have fast-forward pulls if that's your workflow.

I think the nag hints git now shows at various places are for new users in the first place. Hints or confirmations are always a bit annoying and disturbing for "pros". But there are not included and built for you anyway.

Anyway, rewording helps a lot already.

@dscho
Copy link
Member

dscho commented Sep 23, 2023

/add relnote bug The installer no longer claims that "fast-forward or merge" is the default git pull behavior: The default behavior has changed in Git a while ago, to "fast-forward only".

The workflow run was started

github-actions bot pushed a commit that referenced this pull request Sep 23, 2023
The installer [no longer claims that "fast-forward or merge" is the
default `git pull`
behavior](#498): The
default behavior has changed in Git a while ago, to "fast-forward only".

Signed-off-by: gitforwindowshelper[bot] <gitforwindowshelper-bot@users.noreply.github.com>
@dscho dscho merged commit 4641b28 into git-for-windows:main Sep 23, 2023
@dscho
Copy link
Member

dscho commented Sep 23, 2023

@kaste thank you!

@kaste
Copy link
Contributor Author

kaste commented Sep 23, 2023

Yeah!

@kaste kaste deleted the flip-pull-behavior-default branch September 23, 2023 14:27
@goyalyashpal
Copy link

goyalyashpal commented Feb 27, 2024

new users (you can't decide the question if you don't know git)
For new users to git, this pull-behavior question is impossible to answer.
- @ kaste at #498 (comment) & elsewhere

yeahhh, soo true. obviously i too didn't know back then; but have seen at various places in github/gitlab where this behaviour of "merge" on git-pull (likely from these sites' web editors) created lotsss of headaches.

now that i know git-local well, and was just doing the update from v2.39.1 to v2.44.0 now; have changed following things during the install & came here from reading the Git/ReleaseNotes.html. i hadn't changed the symlink one before too, but i guess it wasn't enabled in the windows system developer settings.

git config -l
--- git-config-2023
+++ git-config-2024
@@ -1,5 +1,5 @@
 $ git -v
-git version 2.39.1.windows.1
+git version 2.44.0.windows.1
 
 $ git config -l
 diff.astextplain.textconv=astextplain
@@ -8,14 +8,15 @@
 filter.lfs.process=git-lfs filter-process
 filter.lfs.required=true
 http.sslbackend=openssl
-http.sslcainfo=.../Git/mingw64/ssl/certs/ca-bundle.crt
-core.autocrlf=true
+http.sslcainfo=.../Git/mingw64/etc/ssl/certs/ca-bundle.crt
+core.autocrlf=input
 core.fscache=true
-core.symlinks=false
-pull.rebase=false
+core.symlinks=true
+core.editor="C:\\Program Files\\VSCodium\\bin\\codium" --wait
+pull.rebase=true
 credential.helper=manager
 credential.https://dev.azure.com.usehttppath=true
-init.defaultbranch=master
+init.defaultbranch=main
 credential.https://codeberg.org.provider=generic
 core.repositoryformatversion=0
 core.filemode=false

found from git-for-windows/git#4758, that install options are recorded (nice) at Git/etc/install-options.txt: (didn't know this before, so, no diff available for this).

It would have been nice if the wordings mentioned here were a bit inline with installer-cli (.inf) setting keys (8764c72 and git-for-windows/git#4423)

Git/etc/install-options.txt
$ cat $DPF/Git/etc/install-options.txt | column -s ":" -to " :"
Editor Option                 : VSCodium
Custom Editor Path            : 
Default Branch Option         : main
Path Option                   : Cmd
SSH Option                    : OpenSSH
Tortoise Option               : false
CURL Option                   : OpenSSL
CRLF Option                   : LFOnly
Bash Terminal Option          : MinTTY
Git Pull Behavior Option      : Rebase
Use Credential Manager        : Enabled
Performance Tweaks FSCache    : Enabled
Enable Symlinks               : Enabled
Enable Pseudo Console Support : Disabled
Enable FSMonitor              : Disabled

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.

Installer incorrectly states that (effectively) pull.rebase=false is the default git pull behavior
4 participants