{"thread":{"id":"34382","subject":"[PATCH] cache.h: move remote/connect API out of it","startedAt":"2013-07-08T21:09:59Z","lastAt":"2013-07-09T05:38:11Z","messageCount":4,"participants":["Junio C Hamano","Jeff King"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"222862","messageId":"7v1u78zrko.fsf@alter.siamese.dyndns.org","threadId":"34382","inReplyTo":null,"subject":"[PATCH] cache.h: move remote/connect API out of it","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-07-08T21:09:59Z","receivedAt":"2013-07-08T21:09:59Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"The definition of \"struct ref\" in \"cache.h\", a header file so\ncentral to the system, always confused me.  This structure is not\nabout the local ref used by sha1-name API to name local objects.\n\nIt is what refspecs are expanded into, after finding out what refs\nthe other side has, to define what refs are updated after object\ntransfer succeeds to what values.  It belongs to \"remote.h\" together\nwith \"struct refspec\".\n\nWhile we are at it, also move the types and functions related to the\nGit transport connection to a new header file connect.h\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n\n * I hate to to this kind of code-movement in the middle of the\n   cycle, but every time I follow the push->transport codepath, I\n   become disoriented by these \"struct ref\"s.\n\n builtin/fetch-pack.c   |  2 ++\n builtin/receive-pack.c |  1 +\n builtin/send-pack.c    |  1 +\n cache.h                | 62 --------------------------------------------------\n connect.c              |  1 +\n fetch-pack.c           |  1 +\n fetch-pack.h           |  1 +\n refs.c                 |  8 -------\n remote.c               |  8 +++++++\n remote.h               | 54 +++++++++++++++++++++++++++++++++++++++++++\n send-pack.c            |  1 +\n transport.c            |  2 ++\n transport.h            |  1 +\n upload-pack.c          |  1 +\n 14 files changed, 74 insertions(+), 70 deletions(-)\n\ndiff --git a/builtin/fetch-pack.c b/builtin/fetch-pack.c\nindex aba4465..c6888c6 100644\n--- a/builtin/fetch-pack.c\n+++ b/builtin/fetch-pack.c\n@@ -1,6 +1,8 @@\n #include \"builtin.h\"\n #include \"pkt-line.h\"\n #include \"fetch-pack.h\"\n+#include \"remote.h\"\n+#include \"connect.h\"\n \n static const char fetch_pack_usage[] =\n \"git fetch-pack [--all] [--stdin] [--quiet|-q] [--keep|-k] [--thin] \"\ndiff --git a/builtin/receive-pack.c b/builtin/receive-pack.c\nindex e3eb5fc..7434d9b 100644\n--- a/builtin/receive-pack.c\n+++ b/builtin/receive-pack.c\n@@ -8,6 +8,7 @@\n #include \"commit.h\"\n #include \"object.h\"\n #include \"remote.h\"\n+#include \"connect.h\"\n #include \"transport.h\"\n #include \"string-list.h\"\n #include \"sha1-array.h\"\ndiff --git a/builtin/send-pack.c b/builtin/send-pack.c\nindex 152c4ea..e86d3b5 100644\n--- a/builtin/send-pack.c\n+++ b/builtin/send-pack.c\n@@ -5,6 +5,7 @@\n #include \"sideband.h\"\n #include \"run-command.h\"\n #include \"remote.h\"\n+#include \"connect.h\"\n #include \"send-pack.h\"\n #include \"quote.h\"\n #include \"transport.h\"\ndiff --git a/cache.h b/cache.h\nindex dd0fb33..cb2891d 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -1035,68 +1035,6 @@ struct pack_entry {\n \tstruct packed_git *p;\n };\n \n-struct ref {\n-\tstruct ref *next;\n-\tunsigned char old_sha1[20];\n-\tunsigned char new_sha1[20];\n-\tchar *symref;\n-\tunsigned int\n-\t\tforce:1,\n-\t\tforced_update:1,\n-\t\tdeletion:1,\n-\t\tmatched:1;\n-\n-\t/*\n-\t * Order is important here, as we write to FETCH_HEAD\n-\t * in numeric order. And the default NOT_FOR_MERGE\n-\t * should be 0, so that xcalloc'd structures get it\n-\t * by default.\n-\t */\n-\tenum {\n-\t\tFETCH_HEAD_MERGE = -1,\n-\t\tFETCH_HEAD_NOT_FOR_MERGE = 0,\n-\t\tFETCH_HEAD_IGNORE = 1\n-\t} fetch_head_status;\n-\n-\tenum {\n-\t\tREF_STATUS_NONE = 0,\n-\t\tREF_STATUS_OK,\n-\t\tREF_STATUS_REJECT_NONFASTFORWARD,\n-\t\tREF_STATUS_REJECT_ALREADY_EXISTS,\n-\t\tREF_STATUS_REJECT_NODELETE,\n-\t\tREF_STATUS_REJECT_FETCH_FIRST,\n-\t\tREF_STATUS_REJECT_NEEDS_FORCE,\n-\t\tREF_STATUS_UPTODATE,\n-\t\tREF_STATUS_REMOTE_REJECT,\n-\t\tREF_STATUS_EXPECTING_REPORT\n-\t} status;\n-\tchar *remote_status;\n-\tstruct ref *peer_ref; /* when renaming */\n-\tchar name[FLEX_ARRAY]; /* more */\n-};\n-\n-#define REF_NORMAL\t(1u << 0)\n-#define REF_HEADS\t(1u << 1)\n-#define REF_TAGS\t(1u << 2)\n-\n-extern struct ref *find_ref_by_name(const struct ref *list, const char *name);\n-\n-#define CONNECT_VERBOSE       (1u << 0)\n-extern struct child_process *git_connect(int fd[2], const char *url, const char *prog, int flags);\n-extern int finish_connect(struct child_process *conn);\n-extern int git_connection_is_socket(struct child_process *conn);\n-struct extra_have_objects {\n-\tint nr, alloc;\n-\tunsigned char (*array)[20];\n-};\n-extern struct ref **get_remote_heads(int in, char *src_buf, size_t src_len,\n-\t\t\t\t     struct ref **list, unsigned int flags,\n-\t\t\t\t     struct extra_have_objects *);\n-extern int server_supports(const char *feature);\n-extern int parse_feature_request(const char *features, const char *feature);\n-extern const char *server_feature_value(const char *feature, int *len_ret);\n-extern const char *parse_feature_value(const char *feature_list, const char *feature, int *len_ret);\n-\n extern struct packed_git *parse_pack_index(unsigned char *sha1, const char *idx_path);\n \n /* A hook for count-objects to report invalid files in pack directory */\ndiff --git a/connect.c b/connect.c\nindex a0783d4..a80ebd3 100644\n--- a/connect.c\n+++ b/connect.c\n@@ -5,6 +5,7 @@\n #include \"refs.h\"\n #include \"run-command.h\"\n #include \"remote.h\"\n+#include \"connect.h\"\n #include \"url.h\"\n \n static char *server_capabilities;\ndiff --git a/fetch-pack.c b/fetch-pack.c\nindex abe5ffb..c2bab42 100644\n--- a/fetch-pack.c\n+++ b/fetch-pack.c\n@@ -9,6 +9,7 @@\n #include \"fetch-pack.h\"\n #include \"remote.h\"\n #include \"run-command.h\"\n+#include \"connect.h\"\n #include \"transport.h\"\n #include \"version.h\"\n \ndiff --git a/fetch-pack.h b/fetch-pack.h\nindex 40f08ba..461cbf3 100644\n--- a/fetch-pack.h\n+++ b/fetch-pack.h\n@@ -2,6 +2,7 @@\n #define FETCH_PACK_H\n \n #include \"string-list.h\"\n+#include \"run-command.h\"\n \n struct fetch_pack_args {\n \tconst char *uploadpack;\ndiff --git a/refs.c b/refs.c\nindex 4302206..330060c 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -3193,14 +3193,6 @@ int update_ref(const char *action, const char *refname,\n \treturn 0;\n }\n \n-struct ref *find_ref_by_name(const struct ref *list, const char *name)\n-{\n-\tfor ( ; list; list = list->next)\n-\t\tif (!strcmp(list->name, name))\n-\t\t\treturn (struct ref *)list;\n-\treturn NULL;\n-}\n-\n /*\n  * generate a format suitable for scanf from a ref_rev_parse_rules\n  * rule, that is replace the \"%.*s\" spec with a \"%s\" spec\ndiff --git a/remote.c b/remote.c\nindex 6f57830..b1ff7a2 100644\n--- a/remote.c\n+++ b/remote.c\n@@ -1302,6 +1302,14 @@ static void add_missing_tags(struct ref *src, struct ref **dst, struct ref ***ds\n \tfree(sent_tips.tip);\n }\n \n+struct ref *find_ref_by_name(const struct ref *list, const char *name)\n+{\n+\tfor ( ; list; list = list->next)\n+\t\tif (!strcmp(list->name, name))\n+\t\t\treturn (struct ref *)list;\n+\treturn NULL;\n+}\n+\n /*\n  * Given the set of refs the local repository has, the set of refs the\n  * remote repository has, and the refspec used for push, determine\ndiff --git a/remote.h b/remote.h\nindex cf56724..a850059 100644\n--- a/remote.h\n+++ b/remote.h\n@@ -71,6 +71,52 @@ struct refspec {\n \n extern const struct refspec *tag_refspec;\n \n+struct ref {\n+\tstruct ref *next;\n+\tunsigned char old_sha1[20];\n+\tunsigned char new_sha1[20];\n+\tchar *symref;\n+\tunsigned int\n+\t\tforce:1,\n+\t\tforced_update:1,\n+\t\tdeletion:1,\n+\t\tmatched:1;\n+\n+\t/*\n+\t * Order is important here, as we write to FETCH_HEAD\n+\t * in numeric order. And the default NOT_FOR_MERGE\n+\t * should be 0, so that xcalloc'd structures get it\n+\t * by default.\n+\t */\n+\tenum {\n+\t\tFETCH_HEAD_MERGE = -1,\n+\t\tFETCH_HEAD_NOT_FOR_MERGE = 0,\n+\t\tFETCH_HEAD_IGNORE = 1\n+\t} fetch_head_status;\n+\n+\tenum {\n+\t\tREF_STATUS_NONE = 0,\n+\t\tREF_STATUS_OK,\n+\t\tREF_STATUS_REJECT_NONFASTFORWARD,\n+\t\tREF_STATUS_REJECT_ALREADY_EXISTS,\n+\t\tREF_STATUS_REJECT_NODELETE,\n+\t\tREF_STATUS_REJECT_FETCH_FIRST,\n+\t\tREF_STATUS_REJECT_NEEDS_FORCE,\n+\t\tREF_STATUS_UPTODATE,\n+\t\tREF_STATUS_REMOTE_REJECT,\n+\t\tREF_STATUS_EXPECTING_REPORT\n+\t} status;\n+\tchar *remote_status;\n+\tstruct ref *peer_ref; /* when renaming */\n+\tchar name[FLEX_ARRAY]; /* more */\n+};\n+\n+#define REF_NORMAL\t(1u << 0)\n+#define REF_HEADS\t(1u << 1)\n+#define REF_TAGS\t(1u << 2)\n+\n+extern struct ref *find_ref_by_name(const struct ref *list, const char *name);\n+\n struct ref *alloc_ref(const char *name);\n struct ref *copy_ref(const struct ref *ref);\n struct ref *copy_ref_list(const struct ref *ref);\n@@ -84,6 +130,14 @@ int check_ref_type(const struct ref *ref, int flags);\n  */\n void free_refs(struct ref *ref);\n \n+struct extra_have_objects {\n+\tint nr, alloc;\n+\tunsigned char (*array)[20];\n+};\n+extern struct ref **get_remote_heads(int in, char *src_buf, size_t src_len,\n+\t\t\t\t     struct ref **list, unsigned int flags,\n+\t\t\t\t     struct extra_have_objects *);\n+\n int resolve_remote_symref(struct ref *ref, struct ref *list);\n int ref_newer(const unsigned char *new_sha1, const unsigned char *old_sha1);\n \ndiff --git a/send-pack.c b/send-pack.c\nindex 7d172ef..9a9908c 100644\n--- a/send-pack.c\n+++ b/send-pack.c\n@@ -5,6 +5,7 @@\n #include \"sideband.h\"\n #include \"run-command.h\"\n #include \"remote.h\"\n+#include \"connect.h\"\n #include \"send-pack.h\"\n #include \"quote.h\"\n #include \"transport.h\"\ndiff --git a/transport.c b/transport.c\nindex e15db98..b84dbf0 100644\n--- a/transport.c\n+++ b/transport.c\n@@ -3,6 +3,8 @@\n #include \"run-command.h\"\n #include \"pkt-line.h\"\n #include \"fetch-pack.h\"\n+#include \"remote.h\"\n+#include \"connect.h\"\n #include \"send-pack.h\"\n #include \"walker.h\"\n #include \"bundle.h\"\ndiff --git a/transport.h b/transport.h\nindex ea70ea7..b551f99 100644\n--- a/transport.h\n+++ b/transport.h\n@@ -2,6 +2,7 @@\n #define TRANSPORT_H\n \n #include \"cache.h\"\n+#include \"run-command.h\"\n #include \"remote.h\"\n \n struct git_transport_options {\ndiff --git a/upload-pack.c b/upload-pack.c\nindex 127e59a..b03492e 100644\n--- a/upload-pack.c\n+++ b/upload-pack.c\n@@ -10,6 +10,7 @@\n #include \"revision.h\"\n #include \"list-objects.h\"\n #include \"run-command.h\"\n+#include \"connect.h\"\n #include \"sigchain.h\"\n #include \"version.h\"\n #include \"string-list.h\"\n-- \n1.8.3.2-876-ge3d3f5e\n"},{"id":"222864","messageId":"7vwqp0ybsy.fsf@alter.siamese.dyndns.org","threadId":"34382","inReplyTo":"7v1u78zrko.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] cache.h: move remote/connect API out of it","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-07-08T21:35:57Z","receivedAt":"2013-07-08T21:35:57Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> The definition of \"struct ref\" in \"cache.h\", a header file so\n> central to the system, always confused me.  This structure is not\n> about the local ref used by sha1-name API to name local objects.\n>\n> It is what refspecs are expanded into, after finding out what refs\n> the other side has, to define what refs are updated after object\n> transfer succeeds to what values.  It belongs to \"remote.h\" together\n> with \"struct refspec\".\n>\n> While we are at it, also move the types and functions related to the\n> Git transport connection to a new header file connect.h\n>\n> Signed-off-by: Junio C Hamano <gitster@pobox.com>\n> ---\n>\n>  * I hate to to this kind of code-movement in the middle of the\n>    cycle, but every time I follow the push->transport codepath, I\n>    become disoriented by these \"struct ref\"s.\n\n>\n>  builtin/fetch-pack.c   |  2 ++\n>  builtin/receive-pack.c |  1 +\n>  builtin/send-pack.c    |  1 +\n>  cache.h                | 62 --------------------------------------------------\n>  connect.c              |  1 +\n>  fetch-pack.c           |  1 +\n>  fetch-pack.h           |  1 +\n>  refs.c                 |  8 -------\n>  remote.c               |  8 +++++++\n>  remote.h               | 54 +++++++++++++++++++++++++++++++++++++++++++\n>  send-pack.c            |  1 +\n>  transport.c            |  2 ++\n>  transport.h            |  1 +\n>  upload-pack.c          |  1 +\n>  14 files changed, 74 insertions(+), 70 deletions(-)\n\nOf course, I should have noticed that the math does not work out\nwithout this part:\n\n connect.h | 13 +++++++++++++\n 1 file changed, 13 insertions(+)\n\ndiff --git a/connect.h b/connect.h\nnew file mode 100644\nindex 0000000..9dff25c\n--- /dev/null\n+++ b/connect.h\n@@ -0,0 +1,13 @@\n+#ifndef CONNECT_H\n+#define CONNECT_H\n+\n+#define CONNECT_VERBOSE       (1u << 0)\n+extern struct child_process *git_connect(int fd[2], const char *url, const char *prog, int flags);\n+extern int finish_connect(struct child_process *conn);\n+extern int git_connection_is_socket(struct child_process *conn);\n+extern int server_supports(const char *feature);\n+extern int parse_feature_request(const char *features, const char *feature);\n+extern const char *server_feature_value(const char *feature, int *len_ret);\n+extern const char *parse_feature_value(const char *feature_list, const char *feature, int *len_ret);\n+\n+#endif\n"},{"id":"222875","messageId":"20130709042106.GC27903@sigill.intra.peff.net","threadId":"34382","inReplyTo":"7v1u78zrko.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] cache.h: move remote/connect API out of it","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2013-07-09T04:21:06Z","receivedAt":"2013-07-09T04:21:06Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Jul 08, 2013 at 02:09:59PM -0700, Junio C Hamano wrote:\n\n> The definition of \"struct ref\" in \"cache.h\", a header file so\n> central to the system, always confused me.  This structure is not\n> about the local ref used by sha1-name API to name local objects.\n> [...]\n>  * I hate to to this kind of code-movement in the middle of the\n>    cycle, but every time I follow the push->transport codepath, I\n>    become disoriented by these \"struct ref\"s.\n\nFWIW, this has often bugged me, too. I did not check what fallouts this\nwill have for series in flight, but in general, I think it is a good\nthing to be doing.\n\n-Peff\n"},{"id":"222883","messageId":"7v4nc4xph8.fsf@alter.siamese.dyndns.org","threadId":"34382","inReplyTo":"20130709042106.GC27903@sigill.intra.peff.net","subject":"Re: [PATCH] cache.h: move remote/connect API out of it","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-07-09T05:38:11Z","receivedAt":"2013-07-09T05:38:11Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> On Mon, Jul 08, 2013 at 02:09:59PM -0700, Junio C Hamano wrote:\n>\n>> The definition of \"struct ref\" in \"cache.h\", a header file so\n>> central to the system, always confused me.  This structure is not\n>> about the local ref used by sha1-name API to name local objects.\n>> [...]\n>>  * I hate to to this kind of code-movement in the middle of the\n>>    cycle, but every time I follow the push->transport codepath, I\n>>    become disoriented by these \"struct ref\"s.\n>\n> FWIW, this has often bugged me, too. I did not check what fallouts this\n> will have for series in flight, but in general, I think it is a good\n> thing to be doing.\n\nThe fallout can be seen at the tip of 'pu', which is fairly minimum\nat the moment.\n"}]}