{"thread":{"id":"62841","subject":"[RFC PATCH 0/3] refspec: centralize refspec-related logic","startedAt":"2025-01-22T07:52:27Z","lastAt":"2025-01-24T17:25:49Z","messageCount":9,"participants":["Meet Soni","Patrick Steinhardt","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":3},"messages":[{"id":"511041","messageId":"20250122075154.5697-1-meetsoni3017@gmail.com","threadId":"62841","inReplyTo":null,"subject":"[RFC PATCH 0/3] refspec: centralize refspec-related logic","fromName":"Meet Soni","fromEmail":"meetsoni3017@gmail.com","sentAt":"2025-01-22T07:51:51Z","receivedAt":"2025-01-22T07:52:27Z","isPatch":true,"sender":{"key":"meetsoni3017@gmail.com","avatar":"https://avatars.githubusercontent.com/u/92802561?v=4"},"body":"As Patrick pointed out in [1], the logic related to refspec is currently\nsplit across multiple headers. This patch series addresses that by\nrelocating refspec-related logic from remote to refspec for improved\ncohesion.\n\nThere are a few functions that seem to be on the borderline. They seem\nto have refspec-related logic, but also contain some elements related\nto remote logic, so I'm unsure whether they should be moved.\nSpecifically:\n\n    * get_fetch_map()\n    * count_refspec_match()\n    * check_push_refs()\n    * match_push_refs()\n\n[1]: https://lore.kernel.org/git/ZysQvUyxgdRqjvj2@pks.im/\n\nMeet Soni (3):\n  refspec: relocate omit_name_by_refspec and related functions\n  refspec: relocate query related functions\n  refspec: relocate apply_refspecs and related funtions\n\n refspec.c | 203 ++++++++++++++++++++++++++++++++++++++++++++++++++++++\n refspec.h |  23 +++++++\n remote.c  | 201 -----------------------------------------------------\n remote.h  |  15 ----\n 4 files changed, 226 insertions(+), 216 deletions(-)\n\n\nbase-commit: efff4a85a4fce58b2aa850c6fbf4d8828329f51d\n-- \n2.34.1\n\n"},{"id":"511042","messageId":"20250122075154.5697-2-meetsoni3017@gmail.com","threadId":"62841","inReplyTo":"20250122075154.5697-1-meetsoni3017@gmail.com","subject":"[RFC PATCH 1/3] refspec: relocate omit_name_by_refspec and related functions","fromName":"Meet Soni","fromEmail":"meetsoni3017@gmail.com","sentAt":"2025-01-22T07:51:52Z","receivedAt":"2025-01-22T07:52:59Z","isPatch":true,"sender":{"key":"meetsoni3017@gmail.com","avatar":"https://avatars.githubusercontent.com/u/92802561?v=4"},"body":"Move the functions `omit_name_by_refspec()`, `refspec_match()`, and\n`match_name_with_pattern()` from `remote.c` to `refspec.c`. These\nfunctions focus on refspec matching, so placing them in `refspec.c`\naligns with the separation of concerns. Keep refspec-related logic in\n`refspec.c` and remote-specific logic in `remote.c` for better code\norganization.\n\nSigned-off-by: Meet Soni <meetsoni3017@gmail.com>\n---\n refspec.c | 48 ++++++++++++++++++++++++++++++++++++++++++++++++\n refspec.h |  8 ++++++++\n remote.c  | 48 ------------------------------------------------\n remote.h  |  6 ------\n 4 files changed, 56 insertions(+), 54 deletions(-)\n\ndiff --git a/refspec.c b/refspec.c\nindex 6d86e04442..66989a1d75 100644\n--- a/refspec.c\n+++ b/refspec.c\n@@ -276,3 +276,51 @@ void refspec_ref_prefixes(const struct refspec *rs,\n \t\t}\n \t}\n }\n+\n+int match_name_with_pattern(const char *key, const char *name,\n+\t\t\t\t   const char *value, char **result)\n+{\n+\tconst char *kstar = strchr(key, '*');\n+\tsize_t klen;\n+\tsize_t ksuffixlen;\n+\tsize_t namelen;\n+\tint ret;\n+\tif (!kstar)\n+\t\tdie(_(\"key '%s' of pattern had no '*'\"), key);\n+\tklen = kstar - key;\n+\tksuffixlen = strlen(kstar + 1);\n+\tnamelen = strlen(name);\n+\tret = !strncmp(name, key, klen) && namelen >= klen + ksuffixlen &&\n+\t\t!memcmp(name + namelen - ksuffixlen, kstar + 1, ksuffixlen);\n+\tif (ret && value) {\n+\t\tstruct strbuf sb = STRBUF_INIT;\n+\t\tconst char *vstar = strchr(value, '*');\n+\t\tif (!vstar)\n+\t\t\tdie(_(\"value '%s' of pattern has no '*'\"), value);\n+\t\tstrbuf_add(&sb, value, vstar - value);\n+\t\tstrbuf_add(&sb, name + klen, namelen - klen - ksuffixlen);\n+\t\tstrbuf_addstr(&sb, vstar + 1);\n+\t\t*result = strbuf_detach(&sb, NULL);\n+\t}\n+\treturn ret;\n+}\n+\n+static int refspec_match(const struct refspec_item *refspec,\n+\t\t\t const char *name)\n+{\n+\tif (refspec->pattern)\n+\t\treturn match_name_with_pattern(refspec->src, name, NULL, NULL);\n+\n+\treturn !strcmp(refspec->src, name);\n+}\n+\n+int omit_name_by_refspec(const char *name, struct refspec *rs)\n+{\n+\tint i;\n+\n+\tfor (i = 0; i < rs->nr; i++) {\n+\t\tif (rs->items[i].negative && refspec_match(&rs->items[i], name))\n+\t\t\treturn 1;\n+\t}\n+\treturn 0;\n+}\ndiff --git a/refspec.h b/refspec.h\nindex 69d693c87d..9556d08bd5 100644\n--- a/refspec.h\n+++ b/refspec.h\n@@ -71,4 +71,12 @@ struct strvec;\n void refspec_ref_prefixes(const struct refspec *rs,\n \t\t\t  struct strvec *ref_prefixes);\n \n+/*\n+ * Check whether a name matches any negative refspec in rs. Returns 1 if the\n+ * name matches at least one negative refspec, and 0 otherwise.\n+ */\n+int omit_name_by_refspec(const char *name, struct refspec *rs);\n+int match_name_with_pattern(const char *key, const char *name,\n+\t\t\t\t   const char *value, char **result);\n+\n #endif /* REFSPEC_H */\ndiff --git a/remote.c b/remote.c\nindex 0f6fba8562..40c2418065 100644\n--- a/remote.c\n+++ b/remote.c\n@@ -907,54 +907,6 @@ void ref_push_report_free(struct ref_push_report *report)\n \t}\n }\n \n-static int match_name_with_pattern(const char *key, const char *name,\n-\t\t\t\t   const char *value, char **result)\n-{\n-\tconst char *kstar = strchr(key, '*');\n-\tsize_t klen;\n-\tsize_t ksuffixlen;\n-\tsize_t namelen;\n-\tint ret;\n-\tif (!kstar)\n-\t\tdie(_(\"key '%s' of pattern had no '*'\"), key);\n-\tklen = kstar - key;\n-\tksuffixlen = strlen(kstar + 1);\n-\tnamelen = strlen(name);\n-\tret = !strncmp(name, key, klen) && namelen >= klen + ksuffixlen &&\n-\t\t!memcmp(name + namelen - ksuffixlen, kstar + 1, ksuffixlen);\n-\tif (ret && value) {\n-\t\tstruct strbuf sb = STRBUF_INIT;\n-\t\tconst char *vstar = strchr(value, '*');\n-\t\tif (!vstar)\n-\t\t\tdie(_(\"value '%s' of pattern has no '*'\"), value);\n-\t\tstrbuf_add(&sb, value, vstar - value);\n-\t\tstrbuf_add(&sb, name + klen, namelen - klen - ksuffixlen);\n-\t\tstrbuf_addstr(&sb, vstar + 1);\n-\t\t*result = strbuf_detach(&sb, NULL);\n-\t}\n-\treturn ret;\n-}\n-\n-static int refspec_match(const struct refspec_item *refspec,\n-\t\t\t const char *name)\n-{\n-\tif (refspec->pattern)\n-\t\treturn match_name_with_pattern(refspec->src, name, NULL, NULL);\n-\n-\treturn !strcmp(refspec->src, name);\n-}\n-\n-int omit_name_by_refspec(const char *name, struct refspec *rs)\n-{\n-\tint i;\n-\n-\tfor (i = 0; i < rs->nr; i++) {\n-\t\tif (rs->items[i].negative && refspec_match(&rs->items[i], name))\n-\t\t\treturn 1;\n-\t}\n-\treturn 0;\n-}\n-\n struct ref *apply_negative_refspecs(struct ref *ref_map, struct refspec *rs)\n {\n \tstruct ref **tail;\ndiff --git a/remote.h b/remote.h\nindex bda10dd5c8..0d109fa9c9 100644\n--- a/remote.h\n+++ b/remote.h\n@@ -261,12 +261,6 @@ int resolve_remote_symref(struct ref *ref, struct ref *list);\n  */\n struct ref *ref_remove_duplicates(struct ref *ref_map);\n \n-/*\n- * Check whether a name matches any negative refspec in rs. Returns 1 if the\n- * name matches at least one negative refspec, and 0 otherwise.\n- */\n-int omit_name_by_refspec(const char *name, struct refspec *rs);\n-\n /*\n  * Remove all entries in the input list which match any negative refspec in\n  * the refspec list.\n-- \n2.34.1\n\n"},{"id":"511043","messageId":"20250122075154.5697-3-meetsoni3017@gmail.com","threadId":"62841","inReplyTo":"20250122075154.5697-1-meetsoni3017@gmail.com","subject":"[RFC PATCH 2/3] refspec: relocate query related functions","fromName":"Meet Soni","fromEmail":"meetsoni3017@gmail.com","sentAt":"2025-01-22T07:51:53Z","receivedAt":"2025-01-22T07:53:12Z","isPatch":true,"sender":{"key":"meetsoni3017@gmail.com","avatar":"https://avatars.githubusercontent.com/u/92802561?v=4"},"body":"Move the functions `query_refspecs()`, `query_refspecs_multiple()` and\n`query_matches_negative_refspec()` from `remote.c` to `refspec.c`. These\nfunctions focus on querying refspecs, so centralizing them in `refspec.c`\nimproves code organization by keeping refspec-related logic in one place.\n\nSigned-off-by: Meet Soni <meetsoni3017@gmail.com>\n---\n refspec.c | 123 ++++++++++++++++++++++++++++++++++++++++++++++++++++++\n refspec.h |   7 ++++\n remote.c  | 122 -----------------------------------------------------\n remote.h  |   1 -\n 4 files changed, 130 insertions(+), 123 deletions(-)\n\ndiff --git a/refspec.c b/refspec.c\nindex 66989a1d75..72b3911110 100644\n--- a/refspec.c\n+++ b/refspec.c\n@@ -5,6 +5,7 @@\n #include \"gettext.h\"\n #include \"hash.h\"\n #include \"hex.h\"\n+#include \"string-list.h\"\n #include \"strvec.h\"\n #include \"refs.h\"\n #include \"refspec.h\"\n@@ -324,3 +325,125 @@ int omit_name_by_refspec(const char *name, struct refspec *rs)\n \t}\n \treturn 0;\n }\n+\n+static int query_matches_negative_refspec(struct refspec *rs, struct refspec_item *query)\n+{\n+\tint i, matched_negative = 0;\n+\tint find_src = !query->src;\n+\tstruct string_list reversed = STRING_LIST_INIT_DUP;\n+\tconst char *needle = find_src ? query->dst : query->src;\n+\n+\t/*\n+\t * Check whether the queried ref matches any negative refpsec. If so,\n+\t * then we should ultimately treat this as not matching the query at\n+\t * all.\n+\t *\n+\t * Note that negative refspecs always match the source, but the query\n+\t * item uses the destination. To handle this, we apply pattern\n+\t * refspecs in reverse to figure out if the query source matches any\n+\t * of the negative refspecs.\n+\t *\n+\t * The first loop finds and expands all positive refspecs\n+\t * matched by the queried ref.\n+\t *\n+\t * The second loop checks if any of the results of the first loop\n+\t * match any negative refspec.\n+\t */\n+\tfor (i = 0; i < rs->nr; i++) {\n+\t\tstruct refspec_item *refspec = &rs->items[i];\n+\t\tchar *expn_name;\n+\n+\t\tif (refspec->negative)\n+\t\t\tcontinue;\n+\n+\t\t/* Note the reversal of src and dst */\n+\t\tif (refspec->pattern) {\n+\t\t\tconst char *key = refspec->dst ? refspec->dst : refspec->src;\n+\t\t\tconst char *value = refspec->src;\n+\n+\t\t\tif (match_name_with_pattern(key, needle, value, &expn_name))\n+\t\t\t\tstring_list_append_nodup(&reversed, expn_name);\n+\t\t} else if (refspec->matching) {\n+\t\t\t/* For the special matching refspec, any query should match */\n+\t\t\tstring_list_append(&reversed, needle);\n+\t\t} else if (!refspec->src) {\n+\t\t\tBUG(\"refspec->src should not be null here\");\n+\t\t} else if (!strcmp(needle, refspec->src)) {\n+\t\t\tstring_list_append(&reversed, refspec->src);\n+\t\t}\n+\t}\n+\n+\tfor (i = 0; !matched_negative && i < reversed.nr; i++) {\n+\t\tif (omit_name_by_refspec(reversed.items[i].string, rs))\n+\t\t\tmatched_negative = 1;\n+\t}\n+\n+\tstring_list_clear(&reversed, 0);\n+\n+\treturn matched_negative;\n+}\n+\n+void query_refspecs_multiple(struct refspec *rs,\n+\t\t\t\t    struct refspec_item *query,\n+\t\t\t\t    struct string_list *results)\n+{\n+\tint i;\n+\tint find_src = !query->src;\n+\n+\tif (find_src && !query->dst)\n+\t\tBUG(\"query_refspecs_multiple: need either src or dst\");\n+\n+\tif (query_matches_negative_refspec(rs, query))\n+\t\treturn;\n+\n+\tfor (i = 0; i < rs->nr; i++) {\n+\t\tstruct refspec_item *refspec = &rs->items[i];\n+\t\tconst char *key = find_src ? refspec->dst : refspec->src;\n+\t\tconst char *value = find_src ? refspec->src : refspec->dst;\n+\t\tconst char *needle = find_src ? query->dst : query->src;\n+\t\tchar **result = find_src ? &query->src : &query->dst;\n+\n+\t\tif (!refspec->dst || refspec->negative)\n+\t\t\tcontinue;\n+\t\tif (refspec->pattern) {\n+\t\t\tif (match_name_with_pattern(key, needle, value, result))\n+\t\t\t\tstring_list_append_nodup(results, *result);\n+\t\t} else if (!strcmp(needle, key)) {\n+\t\t\tstring_list_append(results, value);\n+\t\t}\n+\t}\n+}\n+\n+int query_refspecs(struct refspec *rs, struct refspec_item *query)\n+{\n+\tint i;\n+\tint find_src = !query->src;\n+\tconst char *needle = find_src ? query->dst : query->src;\n+\tchar **result = find_src ? &query->src : &query->dst;\n+\n+\tif (find_src && !query->dst)\n+\t\tBUG(\"query_refspecs: need either src or dst\");\n+\n+\tif (query_matches_negative_refspec(rs, query))\n+\t\treturn -1;\n+\n+\tfor (i = 0; i < rs->nr; i++) {\n+\t\tstruct refspec_item *refspec = &rs->items[i];\n+\t\tconst char *key = find_src ? refspec->dst : refspec->src;\n+\t\tconst char *value = find_src ? refspec->src : refspec->dst;\n+\n+\t\tif (!refspec->dst || refspec->negative)\n+\t\t\tcontinue;\n+\t\tif (refspec->pattern) {\n+\t\t\tif (match_name_with_pattern(key, needle, value, result)) {\n+\t\t\t\tquery->force = refspec->force;\n+\t\t\t\treturn 0;\n+\t\t\t}\n+\t\t} else if (!strcmp(needle, key)) {\n+\t\t\t*result = xstrdup(value);\n+\t\t\tquery->force = refspec->force;\n+\t\t\treturn 0;\n+\t\t}\n+\t}\n+\treturn -1;\n+}\ndiff --git a/refspec.h b/refspec.h\nindex 9556d08bd5..d3c97bfdc5 100644\n--- a/refspec.h\n+++ b/refspec.h\n@@ -1,6 +1,8 @@\n #ifndef REFSPEC_H\n #define REFSPEC_H\n \n+#include \"string-list.h\"\n+\n #define TAG_REFSPEC \"refs/tags/*:refs/tags/*\"\n \n /**\n@@ -79,4 +81,9 @@ int omit_name_by_refspec(const char *name, struct refspec *rs);\n int match_name_with_pattern(const char *key, const char *name,\n \t\t\t\t   const char *value, char **result);\n \n+int query_refspecs(struct refspec *rs, struct refspec_item *query);\n+void query_refspecs_multiple(struct refspec *rs,\n+\t\t\t\t    struct refspec_item *query,\n+\t\t\t\t    struct string_list *results);\n+\n #endif /* REFSPEC_H */\ndiff --git a/remote.c b/remote.c\nindex 40c2418065..2c46611821 100644\n--- a/remote.c\n+++ b/remote.c\n@@ -925,128 +925,6 @@ struct ref *apply_negative_refspecs(struct ref *ref_map, struct refspec *rs)\n \treturn ref_map;\n }\n \n-static int query_matches_negative_refspec(struct refspec *rs, struct refspec_item *query)\n-{\n-\tint i, matched_negative = 0;\n-\tint find_src = !query->src;\n-\tstruct string_list reversed = STRING_LIST_INIT_DUP;\n-\tconst char *needle = find_src ? query->dst : query->src;\n-\n-\t/*\n-\t * Check whether the queried ref matches any negative refpsec. If so,\n-\t * then we should ultimately treat this as not matching the query at\n-\t * all.\n-\t *\n-\t * Note that negative refspecs always match the source, but the query\n-\t * item uses the destination. To handle this, we apply pattern\n-\t * refspecs in reverse to figure out if the query source matches any\n-\t * of the negative refspecs.\n-\t *\n-\t * The first loop finds and expands all positive refspecs\n-\t * matched by the queried ref.\n-\t *\n-\t * The second loop checks if any of the results of the first loop\n-\t * match any negative refspec.\n-\t */\n-\tfor (i = 0; i < rs->nr; i++) {\n-\t\tstruct refspec_item *refspec = &rs->items[i];\n-\t\tchar *expn_name;\n-\n-\t\tif (refspec->negative)\n-\t\t\tcontinue;\n-\n-\t\t/* Note the reversal of src and dst */\n-\t\tif (refspec->pattern) {\n-\t\t\tconst char *key = refspec->dst ? refspec->dst : refspec->src;\n-\t\t\tconst char *value = refspec->src;\n-\n-\t\t\tif (match_name_with_pattern(key, needle, value, &expn_name))\n-\t\t\t\tstring_list_append_nodup(&reversed, expn_name);\n-\t\t} else if (refspec->matching) {\n-\t\t\t/* For the special matching refspec, any query should match */\n-\t\t\tstring_list_append(&reversed, needle);\n-\t\t} else if (!refspec->src) {\n-\t\t\tBUG(\"refspec->src should not be null here\");\n-\t\t} else if (!strcmp(needle, refspec->src)) {\n-\t\t\tstring_list_append(&reversed, refspec->src);\n-\t\t}\n-\t}\n-\n-\tfor (i = 0; !matched_negative && i < reversed.nr; i++) {\n-\t\tif (omit_name_by_refspec(reversed.items[i].string, rs))\n-\t\t\tmatched_negative = 1;\n-\t}\n-\n-\tstring_list_clear(&reversed, 0);\n-\n-\treturn matched_negative;\n-}\n-\n-static void query_refspecs_multiple(struct refspec *rs,\n-\t\t\t\t    struct refspec_item *query,\n-\t\t\t\t    struct string_list *results)\n-{\n-\tint i;\n-\tint find_src = !query->src;\n-\n-\tif (find_src && !query->dst)\n-\t\tBUG(\"query_refspecs_multiple: need either src or dst\");\n-\n-\tif (query_matches_negative_refspec(rs, query))\n-\t\treturn;\n-\n-\tfor (i = 0; i < rs->nr; i++) {\n-\t\tstruct refspec_item *refspec = &rs->items[i];\n-\t\tconst char *key = find_src ? refspec->dst : refspec->src;\n-\t\tconst char *value = find_src ? refspec->src : refspec->dst;\n-\t\tconst char *needle = find_src ? query->dst : query->src;\n-\t\tchar **result = find_src ? &query->src : &query->dst;\n-\n-\t\tif (!refspec->dst || refspec->negative)\n-\t\t\tcontinue;\n-\t\tif (refspec->pattern) {\n-\t\t\tif (match_name_with_pattern(key, needle, value, result))\n-\t\t\t\tstring_list_append_nodup(results, *result);\n-\t\t} else if (!strcmp(needle, key)) {\n-\t\t\tstring_list_append(results, value);\n-\t\t}\n-\t}\n-}\n-\n-int query_refspecs(struct refspec *rs, struct refspec_item *query)\n-{\n-\tint i;\n-\tint find_src = !query->src;\n-\tconst char *needle = find_src ? query->dst : query->src;\n-\tchar **result = find_src ? &query->src : &query->dst;\n-\n-\tif (find_src && !query->dst)\n-\t\tBUG(\"query_refspecs: need either src or dst\");\n-\n-\tif (query_matches_negative_refspec(rs, query))\n-\t\treturn -1;\n-\n-\tfor (i = 0; i < rs->nr; i++) {\n-\t\tstruct refspec_item *refspec = &rs->items[i];\n-\t\tconst char *key = find_src ? refspec->dst : refspec->src;\n-\t\tconst char *value = find_src ? refspec->src : refspec->dst;\n-\n-\t\tif (!refspec->dst || refspec->negative)\n-\t\t\tcontinue;\n-\t\tif (refspec->pattern) {\n-\t\t\tif (match_name_with_pattern(key, needle, value, result)) {\n-\t\t\t\tquery->force = refspec->force;\n-\t\t\t\treturn 0;\n-\t\t\t}\n-\t\t} else if (!strcmp(needle, key)) {\n-\t\t\t*result = xstrdup(value);\n-\t\t\tquery->force = refspec->force;\n-\t\t\treturn 0;\n-\t\t}\n-\t}\n-\treturn -1;\n-}\n-\n char *apply_refspecs(struct refspec *rs, const char *name)\n {\n \tstruct refspec_item query;\ndiff --git a/remote.h b/remote.h\nindex 0d109fa9c9..f3da64dc41 100644\n--- a/remote.h\n+++ b/remote.h\n@@ -267,7 +267,6 @@ struct ref *ref_remove_duplicates(struct ref *ref_map);\n  */\n struct ref *apply_negative_refspecs(struct ref *ref_map, struct refspec *rs);\n \n-int query_refspecs(struct refspec *rs, struct refspec_item *query);\n char *apply_refspecs(struct refspec *rs, const char *name);\n \n int check_push_refs(struct ref *src, struct refspec *rs);\n-- \n2.34.1\n\n"},{"id":"511044","messageId":"20250122075154.5697-4-meetsoni3017@gmail.com","threadId":"62841","inReplyTo":"20250122075154.5697-1-meetsoni3017@gmail.com","subject":"[RFC PATCH 3/3] refspec: relocate apply_refspecs and related funtions","fromName":"Meet Soni","fromEmail":"meetsoni3017@gmail.com","sentAt":"2025-01-22T07:51:54Z","receivedAt":"2025-01-22T07:53:16Z","isPatch":true,"sender":{"key":"meetsoni3017@gmail.com","avatar":"https://avatars.githubusercontent.com/u/92802561?v=4"},"body":"Move the functions `apply_refspecs()` and `apply_negative_refspecs()`\nfrom `remote.c` to `refspec.c`. These functions focus on applying\nrefspecs, so centralizing them in `refspec.c` improves code organization\nby keeping refspec-related logic in one place.\n\nSigned-off-by: Meet Soni <meetsoni3017@gmail.com>\n---\n refspec.c | 32 ++++++++++++++++++++++++++++++++\n refspec.h |  8 ++++++++\n remote.c  | 31 -------------------------------\n remote.h  |  8 --------\n 4 files changed, 40 insertions(+), 39 deletions(-)\n\ndiff --git a/refspec.c b/refspec.c\nindex 72b3911110..d279d6032a 100644\n--- a/refspec.c\n+++ b/refspec.c\n@@ -9,6 +9,7 @@\n #include \"strvec.h\"\n #include \"refs.h\"\n #include \"refspec.h\"\n+#include \"remote.h\"\n #include \"strbuf.h\"\n \n /*\n@@ -447,3 +448,34 @@ int query_refspecs(struct refspec *rs, struct refspec_item *query)\n \t}\n \treturn -1;\n }\n+\n+struct ref *apply_negative_refspecs(struct ref *ref_map, struct refspec *rs)\n+{\n+\tstruct ref **tail;\n+\n+\tfor (tail = &ref_map; *tail; ) {\n+\t\tstruct ref *ref = *tail;\n+\n+\t\tif (omit_name_by_refspec(ref->name, rs)) {\n+\t\t\t*tail = ref->next;\n+\t\t\tfree(ref->peer_ref);\n+\t\t\tfree(ref);\n+\t\t} else\n+\t\t\ttail = &ref->next;\n+\t}\n+\n+\treturn ref_map;\n+}\n+\n+char *apply_refspecs(struct refspec *rs, const char *name)\n+{\n+\tstruct refspec_item query;\n+\n+\tmemset(&query, 0, sizeof(struct refspec_item));\n+\tquery.src = (char *)name;\n+\n+\tif (query_refspecs(rs, &query))\n+\t\treturn NULL;\n+\n+\treturn query.dst;\n+}\ndiff --git a/refspec.h b/refspec.h\nindex d3c97bfdc5..294068d226 100644\n--- a/refspec.h\n+++ b/refspec.h\n@@ -86,4 +86,12 @@ void query_refspecs_multiple(struct refspec *rs,\n \t\t\t\t    struct refspec_item *query,\n \t\t\t\t    struct string_list *results);\n \n+/*\n+ * Remove all entries in the input list which match any negative refspec in\n+ * the refspec list.\n+ */\n+struct ref *apply_negative_refspecs(struct ref *ref_map, struct refspec *rs);\n+\n+char *apply_refspecs(struct refspec *rs, const char *name);\n+\n #endif /* REFSPEC_H */\ndiff --git a/remote.c b/remote.c\nindex 2c46611821..641dd1125f 100644\n--- a/remote.c\n+++ b/remote.c\n@@ -907,37 +907,6 @@ void ref_push_report_free(struct ref_push_report *report)\n \t}\n }\n \n-struct ref *apply_negative_refspecs(struct ref *ref_map, struct refspec *rs)\n-{\n-\tstruct ref **tail;\n-\n-\tfor (tail = &ref_map; *tail; ) {\n-\t\tstruct ref *ref = *tail;\n-\n-\t\tif (omit_name_by_refspec(ref->name, rs)) {\n-\t\t\t*tail = ref->next;\n-\t\t\tfree(ref->peer_ref);\n-\t\t\tfree(ref);\n-\t\t} else\n-\t\t\ttail = &ref->next;\n-\t}\n-\n-\treturn ref_map;\n-}\n-\n-char *apply_refspecs(struct refspec *rs, const char *name)\n-{\n-\tstruct refspec_item query;\n-\n-\tmemset(&query, 0, sizeof(struct refspec_item));\n-\tquery.src = (char *)name;\n-\n-\tif (query_refspecs(rs, &query))\n-\t\treturn NULL;\n-\n-\treturn query.dst;\n-}\n-\n int remote_find_tracking(struct remote *remote, struct refspec_item *refspec)\n {\n \treturn query_refspecs(&remote->fetch, refspec);\ndiff --git a/remote.h b/remote.h\nindex f3da64dc41..b4bb16af0e 100644\n--- a/remote.h\n+++ b/remote.h\n@@ -261,14 +261,6 @@ int resolve_remote_symref(struct ref *ref, struct ref *list);\n  */\n struct ref *ref_remove_duplicates(struct ref *ref_map);\n \n-/*\n- * Remove all entries in the input list which match any negative refspec in\n- * the refspec list.\n- */\n-struct ref *apply_negative_refspecs(struct ref *ref_map, struct refspec *rs);\n-\n-char *apply_refspecs(struct refspec *rs, const char *name);\n-\n int check_push_refs(struct ref *src, struct refspec *rs);\n int match_push_refs(struct ref *src, struct ref **dst,\n \t\t    struct refspec *rs, int flags);\n-- \n2.34.1\n\n"},{"id":"511146","messageId":"Z5NI4uUaeHknckJY@pks.im","threadId":"62841","inReplyTo":"20250122075154.5697-1-meetsoni3017@gmail.com","subject":"Re: [RFC PATCH 0/3] refspec: centralize refspec-related logic","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-01-24T08:01:38Z","receivedAt":"2025-01-24T08:01:43Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Wed, Jan 22, 2025 at 01:21:51PM +0530, Meet Soni wrote:\n> As Patrick pointed out in [1], the logic related to refspec is currently\n> split across multiple headers. This patch series addresses that by\n> relocating refspec-related logic from remote to refspec for improved\n> cohesion.\n> \n> There are a few functions that seem to be on the borderline. They seem\n> to have refspec-related logic, but also contain some elements related\n> to remote logic, so I'm unsure whether they should be moved.\n> Specifically:\n> \n>     * get_fetch_map()\n>     * count_refspec_match()\n>     * check_push_refs()\n>     * match_push_refs()\n> \n> [1]: https://lore.kernel.org/git/ZysQvUyxgdRqjvj2@pks.im/\n\nThanks for this series! A couple comments from my side, but I think that\nthe overall goal of this series is sensible (well, I seemingly proposed\nit at one point, so no surprise there).\n\nPatrick\n"},{"id":"511147","messageId":"Z5NI5jVxcNtPQmi8@pks.im","threadId":"62841","inReplyTo":"20250122075154.5697-2-meetsoni3017@gmail.com","subject":"Re: [RFC PATCH 1/3] refspec: relocate omit_name_by_refspec and related functions","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-01-24T08:01:42Z","receivedAt":"2025-01-24T08:01:47Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Wed, Jan 22, 2025 at 01:21:52PM +0530, Meet Soni wrote:\n> diff --git a/refspec.h b/refspec.h\n> index 69d693c87d..9556d08bd5 100644\n> --- a/refspec.h\n> +++ b/refspec.h\n> @@ -71,4 +71,12 @@ struct strvec;\n>  void refspec_ref_prefixes(const struct refspec *rs,\n>  \t\t\t  struct strvec *ref_prefixes);\n>  \n> +/*\n> + * Check whether a name matches any negative refspec in rs. Returns 1 if the\n> + * name matches at least one negative refspec, and 0 otherwise.\n> + */\n> +int omit_name_by_refspec(const char *name, struct refspec *rs);\n> +int match_name_with_pattern(const char *key, const char *name,\n> +\t\t\t\t   const char *value, char **result);\n\nReading the signature of `match_name_with_pattern()` I wouldn't have any\nclue how to use it. Could we maybe add some documentation to it now that\nwe expose it?\n\n>  #endif /* REFSPEC_H */\n> diff --git a/remote.c b/remote.c\n> index 0f6fba8562..40c2418065 100644\n> --- a/remote.c\n> +++ b/remote.c\n> @@ -907,54 +907,6 @@ void ref_push_report_free(struct ref_push_report *report)\n>  \t}\n>  }\n>  \n> -static int match_name_with_pattern(const char *key, const char *name,\n> -\t\t\t\t   const char *value, char **result)\n> -{\n\nIt's a bit unfortunate that we have to convert this static function to a\nnon-static one, but I guess it makes sense in the bigger picture of what\nthis series wants to achieve.\n\nPatrick\n"},{"id":"511148","messageId":"Z5NI60sD8QnQ89W3@pks.im","threadId":"62841","inReplyTo":"20250122075154.5697-3-meetsoni3017@gmail.com","subject":"Re: [RFC PATCH 2/3] refspec: relocate query related functions","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-01-24T08:01:47Z","receivedAt":"2025-01-24T08:01:51Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Wed, Jan 22, 2025 at 01:21:53PM +0530, Meet Soni wrote:\n> diff --git a/refspec.h b/refspec.h\n> index 9556d08bd5..d3c97bfdc5 100644\n> --- a/refspec.h\n> +++ b/refspec.h\n> @@ -1,6 +1,8 @@\n>  #ifndef REFSPEC_H\n>  #define REFSPEC_H\n>  \n> +#include \"string-list.h\"\n> +\n\nWe can avoid including this header by adding a forward-declaration of\n`struct string_list` instead.\n\n> @@ -79,4 +81,9 @@ int omit_name_by_refspec(const char *name, struct refspec *rs);\n>  int match_name_with_pattern(const char *key, const char *name,\n>  \t\t\t\t   const char *value, char **result);\n>  \n> +int query_refspecs(struct refspec *rs, struct refspec_item *query);\n> +void query_refspecs_multiple(struct refspec *rs,\n> +\t\t\t\t    struct refspec_item *query,\n> +\t\t\t\t    struct string_list *results);\n> +\n>  #endif /* REFSPEC_H */\n\nIt would be nice to add some docs here, too.\n\nPatrick\n"},{"id":"511149","messageId":"Z5NI7vQ66aSCgjjg@pks.im","threadId":"62841","inReplyTo":"20250122075154.5697-4-meetsoni3017@gmail.com","subject":"Re: [RFC PATCH 3/3] refspec: relocate apply_refspecs and related funtions","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-01-24T08:01:50Z","receivedAt":"2025-01-24T08:01:54Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Wed, Jan 22, 2025 at 01:21:54PM +0530, Meet Soni wrote:\n> diff --git a/refspec.h b/refspec.h\n> index d3c97bfdc5..294068d226 100644\n> --- a/refspec.h\n> +++ b/refspec.h\n> @@ -86,4 +86,12 @@ void query_refspecs_multiple(struct refspec *rs,\n>  \t\t\t\t    struct refspec_item *query,\n>  \t\t\t\t    struct string_list *results);\n>  \n> +/*\n> + * Remove all entries in the input list which match any negative refspec in\n> + * the refspec list.\n> + */\n> +struct ref *apply_negative_refspecs(struct ref *ref_map, struct refspec *rs);\n> +\n> +char *apply_refspecs(struct refspec *rs, const char *name);\n> +\n>  #endif /* REFSPEC_H */\n\nSame comment here regarding the documentation of `apply_refspecs()`.\n\nPatrick\n"},{"id":"511180","messageId":"xmqq1pws15ut.fsf@gitster.g","threadId":"62841","inReplyTo":"Z5NI4uUaeHknckJY@pks.im","subject":"Re: [RFC PATCH 0/3] refspec: centralize refspec-related logic","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-01-24T17:25:46Z","receivedAt":"2025-01-24T17:25:49Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Patrick Steinhardt <ps@pks.im> writes:\n\n> On Wed, Jan 22, 2025 at 01:21:51PM +0530, Meet Soni wrote:\n>> As Patrick pointed out in [1], the logic related to refspec is currently\n>> split across multiple headers. This patch series addresses that by\n>> relocating refspec-related logic from remote to refspec for improved\n>> cohesion.\n>> \n>> There are a few functions that seem to be on the borderline. They seem\n>> to have refspec-related logic, but also contain some elements related\n>> to remote logic, so I'm unsure whether they should be moved.\n>> Specifically:\n>> \n>>     * get_fetch_map()\n>>     * count_refspec_match()\n>>     * check_push_refs()\n>>     * match_push_refs()\n>> \n>> [1]: https://lore.kernel.org/git/ZysQvUyxgdRqjvj2@pks.im/\n>\n> Thanks for this series! A couple comments from my side, but I think that\n> the overall goal of this series is sensible (well, I seemingly proposed\n> it at one point, so no surprise there).\n\n;-).  Nicely done.\n"}]}