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

Make git push --force-with-lease safer #640

Open
dscho opened this issue May 20, 2020 · 13 comments
Open

Make git push --force-with-lease safer #640

dscho opened this issue May 20, 2020 · 13 comments
Labels
leftoverbits From the Git mailing list: https://lore.kernel.org/git/?q=%23leftoverbits

Comments

@dscho
Copy link
Member

dscho commented May 20, 2020

The --force-with-lease option ensures that you can only force-push updates where your remote-tracking ref agrees with the remote ref.

Obviously, the idea was that if you call git pull <remote>, you will have seen and incorporated the remote commits, even if you rebased and the result therefore does not fast-forward.

However, one rather common setup is ignored by this idea: Git UIs often "helpfully" fetch in the background. And that does not update the local branch, therefore it is quite possible to lose commits inadvertently.

On the mailing list, I proposed the following strategy:

The new behavior would look at the reflog, much as the --fork-point option of git rebase: in addition to the regular --orce-with-lease server-side checks, a client-side check first verifies that the remote-tracking branch is reachable at least from one of the items in the reflog of the branch we are about to push.

That is, it would ensure that even if we rebased locally, we did incorporate the tip commit of the remote-tracking branch, at some stage.

Granted, there are probably cases where you would fetch, look at the remote-tracking branch, and reject those changes without integrating those into the local branch. In that case, you would want to relax to the current behavior of --force-with-lease. But I would expect that to happen only rarely.

The safety by the proposed behavior would make it a lot easier to accept a config setting that makes this the default.

@dscho dscho added the leftoverbits From the Git mailing list: https://lore.kernel.org/git/?q=%23leftoverbits label May 20, 2020
@periperidip
Copy link

periperidip commented Feb 11, 2021

Hi @dscho ! Is this issue still unsolved? I studied this and it seemed very interesting to me. I would like to work on this.

@periperidip
Copy link

From what I have inferred, the issue is that many Git UIs or sometimes even the user may unknowingly fetch the latest status of the remote ref but will not compare it with our local branch before pushing since the user does not ask so.

Right now, our job is to make sure that the local incorporates the tip of the remote, essentially doing a fetch first and then comparing before pushing, is that correct?

I am not able to understand how this happens as you quote here:

In that case, you did not see what was fetched, and you might have missed
updates, and you will overwrite them, even if you tried to be careful by
using --for-ce-with-lease.

How will we overwrite any updates when the remote ref won't match the local ref when we use the --force-with-lease option?

@dscho
Copy link
Member Author

dscho commented Feb 12, 2021

I think that this has been addressed via the --force-if-included option (at least that was the idea, although I seem to be unable to make --force-if-included do what I want over here).

@dscho
Copy link
Member Author

dscho commented Feb 12, 2021

@periperidip and if you're looking for work, how about finishing ss/submodule-add-in-c? It was just kicked out because it has been stale for so long.

@periperidip
Copy link

I think that this has been addressed via the --force-if-included option (at least that was the idea, although I seem to be unable to make --force-if-included do what I want over here).

What exactly is missing would you like to talk about it a little bit here?

@periperidip
Copy link

@periperidip and if you're looking for work, how about finishing ss/submodule-add-in-c? It was just kicked out because it has been stale for so long.

I am quite bored with the whole work. I was thinking of trying something else with Git. Plus, that work has been finalised as a project for the upcoming GSoC too.

Do you have any other stuff I could work on? Any issues which are left or any discussions which are going on? I was also thinking of working on some NEEDSWORK topics in the Git code. What do you think?

@dscho
Copy link
Member Author

dscho commented Feb 15, 2021

if you're looking for work, how about finishing ss/submodule-add-in-c? It was just kicked out because it has been stale for so long.

I am quite bored with the whole work. I was thinking of trying something else with Git. Plus, that work has been finalised as a project for the upcoming GSoC too.

In that case, I do not want you to work on any issue that concerns me personally, lest you get bored on working on that, too ;-)

@chinmoy12c
Copy link

@periperidip and if you're looking for work, how about finishing ss/submodule-add-in-c? It was just kicked out because it has been stale for so long.

@dscho, could you please direct me to the work that was done previously on this (and any other relevant links maybe). Maybe I could work on it and try to finish it :).

@dscho
Copy link
Member Author

dscho commented Mar 27, 2021

@periperidip and if you're looking for work, how about finishing ss/submodule-add-in-c? It was just kicked out because it has been stale for so long.

@dscho, could you please direct me to the work that was done previously on this (and any other relevant links maybe). Maybe I could work on it and try to finish it :).

Sure. https://lore.kernel.org/git/20201214231939.644175-1-periperidip@gmail.com/ seems to be the latest attempt, and git@1861aa4#diff-1e38796093f163084858763921ead3459e09087f85af3e4354f3401c1641392dR1158-R1169 has the information what still needs to be done.

But please only work on this if you find it personally satisfying or interesting.

@chinmoy12c
Copy link

@periperidip and if you're looking for work, how about finishing ss/submodule-add-in-c? It was just kicked out because it has been stale for so long.

@dscho, could you please direct me to the work that was done previously on this (and any other relevant links maybe). Maybe I could work on it and try to finish it :).

Sure. https://lore.kernel.org/git/20201214231939.644175-1-periperidip@gmail.com/ seems to be the latest attempt, and git@1861aa4#diff-1e38796093f163084858763921ead3459e09087f85af3e4354f3401c1641392dR1158-R1169 has the information what still needs to be done.

Thanks 😄

But please only work on this if you find it personally satisfying or interesting.

Sure, it would help me dive deep into the codebase and understand it properly :).

@dscho
Copy link
Member Author

dscho commented Mar 28, 2021

I guess if it was up to me, I'd not work on submodules, unless I had committed to it. In my mind, submodules are a mistake by design. Therefore, if I were to work in this space, I'd try to design something with the same goal (allowing not to check out a given set of subdirectories) around partial clone and git subtree.

@chinmoy12c
Copy link

I guess if it was up to me, I'd not work on submodules, unless I had committed to it. In my mind, submodules are a mistake by design. Therefore, if I were to work in this space, I'd try to design something with the same goal (allowing not to check out a given set of subdirectories) around partial clone and git subtree.

Okay, then I'd rather find a different issue to work on.

Thanks 😄

@dscho
Copy link
Member Author

dscho commented Mar 28, 2021

I mean, I don't want to discourage you from working on submodules, at the same time I have to admit that I find other parts of Git much more rewarding.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
leftoverbits From the Git mailing list: https://lore.kernel.org/git/?q=%23leftoverbits
Projects
None yet
Development

No branches or pull requests

3 participants