{"thread":{"id":"62991","subject":"git client erroneously downloads files when --filter=blob:none and filtering unsupported","startedAt":"2025-02-22T15:01:33Z","lastAt":"2025-02-28T20:27:17Z","messageCount":3,"participants":["John Giorshev","Jeff King"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"512872","messageId":"CADJmAEiE=9Ar8CJFRWeN9f0i7TVDOF7VcV+4f-UaPw0m6Frh6w@mail.gmail.com","threadId":"62991","inReplyTo":null,"subject":"git client erroneously downloads files when --filter=blob:none and filtering unsupported","fromName":"John Giorshev","fromEmail":"john.giorshev1@gmail.com","sentAt":"2025-02-22T15:01:22Z","receivedAt":"2025-02-22T15:01:33Z","isPatch":false,"sender":{"key":"john.giorshev1@gmail.com","avatar":null},"body":"The git client, in my opinion, erroneously downloads files when\n--filter=blob:none is specified and the server does not support\nfiltering. I created a related question on this before coming here:\n\nhttps://stackoverflow.com/q/79413099/15534181\n\nInstead of giving a warning, it should instead error and exit. From a\nuser perspective, when I request \"don't pull down the file contents\"\nand it does it anyway under some circumstances, this is unexpected. In\nmy case it caused performance degradations on a repo scanner.\n\nI propose something like this:\n\nhttps://github.com/jagprog5/git/commit/c4bd8c9640c1491dc6e23acf31fa0230485b68b1\n\nThis is not backwards compatible. My question is, how best should this\nbe handled?  Is this breaking change ok? Or should there instead be a\nnew CLI arg or config which enabled this new behaviour. Looking for\nadvice, thanks.\n"},{"id":"512938","messageId":"20250225013227.GB752084@coredump.intra.peff.net","threadId":"62991","inReplyTo":"CADJmAEiE=9Ar8CJFRWeN9f0i7TVDOF7VcV+4f-UaPw0m6Frh6w@mail.gmail.com","subject":"Re: git client erroneously downloads files when --filter=blob:none and filtering unsupported","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2025-02-25T01:32:27Z","receivedAt":"2025-02-25T01:32:28Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sat, Feb 22, 2025 at 10:01:22AM -0500, John Giorshev wrote:\n\n> The git client, in my opinion, erroneously downloads files when\n> --filter=blob:none is specified and the server does not support\n> filtering. I created a related question on this before coming here:\n> \n> https://stackoverflow.com/q/79413099/15534181\n> \n> Instead of giving a warning, it should instead error and exit. From a\n> user perspective, when I request \"don't pull down the file contents\"\n> and it does it anyway under some circumstances, this is unexpected. In\n> my case it caused performance degradations on a repo scanner.\n> \n> I propose something like this:\n> \n> https://github.com/jagprog5/git/commit/c4bd8c9640c1491dc6e23acf31fa0230485b68b1\n> \n> This is not backwards compatible. My question is, how best should this\n> be handled?  Is this breaking change ok? Or should there instead be a\n> new CLI arg or config which enabled this new behaviour. Looking for\n> advice, thanks.\n\nI could see arguments going either way:\n\n  - you asked for no blobs, but it's just an optimization, so we can\n    complete the operation for a bit more expense. It should be a\n    warning. That makes it safe just sprinkle \"--filter=blob:none\"\n    wherever you like, and sometimes things get faster and sometimes\n    not.\n\n  - downloading the blobs is so expensive that it's better to fail than\n    spend resources on something that will probably fail eventually\n    anyway.\n\nWhich implies to me it should perhaps be configurable. And then that\ngives you a backwards-compatibility solution, too. Step 1 is to add the\nconfig. Step 2 may eventually be to flip the default, and there the\nconfig option gives people an escape hatch if they like the old\nbehavior.\n\nAll that said, I wondered if there was another similar case: when the\nserver supports filters but your particular filter is not allowed.\n\nE.g., if the server has config like this:\n\n  [uploadpackfilter \"blob:none\"]\n  allow = false\n\nBut then interestingly, we already consider that a fatal error!\n\n  $ git clone --filter=blob:none --no-local /path/to/repo\n  fatal: filter 'blob:none' not supported\n  fatal: remote error: filter 'blob:none' not supported\n\nSo I dunno. Maybe nobody actually cares about continuing with a warning\nfor this case. OTOH, I doubt anybody would forbid _just_ blob:none; it's\nthe cheapest filter to support. So it's likely that nobody has run into\nit. But the behavior would be the same with something like sparse:oid,\nwhich is likely to be forbidden because it's expensive. So:\n\n  $ git clone --filter=sparse:oid=foo https://github.com/git/git\n  Cloning into 'git'...\n  fatal: remote error: filter 'sparse:oid' not supported\n\nWhich maybe implies that worrying about config or backwards\ncompatibility is not worth it.\n\n-Peff\n"},{"id":"513296","messageId":"CADJmAEgCRdWzG6kUXS4FHJDQkRmMhQHmTbpHJ0w5q8_WyLqNYQ@mail.gmail.com","threadId":"62991","inReplyTo":"20250225013227.GB752084@coredump.intra.peff.net","subject":"Re: git client erroneously downloads files when --filter=blob:none and filtering unsupported","fromName":"John Giorshev","fromEmail":"john.giorshev1@gmail.com","sentAt":"2025-02-28T20:27:06Z","receivedAt":"2025-02-28T20:27:17Z","isPatch":false,"sender":{"key":"john.giorshev1@gmail.com","avatar":null},"body":"Thanks for the reply, I appreciate it.\n\nMy understanding from this is that it might be ok to send the patch as\nis and not be backward compatible. Or, maybe we need feedback from\nothers?\n\nCreated: https://github.com/gitgitgadget/git/pull/1869\n\nOn Mon, Feb 24, 2025 at 8:32 PM Jeff King <peff@peff.net> wrote:\n>\n> On Sat, Feb 22, 2025 at 10:01:22AM -0500, John Giorshev wrote:\n>\n> > The git client, in my opinion, erroneously downloads files when\n> > --filter=blob:none is specified and the server does not support\n> > filtering. I created a related question on this before coming here:\n> >\n> > https://stackoverflow.com/q/79413099/15534181\n> >\n> > Instead of giving a warning, it should instead error and exit. From a\n> > user perspective, when I request \"don't pull down the file contents\"\n> > and it does it anyway under some circumstances, this is unexpected. In\n> > my case it caused performance degradations on a repo scanner.\n> >\n> > I propose something like this:\n> >\n> > https://github.com/jagprog5/git/commit/c4bd8c9640c1491dc6e23acf31fa0230485b68b1\n> >\n> > This is not backwards compatible. My question is, how best should this\n> > be handled?  Is this breaking change ok? Or should there instead be a\n> > new CLI arg or config which enabled this new behaviour. Looking for\n> > advice, thanks.\n>\n> I could see arguments going either way:\n>\n>   - you asked for no blobs, but it's just an optimization, so we can\n>     complete the operation for a bit more expense. It should be a\n>     warning. That makes it safe just sprinkle \"--filter=blob:none\"\n>     wherever you like, and sometimes things get faster and sometimes\n>     not.\n>\n>   - downloading the blobs is so expensive that it's better to fail than\n>     spend resources on something that will probably fail eventually\n>     anyway.\n>\n> Which implies to me it should perhaps be configurable. And then that\n> gives you a backwards-compatibility solution, too. Step 1 is to add the\n> config. Step 2 may eventually be to flip the default, and there the\n> config option gives people an escape hatch if they like the old\n> behavior.\n>\n> All that said, I wondered if there was another similar case: when the\n> server supports filters but your particular filter is not allowed.\n>\n> E.g., if the server has config like this:\n>\n>   [uploadpackfilter \"blob:none\"]\n>   allow = false\n>\n> But then interestingly, we already consider that a fatal error!\n>\n>   $ git clone --filter=blob:none --no-local /path/to/repo\n>   fatal: filter 'blob:none' not supported\n>   fatal: remote error: filter 'blob:none' not supported\n>\n> So I dunno. Maybe nobody actually cares about continuing with a warning\n> for this case. OTOH, I doubt anybody would forbid _just_ blob:none; it's\n> the cheapest filter to support. So it's likely that nobody has run into\n> it. But the behavior would be the same with something like sparse:oid,\n> which is likely to be forbidden because it's expensive. So:\n>\n>   $ git clone --filter=sparse:oid=foo https://github.com/git/git\n>   Cloning into 'git'...\n>   fatal: remote error: filter 'sparse:oid' not supported\n>\n> Which maybe implies that worrying about config or backwards\n> compatibility is not worth it.\n>\n> -Peff\n"}]}