Re: [PATCH 19/26] upload-pack: introduce fetch server command

[Stefan Beller]

On Tue, Jan 2, 2018 at 4:18 PM, Brandon Williams  wrote:
> Introduce the 'fetch' server command.
>
> Signed-off-by: Brandon Williams 
> ---
>  Documentation/technical/protocol-v2.txt |  14 ++
>  serve.c |   2 +
>  upload-pack.c   | 290 
> 
>  upload-pack.h   |   9 +
>  4 files changed, 315 insertions(+)
>  create mode 100644 upload-pack.h
>
> diff --git a/Documentation/technical/protocol-v2.txt 
> b/Documentation/technical/protocol-v2.txt
> index 5f4d0e719..2a8e2f226 100644
> --- a/Documentation/technical/protocol-v2.txt
> +++ b/Documentation/technical/protocol-v2.txt
> @@ -115,3 +115,17 @@ The output of ls-refs is as follows:
>
>  symref = PKT-LINE("symref" SP symbolic-ref SP resolved-ref LF)
>  shallow = PKT-LINE("shallow" SP obj-id LF)
> +
> + Fetch
> +---
> +
> +Fetch will need to be a modified version of the v1 fetch protocol.  Some
> +potential areas for improvement are: Ref-in-want, CDN offloading,
> +Fetch-options.
> +
> +Since we'll have an 'ls-ref' service we can eliminate the need of fetch
> +to perform a ref-advertisement, instead a client can run the 'ls-refs'
> +service first, in order to find out what refs the server has, and then
> +request those refs directly using the fetch service.
> +
> +//TODO Flesh out the design

TODO: actually do it. ;)

a couple notes from the discussion in office:
* Could we split fetch into multiple phases
  (negotiation + getting the pack)
* negotiation could be reused in forced push to
  minimize the pack to be sent
* negotiation in a half duplex is actually better
  called 'discovery', which discovers about the set
  of objects available on the remote side.
  (the opposite would be reveal, or 'ask-for-discovery', which
  is could be used for a symmetric design of fetch and push)


> diff --git a/serve.c b/serve.c
> index 88d548410..ca3bb7190 100644
> --- a/serve.c
> +++ b/serve.c
> @@ -6,6 +6,7 @@
>  #include "argv-array.h"
>  #include "ls-refs.h"
>  #include "serve.h"
> +#include "upload-pack.h"
>
>  static int always_advertise(struct repository *r,
> struct strbuf *value)
> @@ -46,6 +47,7 @@ static struct protocol_capability capabilities[] = {
> { "agent", agent_advertise, NULL },
> { "stateless-rpc", always_advertise, NULL },
> { "ls-refs", always_advertise, ls_refs },
> +   { "fetch", always_advertise, upload_pack_v2 },
>  };
>
>  static void advertise_capabilities(void)
> diff --git a/upload-pack.c b/upload-pack.c
> index 2ca60d27c..c41f6f528 100644
> --- a/upload-pack.c
> +++ b/upload-pack.c
> @@ -20,6 +20,7 @@
>  #include "prio-queue.h"
>  #include "protocol.h"
>  #include "serve.h"
> +#include "upload-pack.h"
>
>  static const char * const upload_pack_usage[] = {
> N_("git upload-pack [] "),
> @@ -1040,6 +1041,295 @@ static void upload_pack(void)
> }
>  }
>
> +struct upload_pack_data {
> +   struct object_array wants;
> +   struct oid_array haves;
> +
> +   unsigned stateless_rpc : 1;
> +
> +   unsigned use_thin_pack : 1;
> +   unsigned use_ofs_delta : 1;
> +   unsigned no_progress : 1;
> +   unsigned use_include_tag : 1;
> +   unsigned done : 1;
> +};
> +
> +#define UPLOAD_PACK_DATA_INIT { OBJECT_ARRAY_INIT, OID_ARRAY_INIT, 0, 0, 0, 
> 0, 0, 0 }
> +
> +static void upload_pack_data_clear(struct upload_pack_data *data)
> +{
> +   object_array_clear(>wants);
> +   oid_array_clear(>haves);
> +}
> +
> +static int parse_want(const char *line)
> +{
> +   const char *arg;
> +   if (skip_prefix(line, "want ", )) {
> +   struct object_id oid;
> +   struct object *o;
> +
> +   if (get_oid_hex(arg, ))
> +   die("git upload-pack: protocol error, "
> +   "expected to get oid, not '%s'", line);
> +
> +   o = parse_object();
> +   if (!o) {
> +   packet_write_fmt(1,
> +"ERR upload-pack: not our ref %s",
> +oid_to_hex());
> +   die("git upload-pack: not our ref %s",
> +   oid_to_hex());
> +   }
> +
> +   if (!(o->flags & WANTED)) {
> +   o->flags |= WANTED;
> +   add_object_array(o, NULL, _obj);
> +   }
> +
> +   return 1;
> +   }
> +
> +   return 0;
> +}
> +
> +static int parse_have(const char *line, struct oid_array *haves)
> +{
> +   const char *arg;
> +   if (skip_prefix(line, "have ", )) {
> +   struct object_id oid;
> +
> +   if (get_oid_hex(arg, ))
> +   die("git upload-pack: expected SHA1 object, got 
> '%s'", arg);
> +   oid_array_append(haves, );
> +   return 1;
> +

[PATCH 19/26] upload-pack: introduce fetch server command

[Brandon Williams]

Introduce the 'fetch' server command.

Signed-off-by: Brandon Williams 
---
 Documentation/technical/protocol-v2.txt |  14 ++
 serve.c |   2 +
 upload-pack.c   | 290 
 upload-pack.h   |   9 +
 4 files changed, 315 insertions(+)
 create mode 100644 upload-pack.h

diff --git a/Documentation/technical/protocol-v2.txt 
b/Documentation/technical/protocol-v2.txt
index 5f4d0e719..2a8e2f226 100644
--- a/Documentation/technical/protocol-v2.txt
+++ b/Documentation/technical/protocol-v2.txt
@@ -115,3 +115,17 @@ The output of ls-refs is as follows:
 
 symref = PKT-LINE("symref" SP symbolic-ref SP resolved-ref LF)
 shallow = PKT-LINE("shallow" SP obj-id LF)
+
+ Fetch
+---
+
+Fetch will need to be a modified version of the v1 fetch protocol.  Some
+potential areas for improvement are: Ref-in-want, CDN offloading,
+Fetch-options.
+
+Since we'll have an 'ls-ref' service we can eliminate the need of fetch
+to perform a ref-advertisement, instead a client can run the 'ls-refs'
+service first, in order to find out what refs the server has, and then
+request those refs directly using the fetch service.
+
+//TODO Flesh out the design
diff --git a/serve.c b/serve.c
index 88d548410..ca3bb7190 100644
--- a/serve.c
+++ b/serve.c
@@ -6,6 +6,7 @@
 #include "argv-array.h"
 #include "ls-refs.h"
 #include "serve.h"
+#include "upload-pack.h"
 
 static int always_advertise(struct repository *r,
struct strbuf *value)
@@ -46,6 +47,7 @@ static struct protocol_capability capabilities[] = {
{ "agent", agent_advertise, NULL },
{ "stateless-rpc", always_advertise, NULL },
{ "ls-refs", always_advertise, ls_refs },
+   { "fetch", always_advertise, upload_pack_v2 },
 };
 
 static void advertise_capabilities(void)
diff --git a/upload-pack.c b/upload-pack.c
index 2ca60d27c..c41f6f528 100644
--- a/upload-pack.c
+++ b/upload-pack.c
@@ -20,6 +20,7 @@
 #include "prio-queue.h"
 #include "protocol.h"
 #include "serve.h"
+#include "upload-pack.h"
 
 static const char * const upload_pack_usage[] = {
N_("git upload-pack [] "),
@@ -1040,6 +1041,295 @@ static void upload_pack(void)
}
 }
 
+struct upload_pack_data {
+   struct object_array wants;
+   struct oid_array haves;
+
+   unsigned stateless_rpc : 1;
+
+   unsigned use_thin_pack : 1;
+   unsigned use_ofs_delta : 1;
+   unsigned no_progress : 1;
+   unsigned use_include_tag : 1;
+   unsigned done : 1;
+};
+
+#define UPLOAD_PACK_DATA_INIT { OBJECT_ARRAY_INIT, OID_ARRAY_INIT, 0, 0, 0, 0, 
0, 0 }
+
+static void upload_pack_data_clear(struct upload_pack_data *data)
+{
+   object_array_clear(>wants);
+   oid_array_clear(>haves);
+}
+
+static int parse_want(const char *line)
+{
+   const char *arg;
+   if (skip_prefix(line, "want ", )) {
+   struct object_id oid;
+   struct object *o;
+
+   if (get_oid_hex(arg, ))
+   die("git upload-pack: protocol error, "
+   "expected to get oid, not '%s'", line);
+
+   o = parse_object();
+   if (!o) {
+   packet_write_fmt(1,
+"ERR upload-pack: not our ref %s",
+oid_to_hex());
+   die("git upload-pack: not our ref %s",
+   oid_to_hex());
+   }
+
+   if (!(o->flags & WANTED)) {
+   o->flags |= WANTED;
+   add_object_array(o, NULL, _obj);
+   }
+
+   return 1;
+   }
+
+   return 0;
+}
+
+static int parse_have(const char *line, struct oid_array *haves)
+{
+   const char *arg;
+   if (skip_prefix(line, "have ", )) {
+   struct object_id oid;
+
+   if (get_oid_hex(arg, ))
+   die("git upload-pack: expected SHA1 object, got '%s'", 
arg);
+   oid_array_append(haves, );
+   return 1;
+   }
+
+   return 0;
+}
+
+static void process_args(struct argv_array *args, struct upload_pack_data 
*data)
+{
+   int i;
+
+   for (i = 0; i < args->argc; i++) {
+   const char *arg = args->argv[i];
+
+   /* process want */
+   if (parse_want(arg))
+   continue;
+   /* process have line */
+   if (parse_have(arg, >haves))
+   continue;
+
+   /* process args like thin-pack */
+   if (!strcmp(arg, "thin-pack")) {
+   use_thin_pack = 1;
+   continue;
+   }
+   if (!strcmp(arg, "ofs-delta")) {
+   use_ofs_delta = 1;
+   continue;
+   }
+   if (!strcmp(arg,