Re: [PATCH] send-pack: don't send a thin pack to a server which doesn't support it

2013-11-23 Thread Jeff King
On Sat, Nov 23, 2013 at 05:07:55PM +0100, Carlos Martín Nieto wrote:

> Up to now git has assumed that all servers are able to fix thin
> packs. This is however not always the case.
> 
> Document the 'no-thin' capability and prevent send-pack from generating
> a thin pack if the server advertises it.
> ---
> 
> This is a re-roll of the series I sent earlier this month, switching
> it around by adding the "no-thin"

Thanks, I think this moves in the right direction.

I wonder if we want to call it "no-thin-pack" just for consistency with
the affirmative version in upload-pack.

> +The upload-pack server advertises 'thin-pack' when it can generate and
> +send a thin pack. The receive-pack server advertises 'no-thin' if
> +it does not know how to "thicken" the pack it receives.
> +
> +A client requests the 'thin-pack' capability when it understands how
> +to "thicken" it.
>  
>  Client MUST NOT request 'thin-pack' capability if it cannot turn a thin
>  pack into a self-contained pack.
>  
> +Client MUST NOT send a thin pack if the server advertises the
> +'no-thin' capability.

As somebody who participated in the discussion, I know why one is in the
affirmative and one is in the negative. But I think it might help a
reader of the spec to emphasize the difference, and to put the client
behavior for each alongside the server behavior, like:

  The upload-pack server advertises 'thin-pack' when it can generate and
  send a thin pack. A client requests the 'thin-pack' capability when it
  understands how to "thicken" it, notifying the server that it can
  receive such a pack. A client MUST NOT request the 'thin-pack'
  capability if it cannot turn a thin pack into a self-contained pack.

  Receive-pack, on the other hand, is assumed by default to be able to
  handle thin packs, but can ask the client not to use the feature by
  advertising the 'no-thin' capability. A client MUST NOT send a thin
  pack if the server advertises the 'no-thin' capability.

  The reasons for this asymmetry are historical. The receive-pack
  program did not exist until after the invention of thin packs, so
  historically the reference implementation of receive-pack always
  understood thin packs. Adding 'no-thin' later allowed receive-pack to
  disable the feature in a backwards-compatible manner.

-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] send-pack: don't send a thin pack to a server which doesn't support it

2013-11-23 Thread Carlos Martín Nieto
On Sat, 2013-11-23 at 17:07 +0100, Carlos Martín Nieto wrote:
> Up to now git has assumed that all servers are able to fix thin
> packs. This is however not always the case.
> 
> Document the 'no-thin' capability and prevent send-pack from generating
> a thin pack if the server advertises it.

Sorry,

Signed-off-by: Carlos Martín Nieto 

> ---
> 
> This is a re-roll of the series I sent earlier this month, switching
> it around by adding the "no-thin" 
> 
>  Documentation/technical/protocol-capabilities.txt | 20 +++-
>  send-pack.c   |  2 ++
>  2 files changed, 17 insertions(+), 5 deletions(-)
> 
> diff --git a/Documentation/technical/protocol-capabilities.txt 
> b/Documentation/technical/protocol-capabilities.txt
> index fd8ffa5..3a75e79 100644
> --- a/Documentation/technical/protocol-capabilities.txt
> +++ b/Documentation/technical/protocol-capabilities.txt
> @@ -72,15 +72,25 @@ interleaved with S-R-Q.
>  thin-pack
>  -
>  
> -This capability means that the server can send a 'thin' pack, a pack
> -which does not contain base objects; if those base objects are available
> -on client side. Client requests 'thin-pack' capability when it
> -understands how to "thicken" it by adding required delta bases making
> -it self-contained.
> +A thin pack is one with deltas which reference base objects not
> +contained within the pack (but are known to exist at the receiving
> +end). This can reduce the network traffic significantly, but it
> +requires the receiving end to know how to "thicken" these packs by
> +adding the missing bases to the pack.
> +
> +The upload-pack server advertises 'thin-pack' when it can generate and
> +send a thin pack. The receive-pack server advertises 'no-thin' if
> +it does not know how to "thicken" the pack it receives.
> +
> +A client requests the 'thin-pack' capability when it understands how
> +to "thicken" it.
>  
>  Client MUST NOT request 'thin-pack' capability if it cannot turn a thin
>  pack into a self-contained pack.
>  
> +Client MUST NOT send a thin pack if the server advertises the
> +'no-thin' capability.
> +
>  
>  side-band, side-band-64k
>  
> diff --git a/send-pack.c b/send-pack.c
> index 7d172ef..9877eb9 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("no-thin"))
> + args->use_thin_pack = 0;
>  
>   if (!remote_refs) {
>   fprintf(stderr, "No refs in common and none specified; doing 
> nothing.\n"



--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2013-11-23 Thread Carlos Martín Nieto
Up to now git has assumed that all servers are able to fix thin
packs. This is however not always the case.

Document the 'no-thin' capability and prevent send-pack from generating
a thin pack if the server advertises it.
---

This is a re-roll of the series I sent earlier this month, switching
it around by adding the "no-thin" 

 Documentation/technical/protocol-capabilities.txt | 20 +++-
 send-pack.c   |  2 ++
 2 files changed, 17 insertions(+), 5 deletions(-)

diff --git a/Documentation/technical/protocol-capabilities.txt 
b/Documentation/technical/protocol-capabilities.txt
index fd8ffa5..3a75e79 100644
--- a/Documentation/technical/protocol-capabilities.txt
+++ b/Documentation/technical/protocol-capabilities.txt
@@ -72,15 +72,25 @@ interleaved with S-R-Q.
 thin-pack
 -
 
-This capability means that the server can send a 'thin' pack, a pack
-which does not contain base objects; if those base objects are available
-on client side. Client requests 'thin-pack' capability when it
-understands how to "thicken" it by adding required delta bases making
-it self-contained.
+A thin pack is one with deltas which reference base objects not
+contained within the pack (but are known to exist at the receiving
+end). This can reduce the network traffic significantly, but it
+requires the receiving end to know how to "thicken" these packs by
+adding the missing bases to the pack.
+
+The upload-pack server advertises 'thin-pack' when it can generate and
+send a thin pack. The receive-pack server advertises 'no-thin' if
+it does not know how to "thicken" the pack it receives.
+
+A client requests the 'thin-pack' capability when it understands how
+to "thicken" it.
 
 Client MUST NOT request 'thin-pack' capability if it cannot turn a thin
 pack into a self-contained pack.
 
+Client MUST NOT send a thin pack if the server advertises the
+'no-thin' capability.
+
 
 side-band, side-band-64k
 
diff --git a/send-pack.c b/send-pack.c
index 7d172ef..9877eb9 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("no-thin"))
+   args->use_thin_pack = 0;
 
if (!remote_refs) {
fprintf(stderr, "No refs in common and none specified; doing 
nothing.\n"
-- 
1.8.5.rc3.362.gdf10213

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html