All of lore.kernel.org
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Christian Couder <christian.couder@gmail.com>
Cc: git@vger.kernel.org,  John Cai <johncai86@gmail.com>,
	 Patrick Steinhardt <ps@pks.im>,
	 Christian Couder <chriscool@tuxfamily.org>
Subject: Re: [PATCH v2 3/3] upload-pack: allow configuring a missing-action
Date: Tue, 28 May 2024 08:54:43 -0700	[thread overview]
Message-ID: <xmqq34q27wzg.fsf@gitster.g> (raw)
In-Reply-To: <CAP8UFD1_aHwbhF12v-miCTWEbbgjtpjTCmkRmFHu4Vusezq6dA@mail.gmail.com> (Christian Couder's message of "Tue, 28 May 2024 12:10:31 +0200")

Christian Couder <christian.couder@gmail.com> writes:

> It's basically the same as when a regular clone or a partial clone or
> a clone using bundle-uri fails or when using a regular bundle fails.
> If it failed because the remote was not properly configured, then that
> config can be fixed. If it fails because the remote doesn't have some
> objects, then maybe the missing objects can be transferred to the
> remote. And so on.

> The feature doesn't create any new kind of failure.

> "it would be nice if there was a capability for the client to say
> that it would like the server to give it information about the
> promisor that it could use, so that the user doesn't have to pass all
> the "remote.my_promisor.XXX" config options on the command like."
>
> and by saying that this could be added later.

Can such an update happen transparently, or does it need changes to
end-user experience?  It is of dubious value to leave the initial
series be incomplete if the latter is the case.

>> Without knowing that, it cannot safely decide that it does not
>> have to send objects that can be obtained from X to C.
>
> In the above command C is asking for a partial clone, as it uses a
> --filter option. This means that C knows very well that it might not
> get from S all the objects needed for a complete object graph.

Hmph.  If C asks a partial clone and S is willing to be the promisor
for C, S is essentially saying that it will serve C any objects on
demand that are reachable from any object it served C in the past,
forever, no?  It might not get from S initially all the objects, but
if it wants later, S promises to let C have them.


> Again when using a regular partial clone omitting the same set of
> objects, C also requests some objects that S doesn't have.  And
> this is not considered an issue or something irresponsible. It
> already works like this.

"S doesn't have" is because S has pruned objects that it shouldn't
have in order to keep the promise it made earlier to C, right?  If
that is the case, I would very much say S is being irresponsible in
that case.

> And then C still has the possibility to configure X as a
> promisor remote and get missing objects from there. So why is it Ok
> when it's done in several steps but not in one?

You are right that S can decide to unilaterally break the promise it
made C, so this update is not making it any worse than giving users
a broken implementation of promisor remotes.  I wouldn't call it OK,
though.

If somebody identifies that even without this series, S can lead to
repository corruption at C by pruning objects it does need to keep
its promise to C, the next action I expect from healthy project is
to try coming up with a mechanism to make it less likely that such a
pruning happens by accident (e.g., by noticing allowAnySHA1InWant as
a sign that the repository has promised others to serve anything
that used to be reachable from anything it historically served,
disabling repack "-d" and instead send the currently unreachable
objects to an archived pack, and something equally silly like that).
It certainly is not to add a new mechanism to make it even easier to
configure S to break promised it made to C.

So, I dunno.

  reply	other threads:[~2024-05-28 15:54 UTC|newest]

Thread overview: 46+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-10-12 13:51 [PATCH 0/3] Implement filtering repacks Christian Couder
2022-10-12 13:51 ` [PATCH 1/3] pack-objects: allow --filter without --stdout Christian Couder
2022-10-12 13:51 ` [PATCH 2/3] repack: add --filter=<filter-spec> option Christian Couder
2022-10-12 13:51 ` [PATCH 3/3] repack: introduce --force to force filtering Christian Couder
2022-10-14 16:46 ` [PATCH 0/3] Implement filtering repacks Junio C Hamano
2022-10-20 11:23   ` Christian Couder
2022-10-28 19:49     ` Taylor Blau
2022-10-28 20:26       ` Junio C Hamano
2022-11-07  9:12         ` Christian Couder
2022-11-07  9:00       ` Christian Couder
2022-10-25 12:28 ` [PATCH v2 0/2] " Christian Couder
2022-10-25 12:28   ` [PATCH v2 1/2] pack-objects: allow --filter without --stdout Christian Couder
2022-10-25 12:28   ` [PATCH v2 2/2] repack: add --filter=<filter-spec> option Christian Couder
2022-10-28 19:54   ` [PATCH v2 0/2] Implement filtering repacks Taylor Blau
2022-11-07  9:29     ` Christian Couder
2022-11-22 17:51   ` [PATCH v3 " Christian Couder
2022-11-22 17:51     ` [PATCH v3 1/2] pack-objects: allow --filter without --stdout Christian Couder
2022-11-22 17:51     ` [PATCH v3 2/2] repack: add --filter=<filter-spec> option Christian Couder
2022-11-23  0:31     ` [PATCH v3 0/2] Implement filtering repacks Junio C Hamano
2022-12-21  3:53       ` Christian Couder
2022-11-23  0:35     ` Junio C Hamano
2022-12-21  4:04     ` [PATCH v4 0/3] " Christian Couder
2022-12-21  4:04       ` [PATCH v4 1/3] pack-objects: allow --filter without --stdout Christian Couder
2023-01-04 14:56         ` Patrick Steinhardt
2022-12-21  4:04       ` [PATCH v4 2/3] repack: add --filter=<filter-spec> option Christian Couder
2023-01-04 14:56         ` Patrick Steinhardt
2023-01-05  1:39           ` Junio C Hamano
2022-12-21  4:04       ` [PATCH v4 3/3] gc: add gc.repackFilter config option Christian Couder
2023-01-04 14:57         ` Patrick Steinhardt
2024-05-15 13:25 ` [PATCH v2 0/3] upload-pack: support a missing-action Christian Couder
2024-05-15 13:25   ` [PATCH v2 1/3] rev-list: refactor --missing=<missing-action> Christian Couder
2024-05-15 16:16     ` Junio C Hamano
2024-05-15 13:25   ` [PATCH v2 2/3] pack-objects: use the missing action API Christian Couder
2024-05-15 16:46     ` Junio C Hamano
2024-05-24 16:40       ` Christian Couder
2024-05-15 13:25   ` [PATCH v2 3/3] upload-pack: allow configuring a missing-action Christian Couder
2024-05-15 17:08     ` Junio C Hamano
2024-05-24 16:41       ` Christian Couder
2024-05-24 21:51         ` Junio C Hamano
2024-05-28 10:10           ` Christian Couder
2024-05-28 15:54             ` Junio C Hamano [this message]
2024-05-31 20:43               ` Christian Couder
2024-06-01  9:43                 ` Junio C Hamano
2024-06-03 15:01                   ` Christian Couder
2024-06-03 17:29                     ` Junio C Hamano
2024-05-15 13:59   ` [PATCH v2 0/3] upload-pack: support " Christian Couder

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=xmqq34q27wzg.fsf@gitster.g \
    --to=gitster@pobox.com \
    --cc=chriscool@tuxfamily.org \
    --cc=christian.couder@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=johncai86@gmail.com \
    --cc=ps@pks.im \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.