[PATCH] push: add remote.pushThin to control thin pack generation
From: brian m. carlson brian.carl...@cpanel.net
Thin packs are enabled when pushing by default, but with a large number
of refs and a fast network, git may spend more time trying to find a
good delta than it would spend simply sending data over the network.
Add a per-remote option, pushThin, that controls the default for pushes
on that remote.
Signed-off-by: brian m. carlson brian.carl...@cpanel.net
---
SOURCES/git/Documentation/config.txt | 6 ++
SOURCES/git/builtin/push.c | 6 --
SOURCES/git/remote.c | 3 +++
SOURCES/git/remote.h | 1 +
SOURCES/git/transport.c | 2 ++
5 files changed, 16 insertions(+), 2 deletions(-)
diff --git a/SOURCES/git/Documentation/config.txt
b/SOURCES/git/Documentation/config.txt
index 9220725..7fededd 100644
--- a/SOURCES/git/Documentation/config.txt
+++ b/SOURCES/git/Documentation/config.txt
@@ -2178,6 +2178,12 @@ remote.name.push::
The default set of refspec for linkgit:git-push[1]. See
linkgit:git-push[1].
+remote.name.pushThin::
+ If true (the default), pass the \--thin option to
+ linkgit:git-pack-objects[1] during push. This results in a smaller
+ pack being sent and can improve push time over slow networks. Over
+ fast networks, setting this value to false may improve performance.
+
remote.name.mirror::
If true, pushing to this remote will automatically behave
as if the `--mirror` option was given on the command line.
diff --git a/SOURCES/git/builtin/push.c b/SOURCES/git/builtin/push.c
index a076b19..ae39677 100644
--- a/SOURCES/git/builtin/push.c
+++ b/SOURCES/git/builtin/push.c
@@ -15,7 +15,7 @@ static const char * const push_usage[] = {
NULL,
};
-static int thin = 1;
+static int thin = -1;
static int deleterefs;
static const char *receivepack;
static int verbosity;
@@ -347,7 +347,9 @@ static int push_with_options(struct transport *transport,
int flags)
if (receivepack)
transport_set_option(transport,
TRANS_OPT_RECEIVEPACK, receivepack);
- transport_set_option(transport, TRANS_OPT_THIN, thin ? yes : NULL);
+
+ if (thin != -1)
+ transport_set_option(transport, TRANS_OPT_THIN, thin ? yes :
NULL);
if (!is_empty_cas(cas)) {
if (!transport-smart_options)
diff --git a/SOURCES/git/remote.c b/SOURCES/git/remote.c
index f624217..54777cb 100644
--- a/SOURCES/git/remote.c
+++ b/SOURCES/git/remote.c
@@ -175,6 +175,7 @@ static struct remote *make_remote(const char *name, int len)
ret = xcalloc(1, sizeof(struct remote));
ret-prune = -1; /* unspecified */
+ ret-push_thin = 1; /* default on */
ALLOC_GROW(remotes, remotes_nr + 1, remotes_alloc);
remotes[remotes_nr++] = ret;
ret-name = xstrndup(name, len);
@@ -445,6 +446,8 @@ static int handle_config(const char *key, const char
*value, void *cb)
if (git_config_string(v, key, value))
return -1;
add_push_refspec(remote, v);
+ } else if (!strcmp(subkey, .pushthin)) {
+ remote-push_thin = git_config_bool(key, value);
} else if (!strcmp(subkey, .fetch)) {
const char *v;
if (git_config_string(v, key, value))
diff --git a/SOURCES/git/remote.h b/SOURCES/git/remote.h
index 8b62efd..badf266 100644
--- a/SOURCES/git/remote.h
+++ b/SOURCES/git/remote.h
@@ -46,6 +46,7 @@ struct remote {
int skip_default_update;
int mirror;
int prune;
+ int push_thin;
const char *receivepack;
const char *uploadpack;
diff --git a/SOURCES/git/transport.c b/SOURCES/git/transport.c
index 70d38e4..2f495fa 100644
--- a/SOURCES/git/transport.c
+++ b/SOURCES/git/transport.c
@@ -987,6 +987,8 @@ struct transport *transport_get(struct remote *remote,
const char *url)
ret-smart_options-receivepack = remote-receivepack;
}
+ transport_set_option(ret, TRANS_OPT_THIN, remote-push_thin ? yes :
NULL);
+
return ret;
}
--
2.2.0
--
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] push: add remote.pushThin to control thin pack generation
On Wed, Dec 10, 2014 at 11:49:49PM +, brian m. carlson wrote:
From: brian m. carlson brian.carl...@cpanel.net
Thin packs are enabled when pushing by default, but with a large number
of refs and a fast network, git may spend more time trying to find a
good delta than it would spend simply sending data over the network.
Add a per-remote option, pushThin, that controls the default for pushes
on that remote.
I just realized this patch doesn't apply outside of the particular repo
I was using. Consider this more of an RFC, since I'd like to avoid
going this route if possible, in favor of a more robust approach.
Signed-off-by: brian m. carlson brian.carl...@cpanel.net
---
SOURCES/git/Documentation/config.txt | 6 ++
SOURCES/git/builtin/push.c | 6 --
SOURCES/git/remote.c | 3 +++
SOURCES/git/remote.h | 1 +
SOURCES/git/transport.c | 2 ++
5 files changed, 16 insertions(+), 2 deletions(-)
diff --git a/SOURCES/git/Documentation/config.txt
b/SOURCES/git/Documentation/config.txt
index 9220725..7fededd 100644
--- a/SOURCES/git/Documentation/config.txt
+++ b/SOURCES/git/Documentation/config.txt
@@ -2178,6 +2178,12 @@ remote.name.push::
The default set of refspec for linkgit:git-push[1]. See
linkgit:git-push[1].
+remote.name.pushThin::
+ If true (the default), pass the \--thin option to
+ linkgit:git-pack-objects[1] during push. This results in a smaller
+ pack being sent and can improve push time over slow networks. Over
+ fast networks, setting this value to false may improve performance.
+
remote.name.mirror::
If true, pushing to this remote will automatically behave
as if the `--mirror` option was given on the command line.
diff --git a/SOURCES/git/builtin/push.c b/SOURCES/git/builtin/push.c
index a076b19..ae39677 100644
--- a/SOURCES/git/builtin/push.c
+++ b/SOURCES/git/builtin/push.c
@@ -15,7 +15,7 @@ static const char * const push_usage[] = {
NULL,
};
-static int thin = 1;
+static int thin = -1;
static int deleterefs;
static const char *receivepack;
static int verbosity;
@@ -347,7 +347,9 @@ static int push_with_options(struct transport *transport,
int flags)
if (receivepack)
transport_set_option(transport,
TRANS_OPT_RECEIVEPACK, receivepack);
- transport_set_option(transport, TRANS_OPT_THIN, thin ? yes : NULL);
+
+ if (thin != -1)
+ transport_set_option(transport, TRANS_OPT_THIN, thin ? yes :
NULL);
if (!is_empty_cas(cas)) {
if (!transport-smart_options)
diff --git a/SOURCES/git/remote.c b/SOURCES/git/remote.c
index f624217..54777cb 100644
--- a/SOURCES/git/remote.c
+++ b/SOURCES/git/remote.c
@@ -175,6 +175,7 @@ static struct remote *make_remote(const char *name, int
len)
ret = xcalloc(1, sizeof(struct remote));
ret-prune = -1; /* unspecified */
+ ret-push_thin = 1; /* default on */
ALLOC_GROW(remotes, remotes_nr + 1, remotes_alloc);
remotes[remotes_nr++] = ret;
ret-name = xstrndup(name, len);
@@ -445,6 +446,8 @@ static int handle_config(const char *key, const char
*value, void *cb)
if (git_config_string(v, key, value))
return -1;
add_push_refspec(remote, v);
+ } else if (!strcmp(subkey, .pushthin)) {
+ remote-push_thin = git_config_bool(key, value);
} else if (!strcmp(subkey, .fetch)) {
const char *v;
if (git_config_string(v, key, value))
diff --git a/SOURCES/git/remote.h b/SOURCES/git/remote.h
index 8b62efd..badf266 100644
--- a/SOURCES/git/remote.h
+++ b/SOURCES/git/remote.h
@@ -46,6 +46,7 @@ struct remote {
int skip_default_update;
int mirror;
int prune;
+ int push_thin;
const char *receivepack;
const char *uploadpack;
diff --git a/SOURCES/git/transport.c b/SOURCES/git/transport.c
index 70d38e4..2f495fa 100644
--- a/SOURCES/git/transport.c
+++ b/SOURCES/git/transport.c
@@ -987,6 +987,8 @@ struct transport *transport_get(struct remote *remote,
const char *url)
ret-smart_options-receivepack = remote-receivepack;
}
+ transport_set_option(ret, TRANS_OPT_THIN, remote-push_thin ? yes :
NULL);
+
return ret;
}
--
2.2.0
--
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
--
brian m. carlson / brian with sandals: Houston, Texas, US
+1 832 623 2791 | http://www.crustytoothpaste.net/~bmc | My opinion only
OpenPGP: RSA v4 4096b: 88AC E9B2 9196 305B A994 7552 F1BA 225C 0223 B187
signature.asc
Description: Digital signature
Thin pack
Hi, What are the difference between pre 1.8.5 and 1.8.5 about thin pack support? Could you describe thin pack? From the doc, it says --thin is default option. Is that true? -- 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: Thin pack
On Thu, Dec 5, 2013 at 8:44 AM, 乙酸鋰 ch3co...@gmail.com wrote: What are the difference between pre 1.8.5 and 1.8.5 about thin pack support? No idea. Could you describe thin pack? It gets a bit technical. Under the hood objects are deltified, only the differences between an object and its base are stored. If both the diff and the base are in a pack, it's a normal pack. If the pack lacks the base, it's thin. This makes thin packs more suited for network transfer because you transfer much less. Imagine you have a 5k file, you fetch another version of the same file that changes 1 line. With thin pack, you receive just that line (not entirely true, but good enough). Without thin pack, you also receive the 5k file you already have because it's the base of the new version. From the doc, it says --thin is default option. Is that true? Yes. -- Duy -- 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 0/2] thin-pack capability for send-pack/receive-pack
On Wed, 2013-11-06 at 15:42 -0800, Shawn Pearce wrote: On Wed, Nov 6, 2013 at 1:41 PM, Carlos Martín Nieto c...@elego.de wrote: On Wed, 2013-11-06 at 12:32 -0800, Junio C Hamano wrote: I'll queue these for now, but I doubt the wisdom of this series, given that the ship has already sailed long time ago. Currently, no third-party implementation of a receiving end can accept thin push, because thin push is not a capability that needs to be checked by the current clients. People will have to wait until the clients with 2/2 patch are widely deployed before starting to use such a receiving end that is incapable of thin push. Wouldn't the world be a better place if instead they used that time waiting to help such a third-party receiving end to implement thin push support? Support in the code isn't always enough. The particular case that brought this on is one where the index-pack implementation can deal with thin packs just fine. This particular service takes the pack which the client sent and does post-processing on it to store it elsewhere. During the receive-pack equivalent, there is no git object db that it can query for the missing base objects. I realise this is pretty a unusual situation. How... odd? At Google we have made effort to ensure servers can accept thin packs, even though its clearly easier to accept non-thin, because clients in the wild already send thin packs and changing the deployed clients is harder than implementing the existing protocol. It is harder, but IMO also more correct, as thin packs are an optimisation that was added somewhat later. Not to say it shouldn't be something you should attempt to do, but it's a trade-off between the complexity of the communication between the pieces and the potential amount of extra data you're willing to put up with. The Google (Code) servers don't just support thin packs, but for upload-pack, they force it upon the client, which is quite frustrating as it won't even tell you why it closes the connection but sends a 500 instead, but that's a different story. If the server can't complete the pack, I guess this also means the client cannot immediately fetch from the server it just pushed to? Not all the details have been worked out yet, but the new history should be converted into the target format before reporting success and closing the connection. The Git frontend/protocol is one way of putting data into the system, but that's not its native data storage format. The database where this is getting stored only has very limited knowledge of git. I'll reroll the series with no-thin as mentioned elsewhere in this thread. cmn -- 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
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
Re: [PATCH] send-pack: don't send a thin pack to a server which doesn't support it
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 c...@elego.de
---
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
Re: [PATCH] send-pack: don't send a thin pack to a server which doesn't support it
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
[PATCH 1/2] receive-pack: advertise thin-pack
upload-pack has long advertised thin-pack, letting the clients request these smaller packs. The client however unconditionally assumes that a server is able to fix thin packs and there is no way of telling the client that this is in fact not the case. Make receive-pack advertise 'thin-pack' in anticipation of the client toggling the assumption and document this capability when used by receive-pack. Signed-off-by: Carlos Martín Nieto c...@elego.de --- Documentation/technical/protocol-capabilities.txt | 20 +++- builtin/receive-pack.c| 2 +- 2 files changed, 16 insertions(+), 6 deletions(-) diff --git a/Documentation/technical/protocol-capabilities.txt b/Documentation/technical/protocol-capabilities.txt index fd8ffa5..4e96d51 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 'thin-pack' when +it knows how to thicken the pack it receives. + +Likewise, the 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 does not advertise this +capability. + side-band, side-band-64k diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c index e3eb5fc..0e35c02 100644 --- a/builtin/receive-pack.c +++ b/builtin/receive-pack.c @@ -132,7 +132,7 @@ static void show_ref(const char *path, const unsigned char *sha1) else packet_write(1, %s %s%c%s%s agent=%s\n, sha1_to_hex(sha1), path, 0, - report-status delete-refs side-band-64k quiet, + report-status delete-refs side-band-64k quiet thin-pack, prefer_ofs_delta ? ofs-delta : , git_user_agent_sanitized()); sent_capabilities = 1; -- 1.8.4.652.g0d6e0ce -- 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 2/2] send-pack: only send a thin pack if the server supports it
In combination a the previous patch making receive-pack advertise the
thin-pack capability, this allows git to push to a server in a
constrained environment which is not able to fix thin packs while taking
advantage of the feature for servers which can.
Signed-off-by: Carlos Martín Nieto c...@elego.de
---
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.652.g0d6e0ce
--
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 0/2] thin-pack capability for send-pack/receive-pack
Hi all, This comes as a result of the discussion starting at [0] about git-push assuming that a server will always support thin packs. Most out there in fact do, but this isn't necessarily the case. Some implementations may not have support for it yet, or the server might be running in an environment where it is not feasible for it to try to fill in the missing objects. Jonathan and Duy mentioned that separate patches for receive-pack and send-pack could let us work around adding this at such a late stage, so here they are. The second patch can maybe lie in waiting for a while. [0] http://thread.gmane.org/gmane.comp.version-control.git/235766/focus=236402 Carlos Martín Nieto (2): receive-pack: advertise thin-pack send-pack: only send a thin pack if the server supports it Documentation/technical/protocol-capabilities.txt | 20 +++- builtin/receive-pack.c| 2 +- send-pack.c | 2 ++ 3 files changed, 18 insertions(+), 6 deletions(-) -- 1.8.4.652.g0d6e0ce -- 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 0/2] thin-pack capability for send-pack/receive-pack
Carlos Martín Nieto c...@elego.de writes: Hi all, This comes as a result of the discussion starting at [0] about git-push assuming that a server will always support thin packs. Most out there in fact do, but this isn't necessarily the case. Some implementations may not have support for it yet, or the server might be running in an environment where it is not feasible for it to try to fill in the missing objects. Jonathan and Duy mentioned that separate patches for receive-pack and send-pack could let us work around adding this at such a late stage, so here they are. The second patch can maybe lie in waiting for a while. I'll queue these for now, but I doubt the wisdom of this series, given that the ship has already sailed long time ago. Currently, no third-party implementation of a receiving end can accept thin push, because thin push is not a capability that needs to be checked by the current clients. People will have to wait until the clients with 2/2 patch are widely deployed before starting to use such a receiving end that is incapable of thin push. Wouldn't the world be a better place if instead they used that time waiting to help such a third-party receiving end to implement thin push support? -- 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 0/2] thin-pack capability for send-pack/receive-pack
On Wed, 2013-11-06 at 12:32 -0800, Junio C Hamano wrote: I'll queue these for now, but I doubt the wisdom of this series, given that the ship has already sailed long time ago. Currently, no third-party implementation of a receiving end can accept thin push, because thin push is not a capability that needs to be checked by the current clients. People will have to wait until the clients with 2/2 patch are widely deployed before starting to use such a receiving end that is incapable of thin push. Wouldn't the world be a better place if instead they used that time waiting to help such a third-party receiving end to implement thin push support? Support in the code isn't always enough. The particular case that brought this on is one where the index-pack implementation can deal with thin packs just fine. This particular service takes the pack which the client sent and does post-processing on it to store it elsewhere. During the receive-pack equivalent, there is no git object db that it can query for the missing base objects. I realise this is pretty a unusual situation. Cheers, cmn -- 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 0/2] thin-pack capability for send-pack/receive-pack
Carlos Martín Nieto c...@elego.de writes: On Wed, 2013-11-06 at 12:32 -0800, Junio C Hamano wrote: I'll queue these for now, but I doubt the wisdom of this series, given that the ship has already sailed long time ago. Currently, no third-party implementation of a receiving end can accept thin push, because thin push is not a capability that needs to be checked by the current clients. People will have to wait until the clients with 2/2 patch are widely deployed before starting to use such a receiving end that is incapable of thin push. Wouldn't the world be a better place if instead they used that time waiting to help such a third-party receiving end to implement thin push support? Support in the code isn't always enough. The particular case that brought this on is one where the index-pack implementation can deal with thin packs just fine. This particular service takes the pack which the client sent and does post-processing on it to store it elsewhere. During the receive-pack equivalent, there is no git object db that it can query for the missing base objects. I realise this is pretty a unusual situation. OK, I agree that it sounds quite niche-y, but it still is sensible. If a receiving end does not want to (this includes it is incapable of doing so, but does not have to be limited to) complete a thin pack, the series will give it such an option in the longer term. Thanks. -- 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 0/2] thin-pack capability for send-pack/receive-pack
On Wed, Nov 06, 2013 at 02:25:50PM -0800, Junio C Hamano wrote: Support in the code isn't always enough. The particular case that brought this on is one where the index-pack implementation can deal with thin packs just fine. This particular service takes the pack which the client sent and does post-processing on it to store it elsewhere. During the receive-pack equivalent, there is no git object db that it can query for the missing base objects. I realise this is pretty a unusual situation. OK, I agree that it sounds quite niche-y, but it still is sensible. If a receiving end does not want to (this includes it is incapable of doing so, but does not have to be limited to) complete a thin pack, the series will give it such an option in the longer term. I wonder if we want to make the flag go in the opposite direction, then. Right now we have no flag, and we assume the other side can handle a thin pack. If we add a thin flag, then the timeline is roughly: 1. Receive-pack starts advertising thin. 2. Send-pack cannot assume lack of thin means the other side cannot handle thin (it might just be an older receive-pack), and keeps sending thin packs. [time passes] 3. Send-pack can safely assume that every server has learned thin and can assume that lack of thin means the server does not want a thin pack. In other words, the benefit happens at step 3, and we do not get any effect until some long assumption time passes. If we instead introduced no-thin, it is more like: 1. Receive-pack starts advertising no-thin (as dictated by circumstances, as Carlos describes). 2. Send-pack which does not understand no-thin will ignore it and send a thin pack. This is the same as now, and the same as step 2 above. 3. An upgraded send-pack will understand no-thin and do as the server asks. So an upgraded client and server can start cooperating immediately, and we do not have to wait for the long assumption time to pass before applying the second half. It is tempting to think about a thin flag because that would be the natural way to have implemented it from the very beginning. But it is not the beginning, and the negative flag is the only way at this point to say if you understand this, please behave differently than we used to (because the status quo is send a thin pack, whether I said it was OK or not). -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 0/2] thin-pack capability for send-pack/receive-pack
On Wed, Nov 6, 2013 at 1:41 PM, Carlos Martín Nieto c...@elego.de wrote: On Wed, 2013-11-06 at 12:32 -0800, Junio C Hamano wrote: I'll queue these for now, but I doubt the wisdom of this series, given that the ship has already sailed long time ago. Currently, no third-party implementation of a receiving end can accept thin push, because thin push is not a capability that needs to be checked by the current clients. People will have to wait until the clients with 2/2 patch are widely deployed before starting to use such a receiving end that is incapable of thin push. Wouldn't the world be a better place if instead they used that time waiting to help such a third-party receiving end to implement thin push support? Support in the code isn't always enough. The particular case that brought this on is one where the index-pack implementation can deal with thin packs just fine. This particular service takes the pack which the client sent and does post-processing on it to store it elsewhere. During the receive-pack equivalent, there is no git object db that it can query for the missing base objects. I realise this is pretty a unusual situation. How... odd? At Google we have made effort to ensure servers can accept thin packs, even though its clearly easier to accept non-thin, because clients in the wild already send thin packs and changing the deployed clients is harder than implementing the existing protocol. If the server can't complete the pack, I guess this also means the client cannot immediately fetch from the server it just pushed to? -- 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 0/2] thin-pack capability for send-pack/receive-pack
On Wed, Nov 6, 2013 at 2:54 PM, Jeff King p...@peff.net wrote: If we instead introduced no-thin, it is more like: 1. Receive-pack starts advertising no-thin (as dictated by circumstances, as Carlos describes). 2. Send-pack which does not understand no-thin will ignore it and send a thin pack. This is the same as now, and the same as step 2 above. 3. An upgraded send-pack will understand no-thin and do as the server asks. So an upgraded client and server can start cooperating immediately, and we do not have to wait for the long assumption time to pass before applying the second half. It is tempting to think about a thin flag because that would be the natural way to have implemented it from the very beginning. But it is not the beginning, and the negative flag is the only way at this point to say if you understand this, please behave differently than we used to (because the status quo is send a thin pack, whether I said it was OK or not). I think the only sane option at this point is a no-thin flag, or just require servers that want to be wire compatible to accept thin packs. -- 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 when the server doesn't support it
On Tue, 2013-10-08 at 15:22 -0700, Jonathan Nieder wrote: 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. 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 -- 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 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 c...@elego.de
---
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
--
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 when the server doesn't support it
On Tue, Oct 8, 2013 at 3:44 PM, Carlos Martín Nieto c...@elego.de wrote:
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, 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
--
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 when the server doesn't support it
Duy Nguyen pclo...@gmail.com writes: On Tue, 2013-10-08 at 16:44 +0700, Duy Nguyen wrote: On Tue, Oct 8, 2013 at 3:44 PM, Carlos Martín Nieto c...@elego.de wrote: 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 Oh, I'd never noticed that when looking though the network traffic. This seems like something that breaks the compatibility that git otherwise tries so hard to maintain. How did it happen that it's fine for the client to assume that the server supports thin packs? 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). Yeah, I had an older version in my PATH and was bitten by that when testing. Having --no-thin and the server's index-pack fail with missing bases is quite worrying when you're the one who wrote most of the server-side code. Having to remember to run 'git push --no-thin' when pushing to one particular project is pretty bad experience as a user and I was hoping to avoid this with newer gits. We could advertise a no-thin-pack extension if a patch to support that would be accepted into mainline git. Cheers, cmn -- 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 when the server doesn't support it
On Tue, Oct 8, 2013 at 4:44 PM, Duy Nguyen pclo...@gmail.com wrote: On Tue, Oct 8, 2013 at 3:44 PM, Carlos Martín Nieto c...@elego.de wrote: 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 -- 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 when the server doesn't support it
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 -- 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 12/21] unpack-objects: recognize end-of-pack in v4 thin pack
Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
builtin/unpack-objects.c | 18 +-
1 file changed, 13 insertions(+), 5 deletions(-)
diff --git a/builtin/unpack-objects.c b/builtin/unpack-objects.c
index 6d0a65c..c9eb31d 100644
--- a/builtin/unpack-objects.c
+++ b/builtin/unpack-objects.c
@@ -20,6 +20,7 @@ static unsigned char buffer[4096];
static unsigned int offset, len;
static off_t consumed_bytes;
static git_SHA_CTX ctx;
+static int packv4;
/*
* When running under --strict mode, objects whose reachability are
@@ -421,7 +422,7 @@ static void unpack_delta_entry(enum object_type type,
unsigned long delta_size,
free(base);
}
-static void unpack_one(unsigned nr)
+static int unpack_one(unsigned nr)
{
unsigned shift;
unsigned char *pack;
@@ -431,6 +432,10 @@ static void unpack_one(unsigned nr)
obj_list[nr].offset = consumed_bytes;
pack = fill(1);
+ if (packv4 *(char*)fill(1) == 0) {
+ use(1);
+ return -1;
+ }
c = *pack;
use(1);
type = (c 4) 7;
@@ -450,18 +455,19 @@ static void unpack_one(unsigned nr)
case OBJ_BLOB:
case OBJ_TAG:
unpack_non_delta_entry(type, size, nr);
- return;
+ break;
case OBJ_REF_DELTA:
case OBJ_OFS_DELTA:
unpack_delta_entry(type, size, nr);
- return;
+ break;
default:
error(bad object type %d, type);
has_errors = 1;
if (recover)
- return;
+ break;
exit(1);
}
+ return 0;
}
static void unpack_all(void)
@@ -477,13 +483,15 @@ static void unpack_all(void)
if (!pack_version_ok(hdr-hdr_version))
die(unknown pack file version %PRIu32,
ntohl(hdr-hdr_version));
+ packv4 = ntohl(hdr-hdr_version) == 4;
use(sizeof(struct pack_header));
if (!quiet)
progress = start_progress(Unpacking objects, nr_objects);
obj_list = xcalloc(nr_objects, sizeof(*obj_list));
for (i = 0; i nr_objects; i++) {
- unpack_one(i);
+ if (unpack_one(i))
+ break;
display_progress(progress, i + 1);
}
stop_progress(progress);
--
1.8.2.82.gc24b958
--
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
