[PATCH] send-pack: don't send a thin pack when the server doesn't support it

Not every server out there supports fixing thin packs, so let's send
them a full pack.

Signed-off-by: Carlos Martín Nieto <redacted>
---

It's not always possible to support thin packs (sometimes there isn't
even an object database to grab bases out of). And in any case git
shouldn't create thin packs if the server hasn't said it knows how to
fix them, as per the point of the extension.

 send-pack.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/send-pack.c b/send-pack.c
index 7d172ef..7b88ac8 100644
--- a/send-pack.c
+++ b/send-pack.c

@@ -205,6 +205,8 @@ int send_pack(struct send_pack_args *args,
 		quiet_supported = 1;
 	if (server_supports("agent"))
 		agent_supported = 1;
+	if (!server_supports("thin-pack"))
+		args->use_thin_pack = 0;
 
 	if (!remote_refs) {
 		fprintf(stderr, "No refs in common and none specified; doing nothing.\n"

-- 
1.8.4.561.g1c3d45d

On Tue, Oct 8, 2013 at 3:44 PM, Carlos Martín Nieto [off-list ref] wrote:

Hmm.. I think this introduces a regression in C Git because
receive-pack never advertises "thin-pack" and so "git push" from now
on will never send thin packs. It's too late now to add thin-pack to
receive-pack, perhaps a new extension "no-thin-pack" for those
servers? Alternatively, just run git push --no-thin (you'll need
f7c815c (push: respect --no-thin - 2013-08-12) though).

        if (!remote_refs) {
                fprintf(stderr, "No refs in common and none specified; doing nothing.\n"

-- 
Duy

On Tue, Oct 8, 2013 at 4:44 PM, Duy Nguyen [off-list ref] wrote:

On Tue, Oct 8, 2013 at 3:44 PM, Carlos Martín Nieto [off-list ref] wrote:

quoted

diff --git a/send-pack.c b/send-pack.c
index 7d172ef..7b88ac8 100644
--- a/send-pack.c
+++ b/send-pack.c

@@ -205,6 +205,8 @@ int send_pack(struct send_pack_args *args,
                quiet_supported = 1;
        if (server_supports("agent"))
                agent_supported = 1;
+       if (!server_supports("thin-pack"))
+               args->use_thin_pack = 0;

Hmm.. I think this introduces a regression in C Git because
receive-pack never advertises "thin-pack" and so "git push" from now
on will never send thin packs. It's too late now to add thin-pack to
receive-pack

Or maybe it's not that late. How about you go with your patch and add
thin-pack capability to receive-pack too?

When new "git push" is used against old server, thin pack is disabled.
But that's not a big deal (I hope). The difference is traffic
increases a bit more, but cpu consumption on the server side is also a
bit less. Pushes are usually small, unless you do initial push, which
is complete anyway. So a bit more or less does not really matter in
practice.

We could mitigate the regression a bit by checking agent string and
enable thin-pack for those versions even if thin-pack is not
advertises. That goes back to v1.7.12. We may do the same for some
other popular git servers. And this hack is maintained like 3-4 years,
enough time for new versions to be deployed, then removed.

Hmm?
-- 
Duy

Duy Nguyen wrote:

Or maybe it's not that late. How about you go with your patch and add
thin-pack capability to receive-pack too?

When new "git push" is used against old server, thin pack is disabled.
But that's not a big deal (I hope).

Could we have separate patches to introduce the server-side capability
and then to request it in the client?  That way, people with old
servers can apply the patch introducing the capability if they want.

The new meaning of the "thin-pack" capability should also be
documented in Documentation/technical/protocol-capabilities.txt.

Done that way and with enough time between the server advertising the
capability and the client looking for it, it seems like a good idea.

My two cents,
Jonathan

On Tue, 2013-10-08 at 15:22 -0700, Jonathan Nieder wrote:

Duy Nguyen wrote:

quoted

Or maybe it's not that late. How about you go with your patch and add
thin-pack capability to receive-pack too?

When new "git push" is used against old server, thin pack is disabled.
But that's not a big deal (I hope).

Could we have separate patches to introduce the server-side capability
and then to request it in the client?  That way, people with old
servers can apply the patch introducing the capability if they want.

That could work.

The new meaning of the "thin-pack" capability should also be
documented in Documentation/technical/protocol-capabilities.txt.

Oh, right; the capability as described is only about the server being
able to generate a thin pack. Wouldn't his mean that git shouldn't
assume that *any* remote can fix thin packs, though? (other than most
servers you do talk to happen to).

Anyway, facts on the ground and all that. I'll prepare some 

Done that way and with enough time between the server advertising the
capability and the client looking for it, it seems like a good idea.

If such patches would be accepted, that would be great. By the time this
all gets merged, we might have thin pack fixing merged into libgit2, but
there will still be uses where fixing them isn't an issue due to other
constraints.


Cheers,
   cmn