git@vger.kernel.org list mirror (unofficial, one of many)
 help / color / mirror / code / Atom feed
From: Christian Couder <christian.couder@gmail.com>
To: Taylor Blau <me@ttaylorr.com>
Cc: git@vger.kernel.org, Junio C Hamano <gitster@pobox.com>,
	John Cai <johncai86@gmail.com>,
	Jonathan Tan <jonathantanmy@google.com>,
	Jonathan Nieder <jrnieder@gmail.com>,
	Derrick Stolee <stolee@gmail.com>
Subject: Re: [PATCH v2 0/2] Implement filtering repacks
Date: Mon, 7 Nov 2022 10:29:51 +0100	[thread overview]
Message-ID: <CAP8UFD2jjQxMsN7VzticiFkrOSLVK6f-f4R1fZU1wnEBpuHapQ@mail.gmail.com> (raw)
In-Reply-To: <Y1wzgOTiKdBcBba0@nand.local>

On Fri, Oct 28, 2022 at 9:54 PM Taylor Blau <me@ttaylorr.com> wrote:
>
> On Tue, Oct 25, 2022 at 02:28:54PM +0200, Christian Couder wrote:
> > So there are only 2 patches now in this v2 series:
> >
> >   - Patch 1/2 is a preparatory patch.
> >
> >   - Patch 2/2 introduces the `--filter=<filter-spec>` option.
>
> One thing that I wasn't clear on in this or the previous round(s) was
> how we handle setting remote.<name>.promisor and partialclonefilter.

Yeah, I agree that it's an interesting question that I overlooked.

> If there is a single remote, then it's obvious that we should set
> promisor to "true" and partialCloneFilter to whatever value of
> `--filter` the user provided when repacking / GCing.

I would be Ok to setting remote.<name>.promisor to true in this case,
but I am not sure we really need to do it.

Maybe the user is mostly interested in reducing the size of the repo
for now and plans to set up a promisor remote afterwards.

Another perhaps better way to handle this would be to just die() if no
remote.<name>.promisor is set to true. This way we can make sure that
users will not forget to set up at least one promisor remote. This
could also give users the opportunity to think about whether their
configured remotes contain all the objects they are going to remove.

About remote.<name>.partialclonefilter I don't think we need to do
anything. Maybe the user would be Ok with having different filters
when fetching and when cleaning up.

> But what happens if there are multiple remotes? Which get the new
> configuration settings modified?

I agree that if we want this feature to modify settings, then there is
no good and simple solution in this case.

> I wonder what breakage happens if we fail to do that (and why such
> breakage isn't yet noticed by CI).

If we want to avoid breakages as much as possible, then die()ing when
no remote.<name>.promisor is set to true seems to be the best
solution, instead of trying to modify settings.

  reply	other threads:[~2022-11-07  9:32 UTC|newest]

Thread overview: 29+ 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 [this message]
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

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

  List information: http://vger.kernel.org/majordomo-info.html

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

  git send-email \
    --in-reply-to=CAP8UFD2jjQxMsN7VzticiFkrOSLVK6f-f4R1fZU1wnEBpuHapQ@mail.gmail.com \
    --to=christian.couder@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=johncai86@gmail.com \
    --cc=jonathantanmy@google.com \
    --cc=jrnieder@gmail.com \
    --cc=me@ttaylorr.com \
    --cc=stolee@gmail.com \
    /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.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).