git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
* [PATCHv5 0/8] git clone: Marry --recursive and --reference
@ 2016-08-15 21:53 Stefan Beller
  2016-08-15 21:53 ` [PATCHv5 1/8] t7408: modernize style Stefan Beller
                   ` (8 more replies)
  0 siblings, 9 replies; 24+ messages in thread
From: Stefan Beller @ 2016-08-15 21:53 UTC (permalink / raw)
  To: gitster; +Cc: git, jrnieder, Jens.Lehmann, Stefan Beller

v5:

Thanks Junio, Ramsay for comments.

As the comments only address programming work as opposed to design, the 
interdiff is rather small:

diff --git a/builtin/clone.c b/builtin/clone.c
index 0593aee..404c5e8 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -285,23 +285,26 @@ static void strip_trailing_slashes(char *dir)
 
 static int add_one_reference(struct string_list_item *item, void *cb_data)
 {
-	struct strbuf sb = STRBUF_INIT;
+	struct strbuf err = STRBUF_INIT;
 	int *required = cb_data;
-	char *ref_git = compute_alternate_path(item->string, &sb);
+	char *ref_git = compute_alternate_path(item->string, &err);
 
 	if (!ref_git) {
 		if (*required)
-			die("%s", sb.buf);
+			die("%s", err.buf);
 		else
 			fprintf(stderr,
 				_("info: Could not add alternate for '%s': %s\n"),
-				item->string, sb.buf);
+				item->string, err.buf);
 	} else {
+		struct strbuf sb = STRBUF_INIT;
 		strbuf_addf(&sb, "%s/objects", ref_git);
 		add_to_alternates_file(sb.buf);
+		strbuf_release(&sb);
 	}
 
-	strbuf_release(&sb);
+	strbuf_release(&err);
+	free(ref_git);
 	return 0;
 }
 
diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index 472b1d9..681b91d 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -484,7 +484,7 @@ struct submodule_alternate_setup {
 #define SUBMODULE_ALTERNATE_SETUP_INIT { NULL, \
 	SUBMODULE_ALTERNATE_ERROR_IGNORE, NULL }
 
-int add_possible_reference_from_superproject(
+static int add_possible_reference_from_superproject(
 		struct alternate_object_database *alt, void *sas_cb)
 {
 	struct submodule_alternate_setup *sas = sas_cb;
diff --git a/sha1_file.c b/sha1_file.c
index 2489653..0fe5aa3 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -435,11 +435,12 @@ char *compute_alternate_path(const char *path, struct strbuf *err)
 {
 	char *ref_git = NULL;
 	const char *repo, *ref_git_s;
-	struct strbuf err_buf = STRBUF_INIT;
+	int seen_error = 0;
 
 	ref_git_s = real_path_if_valid(path);
 	if (!ref_git_s) {
-		strbuf_addf(&err_buf, _("path '%s' does not exist"), path);
+		seen_error = 1;
+		strbuf_addf(err, _("path '%s' does not exist"), path);
 		goto out;
 	} else
 		/*
@@ -462,40 +463,41 @@ char *compute_alternate_path(const char *path, struct strbuf *err)
 		ref_git = ref_git_git;
 	} else if (!is_directory(mkpath("%s/objects", ref_git))) {
 		struct strbuf sb = STRBUF_INIT;
+		seen_error = 1;
 		if (get_common_dir(&sb, ref_git)) {
-			strbuf_addf(&err_buf,
+			strbuf_addf(err,
 				    _("reference repository '%s' as a linked "
 				      "checkout is not supported yet."),
 				    path);
 			goto out;
 		}
 
-		strbuf_addf(&err_buf, _("reference repository '%s' is not a "
+		strbuf_addf(err, _("reference repository '%s' is not a "
 					"local repository."), path);
 		goto out;
 	}
 
 	if (!access(mkpath("%s/shallow", ref_git), F_OK)) {
-		strbuf_addf(&err_buf, _("reference repository '%s' is shallow"),
+		strbuf_addf(err, _("reference repository '%s' is shallow"),
 			    path);
+		seen_error = 1;
 		goto out;
 	}
 
 	if (!access(mkpath("%s/info/grafts", ref_git), F_OK)) {
-		strbuf_addf(&err_buf,
+		strbuf_addf(err,
 			    _("reference repository '%s' is grafted"),
 			    path);
+		seen_error = 1;
 		goto out;
 	}
 
 out:
-	if (err_buf.len) {
-		strbuf_addbuf(err, &err_buf);
+	if (seen_error) {
 		free(ref_git);
 		ref_git = NULL;
 	}
 
-	strbuf_release(&err_buf);
 	return ref_git;
 }
 
Thanks,
Stefan


v4:
Thanks to Junios critial questions regarding the design, I took a step back
to look at the bigger picture, again.

new patches:
  clone: factor out checking for an alternate path
  clone: recursive and reference option triggers submodule alternates
  
The last patch redesigns completely how we approach the problem.
Now there are no new command line options (that relate to the problem
of marrying --recursive and --reference), but instead we communicate
everything via configuration options to have a lasting effect (i.e.
submodule update remembers the decision of the initial setup)

Thanks,
Stefan

v3:

Thanks to Junios critial questions regarding the design, I took a step back
to look at the bigger picture. 

--super-reference sounds confusing. (what is the super referring to?)
So drop that approach.

Instead we'll compute where the reference might be in the superproject scope
and ask the submodule clone operation to consider an optional reference.
If the referenced alternate is not there, we'll just warn about it and
carry on.


* fixed the style in patch 2.

* fixed another bug in the last patch, that is unrelated, but would have helped
  me a lot.

Thanks,
Stefan

 Documentation/git-clone.txt    |   9 ++++++++-
 builtin/clone.c                |  36 ++++++++++++++++++++++++++++--------
 builtin/submodule--helper.c    | 105 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-----------------
 git-submodule.sh               |   2 +-
 t/t7408-submodule-reference.sh | 162 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++----------------------------------------------------------------------
 5 files changed, 217 insertions(+), 97 deletions(-)

v2:
 * fixed the p1,2 cleanups
 * added documentation to patches 5,6
 * improved commit message in v4
 
 Thanks,
 Stefan
 
v1:
 
 Currently when cloning a superproject with --recursive and --reference
 only the superproject learns about its alternates. The submodules are
 cloned independently, which may incur lots of network costs.
 
 Assume that the reference repository has the submodules at the same
 paths as the to-be-cloned submodule and try to setup alternates from
 there.
 
 Some submodules in the referenced superproject may not be there, 
 (they are just not initialized/cloned/checked out), which yields
 an error for now. In future work we may want to soften the alternate
 check and not die in the clone when one of the given alternates doesn't
 exist.
 
 patch 1,2 are modernizing style of t7408, 
 patches 3,4 are not strictly necessary, but I think it is a good thing
 to not leave the submodule related C code in a crippled state (i.e.
 allowing only one reference). The shell code would also need this update,
 but it looked ugly to me, so I postpone it until more of the submodule code
 is written in C. 
 
 Thanks,
 Stefan 

Stefan Beller (6):
  t7408: modernize style
  t7408: merge short tests, factor out testing method
  submodule--helper module-clone: allow multiple references
  submodule--helper update-clone: allow multiple references
  submodule update: add super-reference flag
  clone: reference flag is used for submodules as well

 builtin/clone.c                |  22 ++++--
 builtin/submodule--helper.c    |  45 ++++++++----
 git-submodule.sh               |  12 +++-
 t/t7408-submodule-reference.sh | 153 +++++++++++++++++++++++------------------
 4 files changed, 147 insertions(+), 85 deletions(-)

-- 
2.9.2.572.g9d9644e.dirty


^ permalink raw reply related	[flat|nested] 24+ messages in thread
end of thread, other threads:[~2016-08-31  6:22 UTC | newest]

Thread overview: 24+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-08-15 21:53 [PATCHv5 0/8] git clone: Marry --recursive and --reference Stefan Beller
2016-08-15 21:53 ` [PATCHv5 1/8] t7408: modernize style Stefan Beller
2016-08-23 22:04   ` Jacob Keller
2016-08-15 21:53 ` [PATCHv5 2/8] t7408: merge short tests, factor out testing method Stefan Beller
2016-08-23 22:10   ` Jacob Keller
2016-08-15 21:53 ` [PATCHv5 3/8] submodule--helper module-clone: allow multiple references Stefan Beller
2016-08-23 22:12   ` Jacob Keller
2016-08-15 21:53 ` [PATCHv5 4/8] submodule--helper update-clone: " Stefan Beller
2016-08-23 22:20   ` Jacob Keller
2016-08-15 21:53 ` [PATCHv5 5/8] clone: factor out checking for an alternate path Stefan Beller
2016-08-23 22:29   ` Jacob Keller
2016-08-15 21:53 ` [PATCHv5 6/8] clone: clarify option_reference as required Stefan Beller
2016-08-23 22:31   ` Jacob Keller
2016-08-15 21:53 ` [PATCHv5 7/8] clone: implement optional references Stefan Beller
2016-08-23 22:35   ` Jacob Keller
2016-08-15 21:53 ` [PATCHv5 8/8] clone: recursive and reference option triggers submodule alternates Stefan Beller
2016-08-23 22:43   ` Jacob Keller
2016-08-23 23:03     ` Stefan Beller
2016-08-24  6:29       ` Jacob Keller
2016-08-24 22:52         ` Stefan Beller
2016-08-24 23:37           ` Jacob Keller
2016-08-31  5:04             ` Stefan Beller
2016-08-31  6:21               ` Jacob Keller
2016-08-15 22:30 ` [PATCHv5 0/8] git clone: Marry --recursive and --reference Junio C Hamano

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