[v3,0/5] refspec: centralize refspec-related logic

Message

Meet Soni Feb. 1, 2025, 6:41 a.m. UTC

As Patrick pointed out in [1], the logic related to refspec is currently
split across multiple headers. This patch series addresses that by
renaming and relocating refspec-related logic from remote to refspec for
improved cohesion.

[1]: https://lore.kernel.org/git/[e-mail]/

Specifically, the following changes have been made:

    Refactoring and renaming functions: Functions such as
    omit_name_by_refspec() have been renamed to better reflect their
    functionality. 

    Relocation of functions: Functions that are primarily responsible
    for refspec related functionality, have been relocated from remote.c
    to refspec.c to maintain a clear separation of concerns.

Thank you for considering this patch.
Meet


Meet Soni (5):
  refactor(remote): rename function omit_name_by_refspec
  refspec: relocate refname_matches_negative_refspec_item
  refactor(remote): rename query_refspecs functions
  refspec: relocate matching related functions
  refspec: relocate apply_refspecs and related funtions

 builtin/push.c   |   2 +-
 builtin/remote.c |   2 +-
 refspec.c        | 203 ++++++++++++++++++++++++++++++++++++++++++++++
 refspec.h        |  37 +++++++++
 remote.c         | 205 +----------------------------------------------
 remote.h         |  15 ----
 6 files changed, 244 insertions(+), 220 deletions(-)

Range-diff against v2:
-:  ---------- > 1:  399e59ff67 refactor(remote): rename function omit_name_by_refspec
1:  8e393ea1c2 ! 2:  4109b2bd1c refspec: relocate omit_name_by_refspec and related functions
    @@ Metadata
     Author: Meet Soni <[e-mail]>
     
      ## Commit message ##
    -    refspec: relocate omit_name_by_refspec and related functions
    +    refspec: relocate refname_matches_negative_refspec_item
     
    -    Move the functions `omit_name_by_refspec()`, `refspec_match()`, and
    -    `match_name_with_pattern()` from `remote.c` to `refspec.c`. These
    -    functions focus on refspec matching, so placing them in `refspec.c`
    -    aligns with the separation of concerns. Keep refspec-related logic in
    -    `refspec.c` and remote-specific logic in `remote.c` for better code
    -    organization.
    +    Move the functions `refname_matches_negative_refspec_item()`,
    +    `refspec_match()`, and `match_name_with_pattern()` from `remote.c` to
    +    `refspec.c`. These functions focus on refspec matching, so placing them
    +    in `refspec.c` aligns with the separation of concerns. Keep
    +    refspec-related logic in `refspec.c` and remote-specific logic in
    +    `remote.c` for better code organization.
     
         Signed-off-by: Meet Soni <[e-mail]>
     
    @@ refspec.c: void refspec_ref_prefixes(const struct refspec *rs,
     +	return !strcmp(refspec->src, name);
     +}
     +
    -+int omit_name_by_refspec(const char *name, struct refspec *rs)
    ++int refname_matches_negative_refspec_item(const char *refname, struct refspec *rs)
     +{
     +	int i;
     +
     +	for (i = 0; i < rs->nr; i++) {
    -+		if (rs->items[i].negative && refspec_match(&rs->items[i], name))
    ++		if (rs->items[i].negative && refspec_match(&rs->items[i], refname))
     +			return 1;
     +	}
     +	return 0;
    @@ refspec.h: struct strvec;
      void refspec_ref_prefixes(const struct refspec *rs,
      			  struct strvec *ref_prefixes);
      
    -+/*
    -+ * Check whether a name matches any negative refspec in rs. Returns 1 if the
    -+ * name matches at least one negative refspec, and 0 otherwise.
    -+ */
    -+int omit_name_by_refspec(const char *name, struct refspec *rs);
    ++int refname_matches_negative_refspec_item(const char *refname, struct refspec *rs);
     +
     +/*
     + * Checks whether a name matches a pattern and optionally generates a result.
    @@ remote.c: void ref_push_report_free(struct ref_push_report *report)
     -	return !strcmp(refspec->src, name);
     -}
     -
    --int omit_name_by_refspec(const char *name, struct refspec *rs)
    +-int refname_matches_negative_refspec_item(const char *refname, struct refspec *rs)
     -{
     -	int i;
     -
     -	for (i = 0; i < rs->nr; i++) {
    --		if (rs->items[i].negative && refspec_match(&rs->items[i], name))
    +-		if (rs->items[i].negative && refspec_match(&rs->items[i], refname))
     -			return 1;
     -	}
     -	return 0;
    @@ remote.c: void ref_push_report_free(struct ref_push_report *report)
      struct ref *apply_negative_refspecs(struct ref *ref_map, struct refspec *rs)
      {
      	struct ref **tail;
    -
    - ## remote.h ##
    -@@ remote.h: int resolve_remote_symref(struct ref *ref, struct ref *list);
    -  */
    - struct ref *ref_remove_duplicates(struct ref *ref_map);
    - 
    --/*
    -- * Check whether a name matches any negative refspec in rs. Returns 1 if the
    -- * name matches at least one negative refspec, and 0 otherwise.
    -- */
    --int omit_name_by_refspec(const char *name, struct refspec *rs);
    --
    - /*
    -  * Remove all entries in the input list which match any negative refspec in
    -  * the refspec list.
-:  ---------- > 3:  559224864f refactor(remote): rename query_refspecs functions
2:  ef6edbc15b ! 4:  13e49509fc refspec: relocate query related functions
    @@ Metadata
     Author: Meet Soni <[e-mail]>
     
      ## Commit message ##
    -    refspec: relocate query related functions
    +    refspec: relocate matching related functions
     
    -    Move the functions `query_refspecs()`, `query_refspecs_multiple()` and
    -    `query_matches_negative_refspec()` from `remote.c` to `refspec.c`. These
    -    functions focus on querying refspecs, so centralizing them in `refspec.c`
    -    improves code organization by keeping refspec-related logic in one place.
    +    Move the functions `find_refspec_match()`, `find_all_refspec_matches()`
    +    and `find_negative_refspec_match()` from `remote.c` to `refspec.c`.
    +    These functions focus on matching refspecs, so centralizing them in
    +    `refspec.c` improves code organization by keeping refspec-related logic
    +    in one place.
     
         Signed-off-by: Meet Soni <[e-mail]>
     
    @@ refspec.c
      #include "strvec.h"
      #include "refs.h"
      #include "refspec.h"
    -@@ refspec.c: int omit_name_by_refspec(const char *name, struct refspec *rs)
    +@@ refspec.c: int refname_matches_negative_refspec_item(const char *refname, struct refspec *r
      	}
      	return 0;
      }
     +
    -+static int query_matches_negative_refspec(struct refspec *rs, struct refspec_item *query)
    ++static int find_negative_refspec_match(struct refspec *rs, struct refspec_item *query)
     +{
     +	int i, matched_negative = 0;
     +	int find_src = !query->src;
    @@ refspec.c: int omit_name_by_refspec(const char *name, struct refspec *rs)
     +	}
     +
     +	for (i = 0; !matched_negative && i < reversed.nr; i++) {
    -+		if (omit_name_by_refspec(reversed.items[i].string, rs))
    ++		if (refname_matches_negative_refspec_item(reversed.items[i].string, rs))
     +			matched_negative = 1;
     +	}
     +
    @@ refspec.c: int omit_name_by_refspec(const char *name, struct refspec *rs)
     +	return matched_negative;
     +}
     +
    -+void query_refspecs_multiple(struct refspec *rs,
    ++void find_all_refspec_matches(struct refspec *rs,
     +				    struct refspec_item *query,
     +				    struct string_list *results)
     +{
    @@ refspec.c: int omit_name_by_refspec(const char *name, struct refspec *rs)
     +	int find_src = !query->src;
     +
     +	if (find_src && !query->dst)
    -+		BUG("query_refspecs_multiple: need either src or dst");
    ++		BUG("find_all_refspec_matches: need either src or dst");
     +
    -+	if (query_matches_negative_refspec(rs, query))
    ++	if (find_negative_refspec_match(rs, query))
     +		return;
     +
     +	for (i = 0; i < rs->nr; i++) {
    @@ refspec.c: int omit_name_by_refspec(const char *name, struct refspec *rs)
     +	}
     +}
     +
    -+int query_refspecs(struct refspec *rs, struct refspec_item *query)
    ++int find_refspec_match(struct refspec *rs, struct refspec_item *query)
     +{
     +	int i;
     +	int find_src = !query->src;
    @@ refspec.c: int omit_name_by_refspec(const char *name, struct refspec *rs)
     +	char **result = find_src ? &query->src : &query->dst;
     +
     +	if (find_src && !query->dst)
    -+		BUG("query_refspecs: need either src or dst");
    ++		BUG("find_refspec_match: need either src or dst");
     +
    -+	if (query_matches_negative_refspec(rs, query))
    ++	if (find_negative_refspec_match(rs, query))
     +		return -1;
     +
     +	for (i = 0; i < rs->nr; i++) {
    @@ refspec.h: struct refspec_item {
      #define REFSPEC_FETCH 1
      #define REFSPEC_PUSH 0
      
    -@@ refspec.h: int omit_name_by_refspec(const char *name, struct refspec *rs);
    +@@ refspec.h: int refname_matches_negative_refspec_item(const char *refname, struct refspec *r
      int match_name_with_pattern(const char *key, const char *name,
      				   const char *value, char **result);
      
    @@ refspec.h: int omit_name_by_refspec(const char *name, struct refspec *rs);
     + * Queries a refspec for a match and updates the query item.
     + * Returns 0 on success, -1 if no match is found or negative refspec matches.
     + */
    -+int query_refspecs(struct refspec *rs, struct refspec_item *query);
    ++int find_refspec_match(struct refspec *rs, struct refspec_item *query);
     +
     +/*
     + * Queries a refspec for all matches and appends results to the provided string
     + * list.
     + */
    -+void query_refspecs_multiple(struct refspec *rs,
    ++void find_all_refspec_matches(struct refspec *rs,
     +				    struct refspec_item *query,
     +				    struct string_list *results);
     +
    @@ remote.c: struct ref *apply_negative_refspecs(struct ref *ref_map, struct refspe
      	return ref_map;
      }
      
    --static int query_matches_negative_refspec(struct refspec *rs, struct refspec_item *query)
    +-static int find_negative_refspec_match(struct refspec *rs, struct refspec_item *query)
     -{
     -	int i, matched_negative = 0;
     -	int find_src = !query->src;
    @@ remote.c: struct ref *apply_negative_refspecs(struct ref *ref_map, struct refspe
     -	}
     -
     -	for (i = 0; !matched_negative && i < reversed.nr; i++) {
    --		if (omit_name_by_refspec(reversed.items[i].string, rs))
    +-		if (refname_matches_negative_refspec_item(reversed.items[i].string, rs))
     -			matched_negative = 1;
     -	}
     -
    @@ remote.c: struct ref *apply_negative_refspecs(struct ref *ref_map, struct refspe
     -	return matched_negative;
     -}
     -
    --static void query_refspecs_multiple(struct refspec *rs,
    +-static void find_all_refspec_matches(struct refspec *rs,
     -				    struct refspec_item *query,
     -				    struct string_list *results)
     -{
    @@ remote.c: struct ref *apply_negative_refspecs(struct ref *ref_map, struct refspe
     -	int find_src = !query->src;
     -
     -	if (find_src && !query->dst)
    --		BUG("query_refspecs_multiple: need either src or dst");
    +-		BUG("find_all_refspec_matches: need either src or dst");
     -
    --	if (query_matches_negative_refspec(rs, query))
    +-	if (find_negative_refspec_match(rs, query))
     -		return;
     -
     -	for (i = 0; i < rs->nr; i++) {
    @@ remote.c: struct ref *apply_negative_refspecs(struct ref *ref_map, struct refspe
     -	}
     -}
     -
    --int query_refspecs(struct refspec *rs, struct refspec_item *query)
    +-int find_refspec_match(struct refspec *rs, struct refspec_item *query)
     -{
     -	int i;
     -	int find_src = !query->src;
    @@ remote.c: struct ref *apply_negative_refspecs(struct ref *ref_map, struct refspe
     -	char **result = find_src ? &query->src : &query->dst;
     -
     -	if (find_src && !query->dst)
    --		BUG("query_refspecs: need either src or dst");
    +-		BUG("find_refspec_match: need either src or dst");
     -
    --	if (query_matches_negative_refspec(rs, query))
    +-	if (find_negative_refspec_match(rs, query))
     -		return -1;
     -
     -	for (i = 0; i < rs->nr; i++) {
    @@ remote.c: struct ref *apply_negative_refspecs(struct ref *ref_map, struct refspe
      char *apply_refspecs(struct refspec *rs, const char *name)
      {
      	struct refspec_item query;
    -
    - ## remote.h ##
    -@@ remote.h: struct ref *ref_remove_duplicates(struct ref *ref_map);
    -  */
    - struct ref *apply_negative_refspecs(struct ref *ref_map, struct refspec *rs);
    - 
    --int query_refspecs(struct refspec *rs, struct refspec_item *query);
    - char *apply_refspecs(struct refspec *rs, const char *name);
    - 
    - int check_push_refs(struct ref *src, struct refspec *rs);
3:  ea72647439 ! 5:  891e01be93 refspec: relocate apply_refspecs and related funtions
    @@ refspec.c
      #include "strbuf.h"
      
      /*
    -@@ refspec.c: int query_refspecs(struct refspec *rs, struct refspec_item *query)
    +@@ refspec.c: int find_refspec_match(struct refspec *rs, struct refspec_item *query)
      	}
      	return -1;
      }
    @@ refspec.c: int query_refspecs(struct refspec *rs, struct refspec_item *query)
     +	for (tail = &ref_map; *tail; ) {
     +		struct ref *ref = *tail;
     +
    -+		if (omit_name_by_refspec(ref->name, rs)) {
    ++		if (refname_matches_negative_refspec_item(ref->name, rs)) {
     +			*tail = ref->next;
     +			free(ref->peer_ref);
     +			free(ref);
    @@ refspec.c: int query_refspecs(struct refspec *rs, struct refspec_item *query)
     +	memset(&query, 0, sizeof(struct refspec_item));
     +	query.src = (char *)name;
     +
    -+	if (query_refspecs(rs, &query))
    ++	if (find_refspec_match(rs, &query))
     +		return NULL;
     +
     +	return query.dst;
     +}
     
      ## refspec.h ##
    -@@ refspec.h: void query_refspecs_multiple(struct refspec *rs,
    +@@ refspec.h: void find_all_refspec_matches(struct refspec *rs,
      				    struct refspec_item *query,
      				    struct string_list *results);
      
    @@ refspec.h: void query_refspecs_multiple(struct refspec *rs,
     +struct ref *apply_negative_refspecs(struct ref *ref_map, struct refspec *rs);
     +
     +/*
    -+ * Applies refspecs to a name and returns the corresponding destination.
    -+ * Returns the destination string if a match is found, NULL otherwise.
    ++ * Search for a refspec that matches the given name and return the
    ++ * corresponding destination (dst) if a match is found, NULL otherwise.
     + */
     +char *apply_refspecs(struct refspec *rs, const char *name);
     +
    @@ remote.c: void ref_push_report_free(struct ref_push_report *report)
     -	for (tail = &ref_map; *tail; ) {
     -		struct ref *ref = *tail;
     -
    --		if (omit_name_by_refspec(ref->name, rs)) {
    +-		if (refname_matches_negative_refspec_item(ref->name, rs)) {
     -			*tail = ref->next;
     -			free(ref->peer_ref);
     -			free(ref);
    @@ remote.c: void ref_push_report_free(struct ref_push_report *report)
     -	memset(&query, 0, sizeof(struct refspec_item));
     -	query.src = (char *)name;
     -
    --	if (query_refspecs(rs, &query))
    +-	if (find_refspec_match(rs, &query))
     -		return NULL;
     -
     -	return query.dst;
    @@ remote.c: void ref_push_report_free(struct ref_push_report *report)
     -
      int remote_find_tracking(struct remote *remote, struct refspec_item *refspec)
      {
    - 	return query_refspecs(&remote->fetch, refspec);
    + 	return find_refspec_match(&remote->fetch, refspec);
     
      ## remote.h ##
     @@ remote.h: int resolve_remote_symref(struct ref *ref, struct ref *list);
       */
      struct ref *ref_remove_duplicates(struct ref *ref_map);
      
    +-int refname_matches_negative_refspec_item(const char *refname, struct refspec *rs);
    +-
     -/*
     - * Remove all entries in the input list which match any negative refspec in
     - * the refspec list.
     - */
     -struct ref *apply_negative_refspecs(struct ref *ref_map, struct refspec *rs);
     -
    +-int find_refspec_match(struct refspec *rs, struct refspec_item *query);
     -char *apply_refspecs(struct refspec *rs, const char *name);
     -
      int check_push_refs(struct ref *src, struct refspec *rs);