On 09/12/2020 11:20, Junio C Hamano wrote: > Srinidhi Kaushik <shrinidhi.kaushik@xxxxxxxxx> writes: > > > Add a new option: `--force-if-includes` to `git-push` where forced > > updates are allowed only if the tip of the remote-tracking ref has > > been integrated locally, by verifying if the tip of the remote-tracking > > ref on which a local branch has based on (for a rewrite), is reachable > > from at least one of the `reflog` entries of the local branch about > > to be updated by force on the remote. > > > > This option can also be used with `--force-with-lease` in setups > > where the remote-tracking refs of the repository are implicitly > > updated in the background. > > > > If a local branch is based on a remote ref for a rewrite, and if that > > remote-tracking ref is updated by a push from another repository after > > it has been checked out locally, force updating that branch to remote > > with `--force-with-lease[=<refname>[:expect]]` without specifying the > > "<refname>" or "<expect>" values, can cause the update that happened > > in-between the checkout and forced push to be lost. > > > > Specifying `--force-with-includes` with `--force-with-lease` as an > > ancillary argument at the time of push, ensures that any new updates > > to the remote-tracking refs are integrated locally before allowing a > > forced update. This behavior can enabled by default if the configuration > > option `push.forceIfIncludesWithLease` is set to `true`. > > This step seems to do too many things at once. Split into logical > progression of improvements, or nobody can sensibly review it, I am > afraid. OK, I will break the commit up into smaller change-sets in the next patch. > > diff --git a/Documentation/config/push.txt b/Documentation/config/push.txt > > index f5e5b38c68..1b4948faa0 100644 > > --- a/Documentation/config/push.txt > > +++ b/Documentation/config/push.txt > > @@ -114,3 +114,11 @@ push.recurseSubmodules:: > > specifying '--recurse-submodules=check|on-demand|no'. > > If not set, 'no' is used by default, unless 'submodule.recurse' is > > set (in which case a 'true' value means 'on-demand'). > > + > > +push.forceIfIncludesWithLease:: > > + If set to `true`, adds `--force-if-includes` as an ancillary argument > > + to `--force-with-lease[=<refname>[:<expect>]]`, when "<refname>" or > > + "<expect>" values are unspecified at the time of push. > > ++ > > +Note: Specifying `--no-force-if-includes` to linkgit:git-push[1] as an > > +argument during the time of push does _not_ override this configuration. > > I do not see why you still want to link these two unrelated > features. I may want to do forced push with lease when I know I am > rewinding, and I may want to do a if-included force when needed, but > I do not see why I want to ask the former and implicitly see it also > trigger the latter. I am actually thinking of getting rid of the configuration option altogether. Sadly, I realized that after I sent out the patch. > I haven't seen the details, but if we severe the (unnecessary?) > entanglement of these two features, perhaps this patch will become a > lot smaller and more focused? I dunno. Other than the configuration setting (which will be removed), tthe only parts that overlap are checking to see if "--force-with-lease" specified with `use_tracking` and `use_tracking_for_test` modes only. The new option does not modify the "strict" version of "compare-and-swap" because in that case, it is not needed at all. However, I think it can be cleaned up a bit more; will look into it. Thanks. -- Srinidhi Kaushik
