From: Stefan Beller <sbeller@google.com>
To: sbeller@google.com
Cc: git@vger.kernel.org
Subject: [PATCHv2 0/9] fetch: make sure submodule oids are fetched
Date: Mon, 17 Sep 2018 14:35:50 -0700 [thread overview]
Message-ID: <20180917213559.126404-1-sbeller@google.com> (raw)
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
next prev parent reply other threads:[~2018-09-17 21:36 UTC|newest]
Thread overview: 33+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-09-11 23:49 [PATCH 0/9] fetch: make sure submodule oids are fetched Stefan Beller
2018-09-11 23:49 ` [PATCH 1/9] string-list: add string_list_{pop, last} functions Stefan Beller
2018-09-12 2:24 ` Ramsay Jones
2018-09-12 17:52 ` Junio C Hamano
2018-09-11 23:49 ` [PATCH 2/9] sha1-array: provide oid_array_filter Stefan Beller
2018-09-12 18:02 ` Junio C Hamano
2018-09-11 23:49 ` [PATCH 3/9] submodule.c: fix indentation Stefan Beller
2018-09-12 18:02 ` Junio C Hamano
2018-09-11 23:49 ` [PATCH 4/9] submodule.c: sort changed_submodule_names before searching it Stefan Beller
2018-09-12 18:18 ` Junio C Hamano
2018-09-12 19:06 ` Stefan Beller
2018-09-11 23:49 ` [PATCH 5/9] submodule: move global changed_submodule_names into fetch submodule struct Stefan Beller
2018-09-11 23:49 ` [PATCH 6/9] submodule.c: do not copy around submodule list Stefan Beller
2018-09-12 2:25 ` Ramsay Jones
2018-09-11 23:49 ` [PATCH 7/9] submodule: fetch in submodules git directory instead of in worktree Stefan Beller
2018-09-12 18:36 ` Junio C Hamano
2018-09-13 19:29 ` Stefan Beller
2018-09-11 23:49 ` [PATCH 8/9] fetch: retry fetching submodules if sha1 were not fetched Stefan Beller
2018-09-12 19:03 ` Junio C Hamano
2018-09-11 23:49 ` [PATCH 9/9] builtin/fetch: check for submodule updates for non branch fetches Stefan Beller
2018-09-12 19:20 ` Junio C Hamano
2018-09-17 21:35 ` Stefan Beller [this message]
2018-09-17 21:35 ` [PATCH 1/9] string-list: add string_list_{pop, last} functions Stefan Beller
2018-09-21 22:08 ` [PATCH] " Stefan Beller
2018-09-17 21:35 ` [PATCH 2/9] sha1-array: provide oid_array_filter Stefan Beller
2018-09-17 21:42 ` Stefan Beller
2018-09-17 21:35 ` [PATCH 3/9] submodule.c: fix indentation Stefan Beller
2018-09-17 21:35 ` [PATCH 4/9] submodule.c: sort changed_submodule_names before searching it Stefan Beller
2018-09-17 21:35 ` [PATCH 5/9] submodule: move global changed_submodule_names into fetch submodule struct Stefan Beller
2018-09-17 21:35 ` [PATCH 6/9] submodule.c: do not copy around submodule list Stefan Beller
2018-09-17 21:35 ` [PATCH 7/9] submodule: fetch in submodules git directory instead of in worktree Stefan Beller
2018-09-17 21:35 ` [PATCH 8/9] fetch: retry fetching submodules if needed objects were not fetched Stefan Beller
2018-09-17 21:35 ` [PATCH 9/9] builtin/fetch: check for submodule updates for non branch fetches Stefan Beller
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: http://vger.kernel.org/majordomo-info.html
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20180917213559.126404-1-sbeller@google.com \
--to=sbeller@google.com \
--cc=git@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).