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.

Sign up for GitHub

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

Jump to bottom

Add support for --force-if-includes to force push more safely #187932

Merged
merged 8 commits into from Oct 23, 2023
Merged

Add support for --force-if-includes to force push more safely #187932

merged 8 commits into from Oct 23, 2023

Conversation

tats-u
Copy link
Contributor

@tats-u tats-u commented Jul 14, 2023 •
edited

Fixes #144635
Fixes #190356

--force-if-includes makes the force push even safer if used with --force-with-lease.
This option is available since Git 2.30.0. (Released on Dec 2020. The version in Ubuntu 22.04 is newer.)

man git-push says:

Force an update only if the tip of the remote-tracking ref has been integrated locally.

This option enables a check that verifies if the tip of the remote-tracking ref is reachable from one of the "reflog" entries of the local branch based in it for a rewrite. The check ensures that any updates from the remote have been incorporated locally by rejecting the forced update if that is not the case.

If the option is passed without specifying --force-with-lease, or specified along with --force-with-lease=:, it is a "no-op".

The release note says:

"git push --force-with-lease[=]" can easily be misused to lose commits unless the user takes good care of their own "git fetch".
A new option "--force-if-includes" attempts to ensure that what is being force-pushed was created after examining the commit at the tip of the remote ref that is about to be force-replaced.

git push --force-with-lease --force-if-includes is so long that we hesitate to type.

VS Code asks us if it turns "Git: Autofetch" on by a toast:


https://ruky.me/2021/02/21/dont-allow-vscode-to-auto-fetch-from-repos/

Innocent Git newbies will press "Yes" and get ready for overwriting others' remote commits by innocent force push incompletely taught with rebase by Git "expert" bad at teaching.

This PR is created based on #53286.

image

image

@tats-u tats-u changed the title Add support for --force-if-includes to force push Add support for --force-if-includes to force push more safely Jul 17, 2023
@tats-u
Copy link
Contributor Author

tats-u commented Jul 19, 2023

@lszomoru Could you assign another person if you're too busy to review my PR?

@tats-u
Copy link
Contributor Author

tats-u commented Jul 26, 2023 •
edited

@lszomoru Only --force-with-lease can't protect others' pushes when the auto-fetch is enabled.
Why on earth have you been ignoring this PR for 2 weeks?

@tats-u tats-u mentioned this pull request Aug 13, 2023
Closed
@tats-u
Copy link
Contributor Author

tats-u commented Aug 13, 2023

@lszomoru A month is passed. If you don't want to review my change, bring someone else as soon as possible.

@tats-u
Copy link
Contributor Author

tats-u commented Aug 20, 2023

@joaomoreno @alexr00 @bpasero
Dear maintainers who have reviewed recent PRs of is:pr is:merged assignee:lszomoru -author:lszomoru Git:
@lszomoru seems to be too busy to give me a reply for more than one month, so could you review this PR in favor of him?
He hasn't review any PRs for more than one month.

@joaomoreno
Copy link
Member

@tats-u Please be patient.

@tats-u
Copy link
Contributor Author

tats-u commented Sep 22, 2023 •
edited

@joaomoreno When on earth can I get a review from one of you?

lszomoru
lszomoru reviewed Sep 26, 2023
View reviewed changes
extensions/git/src/git.ts Outdated Show resolved Hide resolved
lszomoru
lszomoru requested changes Sep 26, 2023
View reviewed changes
Copy link
Member

@lszomoru lszomoru left a comment

Choose a reason for hiding this comment

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

Could you please address my comments? Thank you!

@lszomoru lszomoru added the git GIT issues label Sep 26, 2023
@lszomoru lszomoru added this to the October 2023 milestone Sep 26, 2023
@tats-u
Copy link
Contributor Author

tats-u commented Sep 26, 2023

@lszomoru I finished the modification. Could you review my changes again?
Also if you want me to squash commits into one, you can feel free to tell it.

@lszomoru
Copy link
Member

lszomoru commented Oct 3, 2023

@tats-u, I have pushed a commit to make some minor improvements.

@lszomoru lszomoru merged commit 2683aa0 into microsoft:main Oct 23, 2023
6 checks passed
@tats-u tats-u deleted the force-if-includes branch October 24, 2023 11:10
Alex0007 pushed a commit to Alex0007/vscode that referenced this pull request Oct 26, 2023
@tats-u @lszomoru @Alex0007
Add support for --force-if-includes to force push more safely (micr…
1484b06 
…osoft#187932)

* Add support for `--force-if-includes` to force push

* Change force push failed error message

* Separate force push (no with lease) failed error message

* Switch to `"markdownDescription"`

* Add Git version requirement for config description

* Improve error message when safer force push is rejected

* Eliminate the option's effect if Git is too old

* Minor improvements to community contribution

---------

Co-authored-by: Ladislau Szomoru <3372902+lszomoru@users.noreply.github.com>
@github-actions github-actions bot locked and limited conversation to collaborators Dec 8, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@lszomoru lszomoru lszomoru approved these changes

@aeschli aeschli aeschli approved these changes

Assignees

@lszomoru lszomoru

Labels
git GIT issues
Projects
None yet
Milestone
October 2023
4 participants
@tats-u @joaomoreno @lszomoru @aeschli