git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCHv2 0/9] fetch: make sure submodule oids are fetched

From
Stefan Beller <sbeller@google.com>
Date
Sep 17, 2018, 21:35 UTC
Message-ID
<20180917213559.126404-1-sbeller@google.com>
In-Reply-To
<20180911234951.14129-1-sbeller@google.com>
v2:
* extended commit messages,
* plugged a memory leak
* rewrote the patch "sha1-array: provide oid_array_filter" to be much more like 
  object_array_fiter
* fixed a typo pointed out by Ramsay.
The range diff is below.
  
Thanks,
Stefan

v1: Currently when git-fetch is asked to recurse into submodules, it dispatches a plain "git-fetch -C <submodule-dir>" (and some submodule related options such as prefix and recusing strategy, but) without any information of the remote or the tip that should be fetched.

This works surprisingly well in some workflows, not so well in others, which this series aims to fix.

The first patches provide new basic functionality and do some refactoring; the interesting part is in the two last patches.

This was discussed in https://public-inbox.org/git/20180808221752.195419-1-sbeller@google.com/ and I think I addressed all feedback so far.

Stefan Beller (9):
  string-list: add string_list_{pop, last} functions
  sha1-array: provide oid_array_filter
  submodule.c: fix indentation
  submodule.c: sort changed_submodule_names before searching it
  submodule: move global changed_submodule_names into fetch submodule
    struct
  submodule.c: do not copy around submodule list
  submodule: fetch in submodules git directory instead of in worktree
  fetch: retry fetching submodules if needed objects were not fetched
  builtin/fetch: check for submodule updates for non branch fetches
 builtin/fetch.c             |  14 +--
 sha1-array.c                |  17 ++++
 sha1-array.h                |   9 ++
 string-list.c               |  14 +++
 string-list.h               |  15 +++
 submodule.c                 | 191 ++++++++++++++++++++++++++++--------
 t/t5526-fetch-submodules.sh |  23 ++++-
 7 files changed, 236 insertions(+), 47 deletions(-)
git range-diff origin/sb/submodule-recursive-fetch-gets-the-tip...HEAD 
 1:  6fecf7cd01a !  1:  54d31d90734 string-list: add string_list_{pop, last} functions
    @@ -12,8 +12,8 @@
          - string_list_pop() removes the string_list_item at the end of
            the string list.
     
    -     - string_list_push() is not added, but string_list_append() can be
    -       used in its place.
    +     - there is no string_list_push(); string_list_append() can be used
    +       in its place.
     
         You can use them in this pattern:
     
    @@ -26,7 +26,6 @@
     
         Helped-by: Junio C Hamano <gitster@pobox.com>
         Signed-off-by: Stefan Beller <sbeller@google.com>
    -    Signed-off-by: Junio C Hamano <gitster@pobox.com>
     
      diff --git a/string-list.c b/string-list.c
      --- a/string-list.c
    @@ -66,6 +65,10 @@
     + */
     +void string_list_pop(struct string_list *list, int free_util);
     +
    ++/*
    ++ * Returns the last item of the list. As it returns the raw access, do not
    ++ * modify the list while holding onto the returned pointer.
    ++ */
     +static inline struct string_list_item *string_list_last(struct string_list *list)
     +{
     +	return &list->items[list->nr - 1];
 2:  7007a318a68 <  -:  ----------- sha1-array: provide oid_array_filter
 -:  ----------- >  2:  a2bd6ef2bf0 sha1-array: provide oid_array_filter
 3:  807429234ac !  3:  0300c27cbd7 submodule.c: fix indentation
    @@ -6,7 +6,6 @@
         Fix it while we are here.
     
         Signed-off-by: Stefan Beller <sbeller@google.com>
    -    Signed-off-by: Junio C Hamano <gitster@pobox.com>
     
      diff --git a/submodule.c b/submodule.c
      --- a/submodule.c
 4:  f6fa5273af9 !  4:  80cf0221bbe submodule.c: sort changed_submodule_names before searching it
    @@ -12,7 +12,6 @@
         appropriate.
     
         Signed-off-by: Stefan Beller <sbeller@google.com>
    -    Signed-off-by: Junio C Hamano <gitster@pobox.com>
     
      diff --git a/submodule.c b/submodule.c
      --- a/submodule.c
 5:  adf7a2fd203 !  5:  7ddb448b748 submodule: move global changed_submodule_names into fetch submodule struct
    @@ -6,7 +6,6 @@
         part of the struct that is passed around for fetching submodules.
     
         Signed-off-by: Stefan Beller <sbeller@google.com>
    -    Signed-off-by: Junio C Hamano <gitster@pobox.com>
     
      diff --git a/submodule.c b/submodule.c
      --- a/submodule.c
 6:  56c9398589a !  6:  7975a7f1e3b submodule.c: do not copy around submodule list
    @@ -9,12 +9,11 @@
         Instead use the result list directly and prune items afterwards
         using string_list_remove_empty_items.
     
    -    By doin so we'll have access to the util pointer for longer that
    +    By doing so we'll have access to the util pointer for longer that
         contains the commits that we need to fetch, which will be
         useful in a later patch.
     
         Signed-off-by: Stefan Beller <sbeller@google.com>
    -    Signed-off-by: Junio C Hamano <gitster@pobox.com>
     
      diff --git a/submodule.c b/submodule.c
      --- a/submodule.c
 7:  9f70a5f32c9 !  7:  29bc2868f26 submodule: fetch in submodules git directory instead of in worktree
    @@ -3,14 +3,14 @@
         submodule: fetch in submodules git directory instead of in worktree
     
         This patch started as a refactoring to make 'get_next_submodule' more
    -    readable, but upon doing so, I realized that git-fetch actually doesn't
    -    need to be run in the worktree. So let's run it in the git dir instead.
    +    readable, but upon doing so, I realized that "git fetch" of the submodule
    +    actually doesn't need to be run in the submodules worktree. So let's run
    +    it in its git dir instead.
     
         That should pave the way towards fetching submodules that are currently
         not checked out.
     
         Signed-off-by: Stefan Beller <sbeller@google.com>
    -    Signed-off-by: Junio C Hamano <gitster@pobox.com>
     
      diff --git a/submodule.c b/submodule.c
      --- a/submodule.c
    @@ -32,7 +32,7 @@
      	return spf->default_option;
      }
      
    -+static const char *get_submodule_git_dir(struct repository *r, const char *path)
    ++static char *get_submodule_git_dir(struct repository *r, const char *path)
     +{
     +	struct repository subrepo;
     +	const char *ret;
    @@ -87,7 +87,11 @@
      			if (!spf->quiet)
      				strbuf_addf(err, "Fetching submodule %s%s\n",
     @@
    + 			argv_array_push(&cp->args, default_argv);
    + 			argv_array_push(&cp->args, "--submodule-prefix");
      			argv_array_push(&cp->args, submodule_prefix.buf);
    ++
    ++			free(git_dir);
      			ret = 1;
      		}
     -		strbuf_release(&submodule_path);
 8:  bab609b4dc1 !  8:  f837c4a0789 fetch: retry fetching submodules if sha1 were not fetched
    @@ -1,9 +1,9 @@
     Author: Stefan Beller <sbeller@google.com>
     
    -    fetch: retry fetching submodules if sha1 were not fetched
    +    fetch: retry fetching submodules if needed objects were not fetched
     
         Currently when git-fetch is asked to recurse into submodules, it dispatches
    -    a plain "git-fetch -C <submodule-dir>" (and some submodule related options
    +    a plain "git-fetch -C <submodule-dir>" (with some submodule related options
         such as prefix and recusing strategy, but) without any information of the
         remote or the tip that should be fetched.
     
    @@ -16,16 +16,22 @@
         topic downloaded as well. However these submodule changes reside in their
         own repository in their own ref (refs/changes/<int>).
     
    -    Retry fetching a submodule if the object id that the superproject points
    -    to, cannot be found.
    +    Retry fetching a submodule by object name if the object id that the
    +    superproject points to, cannot be found.
     
    -    This doesn't support fetching to FETCH_HEAD yet, but only into a local
    -    branch. To make fetching into FETCH_HEAD work, we need some refactoring
    -    in builtin/fetch.c to adjust the calls to 'check_for_new_submodule_commits'
    -    that is coming in the next patch.
    +    This retrying does not happen when the "git fetch" done at the
    +    superproject is not storing the fetched results in remote
    +    tracking branches (i.e. instead just recording them to
    +    FETCH_HEAD) in this step. A later patch will fix this.
    +
    +    builtin/fetch used to only inspect submodules when they were fetched
    +    "on-demand", as in either on/off case it was clear whether the submodule
    +    needs to be fetched. However to know whether we need to try fetching the
    +    object ids, we need to identify the object names, which is done in this
    +    function check_for_new_submodule_commits(), so we'll also run that code
    +    in case the submodule recursion is set to "on".
     
         Signed-off-by: Stefan Beller <sbeller@google.com>
    -    Signed-off-by: Junio C Hamano <gitster@pobox.com>
     
      diff --git a/builtin/fetch.c b/builtin/fetch.c
      --- a/builtin/fetch.c
 9:  c16d21313f6 !  9:  71f2bb035b1 builtin/fetch: check for submodule updates for non branch fetches
    @@ -2,11 +2,29 @@
     
         builtin/fetch: check for submodule updates for non branch fetches
     
    -    For Gerrit users that use submodules the invocation of fetch without a
    -    branch is their main use case.
    +    Gerrit, the code review tool, has a different workflow than our mailing
    +    list based approach. Usually users upload changes to a Gerrit server and
    +    continuous integration and testing happens by bots. Sometimes however a
    +    user wants to checkout a change locally and look at it locally. For this
    +    use case, Gerrit offers a command line snippet to copy and paste to your
    +    terminal, which looks like
    +
    +      git fetch https://<host>/gerrit refs/changes/<id> &&
    +      git checkout FETCH_HEAD
    +
    +    For Gerrit changes that contain changing submodule gitlinks, it would be
    +    easy to extend both the fetch and checkout with the '--recurse-submodules'
    +    flag, such that this command line snippet would produce the state of a
    +    change locally.
    +
    +    However the functionality added in the previous patch, which would
    +    ensure that we fetch the objects in the submodule that the gitlink pointed
    +    at, only works for remote tracking branches so far, not for FETCH_HEAD.
    +
    +    Make sure that fetching a superproject to its FETCH_HEAD, also respects
    +    the existence checks for objects in the submodule recursion.
     
         Signed-off-by: Stefan Beller <sbeller@google.com>
    -    Signed-off-by: Junio C Hamano <gitster@pobox.com>
     
      diff --git a/builtin/fetch.c b/builtin/fetch.c
      --- a/builtin/fetch.c
Previous: Junio C HamanoNext: Stefan Beller
Message 22 of 33 in “fetch: make sure submodule oids are fetched”
  1. 0/9 fetch: make sure submodule oids are fetchedStefan Beller, Sep 11, 2018
  2. 1/9 string-list: add string_list_{pop, last} functionsStefan Beller, Sep 11, 2018
  3. Ramsay JonesSep 12, 2018
  4. Junio C HamanoSep 12, 2018
  5. 2/9 sha1-array: provide oid_array_filterStefan Beller, Sep 11, 2018
  6. Junio C HamanoSep 12, 2018
  7. 3/9 submodule.c: fix indentationStefan Beller, Sep 11, 2018
  8. Junio C HamanoSep 12, 2018
  9. 4/9 submodule.c: sort changed_submodule_names before searching itStefan Beller, Sep 11, 2018
  10. Junio C HamanoSep 12, 2018
  11. Stefan BellerSep 12, 2018
  12. 5/9 submodule: move global changed_submodule_names into fetch submodule structStefan Beller, Sep 11, 2018
  13. 6/9 submodule.c: do not copy around submodule listStefan Beller, Sep 11, 2018
  14. Ramsay JonesSep 12, 2018
  15. 7/9 submodule: fetch in submodules git directory instead of in worktreeStefan Beller, Sep 11, 2018
  16. Junio C HamanoSep 12, 2018
  17. Stefan BellerSep 13, 2018
  18. 8/9 fetch: retry fetching submodules if sha1 were not fetchedStefan Beller, Sep 11, 2018
  19. Junio C HamanoSep 12, 2018
  20. 9/9 builtin/fetch: check for submodule updates for non branch fetchesStefan Beller, Sep 11, 2018
  21. Junio C HamanoSep 12, 2018
  22. 0/9 fetch: make sure submodule oids are fetchedStefan Beller, Sep 17, 2018
  23. 1/9 string-list: add string_list_{pop, last} functionsStefan Beller, Sep 17, 2018
  24. string-list: add string_list_{pop, last} functionsStefan Beller, Sep 21, 2018
  25. 2/9 sha1-array: provide oid_array_filterStefan Beller, Sep 17, 2018
  26. Stefan BellerSep 17, 2018
  27. 3/9 submodule.c: fix indentationStefan Beller, Sep 17, 2018
  28. 4/9 submodule.c: sort changed_submodule_names before searching itStefan Beller, Sep 17, 2018
  29. 5/9 submodule: move global changed_submodule_names into fetch submodule structStefan Beller, Sep 17, 2018
  30. 6/9 submodule.c: do not copy around submodule listStefan Beller, Sep 17, 2018
  31. 7/9 submodule: fetch in submodules git directory instead of in worktreeStefan Beller, Sep 17, 2018
  32. 8/9 fetch: retry fetching submodules if needed objects were not fetchedStefan Beller, Sep 17, 2018
  33. 9/9 builtin/fetch: check for submodule updates for non branch fetchesStefan Beller, Sep 17, 2018

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.