Skip to content

Commit f2f60a5

Browse files
committed
push: disable lazy --force-with-lease by default
"git push --force-with-lease=<branch>:<expect>" makes sure that there is no unexpected changes to the branch at the remote while you prepare a rewrite based on the old state of the branch. This feature came with an experimental option that allows :<expect> part to be omitted by using the tip of remote-tracking branch that corresponds to the <branch>. It turns out that some people use third-party tools that fetch from remote and update the remote-tracking branches behind users' back, defeating the safety that relies on the stability of the remote-tracking branches. We have some warning text that was meant to sound scary in our documentation, but nevertheless people seem to be bitten. See https://public-inbox.org/git/[email protected]/ for a recent example. Let's disable the forms that rely on the stability of remote-tracking branches by default, and allow users who _know_ their remote-tracking branches are stable to enable it with a configuration variable. This problem was predicted from the very beginning; see 28f5d17 (remote.c: add command line option parser for "--force-with-lease", 2013-07-08). Signed-off-by: Junio C Hamano <[email protected]>
1 parent 50ff9ea commit f2f60a5

File tree

9 files changed

+56
-5
lines changed

9 files changed

+56
-5
lines changed

Documentation/config.txt

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2588,6 +2588,11 @@ new default).
25882588

25892589
--
25902590

2591+
push.allowLazyForceWithLease::
2592+
If set to true, allow the `--force-with-lease` option
2593+
without the expected object name (i.e. expecting the objects
2594+
at the tip of corresponding remote-tracking branches).
2595+
25912596
push.followTags::
25922597
If set to true enable `--follow-tags` option by default. You
25932598
may override this configuration at time of push by specifying

Documentation/git-push.txt

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -214,6 +214,10 @@ Note that all forms other than `--force-with-lease=<refname>:<expect>`
214214
that specifies the expected current value of the ref explicitly are
215215
still experimental and their semantics may change as we gain experience
216216
with this feature.
217+
These experimental forms are disabled by default due to safety concerns.
218+
Read the note on safety below, and if you still think you are not affected,
219+
enable it with the `push.allowLazyForceWithLease` configuration variable
220+
(cf. linkgit:git-config[1]).
217221
+
218222
"--no-force-with-lease" will cancel all the previous --force-with-lease on the
219223
command line.

builtin/send-pack.c

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,11 @@ static void print_helper_status(struct ref *ref)
6868
msg = "stale info";
6969
break;
7070

71+
case REF_STATUS_REJECT_LAZY_CAS:
72+
res = "error";
73+
msg = "lazy force-with-error";
74+
break;
75+
7176
case REF_STATUS_REJECT_ALREADY_EXISTS:
7277
res = "error";
7378
msg = "already exists";

remote.c

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1517,7 +1517,9 @@ void set_ref_status_for_push(struct ref *remote_refs, int send_mirror,
15171517
int force_update)
15181518
{
15191519
struct ref *ref;
1520+
int allow_lazy_cas = 0;
15201521

1522+
git_config_get_bool("push.allowLazyForceWithLease", &allow_lazy_cas);
15211523
for (ref = remote_refs; ref; ref = ref->next) {
15221524
int force_ref_update = ref->force || force_update;
15231525
int reject_reason = 0;
@@ -1544,7 +1546,9 @@ void set_ref_status_for_push(struct ref *remote_refs, int send_mirror,
15441546
* branch.
15451547
*/
15461548
if (ref->expect_old_sha1) {
1547-
if (oidcmp(&ref->old_oid, &ref->old_oid_expect))
1549+
if (!allow_lazy_cas && ref->lazy_cas)
1550+
reject_reason = REF_STATUS_REJECT_LAZY_CAS;
1551+
else if (oidcmp(&ref->old_oid, &ref->old_oid_expect))
15481552
reject_reason = REF_STATUS_REJECT_STALE;
15491553
else
15501554
/* If the ref isn't stale then force the update. */
@@ -2341,10 +2345,13 @@ static void apply_cas(struct push_cas_option *cas,
23412345
if (!refname_match(entry->refname, ref->name))
23422346
continue;
23432347
ref->expect_old_sha1 = 1;
2344-
if (!entry->use_tracking)
2348+
if (!entry->use_tracking) {
23452349
hashcpy(ref->old_oid_expect.hash, cas->entry[i].expect);
2346-
else if (remote_tracking(remote, ref->name, &ref->old_oid_expect))
2347-
oidclr(&ref->old_oid_expect);
2350+
} else {
2351+
ref->lazy_cas = 1;
2352+
if (remote_tracking(remote, ref->name, &ref->old_oid_expect))
2353+
oidclr(&ref->old_oid_expect);
2354+
}
23482355
return;
23492356
}
23502357

@@ -2353,6 +2360,7 @@ static void apply_cas(struct push_cas_option *cas,
23532360
return;
23542361

23552362
ref->expect_old_sha1 = 1;
2363+
ref->lazy_cas = 1;
23562364
if (remote_tracking(remote, ref->name, &ref->old_oid_expect))
23572365
oidclr(&ref->old_oid_expect);
23582366
}

remote.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,7 @@ struct ref {
8989
force:1,
9090
forced_update:1,
9191
expect_old_sha1:1,
92+
lazy_cas:1,
9293
deletion:1;
9394

9495
enum {
@@ -118,6 +119,7 @@ struct ref {
118119
REF_STATUS_REJECT_FETCH_FIRST,
119120
REF_STATUS_REJECT_NEEDS_FORCE,
120121
REF_STATUS_REJECT_STALE,
122+
REF_STATUS_REJECT_LAZY_CAS,
121123
REF_STATUS_REJECT_SHALLOW,
122124
REF_STATUS_UPTODATE,
123125
REF_STATUS_REMOTE_REJECT,

send-pack.c

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -248,6 +248,7 @@ static int check_to_send_update(const struct ref *ref, const struct send_pack_ar
248248
case REF_STATUS_REJECT_FETCH_FIRST:
249249
case REF_STATUS_REJECT_NEEDS_FORCE:
250250
case REF_STATUS_REJECT_STALE:
251+
case REF_STATUS_REJECT_LAZY_CAS:
251252
case REF_STATUS_REJECT_NODELETE:
252253
return CHECK_REF_STATUS_REJECTED;
253254
case REF_STATUS_UPTODATE:

t/t5533-push-cas.sh

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,8 @@ test_expect_success setup '
1717
# create template repository
1818
test_commit A &&
1919
test_commit B &&
20-
test_commit C
20+
test_commit C &&
21+
git config --global push.allowLazyForceWithLease true
2122
'
2223

2324
test_expect_success 'push to update (protected)' '
@@ -91,6 +92,7 @@ test_expect_success 'push to update (allowed)' '
9192
(
9293
cd dst &&
9394
test_commit D &&
95+
git config push.allowLazyForceWithLease false &&
9496
git push --force-with-lease=master:master^ origin master
9597
) &&
9698
git ls-remote dst refs/heads/master >expect &&
@@ -103,6 +105,10 @@ test_expect_success 'push to update (allowed, tracking)' '
103105
(
104106
cd dst &&
105107
test_commit D &&
108+
git config push.allowLazyForceWithLease false &&
109+
test_must_fail git push --force-with-lease=master origin master 2>err &&
110+
grep "lazy force-with-lease" err &&
111+
git config --unset push.allowLazyForceWithLease &&
106112
git push --force-with-lease=master origin master 2>err &&
107113
! grep "forced update" err
108114
) &&
@@ -151,6 +157,10 @@ test_expect_success 'push to delete (allowed)' '
151157
setup_srcdst_basic &&
152158
(
153159
cd dst &&
160+
git config push.allowLazyForceWithLease false &&
161+
test_must_fail git push --force-with-lease=master origin :master 2>err &&
162+
grep "lazy force-with-lease" err &&
163+
git config --unset push.allowLazyForceWithLease &&
154164
git push --force-with-lease=master origin :master 2>err &&
155165
grep deleted err
156166
) &&
@@ -183,6 +193,9 @@ test_expect_success 'cover everything with default force-with-lease (allowed)' '
183193
(
184194
cd dst &&
185195
git fetch &&
196+
git config push.allowLazyForceWithLease false &&
197+
test_must_fail git push --force-with-lease origin master master:naster &&
198+
git config --unset push.allowLazyForceWithLease &&
186199
git push --force-with-lease origin master master:naster
187200
) &&
188201
git ls-remote dst refs/heads/master |
@@ -196,6 +209,9 @@ test_expect_success 'new branch covered by force-with-lease' '
196209
(
197210
cd dst &&
198211
git branch branch master &&
212+
git config push.allowLazyForceWithLease false &&
213+
test_must_fail git push --force-with-lease=branch origin branch &&
214+
git config --unset push.allowLazyForceWithLease &&
199215
git push --force-with-lease=branch origin branch
200216
) &&
201217
git ls-remote dst refs/heads/branch >expect &&

transport-helper.c

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -736,6 +736,10 @@ static int push_update_ref_status(struct strbuf *buf,
736736
status = REF_STATUS_REJECT_STALE;
737737
FREE_AND_NULL(msg);
738738
}
739+
else if (!strcmp(msg, "lazy force-with-error")) {
740+
status = REF_STATUS_REJECT_LAZY_CAS;
741+
FREE_AND_NULL(msg);
742+
}
739743
else if (!strcmp(msg, "forced update")) {
740744
forced = 1;
741745
FREE_AND_NULL(msg);
@@ -847,6 +851,7 @@ static int push_refs_with_push(struct transport *transport,
847851
switch (ref->status) {
848852
case REF_STATUS_REJECT_NONFASTFORWARD:
849853
case REF_STATUS_REJECT_STALE:
854+
case REF_STATUS_REJECT_LAZY_CAS:
850855
case REF_STATUS_REJECT_ALREADY_EXISTS:
851856
case REF_STATUS_UPTODATE:
852857
continue;

transport.c

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -418,6 +418,10 @@ static int print_one_push_status(struct ref *ref, const char *dest, int count,
418418
print_ref_status('!', "[rejected]", ref, ref->peer_ref,
419419
"stale info", porcelain, summary_width);
420420
break;
421+
case REF_STATUS_REJECT_LAZY_CAS:
422+
print_ref_status('!', "[rejected]", ref, ref->peer_ref,
423+
"lazy force-with-lease", porcelain, summary_width);
424+
break;
421425
case REF_STATUS_REJECT_SHALLOW:
422426
print_ref_status('!', "[rejected]", ref, ref->peer_ref,
423427
"new shallow roots not allowed",
@@ -934,6 +938,7 @@ static int run_pre_push_hook(struct transport *transport,
934938
if (!r->peer_ref) continue;
935939
if (r->status == REF_STATUS_REJECT_NONFASTFORWARD) continue;
936940
if (r->status == REF_STATUS_REJECT_STALE) continue;
941+
if (r->status == REF_STATUS_REJECT_LAZY_CAS) continue;
937942
if (r->status == REF_STATUS_UPTODATE) continue;
938943

939944
strbuf_reset(&buf);

0 commit comments

Comments
 (0)