Re: [PATCHv3 1/4] clone: respect additional configured fetch refspecs during initial fetch

From

SZEDER Gábor <szeder.dev@gmail.com>

Date

May 30, 2017, 07:11 UTC

Message-ID

<CAM0VKjnOSxQg_VCBO2cgtbqesmNYx+e_H7m=36PsNWi9K9rQ1Q@mail.gmail.com>

In-Reply-To

<xmqq8tlfdmpg.fsf@gitster.mtv.corp.google.com>

On Tue, May 30, 2017 at 5:53 AM, Junio C Hamano <gitster@pobox.com> wrote:

> So... do we have a further plan for this topic, or are
> the patches more or less good as they are?

Yup, here it comes.

 - I call that function add_and_parse_fetch_refspec(), because, well,
   that's what it does, and
 - I dropped the if (remote->fetch) condition, because the function
   should work when there are no refspecs in the configuration, too.

I also dropped Peff's two patches that were included in v3, because:

 - his last patch doesn't apply anymore, because the variable it frees
   properly doesn't exist anymore, and
 - with that patch gone his second patch is not tangled up with this
   topic, so I think it should go on its own branch.

Interdiff against the first two patch of v3 below.

SZEDER Gábor (2):
  clone: respect additional configured fetch refspecs during initial
    fetch
  Documentation/clone: document ignored configuration variables

 Documentation/git-clone.txt |  4 ++++
 builtin/clone.c             | 34 +++++++++++++++++-----------------
 remote.c                    | 13 +++++++++++++
 remote.h                    |  1 +
 t/t5611-clone-config.sh     | 44 ++++++++++++++++++++++++++++++++++++++++++++
 5 files changed, 79 insertions(+), 17 deletions(-)

diff --git a/builtin/clone.c b/builtin/clone.c
index 4144190da..4157922d8 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -853,16 +853,13 @@ int cmd_clone(int argc, const char **argv, const
char *prefix)
        const struct ref *our_head_points_at;
        struct ref *mapped_refs;
        const struct ref *ref;
-       struct strbuf key = STRBUF_INIT, value = STRBUF_INIT;
+       struct strbuf key = STRBUF_INIT, default_refspec = STRBUF_INIT;
        struct strbuf branch_top = STRBUF_INIT, reflog_msg = STRBUF_INIT;
        struct transport *transport = NULL;
        const char *src_ref_prefix = "refs/heads/";
        struct remote *remote;
        int err = 0, complete_refs_before_fetch = 1;

-       struct refspec *refspec;
-       const char *fetch_pattern;
-
        packet_trace_identity("clone");
        argc = parse_options(argc, argv, prefix, builtin_clone_options,
                             builtin_clone_usage, 0);
@@ -980,7 +977,6 @@ int cmd_clone(int argc, const char **argv, const
char *prefix)
                strbuf_addf(&branch_top, "refs/remotes/%s/", option_origin);
        }

-       strbuf_addf(&value, "+%s*:%s*", src_ref_prefix, branch_top.buf);
        strbuf_addf(&key, "remote.%s.url", option_origin);
        git_config_set(key.buf, repo);
        strbuf_reset(&key);
@@ -988,15 +984,10 @@ int cmd_clone(int argc, const char **argv, const
char *prefix)
        if (option_reference.nr)
                setup_reference();

-       fetch_pattern = value.buf;
-       refspec = parse_fetch_refspec(1, &fetch_pattern);
-
-       strbuf_reset(&value);
-
        remote = remote_get(option_origin);
-       REALLOC_ARRAY(remote->fetch, remote->fetch_refspec_nr + 1);
-       memcpy(remote->fetch+remote->fetch_refspec_nr, refspec,
-              sizeof(*refspec));
+       strbuf_addf(&default_refspec, "+%s*:%s*", src_ref_prefix,
+                   branch_top.buf);
+       add_and_parse_fetch_refspec(remote, default_refspec.buf);

        transport = transport_get(remote, remote->url[0]);
        transport_set_verbosity(transport, option_verbosity, option_progress);
@@ -1039,7 +1030,7 @@ int cmd_clone(int argc, const char **argv, const
char *prefix)

        if (refs) {
                mapped_refs = wanted_peer_refs(refs, remote->fetch,
-                                              remote->fetch_refspec_nr + 1);
+                                              remote->fetch_refspec_nr);
                /*
                 * transport_get_remote_refs() may return refs with null sha-1
                 * in mapped_refs (see struct transport->get_refs_list
@@ -1117,9 +1108,8 @@ int cmd_clone(int argc, const char **argv, const
char *prefix)
        strbuf_release(&reflog_msg);
        strbuf_release(&branch_top);
        strbuf_release(&key);
-       strbuf_release(&value);
+       strbuf_release(&default_refspec);
        junk_mode = JUNK_LEAVE_ALL;

-       free(refspec);
        return err;
 }
diff --git a/remote.c b/remote.c
index ad6c5424e..b8fd09dc9 100644
--- a/remote.c
+++ b/remote.c
@@ -626,6 +626,19 @@ struct refspec *parse_fetch_refspec(int
nr_refspec, const char **refspec)
        return parse_refspec_internal(nr_refspec, refspec, 1, 0);
 }

+void add_and_parse_fetch_refspec(struct remote *remote, const char *refspec)
+{
+       struct refspec *rs;
+
+       add_fetch_refspec(remote, refspec);
+       rs = parse_fetch_refspec(1, &refspec);
+       REALLOC_ARRAY(remote->fetch, remote->fetch_refspec_nr);
+       remote->fetch[remote->fetch_refspec_nr - 1] = *rs;
+
+       /* Not free_refspecs(), as we copied its pointers above */
+       free(rs);
+}
+
 static struct refspec *parse_push_refspec(int nr_refspec, const char **refspec)
 {
        return parse_refspec_internal(nr_refspec, refspec, 0, 0);
diff --git a/remote.h b/remote.h
index 924881169..9ad8c1085 100644
--- a/remote.h
+++ b/remote.h
@@ -164,6 +164,7 @@ struct ref *ref_remove_duplicates(struct ref *ref_map);

 int valid_fetch_refspec(const char *refspec);
 struct refspec *parse_fetch_refspec(int nr_refspec, const char **refspec);
+void add_and_parse_fetch_refspec(struct remote *remote, const char *refspec);

 void free_refspec(int nr_refspec, struct refspec *refspec);