search

sublimehq / sublime_merge

Issue tracker for Sublime Merge
https://www.sublimemerge.com
274 stars 14 forks source link

"push --force-with-lease" should not be allowed when "Automatic Fetching" is enabled. #1846

Open VulumeCode opened 10 months ago

VulumeCode commented 10 months ago

Version info

Description

Sublime Merge makes it easy to get a false sense of security, by allowing the use of "push --force-with-lease" combined with "Automatic Fetching".

From the git docs https://git-scm.com/docs/git-push

A general note on safety: supplying this option without an expected value, i.e. as --force-with-lease or --force-with-lease= interacts very badly with anything that implicitly runs git fetch on the remote to be pushed to in the background, e.g. git fetch origin on your repository in a cronjob.

The protection it offers over --force is ensuring that subsequent changes your work wasn’t based on aren’t clobbered, but this is trivially defeated if some background process is updating refs in the background. We don’t have anything except the remote tracking info to go by as a heuristic for refs you’re expected to have seen & are willing to clobber.

Steps to reproduce

You rebase a branch.

Your friend also rebases the branch and force pushes.

Automatic Fetch happens.

You force push with lease, thinking it's safe. It completes successfully.

Your friend's changes are lost regardless.

Expected behavior

Only force push is available, so you'll only have yourself to blame.

Alternatively, the git docs explain ways to make it safe again, but I don't know how feasible it is to implement that.

themilkman commented 10 months ago

Agree, at least there should be a "smart protection" by SM.

https://github.com/sublimehq/sublime_merge/issues/25#issuecomment-746467740 and some of the comments below target the same topic

themilkman commented 7 months ago

Maybe git maintenance mechanisms could be utilized way to build a solution for the core problem, if I get it correctly ("fetch" stuff, but not updating refs → if SM would check that, it could be done transparently and show it in the UI). Obviously, SM should not run this hourly only. @ratijas, as I you brought this up, too, do you also think that this could work?