* [PATCH 1/2] clone: allow more than one --reference
2011-08-23 1:05 ` [PATCH 0/2] clone-local fixup Junio C Hamano
@ 2011-08-23 1:05 ` Junio C Hamano
2011-08-23 1:05 ` [PATCH 2/2] clone: clone from a repository with relative alternates Junio C Hamano
2011-08-23 3:43 ` [PATCH 0/2] clone-local fixup Hui Wang
2 siblings, 0 replies; 10+ messages in thread
From: Junio C Hamano @ 2011-08-23 1:05 UTC (permalink / raw)
To: git; +Cc: Hui Wang
Also add a test to expose a long-standing bug that is triggered when
cloning with --reference option from a local repository that has its own
alternates. The alternate object stores specified on the command line
are lost, and only alternates copied from the source repository remain.
The bug will be fixed in the next patch.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
builtin/clone.c | 32 ++++++++++++++++++++++++--------
t/t5601-clone.sh | 15 +++++++++++++++
2 files changed, 39 insertions(+), 8 deletions(-)
diff --git a/builtin/clone.c b/builtin/clone.c
index f579794..ee5d651 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -39,13 +39,23 @@ static const char * const builtin_clone_usage[] = {
static int option_no_checkout, option_bare, option_mirror;
static int option_local, option_no_hardlinks, option_shared, option_recursive;
-static char *option_template, *option_reference, *option_depth;
+static char *option_template, *option_depth;
static char *option_origin = NULL;
static char *option_branch = NULL;
static const char *real_git_dir;
static char *option_upload_pack = "git-upload-pack";
static int option_verbosity;
static int option_progress;
+static struct string_list option_reference;
+
+static int opt_parse_reference(const struct option *opt, const char *arg, int unset)
+{
+ struct string_list *option_reference = opt->value;
+ if (!arg)
+ return -1;
+ string_list_append(option_reference, arg);
+ return 0;
+}
static struct option builtin_clone_options[] = {
OPT__VERBOSITY(&option_verbosity),
@@ -71,8 +81,8 @@ static struct option builtin_clone_options[] = {
"initialize submodules in the clone"),
OPT_STRING(0, "template", &option_template, "template-directory",
"directory from which templates will be used"),
- OPT_STRING(0, "reference", &option_reference, "repo",
- "reference repository"),
+ OPT_CALLBACK(0 , "reference", &option_reference, "repo",
+ "reference repository", &opt_parse_reference),
OPT_STRING('o', "origin", &option_origin, "branch",
"use <branch> instead of 'origin' to track upstream"),
OPT_STRING('b', "branch", &option_branch, "branch",
@@ -197,7 +207,7 @@ static void strip_trailing_slashes(char *dir)
*end = '\0';
}
-static void setup_reference(const char *repo)
+static int add_one_reference(struct string_list_item *item, void *cb_data)
{
const char *ref_git;
char *ref_git_copy;
@@ -206,13 +216,13 @@ static void setup_reference(const char *repo)
struct transport *transport;
const struct ref *extra;
- ref_git = real_path(option_reference);
+ ref_git = real_path(item->string);
if (is_directory(mkpath("%s/.git/objects", ref_git)))
ref_git = mkpath("%s/.git", ref_git);
else if (!is_directory(mkpath("%s/objects", ref_git)))
die(_("reference repository '%s' is not a local directory."),
- option_reference);
+ item->string);
ref_git_copy = xstrdup(ref_git);
@@ -227,6 +237,12 @@ static void setup_reference(const char *repo)
transport_disconnect(transport);
free(ref_git_copy);
+ return 0;
+}
+
+static void setup_reference(void)
+{
+ for_each_string_list(&option_reference, add_one_reference, NULL);
}
static void copy_or_link_directory(struct strbuf *src, struct strbuf *dest)
@@ -521,8 +537,8 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
git_config_set(key.buf, repo);
strbuf_reset(&key);
- if (option_reference)
- setup_reference(git_dir);
+ if (option_reference.nr)
+ setup_reference();
fetch_pattern = value.buf;
refspec = parse_fetch_refspec(1, &fetch_pattern);
diff --git a/t/t5601-clone.sh b/t/t5601-clone.sh
index 151ea53..0163ad1 100755
--- a/t/t5601-clone.sh
+++ b/t/t5601-clone.sh
@@ -207,4 +207,19 @@ test_expect_success 'clone separate gitdir where target already exists' '
test_must_fail git clone --separate-git-dir realgitdir src dst
'
+test_expect_failure 'clone --reference from original' '
+ git clone --shared --bare src src-1 &&
+ git clone --bare src src-2 &&
+ git clone --reference=src-2 --bare src-1 target-8 &&
+ grep /src-2/ target-8/objects/info/alternates
+'
+
+test_expect_success 'clone with more than one --reference' '
+ git clone --bare src src-3 &&
+ git clone --bare src src-4 &&
+ git clone --reference=src-3 --reference=src-4 src target-9 &&
+ grep /src-3/ target-9/.git/objects/info/alternates &&
+ grep /src-4/ target-9/.git/objects/info/alternates
+'
+
test_done
--
1.7.6.557.gcee42
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 2/2] clone: clone from a repository with relative alternates
2011-08-23 1:05 ` [PATCH 0/2] clone-local fixup Junio C Hamano
2011-08-23 1:05 ` [PATCH 1/2] clone: allow more than one --reference Junio C Hamano
@ 2011-08-23 1:05 ` Junio C Hamano
2011-08-23 3:43 ` [PATCH 0/2] clone-local fixup Hui Wang
2 siblings, 0 replies; 10+ messages in thread
From: Junio C Hamano @ 2011-08-23 1:05 UTC (permalink / raw)
To: git; +Cc: Hui Wang
Cloning from a local repository blindly copies or hardlinks all the files
under objects/ hierarchy. This results in two issues:
- If the repository cloned has an "objects/info/alternates" file, and the
command line of clone specifies --reference, the ones specified on the
command line get overwritten by the copy from the original repository.
- An entry in a "objects/info/alternates" file can specify the object
stores it borrows objects from as a path relative to the "objects/"
directory. When cloning a repository with such an alternates file, if
the new repository is not sitting next to the original repository, such
relative paths needs to be adjusted so that they can be used in the new
repository.
This updates add_to_alternates_file() to take the path to the alternate
object store, including the "/objects" part at the end (earlier, it was
taking the path to $GIT_DIR and was adding "/objects" itself), as it is
technically possible to specify in objects/info/alternates file the path
of a directory whose name does not end with "/objects".
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
builtin/clone.c | 52 ++++++++++++++++++++++++++++++++++++++++------------
sha1_file.c | 2 +-
t/t5601-clone.sh | 10 +++++++++-
3 files changed, 50 insertions(+), 14 deletions(-)
diff --git a/builtin/clone.c b/builtin/clone.c
index ee5d651..2842707 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -210,8 +210,7 @@ static void strip_trailing_slashes(char *dir)
static int add_one_reference(struct string_list_item *item, void *cb_data)
{
const char *ref_git;
- char *ref_git_copy;
-
+ struct strbuf alternate = STRBUF_INIT;
struct remote *remote;
struct transport *transport;
const struct ref *extra;
@@ -224,19 +223,17 @@ static int add_one_reference(struct string_list_item *item, void *cb_data)
die(_("reference repository '%s' is not a local directory."),
item->string);
- ref_git_copy = xstrdup(ref_git);
-
- add_to_alternates_file(ref_git_copy);
+ strbuf_addf(&alternate, "%s/objects", ref_git);
+ add_to_alternates_file(alternate.buf);
+ strbuf_release(&alternate);
- remote = remote_get(ref_git_copy);
- transport = transport_get(remote, ref_git_copy);
+ remote = remote_get(ref_git);
+ transport = transport_get(remote, ref_git);
for (extra = transport_get_remote_refs(transport); extra;
extra = extra->next)
add_extra_ref(extra->name, extra->old_sha1, 0);
transport_disconnect(transport);
-
- free(ref_git_copy);
return 0;
}
@@ -245,7 +242,32 @@ static void setup_reference(void)
for_each_string_list(&option_reference, add_one_reference, NULL);
}
-static void copy_or_link_directory(struct strbuf *src, struct strbuf *dest)
+static void copy_alternates(struct strbuf *src, struct strbuf *dst,
+ const char *src_repo)
+{
+ /*
+ * Read from the source objects/info/alternates file
+ * and copy the entries to corresponding file in the
+ * destination repository with add_to_alternates_file().
+ * Both src and dst have "$path/objects/info/alternates".
+ */
+ FILE *in = fopen(src->buf, "r");
+ struct strbuf line = STRBUF_INIT;
+
+ while (strbuf_getline(&line, in, '\n') != EOF) {
+ char *abs_path, abs_buf[PATH_MAX];
+ if (!line.len || line.buf[0] == '#')
+ continue;
+ abs_path = mkpath("%s/objects/%s", src_repo, line.buf);
+ normalize_path_copy(abs_buf, abs_path);
+ add_to_alternates_file(abs_buf);
+ }
+ strbuf_release(&line);
+ fclose(in);
+}
+
+static void copy_or_link_directory(struct strbuf *src, struct strbuf *dest,
+ const char *src_repo, int src_baselen)
{
struct dirent *de;
struct stat buf;
@@ -281,7 +303,13 @@ static void copy_or_link_directory(struct strbuf *src, struct strbuf *dest)
}
if (S_ISDIR(buf.st_mode)) {
if (de->d_name[0] != '.')
- copy_or_link_directory(src, dest);
+ copy_or_link_directory(src, dest,
+ src_repo, src_baselen);
+ continue;
+ }
+
+ if (!strcmp(src->buf + src_baselen, "/info/alternates")) {
+ copy_alternates(src, dest, src_repo);
continue;
}
@@ -314,7 +342,7 @@ static const struct ref *clone_local(const char *src_repo,
else {
strbuf_addf(&src, "%s/objects", src_repo);
strbuf_addf(&dest, "%s/objects", dest_repo);
- copy_or_link_directory(&src, &dest);
+ copy_or_link_directory(&src, &dest, src_repo, src.len);
strbuf_release(&src);
strbuf_release(&dest);
}
diff --git a/sha1_file.c b/sha1_file.c
index 064a330..f7c3408 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -380,7 +380,7 @@ void add_to_alternates_file(const char *reference)
{
struct lock_file *lock = xcalloc(1, sizeof(struct lock_file));
int fd = hold_lock_file_for_append(lock, git_path("objects/info/alternates"), LOCK_DIE_ON_ERROR);
- char *alt = mkpath("%s/objects\n", reference);
+ char *alt = mkpath("%s\n", reference);
write_or_die(fd, alt, strlen(alt));
if (commit_lock_file(lock))
die("could not close alternates file");
diff --git a/t/t5601-clone.sh b/t/t5601-clone.sh
index 0163ad1..d87214c 100755
--- a/t/t5601-clone.sh
+++ b/t/t5601-clone.sh
@@ -207,7 +207,7 @@ test_expect_success 'clone separate gitdir where target already exists' '
test_must_fail git clone --separate-git-dir realgitdir src dst
'
-test_expect_failure 'clone --reference from original' '
+test_expect_success 'clone --reference from original' '
git clone --shared --bare src src-1 &&
git clone --bare src src-2 &&
git clone --reference=src-2 --bare src-1 target-8 &&
@@ -222,4 +222,12 @@ test_expect_success 'clone with more than one --reference' '
grep /src-4/ target-9/.git/objects/info/alternates
'
+test_expect_success 'clone from original with relative alternate' '
+ mkdir nest &&
+ git clone --bare src nest/src-5 &&
+ echo ../../../src/.git/objects >nest/src-5/objects/info/alternates &&
+ git clone --bare nest/src-5 target-10 &&
+ grep /src/\\.git/objects target-10/objects/info/alternates
+'
+
test_done
--
1.7.6.557.gcee42
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 0/2] clone-local fixup
2011-08-23 1:05 ` [PATCH 0/2] clone-local fixup Junio C Hamano
2011-08-23 1:05 ` [PATCH 1/2] clone: allow more than one --reference Junio C Hamano
2011-08-23 1:05 ` [PATCH 2/2] clone: clone from a repository with relative alternates Junio C Hamano
@ 2011-08-23 3:43 ` Hui Wang
2011-08-23 16:26 ` Junio C Hamano
2 siblings, 1 reply; 10+ messages in thread
From: Hui Wang @ 2011-08-23 3:43 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Hui Wang
This patch series solved the problem from the root cause. :-)
But there is a little problem in the second patch, i provide an
incremental patch basing on your second patch, if it is fine to you, it
is OK to squash this patch to your second patch.
commit 47e890818fdac5f6493cd0bbaf9da350785a2bca
Author: Hui Wang <jason77.wang@gmail.com>
Date: Tue Aug 23 11:32:26 2011 +0800
clone: don't change absolute path in the copy_alternates
The source alternates may include empty lines, comment lines, relative
paths and absolute paths. Absolute paths don't need to be changed
before added to the destination alternates.
Signed-off-by: Hui Wang <jason77.wang@gmail.com>
diff --git a/builtin/clone.c b/builtin/clone.c
index 2842707..284e325 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -258,6 +258,10 @@ static void copy_alternates(struct strbuf *src,
struct strbuf *dst,
char *abs_path, abs_buf[PATH_MAX];
if (!line.len || line.buf[0] == '#')
continue;
+ if (is_absolute_path(line.buf)) {
+ add_to_alternates_file(line.buf);
+ continue;
+ }
abs_path = mkpath("%s/objects/%s", src_repo, line.buf);
normalize_path_copy(abs_buf, abs_path);
add_to_alternates_file(abs_buf);
Regards,
Hui.
Junio C Hamano wrote:
> So I ended up tackling this myself. I didn't bother moving the
> add_to_alternates_file(), though.
>
> Junio C Hamano (2):
> clone: allow more than one --reference
> clone: clone from a repository with relative alternates
>
> builtin/clone.c | 82 +++++++++++++++++++++++++++++++++++++++++------------
> sha1_file.c | 2 +-
> t/t5601-clone.sh | 23 +++++++++++++++
> 3 files changed, 87 insertions(+), 20 deletions(-)
>
>
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 0/2] clone-local fixup
2011-08-23 3:43 ` [PATCH 0/2] clone-local fixup Hui Wang
@ 2011-08-23 16:26 ` Junio C Hamano
0 siblings, 0 replies; 10+ messages in thread
From: Junio C Hamano @ 2011-08-23 16:26 UTC (permalink / raw)
To: Hui Wang; +Cc: git
Hui Wang <jason77.wang@gmail.com> writes:
> This patch series solved the problem from the root cause. :-)
>
> But there is a little problem in the second patch, i provide an
> incremental patch basing on your second patch, if it is fine to you,
> it is OK to squash this patch to your second patch.
Thanks. We also need the following fix-up, as I changed the semantics of
add_to_alternates_file() to take the path to the "objects" directory.
diff --git a/builtin/clone.c b/builtin/clone.c
index 284e325..63c34d0 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -336,14 +336,17 @@ static const struct ref *clone_local(const char *src_repo,
const char *dest_repo)
{
const struct ref *ret;
- struct strbuf src = STRBUF_INIT;
- struct strbuf dest = STRBUF_INIT;
struct remote *remote;
struct transport *transport;
- if (option_shared)
- add_to_alternates_file(src_repo);
- else {
+ if (option_shared) {
+ struct strbuf alt = STRBUF_INIT;
+ strbuf_addf(&alt, "%s/objects", src_repo);
+ add_to_alternates_file(alt.buf);
+ strbuf_release(&alt);
+ } else {
+ struct strbuf src = STRBUF_INIT;
+ struct strbuf dest = STRBUF_INIT;
strbuf_addf(&src, "%s/objects", src_repo);
strbuf_addf(&dest, "%s/objects", dest_repo);
copy_or_link_directory(&src, &dest, src_repo, src.len);
^ permalink raw reply related [flat|nested] 10+ messages in thread