diff mbox series

[2/3] transport-helper: drop "object-format <algo>" option

Message ID 20240320093740.GB2445682@coredump.intra.peff.net (mailing list archive)
State Accepted
Commit d6f6b433a87a0157b2f1bc45a3758841fc14fb8b
Headers show
Series some transport-helper "option object-format" confusion | expand

Commit Message

Jeff King March 20, 2024, 9:37 a.m. UTC
The documentation in gitremote-helpers.txt claims that helpers should
accept an object-format option from Git whose value is either:

  1. "true", in which case the helper is merely told that Git
     understands the special ":object-format" response, and will send it

  2. an algorithm name that the helper should use

However, Git has never sent the second form, and it's not clear if it
would ever be useful.

When interacting with a remote Git repository, we generally discover
what _their_ object format is, and then decide what to do with a
mismatch (where that is currently just "bail out", but could eventually
be on-the-fly conversion and interop). And that is true for native
protocols, but also for transport helpers like remote-curl that talk to
remote Git repositories.  There we send back an ":object-format" line
telling Git what remote-curl detected on the other side.

And this is true even for pushes (since we get it via receive-pack's
advertisement). And it is even true for dumb-http, as we guess at the
algorithm based on the hash size, due to ac093d0790 (remote-curl: detect
algorithm for dumb HTTP by size, 2020-06-19).

The one case where it _isn't_ true is dumb-http talking to an empty
repository. There we have no clue what the remote hash is, so
remote-curl just sends back its default. If we kept the "object-format
<algo>" form then in theory Git could say "object-format sha256" to
change that default. But it doesn't really accomplish anything. We still
may or may not be mis-matched with the other side. For a fetch that's
OK, since it's by definition a noop. For a push into an empty
repository, it might matter (though the dumb http-push DAV code seems
happy to clobber a remote sha256 info/refs and corrupt the repository).
If we want to pursue making this work, I think we'd be better off
improving detection of the object format of empty repositories over
dumb-http (e.g., an "info/object-format" file).

But what about helpers that _aren't_ talking to another Git repo?
Consider something like git-cinnabar, which is converting on the fly
to/from hg. Most of the heavy lifting is done by fast-import/export, but
some oids may still pass between Git and the helper. Could
"object-format <algo>" be useful to tell the helper what oids we expect
to see?

Possibly, but in practice this isn't necessary. Git-cinnabar for example
already peeks at the local-repo .git/config to check its object-format
(and currently just bails if it is sha256).

So I think the "object-format" extension really is only useful for the
helper telling Git what object-format it found, and not the other way
around.

Note that this patch can't break any remote helpers; we're not changing
the code on the Git side at all, but just bringing the documentation in
line with what Git has always done. It does remove the receiving support
in remote-curl.c, but that code was never actually triggered.

Signed-off-by: Jeff King <peff@peff.net>
---
 Documentation/gitremote-helpers.txt | 7 ++-----
 remote-curl.c                       | 9 ++-------
 2 files changed, 4 insertions(+), 12 deletions(-)
diff mbox series

Patch

diff --git a/Documentation/gitremote-helpers.txt b/Documentation/gitremote-helpers.txt
index 07c8439a6f..0d3f4f37c2 100644
--- a/Documentation/gitremote-helpers.txt
+++ b/Documentation/gitremote-helpers.txt
@@ -542,13 +542,10 @@  set by Git if the remote helper has the 'option' capability.
 	transaction.  If successful, all refs will be updated, or none will.  If the
 	remote side does not support this capability, the push will fail.
 
-'option object-format' {'true'|algorithm}::
-	If 'true', indicate that the caller wants hash algorithm information
+'option object-format true'::
+	Indicate that the caller wants hash algorithm information
 	to be passed back from the remote.  This mode is used when fetching
 	refs.
-+
-If set to an algorithm, indicate that the caller wants to interact with
-the remote side using that algorithm.
 
 SEE ALSO
 --------
diff --git a/remote-curl.c b/remote-curl.c
index 1161dc7fed..31b02b8840 100644
--- a/remote-curl.c
+++ b/remote-curl.c
@@ -211,14 +211,9 @@  static int set_option(const char *name, const char *value)
 		options.filter = xstrdup(value);
 		return 0;
 	} else if (!strcmp(name, "object-format")) {
-		int algo;
 		options.object_format = 1;
-		if (strcmp(value, "true")) {
-			algo = hash_algo_by_name(value);
-			if (algo == GIT_HASH_UNKNOWN)
-				die("unknown object format '%s'", value);
-			options.hash_algo = &hash_algos[algo];
-		}
+		if (strcmp(value, "true"))
+			die(_("unknown value for object-format: %s"), value);
 		return 0;
 	} else {
 		return 1 /* unsupported */;