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

["SZEDER Gábor" <szeder.dev@gmail.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.

The main change since v3 is that clone's default refspec is now added
to the 'struct remote' using a function based on Peff's
recommendation, with slight changes:

 - 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(-)

--
2.13.0.35.g14b6294b1


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);