{"thread":{"id":"12315","subject":"[RFC] Build in clone","startedAt":"2008-02-25T21:12:40Z","lastAt":"2008-03-05T23:56:27Z","messageCount":47,"participants":["Daniel Barkalow","Johan Herland","Johannes Schindelin","Kristian Høgsberg","Junio C Hamano","Santi Béjar","Pierre Habouzit"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"69926","messageId":"alpine.LNX.1.00.0802251604460.19024@iabervon.org","threadId":"12315","inReplyTo":null,"subject":"[RFC] Build in clone","fromName":"Daniel Barkalow","fromEmail":"barkalow@iabervon.org","sentAt":"2008-02-25T21:12:40Z","receivedAt":"2008-02-25T21:12:40Z","isPatch":false,"sender":{"key":"barkalow@iabervon.org","avatar":"https://avatars.githubusercontent.com/u/55364219?v=4"},"body":"This version is still a mess, but it passes all of the tests. I'm\nsomewhat unconvinced by the test ccoverage for clone, however; the\nlast failure I found was actually for which heads get created in a\nbare repository, and it was only failing when there was an extra one\nin a non-bare clone in a test for something entirely different.\n\nThis is largely based on Kristian Høgsberg's version from December, but \nthe introduced warnings and two whitespace errors I haven't located are \nmine.\n\nI'm still working on getting it cleaned up, but I thought it would be good \nto get it some exposure and testing, since people have been talking about \nbuiltin-clone today.\n\nSigned-off-by: Daniel Barkalow \n---\n Makefile | 2 +-\n builtin-clone.c | 598 +++++++++++++++++++++++++\n builtin-init-db.c | 163 ++++---\n builtin.h | 1 +\n cache.h | 5 +\n git-clone.sh => contrib/examples/git-clone.sh | 0 \n environment.c | 6 +\n git.c | 1 +\n 8 files changed, 704 insertions(+), 72 deletions(-)\n create mode 100644 builtin-clone.c\n rename git-clone.sh => contrib/examples/git-clone.sh (100%)\n\ndiff --git a/Makefile b/Makefile\nindex 149343c..c56d9da 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -227,7 +227,6 @@ BASIC_LDFLAGS =\n \n SCRIPT_SH = \\\n \tgit-bisect.sh \\\n-\tgit-clone.sh \\\n \tgit-merge-one-file.sh git-mergetool.sh git-parse-remote.sh \\\n \tgit-pull.sh git-rebase.sh git-rebase--interactive.sh \\\n \tgit-repack.sh git-request-pull.sh \\\n@@ -344,6 +343,7 @@ BUILTIN_OBJS = \\\n \tbuiltin-checkout-index.o \\\n \tbuiltin-check-ref-format.o \\\n \tbuiltin-clean.o \\\n+\tbuiltin-clone.o \\\n \tbuiltin-commit.o \\\n \tbuiltin-commit-tree.o \\\n \tbuiltin-count-objects.o \\\ndiff --git a/builtin-clone.c b/builtin-clone.c\nnew file mode 100644\nindex 0000000..da278f9\n--- /dev/null\n+++ b/builtin-clone.c\n@@ -0,0 +1,598 @@\n+/*\n+ * Builtin \"git clone\"\n+ *\n+ * Copyright (c) 2007 Kristian Høgsberg \n+ * Based on git-commit.sh by Junio C Hamano and Linus Torvalds\n+ *\n+ * Clone a repository into a different directory that does not yet exist.\n+ */\n+\n+#include \"cache.h\"\n+#include \"parse-options.h\"\n+#include \"fetch-pack.h\"\n+#include \"refs.h\"\n+#include \"tree.h\"\n+#include \"tree-walk.h\"\n+#include \"unpack-trees.h\"\n+#include \"transport.h\"\n+#include \"strbuf.h\"\n+#include \"dir.h\"\n+\n+/*\n+ * Overall FIXMEs:\n+ * - respect DB_ENVIRONMENT for .git/objects.\n+ * - error path cleanup of dirs+files.\n+ *\n+ * Implementation notes:\n+ * - dropping use-separate-remote and no-separate-remote compatibility\n+ *\n+ */\n+static const char * const builtin_clone_usage[] = {\n+\t\"git-clone [options] [--] []\",\n+\tNULL\n+};\n+\n+static int option_quiet, option_no_checkout, option_bare;\n+static int option_local, option_no_hardlinks, option_shared;\n+static char *option_template, *option_reference, *option_depth;\n+static char *option_origin = NULL;\n+static char *option_upload_pack = \"git-upload-pack\";\n+\n+static struct option builtin_clone_options[] = {\n+\tOPT__QUIET(&option_quiet),\n+\tOPT_BOOLEAN('n', \"no-checkout\", &option_no_checkout,\n+\t\t \"don't create a checkout\"),\n+\tOPT_BOOLEAN(0, \"bare\", &option_bare, \"create a bare repository\"),\n+\tOPT_BOOLEAN(0, \"naked\", &option_bare, \"create a bare repository\"),\n+\tOPT_BOOLEAN('l', \"local\", &option_local,\n+\t\t \"to clone from a local repository\"),\n+\tOPT_BOOLEAN(0, \"no-hardlinks\", &option_no_hardlinks,\n+\t\t \"don't use local hardlinks, always copy\"),\n+\tOPT_BOOLEAN('s', \"shared\", &option_shared,\n+\t\t \"setup as shared repository\"),\n+\tOPT_STRING(0, \"template\", &option_template, \"path\",\n+\t\t \"path the template repository\"),\n+\tOPT_STRING(0, \"reference\", &option_reference, \"repo\",\n+\t\t \"reference repository\"),\n+\tOPT_STRING('o', \"origin\", &option_origin, \"branch\",\n+\t\t \"use instead or 'origin' to track upstream\"),\n+\tOPT_STRING('u', \"upload-pack\", &option_upload_pack, \"path\",\n+\t\t \"path to git-upload-pack on the remote\"),\n+\tOPT_STRING(0, \"depth\", &option_depth, \"depth\",\n+\t\t \"create a shallow clone of that depth\"),\n+\n+\tOPT_END()\n+};\n+\n+static char *get_repo_path(const char *repo)\n+{\n+\tconst char *path;\n+\tstruct stat buf;\n+\n+\tpath = mkpath(\"%s/.git\", repo);\n+\tif (!stat(path, &buf) && S_ISDIR(buf.st_mode))\n+\t\treturn xstrdup(make_absolute_path(path));\n+\n+\tpath = mkpath(\"%s.git\", repo);\n+\tif (!stat(path, &buf) && S_ISDIR(buf.st_mode))\n+\t\treturn xstrdup(make_absolute_path(path));\n+\n+\tif (!stat(repo, &buf) && S_ISDIR(buf.st_mode))\n+\t\treturn xstrdup(make_absolute_path(repo));\n+\t\n+\treturn NULL;\n+}\n+\n+static char *guess_dir_name(const char *repo)\n+{\n+\tconst char *p, *start, *end, *limit;\n+\tint after_slash_or_colon;\n+\n+\t/* Guess dir name from repository: strip trailing '/',\n+\t * strip trailing '[:/]*git', strip leading '.*[/:]'. */\n+\n+\tafter_slash_or_colon = 1;\n+\tlimit = repo + strlen(repo);\n+\tstart = repo;\n+\tend = limit;\n+\tfor (p = repo; p < limit; p++) {\n+\t\tif (!prefixcmp(p, \".git\")) {\n+\t\t\tif (!after_slash_or_colon)\n+\t\t\t\tend = p;\n+\t\t\tp += 3;\n+\t\t} else if (*p == '/' || *p == ':') {\n+\t\t\tif (end == limit)\n+\t\t\t\tend = p;\n+\t\t\tafter_slash_or_colon = 1;\n+\t\t} else if (after_slash_or_colon) {\n+\t\t\tstart = p;\n+\t\t\tend = limit;\n+\t\t\tafter_slash_or_colon = 0;\n+\t\t}\n+\t}\n+\n+\treturn xstrndup(start, end - start);\n+}\n+\n+static void\n+write_alternates_file(const char *repo, const char *reference)\n+{\n+\tchar *file;\n+\tchar *alternates;\n+\tint fd;\n+\n+\tfile = mkpath(\"%s/objects/info/alternates\", repo);\n+\tfd = open(file, O_CREAT | O_WRONLY | O_APPEND, 0666);\n+\tif (fd < 0)\n+\t\tdie(\"failed to create %s\", file);\n+\talternates = mkpath(\"%s/objects\\n\", reference);\n+\twrite_or_die(fd, alternates, strlen(alternates));\n+\tif (close(fd))\n+\t\tdie(\"could not close %s\", file);\n+\tfprintf(stderr, \"Wrote %s to %s\\n\", alternates, file);\n+}\n+\n+static int\n+setup_tmp_ref(const char *refname,\n+\t const unsigned char *sha1, int flags, void *cb_data)\n+{\n+\tconst char *ref_temp = cb_data;\n+\tchar *path;\n+\tstruct lock_file lk;\n+\tstruct ref_lock *rl;\n+\n+\t/*\n+\n+\techo \"$ref_git/objects\" >\"$GIT_DIR/objects/info/alternates\"\n+\t(\n+\t\tGIT_DIR=\"$ref_git\" git for-each-ref \\\n+\t\t\t--format='%(objectname) %(*objectname)'\n+\t) |\n+\twhile read a b\n+\tdo\n+\t\ttest -z \"$a\" ||\n+\t\tgit update-ref \"refs/reference-tmp/$a\" \"$a\"\n+\t\ttest -z \"$b\" ||\n+\t\tgit update-ref \"refs/reference-tmp/$b\" \"$b\"\n+\tdone\n+\n+\t*/\n+\n+\t/* We go a bit out of way to use write_ref_sha1() here. We\n+\t * could just write the ref file directly, since neither\n+\t * locking or reflog really matters here. However, let's use\n+\t * the standard interface for writing refs as much as is\n+\t * possible given that get_git_dir() != the repo we're writing\n+\t * the refs in. */\n+\n+\tprintf(\"%s -> %s/%s\\n\",\n+\t sha1_to_hex(sha1), ref_temp, sha1_to_hex(sha1));\n+\n+\tpath = mkpath(\"%s/%s\", ref_temp, sha1_to_hex(sha1));\n+\trl = xmalloc(sizeof *rl);\n+\trl->force_write = 1;\n+\trl->lk = &lk;\n+\trl->ref_name = xstrdup(sha1_to_hex(sha1));\n+\trl->orig_ref_name = xstrdup(rl->ref_name);\n+\trl->lock_fd = hold_lock_file_for_update(rl->lk, path, 1);\n+\tif (write_ref_sha1(rl, sha1, NULL) < 0)\n+\t\tdie(\"failed to write temporary ref %s\", lk.filename);\n+\n+\treturn 0;\n+}\n+\n+static char *\n+setup_reference(const char *repo)\n+{\n+\tstruct stat buf;\n+\tconst char *ref_git;\n+\tchar *ref_temp;\n+\n+\tif (!option_reference)\n+\t\treturn NULL;\n+\n+\tref_git = make_absolute_path(option_reference);\n+\n+\tif (!stat(mkpath(\"%s/.git/objects\", ref_git), &buf) &&\n+\t S_ISDIR(buf.st_mode))\n+\t\tref_git = mkpath(\"%s/.git\", ref_git);\n+\telse if (stat(mkpath(\"%s/objects\", ref_git), &buf) ||\n+\t\t !S_ISDIR(buf.st_mode))\n+\t\tdie(\"reference repository '%s' is not a local directory.\",\n+\t\t option_reference);\n+\n+\tset_git_dir(ref_git);\n+\n+\twrite_alternates_file(repo, ref_git);\n+\n+\tref_temp = xstrdup(mkpath(\"%s/refs/reference-tmp\", repo));\n+\tif (mkdir(ref_temp, 0777))\n+\t\tdie(\"could not create directory %s\", ref_temp);\n+\tfor_each_ref(setup_tmp_ref, (void *) ref_temp);\n+\n+\treturn ref_temp;\n+}\n+\n+static void\n+cleanup_reference(char *ref_temp)\n+{\n+\tstruct dirent *de;\n+\tDIR *dir;\n+\n+\tif (!ref_temp)\n+\t\treturn;\n+\tdir = opendir(ref_temp);\n+\tif (!dir) {\n+\t\tif (errno == ENOENT)\n+\t\t\treturn;\n+\t\tdie(\"failed to open directory %s\", ref_temp);\n+\t}\n+\n+\twhile ((de = readdir(dir)) != NULL) {\n+\t\tif (de->d_name[0] == '.')\n+\t\t\tcontinue;\n+\t\tunlink(mkpath(\"%s/%s\", ref_temp, de->d_name));\n+\t}\n+\n+\tunlink(ref_temp);\n+\tfree(ref_temp);\n+}\n+\n+static void\n+walk_objects(char *src, char *dest)\n+{\n+\tstruct dirent *de;\n+\tstruct stat buf;\n+\tint src_len, dest_len;\n+\tDIR *dir;\n+\n+\tdir = opendir(src);\n+\tif (!dir)\n+\t\tdie(\"failed to open %s\\n\", src);\n+\n+\tif (mkdir(dest, 0777)) {\n+\t\tif (errno != EEXIST)\n+\t\t\tdie(\"failed to create directory %s\\n\", dest);\n+\t\telse if (stat(dest, &buf))\n+\t\t\tdie(\"failed to stat %s\\n\", dest);\n+\t\telse if (!S_ISDIR(buf.st_mode))\n+\t\t\tdie(\"%s exists and is not a directory\\n\", dest);\n+\t}\n+\n+\tsrc_len = strlen(src);\n+\tsrc[src_len] = '/';\n+\tdest_len = strlen(dest);\n+\tdest[dest_len] = '/';\n+\n+\twhile ((de = readdir(dir)) != NULL) {\n+\t\tstrcpy(src + src_len + 1, de->d_name);\n+\t\tstrcpy(dest + dest_len + 1, de->d_name);\n+\t\tif (stat(src, &buf)) {\n+\t\t\tfprintf(stderr, \"failed to stat %s, ignoring\\n\", src);\n+\t\t\tcontinue;\n+\t\t}\n+\t\tif (S_ISDIR(buf.st_mode)) {\n+\t\t\tif (de->d_name[0] != '.')\n+\t\t\t\twalk_objects(src, dest);\n+\t\t\tcontinue;\n+\t\t}\n+\n+\t\tif (unlink(dest) && errno != ENOENT)\n+\t\t\tdie(\"failed to unlink %s\\n\", dest);\n+\t\tif (option_no_hardlinks) {\n+\t\t\tif (copy_file(dest, src, 0666))\n+\t\t\t\tdie(\"failed to copy file to %s\\n\", dest);\n+\t\t} else {\n+\t\t\tif (link(src, dest))\n+\t\t\t\tdie(\"failed to create link %s\\n\", dest);\n+\t\t}\n+\t}\n+}\n+\n+static const struct ref *\n+clone_local(const char *src_repo, const char *dest_repo)\n+{\n+\tconst struct ref *ret;\n+\tchar src[PATH_MAX];\n+\tchar dest[PATH_MAX];\n+\tstruct remote *remote;\n+\tstruct transport *transport;\n+\n+\tif (option_shared) {\n+\t\twrite_alternates_file(dest_repo, src_repo);\n+\t} else {\n+\t\tsnprintf(src, PATH_MAX, \"%s/objects\", src_repo);\n+\t\tsnprintf(dest, PATH_MAX, \"%s/objects\", dest_repo);\n+\t\twalk_objects(src, dest);\n+\t}\n+\n+\tfprintf(stderr, \"Get for %s\\n\", src_repo);\n+\tremote = remote_get(src_repo);\n+\ttransport = transport_get(remote, src_repo);\n+\tret = transport_get_remote_refs(transport);\n+\ttransport_disconnect(transport);\n+\treturn ret;\n+}\n+\n+static const char *junk_work_tree;\n+static const char *junk_git_dir;\n+pid_t clone_pid;\n+\n+static void remove_junk(void)\n+{\n+\tstruct strbuf sb;\n+\tif (getpid() != clone_pid)\n+\t\treturn;\n+\tstrbuf_init(&sb, 0);\n+\tif (junk_git_dir) {\n+\t\tfprintf(stderr, \"Remove junk %s\\n\", junk_git_dir);\n+\t\tstrbuf_addstr(&sb, junk_git_dir);\n+\t\tremove_dir_recursively(&sb, 0);\n+\t\tstrbuf_reset(&sb);\n+\t}\n+\tif (junk_work_tree) {\n+\t\tfprintf(stderr, \"Remove junk %s\\n\", junk_work_tree);\n+\t\tstrbuf_addstr(&sb, junk_work_tree);\n+\t\tremove_dir_recursively(&sb, 0);\n+\t\tstrbuf_reset(&sb);\n+\t}\n+}\n+\n+int cmd_clone(int argc, const char **argv, const char *prefix)\n+{\n+\tint use_local_hardlinks = 1;\n+\tint use_separate_remote = 1;\n+\tstruct stat buf;\n+\tconst char *repo, *work_tree, *git_dir;\n+\tchar *path, *dir, *head, *ref_temp;\n+\tstruct ref *refs, *r, *remote_head, *head_points_at, *remote_master;\n+\tchar branch_top[256], key[256], refname[256], value[256];\n+\n+\tclone_pid = getpid();\n+\n+\targc = parse_options(argc, argv, builtin_clone_options,\n+\t\t\t builtin_clone_usage, 0);\n+\n+\tif (argc == 0)\n+\t\tdie(\"You must specify a repository to clone.\");\n+\n+\tif (option_no_hardlinks)\n+\t\tuse_local_hardlinks = 0;\n+\n+\tif (option_bare) {\n+\t\tif (option_origin)\n+\t\t\tdie(\"--bare and --origin %s options are incompatible.\",\n+\t\t\t option_origin);\n+\t\toption_no_checkout = 1;\n+\t\tuse_separate_remote = 0;\n+\t}\n+\n+\tif (!option_origin)\n+\t\toption_origin = \"origin\";\n+\n+\trepo = argv[0];\n+\tpath = get_repo_path(repo);\n+\n+\tif (argc == 2) {\n+\t\tdir = xstrdup(argv[1]);\n+\t} else {\n+\t\tdir = guess_dir_name(repo);\n+\t}\n+\n+\tif (!stat(dir, &buf))\n+\t\tdie(\"destination directory '%s' already exists.\", dir);\n+\n+\tif (option_bare)\n+\t\twork_tree = NULL;\n+\telse {\n+\t\twork_tree = getenv(\"GIT_WORK_TREE\");\n+\t\tif (work_tree && !stat(work_tree, &buf))\n+\t\t\tdie(\"working tree '%s' already exists.\", work_tree);\n+\t}\n+\n+\tatexit(remove_junk);\n+\n+\tif (option_bare || work_tree)\n+\t\tgit_dir = xstrdup(dir);\n+\telse {\n+\t\twork_tree = dir;\n+\t\tgit_dir = xstrdup(mkpath(\"%s/.git\", dir));\n+\t}\n+\n+\tif (!option_bare) {\n+\t\tif (mkdir(work_tree, 0755))\n+\t\t\tdie(\"could not create work tree dir '%s'.\", work_tree);\n+\t\tset_git_work_tree(work_tree);\n+\t\tjunk_work_tree = work_tree;\n+\t}\n+\n+\tsetenv(CONFIG_ENVIRONMENT, xstrdup(mkpath(\"%s/config\", git_dir)), 1);\n+\n+\t//set_git_dir(make_absolute_path(git_dir));\n+\n+\tfprintf(stderr, \"Initialize %s\\n\", git_dir);\n+\tinit_db(git_dir, option_template, work_tree,\n+\t\toption_quiet ? INIT_DB_QUIET : 0);\n+\tjunk_git_dir = git_dir;\n+\tfprintf(stderr, \"Okay\\n\");\n+\n+\t/* This calls set_git_dir for the reference repo so we can get\n+\t * the refs there. Thus, call this before calling\n+\t * set_git_dir() on the repo we're setting up. */\n+\tref_temp = setup_reference(git_dir);\n+\n+\tset_git_dir(make_absolute_path(git_dir));\n+\n+\tif (option_bare)\n+\t\tgit_config_set(\"core.bare\", \"true\");\n+\n+\tif (path != NULL) {\n+\t\trefs = clone_local(path, git_dir);\n+\t\trepo = make_absolute_path(path);\n+\t} else {\n+\t\tstruct remote *remote = remote_get(argv[0]);\n+\t\tstruct transport *transport = transport_get(remote, argv[0]);\n+\t\tconst struct ref *show;\n+\n+\t\ttransport_set_option(transport, TRANS_OPT_KEEP, \"yes\");\n+\n+\t\tif (option_depth)\n+\t\t\ttransport_set_option(transport, TRANS_OPT_DEPTH,\n+\t\t\t\t\t option_depth);\n+\n+\t\tif (option_quiet)\n+\t\t\ttransport->verbose = -1;\n+\n+\t\t//args.no_progress = 1;\n+\n+\t\tfprintf(stderr, \"Get refs for %s\\n\", argv[0]);\n+\t\trefs = transport_get_remote_refs(transport);\n+\n+\t\ttransport_fetch_refs(transport, refs);\n+\t}\n+\n+\tcleanup_reference(ref_temp);\n+\n+\tif (option_bare)\n+\t\tstrcpy(branch_top, \"refs/heads\");\n+\telse\n+\t\tsnprintf(branch_top, sizeof branch_top,\n+\t\t\t \"refs/remotes/%s\", option_origin);\n+\n+\tprintf(\"%p\\n\", refs);\n+\tremote_head = NULL;\n+\tremote_master = NULL;\n+\tfor (r = refs; r; r = r->next) {\n+\t\tfprintf(stderr, \"%s\\n\",\tr->name);\n+\t\tif (strlen(r->name) >= 3 &&\n+\t\t !strcmp(r->name + strlen(r->name) - 3, \"^{}\"))\n+\t\t\tcontinue;\n+\t\tif (!strcmp(r->name, \"HEAD\")) {\n+\t\t\tremote_head = r;\n+\t\t\tif (option_bare)\n+\t\t\t\tcontinue;\n+\t\t\tsnprintf(refname, sizeof refname,\n+\t\t\t\t \"%s/HEAD\", branch_top);\n+\t\t} else {\n+\t\t\tif (!strcmp(r->name, \"refs/heads/master\"))\n+\t\t\t\tremote_master = r;\n+\n+\t\t\tif (!prefixcmp(r->name, \"refs/heads/\"))\n+\t\t\t\tsnprintf(refname, sizeof refname,\n+\t\t\t\t\t \"%s/%s\", branch_top, r->name + 11);\n+\t\t\telse if (!prefixcmp(r->name, \"refs/tags/\"))\n+\t\t\t\tsnprintf(refname, sizeof refname,\n+\t\t\t\t\t \"refs/tags/%s\", r->name + 10);\n+\t\t\telse\n+\t\t\t\tcontinue;\n+\t\t}\n+\n+\t\tupdate_ref(\"clone from $repo\",\n+\t\t\t refname, r->old_sha1, NULL, 0, DIE_ON_ERR);\n+\t}\n+\n+\thead_points_at = NULL;\n+\tif (!remote_head) {\n+\t\t/* If there isn't one, oh well. */\n+\t} else if (remote_master && !hashcmp(remote_master->old_sha1,\n+\t\t\t\t remote_head->old_sha1)) {\n+\t\t/* If refs/heads/master could be right, it is. */\n+\t\thead_points_at = remote_master;\n+\t} else\n+\t\tfor (r = refs; r; r = r->next) {\n+\t\t\tif (r != remote_head &&\n+\t\t\t !hashcmp(r->old_sha1, remote_head->old_sha1)) {\n+\t\t\t\thead_points_at = r;\n+\t\t\t\tprintf(\"head points at %s\\n\", r->name);\n+\t\t\t\tbreak;\n+\t\t\t}\n+\t\t}\n+\n+\t/* FIXME: What about the \"Uh-oh, the remote told us...\" case? */\n+\tif (!option_bare) {\n+\t\tsnprintf(key, sizeof key, \"remote.%s.url\", option_origin);\n+\t\tgit_config_set(key, repo);\n+\t\tsnprintf(key, sizeof key, \"remote.%s.fetch\", option_origin);\n+\t\tsnprintf(value, sizeof value, \"+refs/heads/*:%s/*\", branch_top);\n+\n+\t\tgit_config_set_multivar(key, value, \"^$\", 0);\n+\t}\n+\n+\tif (option_bare) {\n+\t\tif (head_points_at) {\n+\t\t\t/* Local default branch */\n+\t\t\tcreate_symref(\"HEAD\", head_points_at->name, NULL);\n+\t\t}\n+\t\tjunk_work_tree = NULL;\n+\t\tjunk_git_dir = NULL;\n+\t\treturn 0;\n+\t}\n+\n+\tif (head_points_at) {\n+\t\tif (strrchr(head_points_at->name, '/'))\n+\t\t\thead = strrchr(head_points_at->name, '/') + 1;\n+\t\telse\n+\t\t\thead = head_points_at->name;\n+\n+\t\t/* Local default branch */\n+\t\tcreate_symref(\"HEAD\", head_points_at->name, NULL);\n+\n+\t\t/* Tracking branch for the primary branch at the remote. */\n+\t\tupdate_ref(NULL, \"HEAD\", head_points_at->old_sha1,\n+\t\t\t NULL, 0, DIE_ON_ERR);\n+\t/*\n+\t\trm -f \"refs/remotes/$origin/HEAD\"\n+\t\tgit symbolic-ref \"refs/remotes/$origin/HEAD\" \\\n+\t\t\t\"refs/remotes/$origin/$head_points_at\" &&\n+\t*/\n+\n+\t\tsnprintf(key, sizeof key, \"branch.%s.remote\", head);\n+\t\tgit_config_set(key, option_origin);\n+\t\tsnprintf(key, sizeof key, \"branch.%s.merge\", head);\n+\t\tgit_config_set(key, head_points_at->name);\n+\t} else if (remote_head) {\n+\t\t/* Source had detached HEAD pointing somewhere. */\n+\t\tupdate_ref(\"clone from $repo\", \"HEAD\", remote_head->old_sha1,\n+\t\t\t NULL, REF_NODEREF, DIE_ON_ERR);\n+\t} else {\n+\t\t/* Nothing to checkout out */\n+\t\tif (!option_no_checkout)\n+\t\t\tfprintf(stderr, \"Warning: Remote HEAD refers to nonexistent ref, unable to checkout.\\n\");\n+\t\toption_no_checkout = 1;\n+\t}\n+\n+\tif (!option_no_checkout) {\n+\t\tstruct lock_file lock_file;\n+\t\tstruct unpack_trees_options opts;\n+\t\tstruct tree *tree;\n+\t\tstruct tree_desc t[2];\n+\t\tint fd;\n+\n+\t\t/* We need to be in the new work tree for the checkout */\n+\t\tsetup_work_tree();\n+\n+\t\tfprintf(stderr, \"work tree now %s\\n\", get_git_work_tree());\n+\n+\t\tfd = hold_locked_index(&lock_file, 1);\n+\n+\t\tmemset(&opts, 0, sizeof opts);\n+\t\topts.update = 1;\n+\t\topts.verbose_update = !option_quiet;\n+\t\topts.merge = 1;\n+\t\topts.fn = twoway_merge;\n+\n+\t\ttree = parse_tree_indirect(remote_head->old_sha1);\n+\t\tparse_tree(tree);\n+\t\tinit_tree_desc(&t[0], tree->buffer, tree->size);\n+\t\tinit_tree_desc(&t[1], tree->buffer, tree->size);\n+\t\tunpack_trees(2, t, &opts);\n+\n+\t\tif (write_cache(fd, active_cache, active_nr) ||\n+\t\t commit_locked_index(&lock_file))\n+\t\t\tdie(\"unable to write new index file\");\n+\t}\n+\t\n+\tjunk_work_tree = NULL;\n+\tjunk_git_dir = NULL;\n+\treturn 0;\n+}\ndiff --git a/builtin-init-db.c b/builtin-init-db.c\nindex 79eaf8d..bc74188 100644\n--- a/builtin-init-db.c\n+++ b/builtin-init-db.c\n@@ -21,6 +21,7 @@ static void safe_create_dir(const char *dir, int share)\n {\n \tif (mkdir(dir, 0777) < 0) {\n \t\tif (errno != EEXIST) {\n+\t\t\tfprintf(stderr, \"When creating GIT_DIR\\n\");\n \t\t\tperror(dir);\n \t\t\texit(1);\n \t\t}\n@@ -176,6 +177,7 @@ static int create_default_files(const char *git_dir, const char *template_path)\n \tif (len > sizeof(path)-50)\n \t\tdie(\"insane git directory %s\", git_dir);\n \tmemcpy(path, git_dir, len);\n+\tgit_dir = make_absolute_path(git_dir);\n \n \tif (len && path[len-1] != '/')\n \t\tpath[len++] = '/';\n@@ -250,8 +252,12 @@ static int create_default_files(const char *git_dir, const char *template_path)\n \t\t/* allow template config file to override the default */\n \t\tif (log_all_ref_updates == -1)\n \t\t git_config_set(\"core.logallrefupdates\", \"true\");\n-\t\tif (work_tree != git_work_tree_cfg)\n+\t\tif (prefixcmp(git_dir, work_tree) ||\n+\t\t strcmp(git_dir + strlen(work_tree), \"/.git\")) {\n+\t\t\tfprintf(stderr, \"Is %s; would be %s\\n\", work_tree,\n+\t\t\t\tgit_dir);\n \t\t\tgit_config_set(\"core.worktree\", work_tree);\n+\t\t}\n \t}\n \n \t/* Check if symlink is supported in the work tree */\n@@ -271,42 +277,93 @@ static int create_default_files(const char *git_dir, const char *template_path)\n \treturn reinit;\n }\n \n-static void guess_repository_type(const char *git_dir)\n+static int guess_repository_type(const char *git_dir)\n {\n \tchar cwd[PATH_MAX];\n \tconst char *slash;\n \n-\tif (0 <= is_bare_repository_cfg)\n-\t\treturn;\n-\tif (!git_dir)\n-\t\treturn;\n-\n \t/*\n \t * \"GIT_DIR=. git init\" is always bare.\n \t * \"GIT_DIR=`pwd` git init\" too.\n \t */\n \tif (!strcmp(\".\", git_dir))\n-\t\tgoto force_bare;\n+\t\treturn 1;\n \tif (!getcwd(cwd, sizeof(cwd)))\n \t\tdie(\"cannot tell cwd\");\n \tif (!strcmp(git_dir, cwd))\n-\t\tgoto force_bare;\n+\t\treturn 1;\n \t/*\n \t * \"GIT_DIR=.git or GIT_DIR=something/.git is usually not.\n \t */\n \tif (!strcmp(git_dir, \".git\"))\n-\t\treturn;\n+\t\treturn 0;\n \tslash = strrchr(git_dir, '/');\n \tif (slash && !strcmp(slash, \"/.git\"))\n-\t\treturn;\n+\t\treturn 0;\n \n \t/*\n \t * Otherwise it is often bare. At this point\n \t * we are just guessing.\n \t */\n- force_bare:\n-\tis_bare_repository_cfg = 1;\n-\treturn;\n+\treturn 1;\n+}\n+\n+int init_db(const char *git_dir, const char *template_dir, const char *work_dir,\n+\t unsigned int flags)\n+{\n+\tconst char *sha1_dir;\n+\tchar *path;\n+\tint len, reinit;\n+\n+\tsafe_create_dir(git_dir, 0);\n+\n+\tset_git_dir(make_absolute_path(git_dir));\n+\n+\t/* Check to see if the repository version is right.\n+\t * Note that a newly created repository does not have\n+\t * config file, so this will not fail. What we are catching\n+\t * is an attempt to reinitialize new repository with an old tool.\n+\t */\n+\tcheck_repository_format();\n+\n+\treinit = create_default_files(git_dir, template_dir);\n+\n+\t/*\n+\t * And set up the object store. Don't use\n+\t * get_object_directory() here, since we're initializing\n+\t * relative to git_dir, not $GIT_DIR.\n+\t */\n+\tsha1_dir = getenv(DB_ENVIRONMENT);\n+\tif (!sha1_dir)\n+\t\tsha1_dir = mkpath(\"%s/objects\", git_dir);\n+\tlen = strlen(sha1_dir);\n+\tpath = xmalloc(len + 40);\n+\tmemcpy(path, sha1_dir, len);\n+\n+\tsafe_create_dir(sha1_dir, 1);\n+\tstrcpy(path+len, \"/pack\");\n+\tsafe_create_dir(path, 1);\n+\tstrcpy(path+len, \"/info\");\n+\tsafe_create_dir(path, 1);\n+\n+\tif (shared_repository) {\n+\t\tchar buf[10];\n+\t\t/* We do not spell \"group\" and such, so that\n+\t\t * the configuration can be read by older version\n+\t\t * of git.\n+\t\t */\n+\t\tsprintf(buf, \"%d\", shared_repository);\n+\t\tgit_config_set(\"core.sharedrepository\", buf);\n+\t\tgit_config_set(\"receive.denyNonFastforwards\", \"true\");\n+\t}\n+\n+\tif (!(flags & INIT_DB_QUIET))\n+\t\tprintf(\"%s%s Git repository in %s/\\n\",\n+\t\t reinit ? \"Reinitialized existing\" : \"Initialized empty\",\n+\t\t shared_repository ? \" shared\" : \"\",\n+\t\t git_dir);\n+\n+\treturn 0;\n }\n \n static const char init_db_usage[] =\n@@ -320,12 +377,10 @@ static const char init_db_usage[] =\n */\n int cmd_init_db(int argc, const char **argv, const char *prefix)\n {\n-\tconst char *git_dir;\n-\tconst char *sha1_dir;\n+\tconst char *git_dir, *work_tree;\n \tconst char *template_dir = NULL;\n-\tchar *path;\n-\tint len, i, reinit;\n-\tint quiet = 0;\n+\tunsigned int flags = 0;\n+\tint i;\n \n \tfor (i = 1; i < argc; i++, argv++) {\n \t\tconst char *arg = argv[1];\n@@ -336,7 +391,7 @@ int cmd_init_db(int argc, const char **argv, const char *prefix)\n \t\telse if (!prefixcmp(arg, \"--shared=\"))\n \t\t\tshared_repository = git_config_perm(\"arg\", arg+9);\n \t\telse if (!strcmp(arg, \"-q\") || !strcmp(arg, \"--quiet\"))\n-\t\t quiet = 1;\n+\t\t\tflags |= INIT_DB_QUIET;\n \t\telse\n \t\t\tusage(init_db_usage);\n \t}\n@@ -353,64 +408,30 @@ int cmd_init_db(int argc, const char **argv, const char *prefix)\n \t\t GIT_WORK_TREE_ENVIRONMENT,\n \t\t GIT_DIR_ENVIRONMENT);\n \n-\tguess_repository_type(git_dir);\n-\n-\tif (is_bare_repository_cfg <= 0) {\n-\t\tgit_work_tree_cfg = xcalloc(PATH_MAX, 1);\n-\t\tif (!getcwd(git_work_tree_cfg, PATH_MAX))\n-\t\t\tdie (\"Cannot access current working directory.\");\n-\t\tif (access(get_git_work_tree(), X_OK))\n-\t\t\tdie (\"Cannot access work tree '%s'\",\n-\t\t\t get_git_work_tree());\n-\t}\n-\n \t/*\n \t * Set up the default .git directory contents\n \t */\n-\tgit_dir = getenv(GIT_DIR_ENVIRONMENT);\n \tif (!git_dir)\n \t\tgit_dir = DEFAULT_GIT_DIR_ENVIRONMENT;\n-\tsafe_create_dir(git_dir, 0);\n-\n-\t/* Check to see if the repository version is right.\n-\t * Note that a newly created repository does not have\n-\t * config file, so this will not fail. What we are catching\n-\t * is an attempt to reinitialize new repository with an old tool.\n-\t */\n-\tcheck_repository_format();\n-\n-\treinit = create_default_files(git_dir, template_dir);\n-\n-\t/*\n-\t * And set up the object store.\n-\t */\n-\tsha1_dir = get_object_directory();\n-\tlen = strlen(sha1_dir);\n-\tpath = xmalloc(len + 40);\n-\tmemcpy(path, sha1_dir, len);\n \n-\tsafe_create_dir(sha1_dir, 1);\n-\tstrcpy(path+len, \"/pack\");\n-\tsafe_create_dir(path, 1);\n-\tstrcpy(path+len, \"/info\");\n-\tsafe_create_dir(path, 1);\n+\tif (is_bare_repository_cfg < 0)\n+\t\tis_bare_repository_cfg = guess_repository_type(git_dir);\n \n-\tif (shared_repository) {\n-\t\tchar buf[10];\n-\t\t/* We do not spell \"group\" and such, so that\n-\t\t * the configuration can be read by older version\n-\t\t * of git.\n-\t\t */\n-\t\tsprintf(buf, \"%d\", shared_repository);\n-\t\tgit_config_set(\"core.sharedrepository\", buf);\n-\t\tgit_config_set(\"receive.denyNonFastforwards\", \"true\");\n+\tif (!is_bare_repository_cfg) {\n+\t\tif (git_dir) {\n+\t\t\tconst char *git_dir_parent = strrchr(git_dir, '/');\n+\t\t\tif (git_dir_parent)\n+\t\t\t\tgit_work_tree_cfg = strdup(make_absolute_path(xstrndup(git_dir, git_dir_parent - git_dir)));\n+\t\t}\n+\t\tif (!git_work_tree_cfg) {\n+\t\t\tgit_work_tree_cfg = xcalloc(PATH_MAX, 1);\n+\t\t\tif (!getcwd(git_work_tree_cfg, PATH_MAX))\n+\t\t\t\tdie (\"Cannot access current working directory.\");\n+\t\t}\n+\t\tif (access(get_git_work_tree(), X_OK))\n+\t\t\tdie (\"Cannot access work tree '%s'\",\n+\t\t\t get_git_work_tree());\n \t}\n \n-\tif (!quiet)\n-\t\tprintf(\"%s%s Git repository in %s/\\n\",\n-\t\t reinit ? \"Reinitialized existing\" : \"Initialized empty\",\n-\t\t shared_repository ? \" shared\" : \"\",\n-\t\t git_dir);\n-\n-\treturn 0;\n+\treturn init_db(git_dir, template_dir, git_work_tree_cfg, flags);\n }\ndiff --git a/builtin.h b/builtin.h\nindex 674c8a1..4a382b5 100644\n--- a/builtin.h\n+++ b/builtin.h\n@@ -24,6 +24,7 @@ extern int cmd_check_attr(int argc, const char **argv, const char *prefix);\n extern int cmd_check_ref_format(int argc, const char **argv, const char *prefix);\n extern int cmd_cherry(int argc, const char **argv, const char *prefix);\n extern int cmd_cherry_pick(int argc, const char **argv, const char *prefix);\n+extern int cmd_clone(int argc, const char **argv, const char *prefix);\n extern int cmd_clean(int argc, const char **argv, const char *prefix);\n extern int cmd_commit(int argc, const char **argv, const char *prefix);\n extern int cmd_commit_tree(int argc, const char **argv, const char *prefix);\ndiff --git a/cache.h b/cache.h\nindex 660ea04..6d806c4 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -322,6 +322,11 @@ extern const char *prefix_filename(const char *prefix, int len, const char *path\n extern void verify_filename(const char *prefix, const char *name);\n extern void verify_non_filename(const char *prefix, const char *name);\n \n+#define INIT_DB_QUIET 0x0001\n+\n+extern int init_db(const char *git_dir, const char *template_dir,\n+\t\t const char *work_dir, unsigned int flags);\n+\n #define alloc_nr(x) (((x)+16)*3/2)\n \n /*\ndiff --git a/git-clone.sh b/contrib/examples/git-clone.sh\nsimilarity index 100%\nrename from git-clone.sh\nrename to contrib/examples/git-clone.sh\ndiff --git a/environment.c b/environment.c\nindex 6739a3f..d6c6a6b 100644\n--- a/environment.c\n+++ b/environment.c\n@@ -81,6 +81,12 @@ const char *get_git_dir(void)\n \treturn git_dir;\n }\n \n+void set_git_work_tree(const char *new_work_tree)\n+{\n+\tget_git_work_tree(); /* make sure it's initialized */\n+\twork_tree = xstrdup(make_absolute_path(new_work_tree));\n+}\n+\n const char *get_git_work_tree(void)\n {\n \tstatic int initialized = 0;\ndiff --git a/git.c b/git.c\nindex 9cca81a..f32883a 100644\n--- a/git.c\n+++ b/git.c\n@@ -285,6 +285,7 @@ static void handle_internal_command(int argc, const char **argv)\n \t\t{ \"check-attr\", cmd_check_attr, RUN_SETUP | NEED_WORK_TREE },\n \t\t{ \"cherry\", cmd_cherry, RUN_SETUP },\n \t\t{ \"cherry-pick\", cmd_cherry_pick, RUN_SETUP | NEED_WORK_TREE },\n+\t\t{ \"clone\", cmd_clone },\n \t\t{ \"clean\", cmd_clean, RUN_SETUP | NEED_WORK_TREE },\n \t\t{ \"commit\", cmd_commit, RUN_SETUP | NEED_WORK_TREE },\n \t\t{ \"commit-tree\", cmd_commit_tree, RUN_SETUP },\n-- \n1.5.4.2.261.g851a5.dirty"},{"id":"69957","messageId":"200802260321.14038.johan@herland.net","threadId":"12315","inReplyTo":"alpine.LNX.1.00.0802251604460.19024@iabervon.org","subject":"Re: [RFC] Build in clone","fromName":"Johan Herland","fromEmail":"johan@herland.net","sentAt":"2008-02-26T02:21:13Z","receivedAt":"2008-02-26T02:21:13Z","isPatch":false,"sender":{"key":"johan@herland.net","avatar":"https://avatars.githubusercontent.com/u/547031?v=4"},"body":"On Monday 25 February 2008, Daniel Barkalow wrote:\n> This version is still a mess, but it passes all of the tests.\n\nNot for me:\n*** t5700-clone-reference.sh ***\n* ok 1: preparing first repository\n* ok 2: preparing second repository\n* FAIL 3: cloning with reference (-l -s)\n git clone -l -s --reference B A C\n* ok 4: existence of info/alternates\n* ok 5: pulling from reference\n* ok 6: that reference gets used\n* FAIL 7: cloning with reference (no -l -s)\n git clone --reference B file://`pwd`/A D\n* ok 8: existence of info/alternates\n* ok 9: pulling from reference\n* ok 10: that reference gets used\n* ok 11: updating origin\n* ok 12: pulling changes from origin\n* ok 13: that alternate to origin gets used\n* ok 14: pulling changes from origin\n* ok 15: check objects expected to exist locally\n* failed 2 among 15 test(s)\nmake[1]: *** [t5700-clone-reference.sh] Error 1\n\n> I'm somewhat unconvinced by the test ccoverage for clone, however; the\n> last failure I found was actually for which heads get created in a\n> bare repository, and it was only failing when there was an extra one\n> in a non-bare clone in a test for something entirely different.\n> \n> This is largely based on Kristian Høgsberg's version from December, but \n> the introduced warnings and two whitespace errors I haven't located are \n> mine.\n> \n> I'm still working on getting it cleaned up, but I thought it would be good \n> to get it some exposure and testing, since people have been talking about \n> builtin-clone today.\n\nOther than the failing tests, it seems to work fairly well. I've been\nplaying around with it for a few minutes, and on a test repo I have with\n1001 branches and 10000 tags, it cuts down the runtime of a local git-clone\nfrom 25 seconds to ~1.5 seconds. (simply by eliminating the overhead of\ninvoking git-update-ref for every single ref) :)\n\nI've tried to test this by diffing a cloned repo against an equivalent\nclone done by the old script. Below I pasted in a few immediate fixes I\nfound. With these fixes, the only remaining diff between the clones is\nthat refs/remotes/origin/HEAD used to be a symbolic ref (with no reflog),\nbut is now a \"regular\" ref (with reflog).\nThe fixes are, in order of importance:\n- Call git_config(git_default_config) in order to properly set up\n user.name and user.email for reflogs (This BREAKS test #9 in\n t1020-subdirectory.sh. Have yet to figure out why)\n- Fix \"clone from $repo\" reflog messages (using strbufs; something tells\n me more of this code would benefit from using strbufs)\n- Høgsberg's name should be in UTF-8 (not sure if this will survive this\n mail)\n- The two whitespace errors you mentioned\n\nI'm sorry that my patch below sucks from a style POV. Feel free to ignore.\nWill redo when it's not in the middle of the night.\n\n\nHave fun! :)\n\n...Johan\n\n-8<----------------8<---------------------8<-\n[PATCH] WIP: Minor fixes on top of builtin-clone\n\nSigned-off-by: Johan Herland \n---\n builtin-clone.c | 19 +++++++++++++------\n 1 files changed, 13 insertions(+), 6 deletions(-)\n\ndiff --git a/builtin-clone.c b/builtin-clone.c\nindex 5aa75e1..7eed340 100644\n--- a/builtin-clone.c\n+++ b/builtin-clone.c\n@@ -1,7 +1,7 @@\n /*\n * Builtin \"git clone\"\n *\n- * Copyright (c) 2007 Kristian Høgsberg \n+ * Copyright (c) 2007 Kristian Høgsberg \n * Based on git-commit.sh by Junio C Hamano and Linus Torvalds\n *\n * Clone a repository into a different directory that does not yet exist.\n@@ -79,7 +79,7 @@ static char *get_repo_path(const char *repo)\n \n \tif (!stat(repo, &buf) && S_ISDIR(buf.st_mode))\n \t\treturn xstrdup(make_absolute_path(repo));\n-\t\n+\n \treturn NULL;\n }\n \n@@ -347,6 +347,9 @@ int cmd_clone(int argc, const char **argv, const char *prefix)\n \tchar *path, *dir, *head, *ref_temp;\n \tstruct ref *refs, *r, *remote_head, *head_points_at, *remote_master;\n \tchar branch_top[256], key[256], refname[256], value[256];\n+\tstruct strbuf reflog_msg;\n+\n+\tgit_config(git_default_config);\n \n \tclone_pid = getpid();\n \n@@ -459,6 +462,9 @@ int cmd_clone(int argc, const char **argv, const char *prefix)\n \t\tsnprintf(branch_top, sizeof branch_top,\n \t\t\t \"refs/remotes/%s\", option_origin);\n \n+\tstrbuf_init(&reflog_msg, strlen(repo) + 12);\n+\tstrbuf_addf(&reflog_msg, \"clone: from %s\", repo);\n+\n \tprintf(\"%p\\n\", refs);\n \tremote_head = NULL;\n \tremote_master = NULL;\n@@ -487,7 +493,7 @@ int cmd_clone(int argc, const char **argv, const char *prefix)\n \t\t\t\tcontinue;\n \t\t}\n \n-\t\tupdate_ref(\"clone from $repo\",\n+\t\tupdate_ref(reflog_msg.buf,\n \t\t\t refname, r->old_sha1, NULL, 0, DIE_ON_ERR);\n \t}\n \n@@ -495,7 +501,7 @@ int cmd_clone(int argc, const char **argv, const char *prefix)\n \tif (!remote_head) {\n \t\t/* If there isn't one, oh well. */\n \t} else if (remote_master && !hashcmp(remote_master->old_sha1,\n-\t\t\t\t remote_head->old_sha1)) {\n+\t\t\t\t\t remote_head->old_sha1)) {\n \t\t/* If refs/heads/master could be right, it is. */\n \t\thead_points_at = remote_master;\n \t} else\n@@ -552,7 +558,7 @@ int cmd_clone(int argc, const char **argv, const char *prefix)\n \t\tgit_config_set(key, head_points_at->name);\n \t} else if (remote_head) {\n \t\t/* Source had detached HEAD pointing somewhere. */\n-\t\tupdate_ref(\"clone from $repo\", \"HEAD\", remote_head->old_sha1,\n+\t\tupdate_ref(reflog_msg.buf, \"HEAD\", remote_head->old_sha1,\n \t\t\t NULL, REF_NODEREF, DIE_ON_ERR);\n \t} else {\n \t\t/* Nothing to checkout out */\n@@ -591,7 +597,8 @@ int cmd_clone(int argc, const char **argv, const char *prefix)\n \t\t commit_locked_index(&lock_file))\n \t\t\tdie(\"unable to write new index file\");\n \t}\n-\t\n+\n+\tstrbuf_release(&reflog_msg);\n \tjunk_work_tree = NULL;\n \tjunk_git_dir = NULL;\n \treturn 0;\n-- \n1.5.4.3.328.gcaed\n"},{"id":"69975","messageId":"alpine.LSU.1.00.0802261111460.17164@racer.site","threadId":"12315","inReplyTo":"200802260321.14038.johan@herland.net","subject":"Re: [RFC] Build in clone","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2008-02-26T11:14:19Z","receivedAt":"2008-02-26T11:14:19Z","isPatch":false,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Tue, 26 Feb 2008, Johan Herland wrote:\n\n> On Monday 25 February 2008, Daniel Barkalow wrote:\n> > This version is still a mess, but it passes all of the tests.\n> \n> Not for me:\n> *** t5700-clone-reference.sh ***\n> * ok 1: preparing first repository\n> * ok 2: preparing second repository\n> * FAIL 3: cloning with reference (-l -s)\n> git clone -l -s --reference B A C\n\nWhich machine? What is your base (next or master)? What does the verbose \noutput look like?\n\n> diff --git a/builtin-clone.c b/builtin-clone.c\n> index 5aa75e1..7eed340 100644\n> --- a/builtin-clone.c\n> +++ b/builtin-clone.c\n> @@ -1,7 +1,7 @@\n> /*\n> * Builtin \"git clone\"\n> *\n> - * Copyright (c) 2007 Kristian Høgsberg \n> + * Copyright (c) 2007 Kristian Høgsberg \n\nThis is almost certainly wrong.\n\n> @@ -79,7 +79,7 @@ static char *get_repo_path(const char *repo)\n> \n> \tif (!stat(repo, &buf) && S_ISDIR(buf.st_mode))\n> \t\treturn xstrdup(make_absolute_path(repo));\n> -\t\n> +\n\nTrailing whitespace. Okay.\n\n> @@ -347,6 +347,9 @@ int cmd_clone(int argc, const char **argv, const char *prefix)\n> \tchar *path, *dir, *head, *ref_temp;\n> \tstruct ref *refs, *r, *remote_head, *head_points_at, *remote_master;\n> \tchar branch_top[256], key[256], refname[256], value[256];\n> +\tstruct strbuf reflog_msg;\n> +\n> +\tgit_config(git_default_config);\n\nGood catch.\n\n> @@ -495,7 +501,7 @@ int cmd_clone(int argc, const char **argv, const char *prefix)\n> \tif (!remote_head) {\n> \t\t/* If there isn't one, oh well. */\n> \t} else if (remote_master && !hashcmp(remote_master->old_sha1,\n> -\t\t\t\t remote_head->old_sha1)) {\n> +\t\t\t\t\t remote_head->old_sha1)) {\n\nI am not so sure about this change. It is unneeded, and distracts from \nthe rest.\n\nThanks,\nDscho\n"},{"id":"69982","messageId":"200802261319.41862.johan@herland.net","threadId":"12315","inReplyTo":"alpine.LSU.1.00.0802261111460.17164@racer.site","subject":"Re: [RFC] Build in clone","fromName":"Johan Herland","fromEmail":"johan@herland.net","sentAt":"2008-02-26T12:19:41Z","receivedAt":"2008-02-26T12:19:41Z","isPatch":false,"sender":{"key":"johan@herland.net","avatar":"https://avatars.githubusercontent.com/u/547031?v=4"},"body":"On Tuesday 26 February 2008, Johannes Schindelin wrote:\n> Hi,\n> \n> On Tue, 26 Feb 2008, Johan Herland wrote:\n> \n> > On Monday 25 February 2008, Daniel Barkalow wrote:\n> > > This version is still a mess, but it passes all of the tests.\n> > \n> > Not for me:\n> > *** t5700-clone-reference.sh ***\n> > * ok 1: preparing first repository\n> > * ok 2: preparing second repository\n> > * FAIL 3: cloning with reference (-l -s)\n> > git clone -l -s --reference B A C\n> \n> Which machine? What is your base (next or master)? What does the verbose \n> output look like?\n\nI'm at work now (with a different machine), and when I try to reproduce\nI get either 4 errors (with \"make test\") or 7 errors (with\n\"sh ./t5700-clone-reference.sh\"). See [1] for verbose output.\nThis is with next (v1.5.4.3-342-g99e8cb6) + Daniel's patch on a 64-bit\nGentoo Linux on top of Intel Core 2 Duo with 4GB RAM. My home box is\nsimilar (Quad instead of Duo). Without Daniel's patch, the entire test\nsuite passes.\n\n> > diff --git a/builtin-clone.c b/builtin-clone.c\n> > index 5aa75e1..7eed340 100644\n> > --- a/builtin-clone.c\n> > +++ b/builtin-clone.c\n> > @@ -1,7 +1,7 @@\n> > /*\n> > * Builtin \"git clone\"\n> > *\n> > - * Copyright (c) 2007 Kristian Høgsberg \n> > + * Copyright (c) 2007 Kristian Høgsberg \n> \n> This is almost certainly wrong.\n\nYes. But the intent is valid: His name should be in UTF-8; not i latin-1.\n\n> > @@ -79,7 +79,7 @@ static char *get_repo_path(const char *repo)\n> > \n> > \tif (!stat(repo, &buf) && S_ISDIR(buf.st_mode))\n> > \t\treturn xstrdup(make_absolute_path(repo));\n> > -\t\n> > +\n> \n> Trailing whitespace. Okay.\n> \n> > @@ -347,6 +347,9 @@ int cmd_clone(int argc, const char **argv, const char *prefix)\n> > \tchar *path, *dir, *head, *ref_temp;\n> > \tstruct ref *refs, *r, *remote_head, *head_points_at, *remote_master;\n> > \tchar branch_top[256], key[256], refname[256], value[256];\n> > +\tstruct strbuf reflog_msg;\n> > +\n> > +\tgit_config(git_default_config);\n> \n> Good catch.\n\nThanks.\n\n> > @@ -495,7 +501,7 @@ int cmd_clone(int argc, const char **argv, const char *prefix)\n> > \tif (!remote_head) {\n> > \t\t/* If there isn't one, oh well. */\n> > \t} else if (remote_master && !hashcmp(remote_master->old_sha1,\n> > -\t\t\t\t remote_head->old_sha1)) {\n> > +\t\t\t\t\t remote_head->old_sha1)) {\n> \n> I am not so sure about this change. It is unneeded, and distracts from \n> the rest.\n\nYes, if this were meant for inclusion, I'd agree. However, as I originally\nstated above the patch, this was pretty much a verbatim dump of my buffers\nwith the immediate issues I found. It was meant as (hopefully useful)\nfeedback to Daniel, and not for any kind of consumption by git proper.\n\nStill, I should probably have taken the extra minute to filter out unneeded\nwhitespace churn.\n\n\nHave fun! :)\n\n...Johan\n\n\n[1] Verbose output from t5700-clone-reference.sh:\n\n$ ./t5700-clone-reference.sh --verbose\n* expecting success: test_create_repo A && cd A &&\necho first > file1 &&\ngit add file1 &&\ngit commit -m initial\nCreated initial commit 8c40535: initial\n 1 files changed, 1 insertions(+), 0 deletions(-)\n create mode 100644 file1\n* ok 1: preparing first repository\n\n* expecting success: git clone A B && cd B &&\necho second > file2 &&\ngit add file2 &&\ngit commit -m addition &&\ngit repack -a -d &&\ngit prune\nInitialize B/.git\nInitialized empty Git repository in B/.git/\nOkay\nGet for /home/johanh/git-test/git/t/trash/A/.git\n0x714f50\nHEAD\nrefs/heads/master\nwork tree now /home/johanh/git-test/git/t/trash/B\nCreated commit 4f5d964: addition\n 1 files changed, 1 insertions(+), 0 deletions(-)\n create mode 100644 file2\nCounting objects: 6, done.\nCompressing objects: 100% (3/3), done.\nWriting objects: 100% (6/6), done.\nTotal 6 (delta 0), reused 0 (delta 0)\n* ok 2: preparing second repository\n\n* expecting success: git clone -l -s --reference B A C\nInitialize C/.git\nInitialized empty Git repository in C/.git/\nOkay\nWrote /home/johanh/git-test/git/t/trash/B/.git/objects\n to C/.git/objects/info/alternates\n4f5d96490c0371a2814efb3203ed699ef4814fda -> C/.git/refs/reference-tmp/4f5d96490c0371a2814efb3203ed699ef4814fda\nerror: refs/remotes/origin/HEAD does not point to a valid object!\nerror: Z;n�+ does not point to a valid object!\nerror: Z;n�+ does not point to a valid object!\nerror: �p does not point to a valid object!\nerror: Z;n�+ does not point to a valid object!\nerror: Z;n�+ does not point to a valid object!\nerror: Y;n�+ does not point to a valid object!\nerror: q does not point to a valid object!\n./test-lib.sh: line 193: 32376 Segmentation fault git clone -l -s --reference B A C\n* FAIL 3: cloning with reference (-l -s)\n git clone -l -s --reference B A C\n\n* expecting success: test `wc -l expected &&\ngit count-objects > current &&\ndiff expected current\n* ok 6: that reference gets used\n\n* expecting success: git clone --reference B file://`pwd`/A D\nInitialize D/.git\nInitialized empty Git repository in D/.git/\nOkay\nWrote /home/johanh/git-test/git/t/trash/B/.git/objects\n to D/.git/objects/info/alternates\n4f5d96490c0371a2814efb3203ed699ef4814fda -> D/.git/refs/reference-tmp/4f5d96490c0371a2814efb3203ed699ef4814fda\nerror: refs/remotes/origin/HEAD does not point to a valid object!\n�+ does not point to a valid object!\n�+ does not point to a valid object!\nerror: �p does not point to a valid object!\n�+ does not point to a valid object!\n�+ does not point to a valid object!\nerror: does not point to a valid object!\n./test-lib.sh: line 193: 32440 Segmentation fault git clone --reference B file://`pwd`/A D\n* FAIL 7: cloning with reference (no -l -s)\n git clone --reference B file://`pwd`/A D\n\n* expecting success: test `wc -l expected &&\ngit count-objects > current &&\ndiff expected current\n* ok 10: that reference gets used\n\n* expecting success: cd A &&\necho third > file3 &&\ngit add file3 &&\ngit commit -m update &&\ngit repack -a -d &&\ngit prune\nCreated commit 20c3827: update\n 1 files changed, 1 insertions(+), 0 deletions(-)\n create mode 100644 file3\nCounting objects: 6, done.\nCompressing objects: 100% (3/3), done.\nWriting objects: 100% (6/6), done.\nTotal 6 (delta 0), reused 0 (delta 0)\n* ok 11: updating origin\n\n* expecting success: cd C &&\ngit pull origin\nfatal: 'origin': unable to chdir or not a git archive\nfatal: The remote end hung up unexpectedly\n* FAIL 12: pulling changes from origin\n cd C &&\n git pull origin\n\n* expecting success: cd C &&\necho \"2 objects\" > expected &&\ngit count-objects | cut -d, -f1 > current &&\ndiff expected current\n1c1\n< 2 objects\n---\n> 0 objects\n* FAIL 13: that alternate to origin gets used\n cd C &&\n echo \"2 objects\" > expected &&\n git count-objects | cut -d, -f1 > current &&\n diff expected current\n\n* expecting success: cd D &&\ngit pull origin\nfatal: 'origin': unable to chdir or not a git archive\nfatal: The remote end hung up unexpectedly\n* FAIL 14: pulling changes from origin\n cd D &&\n git pull origin\n\n* expecting success: cd D &&\necho \"5 objects\" > expected &&\ngit count-objects | cut -d, -f1 > current &&\ndiff expected current\n1c1\n< 5 objects\n---\n> 0 objects\n* FAIL 15: check objects expected to exist locally\n cd D &&\n echo \"5 objects\" > expected &&\n git count-objects | cut -d, -f1 > current &&\n diff expected current\n\n* failed 7 among 15 test(s)\n\n\n-- \nJohan Herland, \nwww.herland.net\n"},{"id":"69991","messageId":"200802261358.33357.johan@herland.net","threadId":"12315","inReplyTo":"200802261319.41862.johan@herland.net","subject":"Re: [RFC] Build in clone","fromName":"Johan Herland","fromEmail":"johan@herland.net","sentAt":"2008-02-26T12:58:33Z","receivedAt":"2008-02-26T12:58:33Z","isPatch":false,"sender":{"key":"johan@herland.net","avatar":"https://avatars.githubusercontent.com/u/547031?v=4"},"body":"On Tuesday 26 February 2008, Johan Herland wrote:\n> [1] Verbose output from t5700-clone-reference.sh:\n> \n> $ ./t5700-clone-reference.sh --verbose\n> * expecting success: test_create_repo A && cd A &&\n> echo first > file1 &&\n> git add file1 &&\n> git commit -m initial\n> Created initial commit 8c40535: initial\n> 1 files changed, 1 insertions(+), 0 deletions(-)\n> create mode 100644 file1\n> * ok 1: preparing first repository\n> \n> * expecting success: git clone A B && cd B &&\n> echo second > file2 &&\n> git add file2 &&\n> git commit -m addition &&\n> git repack -a -d &&\n> git prune\n> Initialize B/.git\n> Initialized empty Git repository in B/.git/\n> Okay\n> Get for /home/johanh/git-test/git/t/trash/A/.git\n> 0x714f50\n> HEAD\n> refs/heads/master\n> work tree now /home/johanh/git-test/git/t/trash/B\n> Created commit 4f5d964: addition\n> 1 files changed, 1 insertions(+), 0 deletions(-)\n> create mode 100644 file2\n> Counting objects: 6, done.\n> Compressing objects: 100% (3/3), done.\n> Writing objects: 100% (6/6), done.\n> Total 6 (delta 0), reused 0 (delta 0)\n> * ok 2: preparing second repository\n> \n> * expecting success: git clone -l -s --reference B A C\n> Initialize C/.git\n> Initialized empty Git repository in C/.git/\n> Okay\n> Wrote /home/johanh/git-test/git/t/trash/B/.git/objects\n> to C/.git/objects/info/alternates\n> 4f5d96490c0371a2814efb3203ed699ef4814fda -> C/.git/refs/reference-tmp/4f5d96490c0371a2814efb3203ed699ef4814fda\n> error: refs/remotes/origin/HEAD does not point to a valid object!\n> error: Z;n�+ does not point to a valid object!\n> error: Z;n�+ does not point to a valid object!\n> error: �p does not point to a valid object!\n> error: Z;n�+ does not point to a valid object!\n> error: Z;n�+ does not point to a valid object!\n> error: Y;n�+ does not point to a valid object!\n> error: q does not point to a valid object!\n> ./test-lib.sh: line 193: 32376 Segmentation fault git clone -l -s --reference B A C\n> * FAIL 3: cloning with reference (-l -s)\n> git clone -l -s --reference B A C\n\nRunning this test with GDB, I get the following backtrace:\n\n#0 0x0000000000474b87 in is_null_sha1 (sha1=0x100000008
) at cache.h:464\n#1 0x0000000000474ad3 in do_one_ref (base=0x4dc8ff \"refs/\", fn=0x419471 , trim=0, cb_data=0x7498d0, entry=0xffffffff) at refs.c:474\n#2 0x0000000000474e28 in do_for_each_ref (base=0x4dc8ff \"refs/\", fn=0x419471 , trim=0, cb_data=0x7498d0) at refs.c:558\n#3 0x0000000000474ecd in for_each_ref (fn=0x419471 , cb_data=0x7498d0) at refs.c:580\n#4 0x0000000000419706 in setup_reference (repo=0x745070 \"C/.git\") at builtin-clone.c:211\n#5 0x0000000000419fce in cmd_clone (argc=2, argv=0x7fff7a282fa0, prefix=0x0) at builtin-clone.c:422\n#6 0x0000000000404ba3 in run_command (p=0x6ff710, argc=7, argv=0x7fff7a282fa0) at git.c:248\n#7 0x0000000000404d55 in handle_internal_command (argc=7, argv=0x7fff7a282fa0) at git.c:378\n#8 0x0000000000404ebe in main (argc=7, argv=0x7fff7a282fa0) at git.c:442\n\nSeems the \"loose\" ref_list in do_for_each_ref() becomes corrupted.\n\nWill investigate more later.\n\n\n...Johan\n\n-- \nJohan Herland, \nwww.herland.net\n"},{"id":"69992","messageId":"200802261437.18950.johan@herland.net","threadId":"12315","inReplyTo":"200802261358.33357.johan@herland.net","subject":"Re: [RFC] Build in clone","fromName":"Johan Herland","fromEmail":"johan@herland.net","sentAt":"2008-02-26T13:37:18Z","receivedAt":"2008-02-26T13:37:18Z","isPatch":false,"sender":{"key":"johan@herland.net","avatar":"https://avatars.githubusercontent.com/u/547031?v=4"},"body":"On Tuesday 26 February 2008, Johan Herland wrote:\n> Running this test with GDB, I get the following backtrace:\n> \n> #0 0x0000000000474b87 in is_null_sha1 (sha1=0x100000008
) at cache.h:464\n> #1 0x0000000000474ad3 in do_one_ref (base=0x4dc8ff \"refs/\", fn=0x419471 , trim=0, cb_data=0x7498d0, entry=0xffffffff) at refs.c:474\n> #2 0x0000000000474e28 in do_for_each_ref (base=0x4dc8ff \"refs/\", fn=0x419471 , trim=0, cb_data=0x7498d0) at refs.c:558\n> #3 0x0000000000474ecd in for_each_ref (fn=0x419471 , cb_data=0x7498d0) at refs.c:580\n> #4 0x0000000000419706 in setup_reference (repo=0x745070 \"C/.git\") at builtin-clone.c:211\n> #5 0x0000000000419fce in cmd_clone (argc=2, argv=0x7fff7a282fa0, prefix=0x0) at builtin-clone.c:422\n> #6 0x0000000000404ba3 in run_command (p=0x6ff710, argc=7, argv=0x7fff7a282fa0) at git.c:248\n> #7 0x0000000000404d55 in handle_internal_command (argc=7, argv=0x7fff7a282fa0) at git.c:378\n> #8 0x0000000000404ebe in main (argc=7, argv=0x7fff7a282fa0) at git.c:442\n> \n> Seems the \"loose\" ref_list in do_for_each_ref() becomes corrupted.\n\n...and the corruption is done when setup_tmp_ref() calls write_ref_sha1()\nwhich calls invalidate_cached_refs() (which frees the ref_list that\ndo_for_each_ref() is iterating over).\n\nNot sure how to best solve this. Maybe setup_tmp_ref() shouldn't use\nwrite_ref_sha1(), but write the ref file directly instead, as hinted\nat in a comment in setup_tmp_ref()?\n\n\n...Johan\n\n-- \nJohan Herland, \nwww.herland.net\n"},{"id":"70001","messageId":"200802261635.51407.johan@herland.net","threadId":"12315","inReplyTo":"200802261437.18950.johan@herland.net","subject":"[PATCH] Fix premature free of ref_lists while writing temporary refs to file","fromName":"Johan Herland","fromEmail":"johan@herland.net","sentAt":"2008-02-26T15:35:51Z","receivedAt":"2008-02-26T15:35:51Z","isPatch":true,"sender":{"key":"johan@herland.net","avatar":"https://avatars.githubusercontent.com/u/547031?v=4"},"body":"We cannot call write_ref_sha1() from within a for_each_ref() callback, since\nit will free() the ref_list that the for_each_ref() is currently traversing.\n\nTherefore rewrite setup_tmp_ref() to not call write_ref_sha1(), as already\nhinted at in a comment.\n\nThis causes the t5700-clone-reference testcases to pass for me.\n\nSigned-off-by: Johan Herland \n---\n\nOn Tuesday 26 February 2008, Johan Herland wrote:\n> On Tuesday 26 February 2008, Johan Herland wrote:\n> > Running this test with GDB, I get the following backtrace:\n> > \n> > #0 0x0000000000474b87 in is_null_sha1 (sha1=0x100000008
) at cache.h:464\n> > #1 0x0000000000474ad3 in do_one_ref (base=0x4dc8ff \"refs/\", fn=0x419471 , trim=0, cb_data=0x7498d0, entry=0xffffffff) at refs.c:474\n> > #2 0x0000000000474e28 in do_for_each_ref (base=0x4dc8ff \"refs/\", fn=0x419471 , trim=0, cb_data=0x7498d0) at refs.c:558\n> > #3 0x0000000000474ecd in for_each_ref (fn=0x419471 , cb_data=0x7498d0) at refs.c:580\n> > #4 0x0000000000419706 in setup_reference (repo=0x745070 \"C/.git\") at builtin-clone.c:211\n> > #5 0x0000000000419fce in cmd_clone (argc=2, argv=0x7fff7a282fa0, prefix=0x0) at builtin-clone.c:422\n> > #6 0x0000000000404ba3 in run_command (p=0x6ff710, argc=7, argv=0x7fff7a282fa0) at git.c:248\n> > #7 0x0000000000404d55 in handle_internal_command (argc=7, argv=0x7fff7a282fa0) at git.c:378\n> > #8 0x0000000000404ebe in main (argc=7, argv=0x7fff7a282fa0) at git.c:442\n> > \n> > Seems the \"loose\" ref_list in do_for_each_ref() becomes corrupted.\n> \n> ...and the corruption is done when setup_tmp_ref() calls write_ref_sha1()\n> which calls invalidate_cached_refs() (which frees the ref_list that\n> do_for_each_ref() is iterating over).\n> \n> Not sure how to best solve this. Maybe setup_tmp_ref() shouldn't use\n> write_ref_sha1(), but write the ref file directly instead, as hinted\n> at in a comment in setup_tmp_ref()?\n\nHere is a shot at fixing this, although I'm not sure it's the best way\nof doing so.\n\n\nHave fun! :)\n\n...Johan\n\n builtin-clone.c | 43 ++++++++++++++++++++-----------------------\n 1 files changed, 20 insertions(+), 23 deletions(-)\n\ndiff --git a/builtin-clone.c b/builtin-clone.c\nindex 6e34e52..d5baffc 100644\n--- a/builtin-clone.c\n+++ b/builtin-clone.c\n@@ -136,14 +136,12 @@ static int\n setup_tmp_ref(const char *refname,\n \t const unsigned char *sha1, int flags, void *cb_data)\n {\n-\tconst char *ref_temp = cb_data;\n+\tconst char *ref_temp = cb_data, *sha1_hex = sha1_to_hex(sha1);\n \tchar *path;\n-\tstruct lock_file lk;\n-\tstruct ref_lock *rl;\n+\tint fd;\n \n \t/*\n \n-\techo \"$ref_git/objects\" >\"$GIT_DIR/objects/info/alternates\"\n \t(\n \t\tGIT_DIR=\"$ref_git\" git for-each-ref \\\n \t\t\t--format='%(objectname) %(*objectname)'\n@@ -158,25 +156,24 @@ setup_tmp_ref(const char *refname,\n \n \t*/\n \n-\t/* We go a bit out of way to use write_ref_sha1() here. We\n-\t * could just write the ref file directly, since neither\n-\t * locking or reflog really matters here. However, let's use\n-\t * the standard interface for writing refs as much as is\n-\t * possible given that get_git_dir() != the repo we're writing\n-\t * the refs in. */\n-\n-\tprintf(\"%s -> %s/%s\\n\",\n-\t sha1_to_hex(sha1), ref_temp, sha1_to_hex(sha1));\n-\n-\tpath = mkpath(\"%s/%s\", ref_temp, sha1_to_hex(sha1));\n-\trl = xmalloc(sizeof *rl);\n-\trl->force_write = 1;\n-\trl->lk = &lk;\n-\trl->ref_name = xstrdup(sha1_to_hex(sha1));\n-\trl->orig_ref_name = xstrdup(rl->ref_name);\n-\trl->lock_fd = hold_lock_file_for_update(rl->lk, path, 1);\n-\tif (write_ref_sha1(rl, sha1, NULL) < 0)\n-\t\tdie(\"failed to write temporary ref %s\", lk.filename);\n+\t/* Write the ref file directly, since neither locking or reflog really\n+\t * matters here. We should probably use some standard interface for\n+\t * writing refs here, although write_ref_sha1() does not work.\n+\t * (It frees the ref_list that is currently being iterated by\n+\t * for_each_ref().) Keep in mind that get_git_dir() != the repo we're\n+\t * writing the refs in. */\n+\n+\tpath = mkpath(\"%s/%s\", ref_temp, sha1_hex);\n+\n+\tprintf(\"%s -> %s\\n\", sha1_hex, path);\n+\n+\tfd = open(path, O_CREAT | O_WRONLY, 0666);\n+\tif (fd < 0)\n+\t\tdie(\"failed to create %s\", path);\n+\twrite_or_die(fd, sha1_hex, strlen(sha1_hex));\n+\tif (close(fd))\n+\t\tdie(\"could not close %s\", path);\n+\tfprintf(stderr, \"Wrote %s to %s\\n\", sha1_hex, path);\n \n \treturn 0;\n }\n-- \n1.5.4.3.342.g99e8\n"},{"id":"70002","messageId":"200802261640.48770.johan@herland.net","threadId":"12315","inReplyTo":"200802260321.14038.johan@herland.net","subject":"[PATCH] Fix premature call to git_config() causing t1020-subdirectory to fail","fromName":"Johan Herland","fromEmail":"johan@herland.net","sentAt":"2008-02-26T15:40:48Z","receivedAt":"2008-02-26T15:40:48Z","isPatch":true,"sender":{"key":"johan@herland.net","avatar":"https://avatars.githubusercontent.com/u/547031?v=4"},"body":"We need to call git_config(git_default_config) in order to get user.name and\nuser.email (so that reflogs will be correct), but if we do it too early, it\ninterferes with the setup of reference repos. Therefore, move git_config()\ncall to _after_ the reference has been setup (but before we start writing\nreflogs). However, in order for git_config() to read in the global\nconfiguration at that point, we must unset CONFIG_ENVIRONMENT.\n\nThere are probably better ways of resolving this issue.\n\nSigned-off-by: Johan Herland \n---\n\nOn Tuesday 26 February 2008, Johan Herland wrote:\n> - Call git_config(git_default_config) in order to properly set up\n> user.name and user.email for reflogs (This BREAKS test #9 in\n> t1020-subdirectory.sh. Have yet to figure out why)\n\nHere is a fix for this breakage, although I think it's ugly as hell.\n\nBut with this fix, and the other one I just sent out for the\nfor_each_ref() corruption, the whole test suite finally passes on my\nbox.\n\nFeel free to incorporate this into the further builtin-clone work,\nor ignore it, and find better ways of solving these issues.\n\n\nHave fun! :)\n\n...Johan\n\n\n builtin-clone.c | 6 ++++--\n 1 files changed, 4 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin-clone.c b/builtin-clone.c\nindex d5baffc..c02a04c 100644\n--- a/builtin-clone.c\n+++ b/builtin-clone.c\n@@ -346,8 +346,6 @@ int cmd_clone(int argc, const char **argv, const char *prefix)\n \tchar branch_top[256], key[256], refname[256], value[256];\n \tstruct strbuf reflog_msg;\n \n-\tgit_config(git_default_config);\n-\n \tclone_pid = getpid();\n \n \targc = parse_options(argc, argv, builtin_clone_options,\n@@ -423,6 +421,10 @@ int cmd_clone(int argc, const char **argv, const char *prefix)\n \n \tset_git_dir(make_absolute_path(git_dir));\n \n+\t/* This must happen _after_ git_dir has been set up */\n+\tunsetenv(CONFIG_ENVIRONMENT); /* need user/email from global config */\n+\tgit_config(git_default_config);\n+\n \tif (option_bare)\n \t\tgit_config_set(\"core.bare\", \"true\");\n \n-- \n1.5.4.3.342.g99e8\n"},{"id":"70003","messageId":"alpine.LSU.1.00.0802261542080.22527@racer.site","threadId":"12315","inReplyTo":"200802261635.51407.johan@herland.net","subject":"Re: [PATCH] Fix premature free of ref_lists while writing temporary refs to file","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2008-02-26T15:42:55Z","receivedAt":"2008-02-26T15:42:55Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Tue, 26 Feb 2008, Johan Herland wrote:\n\n> We cannot call write_ref_sha1() from within a for_each_ref() callback, \n> since it will free() the ref_list that the for_each_ref() is currently \n> traversing.\n> \n> Therefore rewrite setup_tmp_ref() to not call write_ref_sha1(), as \n> already hinted at in a comment.\n\nI guess the reason was to use a much of an API as possible.\n\nIf you already avoid that, why not write into .git/packed-refs directly?\n\nCiao,\nDscho\n"},{"id":"70004","messageId":"alpine.LSU.1.00.0802261547060.22527@racer.site","threadId":"12315","inReplyTo":"200802261640.48770.johan@herland.net","subject":"Re: [PATCH] Fix premature call to git_config() causing t1020-subdirectory to fail","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2008-02-26T15:47:42Z","receivedAt":"2008-02-26T15:47:42Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Tue, 26 Feb 2008, Johan Herland wrote:\n\n> On Tuesday 26 February 2008, Johan Herland wrote:\n> > - Call git_config(git_default_config) in order to properly set up\n> > user.name and user.email for reflogs (This BREAKS test #9 in\n> > t1020-subdirectory.sh. Have yet to figure out why)\n> \n> Here is a fix for this breakage, although I think it's ugly as hell.\n\nI think it is not ugly, but correct.\n\nCiao,\nDscho\n"},{"id":"70009","messageId":"200802261817.50673.johan@herland.net","threadId":"12315","inReplyTo":"alpine.LSU.1.00.0802261542080.22527@racer.site","subject":"Re: [PATCH] Fix premature free of ref_lists while writing temporary refs to file","fromName":"Johan Herland","fromEmail":"johan@herland.net","sentAt":"2008-02-26T17:17:50Z","receivedAt":"2008-02-26T17:17:50Z","isPatch":true,"sender":{"key":"johan@herland.net","avatar":"https://avatars.githubusercontent.com/u/547031?v=4"},"body":"On Tuesday 26 February 2008, Johannes Schindelin wrote:\n> Hi,\n> \n> On Tue, 26 Feb 2008, Johan Herland wrote:\n> \n> > We cannot call write_ref_sha1() from within a for_each_ref() callback, \n> > since it will free() the ref_list that the for_each_ref() is currently \n> > traversing.\n> > \n> > Therefore rewrite setup_tmp_ref() to not call write_ref_sha1(), as \n> > already hinted at in a comment.\n> \n> I guess the reason was to use a much of an API as possible.\n> \n> If you already avoid that, why not write into .git/packed-refs directly?\n\nThat's exactly what I'm planning. ;-)\n\n...just wanted to get the existing code working first.\n\n\n...Johan\n\n-- \nJohan Herland, \nwww.herland.net\n"},{"id":"70010","messageId":"alpine.LNX.1.00.0802261128360.19024@iabervon.org","threadId":"12315","inReplyTo":"200802260321.14038.johan@herland.net","subject":"Re: [RFC] Build in clone","fromName":"Daniel Barkalow","fromEmail":"barkalow@iabervon.org","sentAt":"2008-02-26T17:36:51Z","receivedAt":"2008-02-26T17:36:51Z","isPatch":false,"sender":{"key":"barkalow@iabervon.org","avatar":"https://avatars.githubusercontent.com/u/55364219?v=4"},"body":"On Tue, 26 Feb 2008, Johan Herland wrote:\n\n> Other than the failing tests, it seems to work fairly well. I've been\n> playing around with it for a few minutes, and on a test repo I have with\n> 1001 branches and 10000 tags, it cuts down the runtime of a local git-clone\n> from 25 seconds to ~1.5 seconds. (simply by eliminating the overhead of\n> invoking git-update-ref for every single ref) :)\n\nGood to hear. A certain amount of the point is performance, and I've only \ngot relatively simple repositories on Linux to test with, where everything \nis too fast to tell anyway.\n\n> I've tried to test this by diffing a cloned repo against an equivalent\n> clone done by the old script. Below I pasted in a few immediate fixes I\n> found. With these fixes, the only remaining diff between the clones is\n> that refs/remotes/origin/HEAD used to be a symbolic ref (with no reflog),\n> but is now a \"regular\" ref (with reflog).\n\nI think that's just a call to the wrong function (and a lack of very very \nexplicit documentation).\n\n> The fixes are, in order of importance:\n> - Call git_config(git_default_config) in order to properly set up\n> user.name and user.email for reflogs (This BREAKS test #9 in\n> t1020-subdirectory.sh. Have yet to figure out why)\n\nI should have read email last night; I could have identified a bunch of \nthe odd errors for you, but you've figured most of them out by now.\n\nI need to look into the config system further; things should be configured \nsuch that the local config is in the new directory and the global config \nis unchanged. If no environment variable is set and pwd is a certain way, \ngit_config_set() will write to the wrong file.\n\n> - Fix \"clone from $repo\" reflog messages (using strbufs; something tells\n> me more of this code would benefit from using strbufs)\n\nMost likely. I think Kristian wrote most of this before strbuf existed or \nsomething of the sort.\n\n> - Høgsberg's name should be in UTF-8 (not sure if this will survive this\n> mail)\n\nIt is for me; I bet it didn't survive the mail from me to you. (That is, \nyour mailer got my UTF-8 message and converted it to Latin-1 on export, \nand so applying it gave a Latin-1 ø in your tree, which you changed to \nUTF-8, and then your mailer again thought your patch was changing \none Latin-1 character to two, and sent the patch like that.)\n\n> - The two whitespace errors you mentioned\n> \n> I'm sorry that my patch below sucks from a style POV. Feel free to ignore.\n> Will redo when it's not in the middle of the night.\n\nI haven't gotten to the level of style in this code myself, so no worries \nthere. :) And whitespace noise is welcome at this point, since I'm still \nincorporating changes and evaluating the resulting state, instead of \nevaluating the changes themselves.\n\n\t-Daniel\n*This .sig left intentionally blank*"},{"id":"70016","messageId":"1204052015.11329.5.camel@gaara.boston.redhat.com","threadId":"12315","inReplyTo":"alpine.LNX.1.00.0802261128360.19024@iabervon.org","subject":"Re: [RFC] Build in clone","fromName":"Kristian Høgsberg","fromEmail":"krh@redhat.com","sentAt":"2008-02-26T18:53:35Z","receivedAt":"2008-02-26T18:53:35Z","isPatch":false,"sender":{"key":"krh@redhat.com","avatar":"https://gravatar.com/avatar/763dee6f9594ac474f725b137a39565792928e583ddf59b32befc2907409027e?d=mp&s=160"},"body":"On Tue, 2008-02-26 at 12:36 -0500, Daniel Barkalow wrote:\n> On Tue, 26 Feb 2008, Johan Herland wrote:\n> \n> > Other than the failing tests, it seems to work fairly well. I've been\n> > playing around with it for a few minutes, and on a test repo I have with\n> > 1001 branches and 10000 tags, it cuts down the runtime of a local git-clone\n> > from 25 seconds to ~1.5 seconds. (simply by eliminating the overhead of\n> > invoking git-update-ref for every single ref) :)\n> \n> Good to hear. A certain amount of the point is performance, and I've only \n> got relatively simple repositories on Linux to test with, where everything \n> is too fast to tell anyway.\n\nYeah, that's pretty cool.\n\n> > - Fix \"clone from $repo\" reflog messages (using strbufs; something tells\n> > me more of this code would benefit from using strbufs)\n> \n> Most likely. I think Kristian wrote most of this before strbuf existed or \n> something of the sort.\n\nNo this was after the strbuf API went in. The \"clone from $repo\"\nmessages I just left at the time because I was lazy, but yeah, they'll\nneed some strbuf love, or maybe just snprintf. I don't agree that\nstrbuf applies very well for the rest of the code. I'm a big fan of the\nstrbuf functionality, but I've been very deliberate about using and not\nusing it.\n\ncheers,\nKristian\n"},{"id":"70037","messageId":"alpine.LNX.1.00.0802261709180.19665@iabervon.org","threadId":"12315","inReplyTo":"200802261640.48770.johan@herland.net","subject":"Re: [PATCH] Fix premature call to git_config() causing t1020-subdirectory to fail","fromName":"Daniel Barkalow","fromEmail":"barkalow@iabervon.org","sentAt":"2008-02-26T22:12:50Z","receivedAt":"2008-02-26T22:12:50Z","isPatch":true,"sender":{"key":"barkalow@iabervon.org","avatar":"https://avatars.githubusercontent.com/u/55364219?v=4"},"body":"On Tue, 26 Feb 2008, Johan Herland wrote:\n\n> We need to call git_config(git_default_config) in order to get user.name and\n> user.email (so that reflogs will be correct), but if we do it too early, it\n> interferes with the setup of reference repos. Therefore, move git_config()\n> call to _after_ the reference has been setup (but before we start writing\n> reflogs). However, in order for git_config() to read in the global\n> configuration at that point, we must unset CONFIG_ENVIRONMENT.\n> \n> There are probably better ways of resolving this issue.\n> \n> Signed-off-by: Johan Herland \n> ---\n> \n> On Tuesday 26 February 2008, Johan Herland wrote:\n> > - Call git_config(git_default_config) in order to properly set up\n> > user.name and user.email for reflogs (This BREAKS test #9 in\n> > t1020-subdirectory.sh. Have yet to figure out why)\n> \n> Here is a fix for this breakage, although I think it's ugly as hell.\n> \n> But with this fix, and the other one I just sent out for the\n> for_each_ref() corruption, the whole test suite finally passes on my\n> box.\n> \n> Feel free to incorporate this into the further builtin-clone work,\n> or ignore it, and find better ways of solving these issues.\n\nActually, I think I'll be leaving CONFIG_ENVIRONMENT alone entirely; I was \nonly using it to override the setting that t5505 uses, but t5505 is just \nwrong to set it. So this is the right placement of git_config(), and the \nsetenv and unsetenv aren't needed.\n\n\t-Daniel\n*This .sig left intentionally blank*\n"},{"id":"70040","messageId":"alpine.LSU.1.00.0802262239200.22527@racer.site","threadId":"12315","inReplyTo":"alpine.LNX.1.00.0802261709180.19665@iabervon.org","subject":"Re: [PATCH] Fix premature call to git_config() causing t1020-subdirectory to fail","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2008-02-26T22:40:34Z","receivedAt":"2008-02-26T22:40:34Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Tue, 26 Feb 2008, Daniel Barkalow wrote:\n\n> Actually, I think I'll be leaving CONFIG_ENVIRONMENT alone entirely; I \n> was only using it to override the setting that t5505 uses, but t5505 is \n> just wrong to set it. So this is the right placement of git_config(), \n> and the setenv and unsetenv aren't needed.\n\nWell, existing git-clone.sh sets GIT_CONFIG. So we have to unset any \nexisting GIT_CONFIG at least.\n\nCiao,\nDscho\n"},{"id":"70041","messageId":"alpine.LNX.1.00.0802261742260.19665@iabervon.org","threadId":"12315","inReplyTo":"alpine.LSU.1.00.0802262239200.22527@racer.site","subject":"Re: [PATCH] Fix premature call to git_config() causing t1020-subdirectory to fail","fromName":"Daniel Barkalow","fromEmail":"barkalow@iabervon.org","sentAt":"2008-02-26T22:49:29Z","receivedAt":"2008-02-26T22:49:29Z","isPatch":true,"sender":{"key":"barkalow@iabervon.org","avatar":"https://avatars.githubusercontent.com/u/55364219?v=4"},"body":"On Tue, 26 Feb 2008, Johannes Schindelin wrote:\n\n> Hi,\n> \n> On Tue, 26 Feb 2008, Daniel Barkalow wrote:\n> \n> > Actually, I think I'll be leaving CONFIG_ENVIRONMENT alone entirely; I \n> > was only using it to override the setting that t5505 uses, but t5505 is \n> > just wrong to set it. So this is the right placement of git_config(), \n> > and the setenv and unsetenv aren't needed.\n> \n> Well, existing git-clone.sh sets GIT_CONFIG. So we have to unset any \n> existing GIT_CONFIG at least.\n\nAs far as I can tell, that's a flaw in git-clone.sh; if the user has set \nGIT_CONFIG, it shouldn't be the case that every program other than \ngit-clone obeys it while git-clone ignores it. (On the other hand, \npossibly every program other than git-config should ignore it, since it's \nonly documented as affecting git-config.) git-clone.sh only sets it, I \nthink, because it runs programs from the wrong context for them to do the \nright thing by default, not because it's specifically trying to override a \nuser-provided setting.\n\n\t-Daniel\n*This .sig left intentionally blank*.\n"},{"id":"70045","messageId":"alpine.LNX.1.00.0802261752160.19665@iabervon.org","threadId":"12315","inReplyTo":"alpine.LSU.1.00.0802261542080.22527@racer.site","subject":"Re: [PATCH] Fix premature free of ref_lists while writing temporary refs to file","fromName":"Daniel Barkalow","fromEmail":"barkalow@iabervon.org","sentAt":"2008-02-26T23:07:36Z","receivedAt":"2008-02-26T23:07:36Z","isPatch":true,"sender":{"key":"barkalow@iabervon.org","avatar":"https://avatars.githubusercontent.com/u/55364219?v=4"},"body":"On Tue, 26 Feb 2008, Johannes Schindelin wrote:\n\n> Hi,\n> \n> On Tue, 26 Feb 2008, Johan Herland wrote:\n> \n> > We cannot call write_ref_sha1() from within a for_each_ref() callback, \n> > since it will free() the ref_list that the for_each_ref() is currently \n> > traversing.\n> > \n> > Therefore rewrite setup_tmp_ref() to not call write_ref_sha1(), as \n> > already hinted at in a comment.\n> \n> I guess the reason was to use a much of an API as possible.\n> \n> If you already avoid that, why not write into .git/packed-refs directly?\n\nActually, it looks to me like the really right thing to do is tell \nfor_each_ref() to also include these refs temporarily, and not actually \nwrite them to disk, read them back, and then delete them.\n\n\t-Daniel\n*This .sig left intentionally blank*\n"},{"id":"70046","messageId":"200802270011.07733.johan@herland.net","threadId":"12315","inReplyTo":"alpine.LNX.1.00.0802261752160.19665@iabervon.org","subject":"Re: [PATCH] Fix premature free of ref_lists while writing temporary refs to file","fromName":"Johan Herland","fromEmail":"johan@herland.net","sentAt":"2008-02-26T23:11:07Z","receivedAt":"2008-02-26T23:11:07Z","isPatch":true,"sender":{"key":"johan@herland.net","avatar":"https://avatars.githubusercontent.com/u/547031?v=4"},"body":"On Wednesday 27 February 2008, Daniel Barkalow wrote:\n> On Tue, 26 Feb 2008, Johannes Schindelin wrote:\n> \n> > Hi,\n> > \n> > On Tue, 26 Feb 2008, Johan Herland wrote:\n> > \n> > > We cannot call write_ref_sha1() from within a for_each_ref() callback, \n> > > since it will free() the ref_list that the for_each_ref() is currently \n> > > traversing.\n> > > \n> > > Therefore rewrite setup_tmp_ref() to not call write_ref_sha1(), as \n> > > already hinted at in a comment.\n> > \n> > I guess the reason was to use a much of an API as possible.\n> > \n> > If you already avoid that, why not write into .git/packed-refs directly?\n> \n> Actually, it looks to me like the really right thing to do is tell \n> for_each_ref() to also include these refs temporarily, and not actually \n> write them to disk, read them back, and then delete them.\n\nI completely agree.\n\n\n...Johan\n\n\n-- \nJohan Herland, \nwww.herland.net\n"},{"id":"70051","messageId":"7vzltn2qsd.fsf@gitster.siamese.dyndns.org","threadId":"12315","inReplyTo":"alpine.LNX.1.00.0802261742260.19665@iabervon.org","subject":"Re: [PATCH] Fix premature call to git_config() causing t1020-subdirectory to fail","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-02-27T00:20:50Z","receivedAt":"2008-02-27T00:20:50Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Daniel Barkalow writes:\n\n> On Tue, 26 Feb 2008, Johannes Schindelin wrote:\n>\n>> Hi,\n>> \n>> On Tue, 26 Feb 2008, Daniel Barkalow wrote:\n>> \n>> > Actually, I think I'll be leaving CONFIG_ENVIRONMENT alone entirely; I \n>> > was only using it to override the setting that t5505 uses, but t5505 is \n>> > just wrong to set it. So this is the right placement of git_config(), \n>> > and the setenv and unsetenv aren't needed.\n>> \n>> Well, existing git-clone.sh sets GIT_CONFIG. So we have to unset any \n>> existing GIT_CONFIG at least.\n>\n> As far as I can tell, that's a flaw in git-clone.sh; if the user has set \n> GIT_CONFIG, it shouldn't be the case that every program other than \n> git-clone obeys it while git-clone ignores it. (On the other hand, \n> possibly every program other than git-config should ignore it, since it's \n> only documented as affecting git-config.) git-clone.sh only sets it, I \n> think, because it runs programs from the wrong context for them to do the \n> right thing by default, not because it's specifically trying to override a \n> user-provided setting.\n\nWhen cloning locally, there are two repositories involved, and\nGIT_CONFIG if exists in the environment talks about the original\none that gets cloned. Without setting GIT_CONFIG explicitly how\nwould you set the configuration that is about the new repository\nclone creates?\n\nAnd to be consistent, if cloning remotely, we should do the\nsame, which means GIT_CONFIG should be reset to point at the\nconfiguration file inside the new repository, be it .git/config\nor $repo/config (if bare).\n\nI think that is the reason behind it.\n"},{"id":"70052","messageId":"alpine.LNX.1.00.0802261933551.19665@iabervon.org","threadId":"12315","inReplyTo":"7vzltn2qsd.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] Fix premature call to git_config() causing t1020-subdirectory to fail","fromName":"Daniel Barkalow","fromEmail":"barkalow@iabervon.org","sentAt":"2008-02-27T00:53:57Z","receivedAt":"2008-02-27T00:53:57Z","isPatch":true,"sender":{"key":"barkalow@iabervon.org","avatar":"https://avatars.githubusercontent.com/u/55364219?v=4"},"body":"On Tue, 26 Feb 2008, Junio C Hamano wrote:\n\n> Daniel Barkalow writes:\n> \n> > On Tue, 26 Feb 2008, Johannes Schindelin wrote:\n> >\n> >> Hi,\n> >> \n> >> On Tue, 26 Feb 2008, Daniel Barkalow wrote:\n> >> \n> >> > Actually, I think I'll be leaving CONFIG_ENVIRONMENT alone entirely; I \n> >> > was only using it to override the setting that t5505 uses, but t5505 is \n> >> > just wrong to set it. So this is the right placement of git_config(), \n> >> > and the setenv and unsetenv aren't needed.\n> >> \n> >> Well, existing git-clone.sh sets GIT_CONFIG. So we have to unset any \n> >> existing GIT_CONFIG at least.\n> >\n> > As far as I can tell, that's a flaw in git-clone.sh; if the user has set \n> > GIT_CONFIG, it shouldn't be the case that every program other than \n> > git-clone obeys it while git-clone ignores it. (On the other hand, \n> > possibly every program other than git-config should ignore it, since it's \n> > only documented as affecting git-config.) git-clone.sh only sets it, I \n> > think, because it runs programs from the wrong context for them to do the \n> > right thing by default, not because it's specifically trying to override a \n> > user-provided setting.\n> \n> When cloning locally, there are two repositories involved, and\n> GIT_CONFIG if exists in the environment talks about the original\n> one that gets cloned.\n\nThere's nothing in the documentation to suggest that you can use \nGIT_CONFIG to affect how the old repository is read, or that GIT_CONFIG \ndoesn't affect the new repository. Actually, as far as I can tell, the \nconfiguration of a repository you're cloning (local or remote) doesn't \nmatter at all. Note that GIT_DIR and GIT_WORK_TREE refer to the new repo, \nso it would be surprising for GIT_CONFIG to refer to the old one.\n\n> Without setting GIT_CONFIG explicitly how would you set the \n> configuration that is about the new repository clone creates?\n\nBy setting GIT_DIR, which also makes everything else work right. (We may \nwant to probe the old repository some by setting GIT_DIR to it \ntemporarily, but we can just collect information that way, and then set it \nto the new one to configure it and set it up.)\n\nMy current design is to collect some initial information, create \ndirectories, and then set the work tree and git dir. Then we run fetch, \nconfigure things, etc., in the new context.\n\nI'm not sure why git-clone.sh doesn't just set GIT_DIR to whatever it \ndecides the git dir with be, and leave GIT_CONFIG alone (or unset it, if \nit's expected to mean something different).\n\n\t-Daniel\n*This .sig left intentionally blank*\n"},{"id":"70054","messageId":"7vy79718tn.fsf@gitster.siamese.dyndns.org","threadId":"12315","inReplyTo":"alpine.LNX.1.00.0802261933551.19665@iabervon.org","subject":"Re: [PATCH] Fix premature call to git_config() causing t1020-subdirectory to fail","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-02-27T01:34:12Z","receivedAt":"2008-02-27T01:34:12Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Daniel Barkalow writes:\n\n> There's nothing in the documentation to suggest that you can use \n> GIT_CONFIG to affect how the old repository is read, or that GIT_CONFIG \n> doesn't affect the new repository. Actually, as far as I can tell, the \n> configuration of a repository you're cloning (local or remote) doesn't \n> matter at all. Note that GIT_DIR and GIT_WORK_TREE refer to the new repo, \n> so it would be surprising for GIT_CONFIG to refer to the old one.\n\nThere was a bit of confusion in this discussion.\n\nGIT_DIR the user may have in the environment may refer to the\nold reopsitory before \"git clone\" is invoked, but it should not\nmatter at all, as the origin of the cloning comes from the\ncommand line and that is where we will read from. The scripted\nversion sets GIT_DIR for our own use to point at the new\nrepository upfront and exports it, so we are safe from bogus\nGIT_DIR value the user may have in the environment.\n\nGIT_WORK_TREE naming the new repository feels Ok, as you do not\ncare about the work tree of the original tree when cloning, and\nyou may want to have a say in where the work tree associated\nwith the new repository should go.\n\nGIT_CONFIG the user may have will refer to the old repository\nbefore \"git clone\" is invoked, as there is no new repository\nbuilt yet. But clone does not read from the old config, so \"you\ncan use GIT_CONFIG to read from old repository\" may be true, but\nit does not matter. We won't use it (we do _not_ want to use\nit) to read from the old configuration file.\n\nWe would however want to make sure that we write to the correct\nconfiguration file of the new repository and not some random\nother place, and that's where the environment variable in the\nscripted version comes into the picture.\n\nIn the scripted version, the only way to make sure which exact\nconfiguration file is updated is to set and export GIT_CONFIG\nwhen running \"git config\", so there are a few places that does\nexactly that (e.g. call to git-init and setting of core.bare).\nUnfortunately many codepaths in the scripted version are utterly\ncareless (e.g. setting of remote.\"$origin\".fetch); they should\nmake sure that they protect themselves against GIT_CONFIG the\nuser may have in the environment that point at random places.\n\n> My current design is to collect some initial information, create \n> directories, and then set the work tree and git dir. Then we run fetch, \n> configure things, etc., in the new context.\n\nThat sounds like a clean design to me.\n"},{"id":"70147","messageId":"alpine.LNX.1.00.0802271430130.19665@iabervon.org","threadId":"12315","inReplyTo":"7vy79718tn.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] Fix premature call to git_config() causing t1020-subdirectory to fail","fromName":"Daniel Barkalow","fromEmail":"barkalow@iabervon.org","sentAt":"2008-02-27T19:47:39Z","receivedAt":"2008-02-27T19:47:39Z","isPatch":true,"sender":{"key":"barkalow@iabervon.org","avatar":"https://avatars.githubusercontent.com/u/55364219?v=4"},"body":"On Tue, 26 Feb 2008, Junio C Hamano wrote:\n\n> Daniel Barkalow writes:\n> \n> > There's nothing in the documentation to suggest that you can use \n> > GIT_CONFIG to affect how the old repository is read, or that GIT_CONFIG \n> > doesn't affect the new repository. Actually, as far as I can tell, the \n> > configuration of a repository you're cloning (local or remote) doesn't \n> > matter at all. Note that GIT_DIR and GIT_WORK_TREE refer to the new repo, \n> > so it would be surprising for GIT_CONFIG to refer to the old one.\n> \n> There was a bit of confusion in this discussion.\n> \n> GIT_DIR the user may have in the environment may refer to the\n> old reopsitory before \"git clone\" is invoked, but it should not\n> matter at all, as the origin of the cloning comes from the\n> command line and that is where we will read from. The scripted\n> version sets GIT_DIR for our own use to point at the new\n> repository upfront and exports it, so we are safe from bogus\n> GIT_DIR value the user may have in the environment.\n\nHuh. I think there's a comment in some test or somewhere that made me \nthink that \"GIT_DIR=dest.git git clone foo\" would write to dest.git \ninstead of ./foo/.git, but your description here is accurate.\n\n> GIT_WORK_TREE naming the new repository feels Ok, as you do not\n> care about the work tree of the original tree when cloning, and\n> you may want to have a say in where the work tree associated\n> with the new repository should go.\n\nWe currently definitely support \"GIT_WORK_TREE=work git clone something\", \npretty much explicitly on line 235 of git-clone.sh.\n\n> GIT_CONFIG the user may have will refer to the old repository\n> before \"git clone\" is invoked, as there is no new repository\n> built yet. But clone does not read from the old config, so \"you\n> can use GIT_CONFIG to read from old repository\" may be true, but\n> it does not matter. We won't use it (we do _not_ want to use\n> it) to read from the old configuration file.\n> \n> We would however want to make sure that we write to the correct\n> configuration file of the new repository and not some random\n> other place, and that's where the environment variable in the\n> scripted version comes into the picture.\n> \n> In the scripted version, the only way to make sure which exact\n> configuration file is updated is to set and export GIT_CONFIG\n> when running \"git config\", so there are a few places that does\n> exactly that (e.g. call to git-init and setting of core.bare).\n> Unfortunately many codepaths in the scripted version are utterly\n> careless (e.g. setting of remote.\"$origin\".fetch); they should\n> make sure that they protect themselves against GIT_CONFIG the\n> user may have in the environment that point at random places.\n\nSince it sets GIT_DIR, it also could simply unset GIT_CONFIG, and then \neverything would just write to the config file for the new GIT_DIR. On the \nother hand, if you have GIT_CONFIG exported in your environment, and you \nset up a repository with \"git clone\", and clone unsets or overrides \nGIT_CONFIG, then your new repository will immediately be unusable, because \nclone will set up the config file inside the new repository, but nothing \nyou run after that will look in the new repository, since everything else \nobeys the GIT_CONFIG you still have set.\n\nOn the other hand, I don't see why any git command other than \"git config\" \n(run my the user directly) has any business looking at GIT_CONFIG, since \nit's only mentioned in the man page for git-config, and not in general for \nconfiguration, the wrapper, or other programs.\n\n\t-Daniel\n*This .sig left intentionally blank*\n"},{"id":"70155","messageId":"7vy796rwkb.fsf@gitster.siamese.dyndns.org","threadId":"12315","inReplyTo":"alpine.LNX.1.00.0802271430130.19665@iabervon.org","subject":"Re: [PATCH] Fix premature call to git_config() causing t1020-subdirectory to fail","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-02-27T20:09:08Z","receivedAt":"2008-02-27T20:09:08Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Daniel Barkalow writes:\n\n> Since it sets GIT_DIR, it also could simply unset GIT_CONFIG, and then \n> everything would just write to the config file for the new GIT_DIR. On the \n> other hand, if you have GIT_CONFIG exported in your environment, and you \n> set up a repository with \"git clone\", and clone unsets or overrides \n> GIT_CONFIG, then your new repository will immediately be unusable, because \n> clone will set up the config file inside the new repository, but nothing \n> you run after that will look in the new repository, since everything else \n> obeys the GIT_CONFIG you still have set.\n\nYes, I think an interactive environment that has GIT_CONFIG is\nsimply misconfigured.\n\nBut on the other hand, I could well imagine a script that does\nthis:\n\n\t#!/bin/sh\n\tGIT_CONFIG=$elsewhere; export GIT_CONFIG\n do things to the $elsewhere file via git-config\n git clone $something $new\n talk about the $new in the $elsewhere file via git-config\n\t(\n \tunset GIT_CONFIG ;# I am writing the script carefully!\n\t\tcd $new\n do something inside the clone\n\t)\n talk more about the $new in the $elsewhere file via git-config\n\texit\n \n> On the other hand, I don't see why any git command other than \"git config\" \n> (run my the user directly) has any business looking at GIT_CONFIG, since \n> it's only mentioned in the man page for git-config, and not in general for \n> configuration, the wrapper, or other programs.\n\nI think reading from the configuration file is done by\neverybody, and GIT_CONFIG affects where the information is read\nfrom. Maybe it was a misfeature. I dunno.\n"},{"id":"70156","messageId":"alpine.LNX.1.00.0802271523130.19665@iabervon.org","threadId":"12315","inReplyTo":"7vy796rwkb.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] Fix premature call to git_config() causing t1020-subdirectory to fail","fromName":"Daniel Barkalow","fromEmail":"barkalow@iabervon.org","sentAt":"2008-02-27T20:31:49Z","receivedAt":"2008-02-27T20:31:49Z","isPatch":true,"sender":{"key":"barkalow@iabervon.org","avatar":"https://avatars.githubusercontent.com/u/55364219?v=4"},"body":"On Wed, 27 Feb 2008, Junio C Hamano wrote:\n\n> Daniel Barkalow writes:\n> \n> > Since it sets GIT_DIR, it also could simply unset GIT_CONFIG, and then \n> > everything would just write to the config file for the new GIT_DIR. On the \n> > other hand, if you have GIT_CONFIG exported in your environment, and you \n> > set up a repository with \"git clone\", and clone unsets or overrides \n> > GIT_CONFIG, then your new repository will immediately be unusable, because \n> > clone will set up the config file inside the new repository, but nothing \n> > you run after that will look in the new repository, since everything else \n> > obeys the GIT_CONFIG you still have set.\n> \n> Yes, I think an interactive environment that has GIT_CONFIG is\n> simply misconfigured.\n> \n> But on the other hand, I could well imagine a script that does\n> this:\n> \n> \t#!/bin/sh\n> \tGIT_CONFIG=$elsewhere; export GIT_CONFIG\n> do things to the $elsewhere file via git-config\n> git clone $something $new\n> talk about the $new in the $elsewhere file via git-config\n> \t(\n> \tunset GIT_CONFIG ;# I am writing the script carefully!\n> \t\tcd $new\n> do something inside the clone\n> \t)\n> talk more about the $new in the $elsewhere file via git-config\n> \texit\n\nThat seems counterintuitive to me. If you created $new with init instead \nof clone, entirely different things would happen. And any other git \ncommands outside the subshell would behave oddly. (Actually, the first of \nthese reminds me why I thought git-clone used the caller's $GIT_DIR: \ngit-init does.)\n\n> > On the other hand, I don't see why any git command other than \"git config\" \n> > (run my the user directly) has any business looking at GIT_CONFIG, since \n> > it's only mentioned in the man page for git-config, and not in general for \n> > configuration, the wrapper, or other programs.\n> \n> I think reading from the configuration file is done by\n> everybody, and GIT_CONFIG affects where the information is read\n> from. Maybe it was a misfeature. I dunno.\n\nIt makes sense that it would work that way, but the documentation should \nprobably reflect that in config.txt.\n\n\t-Daniel\n*This .sig left intentionally blank*\n"},{"id":"70582","messageId":"alpine.LSU.1.00.0803020556380.22527@racer.site","threadId":"12315","inReplyTo":"alpine.LNX.1.00.0802261128360.19024@iabervon.org","subject":"[PATCH] builtin-clone: create remotes/origin/HEAD symref, if guessed","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2008-03-02T05:57:58Z","receivedAt":"2008-03-02T05:57:58Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"\nSigned-off-by: Johannes Schindelin \n---\n\n\tOn Tue, 26 Feb 2008, Daniel Barkalow wrote:\n\n\t> On Tue, 26 Feb 2008, Johan Herland wrote:\n\t> \n\t> > I've tried to test this by diffing a cloned repo against an \n\t> > equivalent clone done by the old script. Below I pasted in a \n\t> > few immediate fixes I found. With these fixes, the only \n\t> > remaining diff between the clones is that \n\t> > refs/remotes/origin/HEAD used to be a symbolic ref (with no\n\t> > reflog), but is now a \"regular\" ref (with reflog).\n\t> \n\t> I think that's just a call to the wrong function (and a lack of \n\t> very very explicit documentation).\n\n\tThis fixes it.\n\n builtin-clone.c | 26 ++++++++++++++------------\n 1 files changed, 14 insertions(+), 12 deletions(-)\n\ndiff --git a/builtin-clone.c b/builtin-clone.c\nindex 89ef665..bd09b0f 100644\n--- a/builtin-clone.c\n+++ b/builtin-clone.c\n@@ -518,33 +518,35 @@ int cmd_clone(int argc, const char **argv, const char *prefix)\n \t\tgit_config_set_multivar(key, value, \"^$\", 0);\n \t}\n \n+\tif (head_points_at)\n+\t\t/* Local default branch */\n+\t\tcreate_symref(\"HEAD\", head_points_at->name, NULL);\n+\n \tif (option_bare) {\n-\t\tif (head_points_at) {\n-\t\t\t/* Local default branch */\n-\t\t\tcreate_symref(\"HEAD\", head_points_at->name, NULL);\n-\t\t}\n \t\tjunk_work_tree = NULL;\n \t\tjunk_git_dir = NULL;\n \t\treturn 0;\n \t}\n \n \tif (head_points_at) {\n+\t\tstruct strbuf buf;\n+\n \t\tif (strrchr(head_points_at->name, '/'))\n \t\t\thead = strrchr(head_points_at->name, '/') + 1;\n \t\telse\n \t\t\thead = head_points_at->name;\n \n-\t\t/* Local default branch */\n-\t\tcreate_symref(\"HEAD\", head_points_at->name, NULL);\n"},{"id":"70586","messageId":"alpine.LSU.1.00.0803020622190.22527@racer.site","threadId":"12315","inReplyTo":"alpine.LSU.1.00.0803020556380.22527@racer.site","subject":"[PATCH, fixed] builtin-clone: create remotes/origin/HEAD symref, if guessed","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2008-03-02T06:25:29Z","receivedAt":"2008-03-02T06:25:29Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"\nSigned-off-by: Johannes Schindelin \n---\n\n\tSorry, my previous patch was broken in so many ways. This one\n\tis better, promise.\n\n\tBTW this incidentally fixes the branch..{remote,merge} \n\tsetup: it used to strip all up-to and including a slash from the \n\tref name. This just _happened_ to work, because commonly HEAD is \n\tat \"refs/heads/master\". However, if it is at \"refs/heads/a/b\", it \n\twould fail.\n\n builtin-clone.c | 35 ++++++++++++++++++++---------------\n 1 files changed, 20 insertions(+), 15 deletions(-)\n\ndiff --git a/builtin-clone.c b/builtin-clone.c\nindex 056e8a3..f27d205 100644\n--- a/builtin-clone.c\n+++ b/builtin-clone.c\n@@ -523,33 +523,38 @@ int cmd_clone(int argc, const char **argv, const char *prefix)\n \t\tgit_config_set_multivar(key, value, \"^$\", 0);\n \t}\n \n+\tif (head_points_at)\n+\t\t/* Local default branch */\n+\t\tcreate_symref(\"HEAD\", head_points_at->name, NULL);\n+\n \tif (option_bare) {\n-\t\tif (head_points_at) {\n-\t\t\t/* Local default branch */\n-\t\t\tcreate_symref(\"HEAD\", head_points_at->name, NULL);\n-\t\t}\n \t\tjunk_work_tree = NULL;\n \t\tjunk_git_dir = NULL;\n \t\treturn 0;\n \t}\n \n \tif (head_points_at) {\n-\t\tif (strrchr(head_points_at->name, '/'))\n-\t\t\thead = strrchr(head_points_at->name, '/') + 1;\n-\t\telse\n-\t\t\thead = head_points_at->name;\n+\t\tstruct strbuf head_ref, real_ref;\n \n-\t\t/* Local default branch */\n-\t\tcreate_symref(\"HEAD\", head_points_at->name, NULL);\n+\t\thead = head_points_at->name;\n+\t\tif (!prefixcmp(head, \"refs/heads/\"))\n+\t\t\thead += 11;\n \n \t\t/* Tracking branch for the primary branch at the remote. */\n \t\tupdate_ref(NULL, \"HEAD\", head_points_at->old_sha1,\n \t\t\t NULL, 0, DIE_ON_ERR);\n-\t/*\n-\t\trm -f \"refs/remotes/$origin/HEAD\"\n-\t\tgit symbolic-ref \"refs/remotes/$origin/HEAD\" \\\n-\t\t\t\"refs/remotes/$origin/$head_points_at\" &&\n-\t*/\n+\n+\t\tstrbuf_init(&head_ref, 0);\n+\t\tstrbuf_addstr(&head_ref, branch_top);\n+\t\tstrbuf_addstr(&head_ref, \"/HEAD\");\n+\t\tdelete_ref(head_ref.buf, head_points_at->old_sha1);\n+\t\tstrbuf_init(&real_ref, 0);\n+\t\tstrbuf_addstr(&real_ref, branch_top);\n+\t\tstrbuf_addch(&real_ref, '/');\n+\t\tstrbuf_addstr(&real_ref, head);\n+\t\tcreate_symref(head_ref.buf, real_ref.buf, reflog_msg.buf);\n+\t\tstrbuf_release(&head_ref);\n+\t\tstrbuf_release(&real_ref);\n \n \t\tsnprintf(key, sizeof key, \"branch.%s.remote\", head);\n \t\tgit_config_set(key, option_origin);\n-- \n1.5.4.3.446.gbe8932\n\n\n"},{"id":"70599","messageId":"alpine.LSU.1.00.0803020743170.22527@racer.site","threadId":"12315","inReplyTo":"alpine.LSU.1.00.0803020622190.22527@racer.site","subject":"[PATCH] builtin clone: support bundles","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2008-03-02T07:46:43Z","receivedAt":"2008-03-02T07:46:43Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"\nThis forward-ports c6fef0bb(clone: support cloning full bundles) to the\nbuiltin clone.\n\nSigned-off-by: Johannes Schindelin \n---\n\n\tNow my private tree passes all tests again.\n\n\tDaniel, do you have a branch where you have your current version \n\tof builting clone? Mine is in \"my-next\" of \n\thttp://repo.or.cz/w/git/dscho.git.\n\n builtin-clone.c | 59 ++++++++++++++++++++++++++++++++++--------------------\n 1 files changed, 37 insertions(+), 22 deletions(-)\n\ndiff --git a/builtin-clone.c b/builtin-clone.c\nindex f27d205..29cd09d 100644\n--- a/builtin-clone.c\n+++ b/builtin-clone.c\n@@ -64,42 +64,52 @@ static struct option builtin_clone_options[] = {\n \tOPT_END()\n };\n \n-static char *get_repo_path(const char *repo)\n+static char *get_repo_path(const char *repo, int *is_bundle)\n {\n-\tconst char *path;\n-\tstruct stat buf;\n-\n-\tpath = mkpath(\"%s/.git\", repo);\n-\tif (!stat(path, &buf) && S_ISDIR(buf.st_mode))\n-\t\treturn xstrdup(make_absolute_path(path));\n-\n-\tpath = mkpath(\"%s.git\", repo);\n-\tif (!stat(path, &buf) && S_ISDIR(buf.st_mode))\n-\t\treturn xstrdup(make_absolute_path(path));\n+\tstatic char *suffix[] = { \"/.git\", \".git\", \"\" };\n+\tstatic char *bundle_suffix[] = { \".bundle\", \"\" };\n+\tstruct stat st;\n+\tint i;\n+\n+\tfor (i = 0; i < ARRAY_SIZE(suffix); i++) {\n+\t\tconst char *path;\n+\t\tpath = mkpath(\"%s%s\", repo, suffix[i]);\n+\t\tif (!stat(path, &st) && S_ISDIR(st.st_mode)) {\n+\t\t\t*is_bundle = 0;\n+\t\t\treturn xstrdup(make_absolute_path(path));\n+\t\t}\n+\t}\n \n-\tif (!stat(repo, &buf) && S_ISDIR(buf.st_mode))\n-\t\treturn xstrdup(make_absolute_path(repo));\n+\tfor (i = 0; i < ARRAY_SIZE(bundle_suffix); i++) {\n+\t\tconst char *path;\n+\t\tpath = mkpath(\"%s%s\", repo, bundle_suffix[i]);\n+\t\tif (!stat(path, &st) && S_ISREG(st.st_mode)) {\n+\t\t\t*is_bundle = 1;\n+\t\t\treturn xstrdup(make_absolute_path(path));\n+\t\t}\n+\t}\n \n \treturn NULL;\n }\n \n-static char *guess_dir_name(const char *repo)\n+static char *guess_dir_name(const char *repo, int is_bundle)\n {\n \tconst char *p, *start, *end, *limit;\n \tint after_slash_or_colon;\n \n \t/* Guess dir name from repository: strip trailing '/',\n-\t * strip trailing '[:/]*git', strip leading '.*[/:]'. */\n+\t * strip trailing '[:/]*.{git,bundle}', strip leading '.*[/:]'. */\n \n \tafter_slash_or_colon = 1;\n \tlimit = repo + strlen(repo);\n \tstart = repo;\n \tend = limit;\n \tfor (p = repo; p < limit; p++) {\n-\t\tif (!prefixcmp(p, \".git\")) {\n+\t\tconst char *prefix = is_bundle ? \".bundle\" : \".git\";\n+\t\tif (!prefixcmp(p, prefix)) {\n \t\t\tif (!after_slash_or_colon)\n \t\t\t\tend = p;\n-\t\t\tp += 3;\n+\t\t\tp += strlen(prefix) - 1;\n \t\t} else if (*p == '/' || *p == ':') {\n \t\t\tif (end == limit)\n \t\t\t\tend = p;\n@@ -287,7 +297,7 @@ walk_objects(char *src, char *dest)\n }\n \n static const struct ref *\n-clone_local(const char *src_repo, const char *dest_repo)\n+clone_local(const char *src_repo, const char *dest_repo, int is_bundle)\n {\n \tconst struct ref *ret;\n \tchar src[PATH_MAX];\n@@ -295,7 +305,9 @@ clone_local(const char *src_repo, const char *dest_repo)\n \tstruct remote *remote;\n \tstruct transport *transport;\n \n-\tif (option_shared) {\n+\tif (is_bundle)\n+\t\t; /* do nothing */\n+\telse if (option_shared) {\n \t\twrite_alternates_file(dest_repo, src_repo);\n \t} else {\n \t\tsnprintf(src, PATH_MAX, \"%s/objects\", src_repo);\n@@ -307,6 +319,8 @@ clone_local(const char *src_repo, const char *dest_repo)\n \tremote = remote_get(src_repo);\n \ttransport = transport_get(remote, src_repo);\n \tret = transport_get_remote_refs(transport);\n+\tif (is_bundle && transport_fetch_refs(transport, (struct ref *)ret))\n+\t\tdie (\"Could not read bundle\");\n \ttransport_disconnect(transport);\n \treturn ret;\n }\n@@ -339,6 +353,7 @@ int cmd_clone(int argc, const char **argv, const char *prefix)\n {\n \tint use_local_hardlinks = 1;\n \tint use_separate_remote = 1;\n+\tint is_bundle;\n \tstruct stat buf;\n \tconst char *repo, *work_tree, *git_dir;\n \tchar *path, *dir, *head, *ref_temp;\n@@ -369,12 +384,12 @@ int cmd_clone(int argc, const char **argv, const char *prefix)\n \t\toption_origin = \"origin\";\n \n \trepo = argv[0];\n-\tpath = get_repo_path(repo);\n+\tpath = get_repo_path(repo, &is_bundle);\n \n \tif (argc == 2) {\n \t\tdir = xstrdup(argv[1]);\n \t} else {\n-\t\tdir = guess_dir_name(repo);\n+\t\tdir = guess_dir_name(repo, is_bundle);\n \t}\n \n \tif (!stat(dir, &buf))\n@@ -429,7 +444,7 @@ int cmd_clone(int argc, const char **argv, const char *prefix)\n \t\tgit_config_set(\"core.bare\", \"true\");\n \n \tif (path != NULL) {\n-\t\trefs = clone_local(path, git_dir);\n+\t\trefs = clone_local(path, git_dir, is_bundle);\n \t\trepo = make_absolute_path(path);\n \t} else {\n \t\tstruct remote *remote = remote_get(argv[0]);\n-- \n1.5.4.3.446.gbe8932\n\n\n"},{"id":"70658","messageId":"alpine.LNX.1.00.0803021113390.19665@iabervon.org","threadId":"12315","inReplyTo":"alpine.LSU.1.00.0803020743170.22527@racer.site","subject":"Re: [PATCH] builtin clone: support bundles","fromName":"Daniel Barkalow","fromEmail":"barkalow@iabervon.org","sentAt":"2008-03-02T16:19:04Z","receivedAt":"2008-03-02T16:19:04Z","isPatch":true,"sender":{"key":"barkalow@iabervon.org","avatar":"https://avatars.githubusercontent.com/u/55364219?v=4"},"body":"On Sun, 2 Mar 2008, Johannes Schindelin wrote:\n\n> \tDaniel, do you have a branch where you have your current version \n> \tof builting clone? Mine is in \"my-next\" of \n> \thttp://repo.or.cz/w/git/dscho.git.\n\nI don't yet, and I've been meaning to post my latest, but haven't gotten \nto it. Once I get these two patches integrated, I'll put something up at:\n\ngit://iabervon.org/~barkalow/git.git builtin-clone\n\n\t-Daniel\n*This .sig left intentionally blank*\n"},{"id":"70661","messageId":"alpine.LNX.1.00.0803021128510.19665@iabervon.org","threadId":"12315","inReplyTo":"alpine.LSU.1.00.0803020743170.22527@racer.site","subject":"Re: [PATCH] builtin clone: support bundles","fromName":"Daniel Barkalow","fromEmail":"barkalow@iabervon.org","sentAt":"2008-03-02T16:48:13Z","receivedAt":"2008-03-02T16:48:13Z","isPatch":true,"sender":{"key":"barkalow@iabervon.org","avatar":"https://avatars.githubusercontent.com/u/55364219?v=4"},"body":"On Sun, 2 Mar 2008, Johannes Schindelin wrote:\n\n> +\tfor (i = 0; i < ARRAY_SIZE(bundle_suffix); i++) {\n> +\t\tconst char *path;\n> +\t\tpath = mkpath(\"%s%s\", repo, bundle_suffix[i]);\n> +\t\tif (!stat(path, &st) && S_ISREG(st.st_mode)) {\n> +\t\t\t*is_bundle = 1;\n> +\t\t\treturn xstrdup(make_absolute_path(path));\n\nThe problem I'm seeing in general is that origin/next's make_absolute_path \ndoesn't work on a regular file in the current directory. How are you \ngetting those tests to pass?\n\nIn any case, I've got my current version at\n\ngit://iabervon.org/~barkalow/git.git builtin-clone\n\nThe top patch is possibly the correct change for make_absolute_path(), and \nI've incorporated your changes (except that I'd reorganized a bunch of \nstuff already, making some of them unnecessary and doing some of them \nslightly differently). For example, I just make bundles not count as \nlocal, and let transport.c deal with them in the normal path.\n\n\t-Daniel\n*This .sig left intentionally blank*\n"},{"id":"70672","messageId":"alpine.LSU.1.00.0803021731400.22527@racer.site","threadId":"12315","inReplyTo":"alpine.LNX.1.00.0803021128510.19665@iabervon.org","subject":"Re: [PATCH] builtin clone: support bundles","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2008-03-02T17:34:12Z","receivedAt":"2008-03-02T17:34:12Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Sun, 2 Mar 2008, Daniel Barkalow wrote:\n\n> On Sun, 2 Mar 2008, Johannes Schindelin wrote:\n> \n> > +\tfor (i = 0; i < ARRAY_SIZE(bundle_suffix); i++) {\n> > +\t\tconst char *path;\n> > +\t\tpath = mkpath(\"%s%s\", repo, bundle_suffix[i]);\n> > +\t\tif (!stat(path, &st) && S_ISREG(st.st_mode)) {\n> > +\t\t\t*is_bundle = 1;\n> > +\t\t\treturn xstrdup(make_absolute_path(path));\n> \n> The problem I'm seeing in general is that origin/next's make_absolute_path \n> doesn't work on a regular file in the current directory. How are you \n> getting those tests to pass?\n\nhttp://repo.or.cz/w/git/dscho.git?a=commitdiff;h=d066bd60e5a93d69c47318cf6e71f77c84b737e6\n\n> In any case, I've got my current version at\n> \n> git://iabervon.org/~barkalow/git.git builtin-clone\n\nThanks.\n\n> The top patch is possibly the correct change for make_absolute_path(), \n\nMine has a test now, too...\n\n> and I've incorporated your changes (except that I'd reorganized a bunch \n> of stuff already, making some of them unnecessary and doing some of them \n> slightly differently). For example, I just make bundles not count as \n> local, and let transport.c deal with them in the normal path.\n\nMakes sense.\n\nCiao,\nDscho\n\n"},{"id":"70675","messageId":"7vhcfpnhfk.fsf@gitster.siamese.dyndns.org","threadId":"12315","inReplyTo":"alpine.LSU.1.00.0803021731400.22527@racer.site","subject":"Re: [PATCH] builtin clone: support bundles","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-03-02T17:50:55Z","receivedAt":"2008-03-02T17:50:55Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin writes:\n\n> Hi,\n>\n> On Sun, 2 Mar 2008, Daniel Barkalow wrote:\n>\n>> On Sun, 2 Mar 2008, Johannes Schindelin wrote:\n>> \n>> > +\tfor (i = 0; i < ARRAY_SIZE(bundle_suffix); i++) {\n>> > +\t\tconst char *path;\n>> > +\t\tpath = mkpath(\"%s%s\", repo, bundle_suffix[i]);\n>> > +\t\tif (!stat(path, &st) && S_ISREG(st.st_mode)) {\n>> > +\t\t\t*is_bundle = 1;\n>> > +\t\t\treturn xstrdup(make_absolute_path(path));\n>> \n>> The problem I'm seeing in general is that origin/next's make_absolute_path \n>> doesn't work on a regular file in the current directory. How are you \n>> getting those tests to pass?\n>\n> http://repo.or.cz/w/git/dscho.git?a=commitdiff;h=d066bd60e5a93d69c47318cf6e71f77c84b737e6\n\nFYI, I already queued this for 'master'.\n"},{"id":"70676","messageId":"7vd4qdnh9z.fsf@gitster.siamese.dyndns.org","threadId":"12315","inReplyTo":"7vhcfpnhfk.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] builtin clone: support bundles","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-03-02T17:54:16Z","receivedAt":"2008-03-02T17:54:16Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> Johannes Schindelin writes:\n>\n>> http://repo.or.cz/w/git/dscho.git?a=commitdiff;h=d066bd60e5a93d69c47318cf6e71f77c84b737e6\n>\n> FYI, I already queued this for 'master'.\n\nClarification.\n\nWhat I queued is _not_ the builtin-clone change, but the patch that your\ngitweb URL shows, which is a fix for e5392c5 (Add is_absolute_path() and\nmake_absolute_path()).\n\n"},{"id":"70715","messageId":"8aa486160803021604r25f80e06xe3aecf23eb4fe172@mail.gmail.com","threadId":"12315","inReplyTo":"alpine.LNX.1.00.0803021113390.19665@iabervon.org","subject":"Re: [PATCH] builtin clone: support bundles","fromName":"Santi Béjar","fromEmail":"sbejar@gmail.com","sentAt":"2008-03-03T00:04:11Z","receivedAt":"2008-03-03T00:04:11Z","isPatch":true,"sender":{"key":"santi@agolina.net","avatar":null},"body":"On Sun, Mar 2, 2008 at 5:19 PM, Daniel Barkalow wrote:\n> On Sun, 2 Mar 2008, Johannes Schindelin wrote:\n>\n> > Daniel, do you have a branch where you have your current version\n> > of builting clone? Mine is in \"my-next\" of\n> > http://repo.or.cz/w/git/dscho.git.\n>\n> I don't yet, and I've been meaning to post my latest, but haven't gotten\n> to it. Once I get these two patches integrated, I'll put something up at:\n>\n> git://iabervon.org/~barkalow/git.git builtin-clone\n\nI've tested the bundle modifications in it and they work for me, so:\n\nTested-by: Santi Béjar \n\nat least for the bundles part (022fc130). Thanks Johannes for the forward-port.\n\nSanti\n"},{"id":"70764","messageId":"200803031004.16568.johan@herland.net","threadId":"12315","inReplyTo":"alpine.LNX.1.00.0803021128510.19665@iabervon.org","subject":"[PATCH] Add test for cloning with \"--reference\" repo being a subset of source repo","fromName":"Johan Herland","fromEmail":"johan@herland.net","sentAt":"2008-03-03T09:04:16Z","receivedAt":"2008-03-03T09:04:16Z","isPatch":true,"sender":{"key":"johan@herland.net","avatar":"https://avatars.githubusercontent.com/u/547031?v=4"},"body":"The first test in this series tests \"git clone -l -s --reference B A C\",\nwhere repo B is a superset of repo A (A has one commit, B has the same\ncommit plus another). In this case, all objects to be cloned are already\npresent in B.\n\nHowever, we should also test the case where the \"--reference\" repo is a\n_subset_ of the source repo (e.g. \"git clone -l -s --reference A B C\"),\ni.e. some objects are not available in the \"--reference\" repo, and will\nhave to be found in the source repo.\n\nSigned-off-by: Johan Herland \n---\n\nOn Sunday 02 March 2008, Daniel Barkalow wrote:\n> In any case, I've got my current version at\n> \n> git://iabervon.org/~barkalow/git.git builtin-clone\n\nThanks, it already looks much better than the initial version. :)\n\nHowever, this added test currently fails for me with the following output:\n\nrepo is /home/johan/git/git/t/trash/B/.git\ndir is E\nInitialize E/.git\nInitialized empty Git repository in E/.git/\nOkay\nWrote /home/johan/git/git/t/trash/A/.git/objects\n to E/.git/objects/info/alternates\nWrote /home/johan/git/git/t/trash/B/.git/objects\n to E/.git/objects/info/alternates\nGet for /home/johan/git/git/t/trash/B/.git\nerror: Trying to write ref refs/remotes/origin/master with nonexistant object 276cf9e94287a7c4e6f79b2724460e9650fa4871\nfatal: Cannot update the ref 'refs/remotes/origin/master'.\nRemove junk E/.git\nRemove junk E\n\nThe same test work well with git-clone.sh.\nNot sure what's going on here, yet, but I thought I'd give you a heads up.\n\n...Johan\n\n\n t/t5700-clone-reference.sh | 5 +++++\n 1 files changed, 5 insertions(+), 0 deletions(-)\n\ndiff --git a/t/t5700-clone-reference.sh b/t/t5700-clone-reference.sh\nindex b6a5486..d318780 100755\n--- a/t/t5700-clone-reference.sh\n+++ b/t/t5700-clone-reference.sh\n@@ -113,4 +113,9 @@ diff expected current'\n \n cd \"$base_dir\"\n \n+test_expect_success 'cloning with reference being subset of source (-l -s)' \\\n+'git clone -l -s --reference A B E'\n+\n+cd \"$base_dir\"\n+\n test_done\n-- \n1.5.4.3.328.gcaed\n\n"},{"id":"70797","messageId":"alpine.LNX.1.00.0803031130090.19665@iabervon.org","threadId":"12315","inReplyTo":"200803031004.16568.johan@herland.net","subject":"Re: [PATCH] Add test for cloning with \"--reference\" repo being a subset of source repo","fromName":"Daniel Barkalow","fromEmail":"barkalow@iabervon.org","sentAt":"2008-03-03T16:36:50Z","receivedAt":"2008-03-03T16:36:50Z","isPatch":true,"sender":{"key":"barkalow@iabervon.org","avatar":"https://avatars.githubusercontent.com/u/55364219?v=4"},"body":"On Mon, 3 Mar 2008, Johan Herland wrote:\n\n> However, we should also test the case where the \"--reference\" repo is a\n> _subset_ of the source repo (e.g. \"git clone -l -s --reference A B C\"),\n> i.e. some objects are not available in the \"--reference\" repo, and will\n> have to be found in the source repo.\n> \n> However, this added test currently fails for me with the following output:\n> \n> repo is /home/johan/git/git/t/trash/B/.git\n> dir is E\n> Initialize E/.git\n> Initialized empty Git repository in E/.git/\n> Okay\n> Wrote /home/johan/git/git/t/trash/A/.git/objects\n> to E/.git/objects/info/alternates\n> Wrote /home/johan/git/git/t/trash/B/.git/objects\n> to E/.git/objects/info/alternates\n> Get for /home/johan/git/git/t/trash/B/.git\n> error: Trying to write ref refs/remotes/origin/master with nonexistant object 276cf9e94287a7c4e6f79b2724460e9650fa4871\n> fatal: Cannot update the ref 'refs/remotes/origin/master'.\n> Remove junk E/.git\n> Remove junk E\n> \n> The same test work well with git-clone.sh.\n> Not sure what's going on here, yet, but I thought I'd give you a heads up.\n\nThanks for the report; I haven't really gone through the local clone \nstuff, and I've altered the use of chdir at various points, so it's quite \npossible that it's not right at all for some cases.\n\n\t-Daniel\n*This .sig left intentionally blank*\n"},{"id":"70803","messageId":"1204563913.4084.3.camel@gaara.boston.redhat.com","threadId":"12315","inReplyTo":"alpine.LSU.1.00.0803020622190.22527@racer.site","subject":"Re: [PATCH, fixed] builtin-clone: create remotes/origin/HEAD symref, if guessed","fromName":"Kristian Høgsberg","fromEmail":"krh@redhat.com","sentAt":"2008-03-03T17:05:13Z","receivedAt":"2008-03-03T17:05:13Z","isPatch":true,"sender":{"key":"krh@redhat.com","avatar":"https://gravatar.com/avatar/763dee6f9594ac474f725b137a39565792928e583ddf59b32befc2907409027e?d=mp&s=160"},"body":"On Sun, 2008-03-02 at 06:25 +0000, Johannes Schindelin wrote:\n> Signed-off-by: Johannes Schindelin \n> ---\n> \n> \tSorry, my previous patch was broken in so many ways. This one\n> \tis better, promise.\n> \n> \tBTW this incidentally fixes the branch..{remote,merge} \n> \tsetup: it used to strip all up-to and including a slash from the \n> \tref name. This just _happened_ to work, because commonly HEAD is \n> \tat \"refs/heads/master\". However, if it is at \"refs/heads/a/b\", it \n> \twould fail.\n> \n> builtin-clone.c | 35 ++++++++++++++++++++---------------\n> 1 files changed, 20 insertions(+), 15 deletions(-)\n> \n> diff --git a/builtin-clone.c b/builtin-clone.c\n> index 056e8a3..f27d205 100644\n> --- a/builtin-clone.c\n> +++ b/builtin-clone.c\n> @@ -523,33 +523,38 @@ int cmd_clone(int argc, const char **argv, const char *prefix)\n> \t\tgit_config_set_multivar(key, value, \"^$\", 0);\n> \t}\n> \n> +\tif (head_points_at)\n> +\t\t/* Local default branch */\n> +\t\tcreate_symref(\"HEAD\", head_points_at->name, NULL);\n> +\n> \tif (option_bare) {\n> -\t\tif (head_points_at) {\n> -\t\t\t/* Local default branch */\n> -\t\t\tcreate_symref(\"HEAD\", head_points_at->name, NULL);\n> -\t\t}\n> \t\tjunk_work_tree = NULL;\n> \t\tjunk_git_dir = NULL;\n> \t\treturn 0;\n> \t}\n> \n> \tif (head_points_at) {\n> -\t\tif (strrchr(head_points_at->name, '/'))\n> -\t\t\thead = strrchr(head_points_at->name, '/') + 1;\n> -\t\telse\n> -\t\t\thead = head_points_at->name;\n> +\t\tstruct strbuf head_ref, real_ref;\n> \n> -\t\t/* Local default branch */\n> -\t\tcreate_symref(\"HEAD\", head_points_at->name, NULL);\n> +\t\thead = head_points_at->name;\n> +\t\tif (!prefixcmp(head, \"refs/heads/\"))\n> +\t\t\thead += 11;\n> \n> \t\t/* Tracking branch for the primary branch at the remote. */\n> \t\tupdate_ref(NULL, \"HEAD\", head_points_at->old_sha1,\n> \t\t\t NULL, 0, DIE_ON_ERR);\n> -\t/*\n> -\t\trm -f \"refs/remotes/$origin/HEAD\"\n> -\t\tgit symbolic-ref \"refs/remotes/$origin/HEAD\" \\\n> -\t\t\t\"refs/remotes/$origin/$head_points_at\" &&\n> -\t*/\n> +\n> +\t\tstrbuf_init(&head_ref, 0);\n> +\t\tstrbuf_addstr(&head_ref, branch_top);\n> +\t\tstrbuf_addstr(&head_ref, \"/HEAD\");\n> +\t\tdelete_ref(head_ref.buf, head_points_at->old_sha1);\n> +\t\tstrbuf_init(&real_ref, 0);\n> +\t\tstrbuf_addstr(&real_ref, branch_top);\n> +\t\tstrbuf_addch(&real_ref, '/');\n> +\t\tstrbuf_addstr(&real_ref, head);\n\nWhat about just using\n\n strbuf_addf(&real_ref, \"%s/%s\", branch_top, head);\n\nAre you worried about performance? :-p\n\nOh and I'm wondering if\n\n strbuf_initf(&real_ref, \"%s/%s\", branch_top, head);\n\nwould be a worthwhile addition to the strbuf API...\n\nKristian\n\n"},{"id":"70805","messageId":"20080303170942.GB23210@artemis.madism.org","threadId":"12315","inReplyTo":"1204563913.4084.3.camel@gaara.boston.redhat.com","subject":"Re: [PATCH, fixed] builtin-clone: create remotes/origin/HEAD symref, if guessed","fromName":"Pierre Habouzit","fromEmail":"madcoder@debian.org","sentAt":"2008-03-03T17:09:43Z","receivedAt":"2008-03-03T17:09:43Z","isPatch":true,"sender":{"key":"madcoder@debian.org","avatar":"https://avatars.githubusercontent.com/u/44708?v=4"},"body":"On Mon, Mar 03, 2008 at 05:05:13PM +0000, Kristian Høgsberg wrote:\n> On Sun, 2008-03-02 at 06:25 +0000, Johannes Schindelin wrote:\n> > +\t\tstrbuf_init(&head_ref, 0);\n> > +\t\tstrbuf_addstr(&head_ref, branch_top);\n> > +\t\tstrbuf_addstr(&head_ref, \"/HEAD\");\n> > +\t\tdelete_ref(head_ref.buf, head_points_at->old_sha1);\n> > +\t\tstrbuf_init(&real_ref, 0);\n> > +\t\tstrbuf_addstr(&real_ref, branch_top);\n> > +\t\tstrbuf_addch(&real_ref, '/');\n> > +\t\tstrbuf_addstr(&real_ref, head);\n> \n> What about just using\n> \n> strbuf_addf(&real_ref, \"%s/%s\", branch_top, head);\n> \n> Are you worried about performance? :-p\n\n If he was he would have used strbuf_init(&real_ref, 1024) or sth like\nthat I assume :)\n\n> Oh and I'm wondering if\n> \n> strbuf_initf(&real_ref, \"%s/%s\", branch_top, head);\n> \n> would be a worthwhile addition to the strbuf API...\n\n I don't think so, unless there are 1289 places in git that would\nbenefit from the shortcut it gives, but I really doubt it.\n\n-- \n·O· Pierre Habouzit\n··O madcoder@debian.org\nOOO http://www.madism.org\n"},{"id":"70806","messageId":"alpine.LSU.1.00.0803031709150.22527@racer.site","threadId":"12315","inReplyTo":"1204563913.4084.3.camel@gaara.boston.redhat.com","subject":"Re: [PATCH, fixed] builtin-clone: create remotes/origin/HEAD symref, if guessed","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2008-03-03T17:10:52Z","receivedAt":"2008-03-03T17:10:52Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Mon, 3 Mar 2008, Kristian Høgsberg wrote:\n\n> On Sun, 2008-03-02 at 06:25 +0000, Johannes Schindelin wrote:\n> > Signed-off-by: Johannes Schindelin \n> > ---\n> > \n> > \tSorry, my previous patch was broken in so many ways. This one\n> > \tis better, promise.\n> > \n> > \tBTW this incidentally fixes the branch..{remote,merge} \n> > \tsetup: it used to strip all up-to and including a slash from the \n> > \tref name. This just _happened_ to work, because commonly HEAD is \n> > \tat \"refs/heads/master\". However, if it is at \"refs/heads/a/b\", it \n> > \twould fail.\n> > \n> > builtin-clone.c | 35 ++++++++++++++++++++---------------\n> > 1 files changed, 20 insertions(+), 15 deletions(-)\n> > \n> > diff --git a/builtin-clone.c b/builtin-clone.c\n> > index 056e8a3..f27d205 100644\n> > --- a/builtin-clone.c\n> > +++ b/builtin-clone.c\n> > @@ -523,33 +523,38 @@ int cmd_clone(int argc, const char **argv, const char *prefix)\n> > \t\tgit_config_set_multivar(key, value, \"^$\", 0);\n> > \t}\n> > \n> > +\tif (head_points_at)\n> > +\t\t/* Local default branch */\n> > +\t\tcreate_symref(\"HEAD\", head_points_at->name, NULL);\n> > +\n> > \tif (option_bare) {\n> > -\t\tif (head_points_at) {\n> > -\t\t\t/* Local default branch */\n> > -\t\t\tcreate_symref(\"HEAD\", head_points_at->name, NULL);\n> > -\t\t}\n> > \t\tjunk_work_tree = NULL;\n> > \t\tjunk_git_dir = NULL;\n> > \t\treturn 0;\n> > \t}\n> > \n> > \tif (head_points_at) {\n> > -\t\tif (strrchr(head_points_at->name, '/'))\n> > -\t\t\thead = strrchr(head_points_at->name, '/') + 1;\n> > -\t\telse\n> > -\t\t\thead = head_points_at->name;\n> > +\t\tstruct strbuf head_ref, real_ref;\n> > \n> > -\t\t/* Local default branch */\n> > -\t\tcreate_symref(\"HEAD\", head_points_at->name, NULL);\n> > +\t\thead = head_points_at->name;\n> > +\t\tif (!prefixcmp(head, \"refs/heads/\"))\n> > +\t\t\thead += 11;\n> > \n> > \t\t/* Tracking branch for the primary branch at the remote. */\n> > \t\tupdate_ref(NULL, \"HEAD\", head_points_at->old_sha1,\n> > \t\t\t NULL, 0, DIE_ON_ERR);\n> > -\t/*\n> > -\t\trm -f \"refs/remotes/$origin/HEAD\"\n> > -\t\tgit symbolic-ref \"refs/remotes/$origin/HEAD\" \\\n> > -\t\t\t\"refs/remotes/$origin/$head_points_at\" &&\n> > -\t*/\n> > +\n> > +\t\tstrbuf_init(&head_ref, 0);\n> > +\t\tstrbuf_addstr(&head_ref, branch_top);\n> > +\t\tstrbuf_addstr(&head_ref, \"/HEAD\");\n> > +\t\tdelete_ref(head_ref.buf, head_points_at->old_sha1);\n> > +\t\tstrbuf_init(&real_ref, 0);\n> > +\t\tstrbuf_addstr(&real_ref, branch_top);\n> > +\t\tstrbuf_addch(&real_ref, '/');\n> > +\t\tstrbuf_addstr(&real_ref, head);\n> \n> What about just using\n> \n> strbuf_addf(&real_ref, \"%s/%s\", branch_top, head);\n> \n> Are you worried about performance? :-p\n\nYou know, just after sending, I thought the same.\n\n> Oh and I'm wondering if\n> \n> strbuf_initf(&real_ref, \"%s/%s\", branch_top, head);\n> \n> would be a worthwhile addition to the strbuf API...\n\nAnd exactly this was crossing my mind, too, as\n\nstatic inline int strbuf_initf(struct strbuf *buf, const char *format, ...)\n{\n\tstrbuf_init(buf, strlen(format));\n\treturn strbuf_addf(format, ...);\n}\n\n(just a sketch, but you get the idea...)\n\nCiao,\nDscho\n"},{"id":"70812","messageId":"200803031841.47302.johan@herland.net","threadId":"12315","inReplyTo":"1204563913.4084.3.camel@gaara.boston.redhat.com","subject":"Re: [PATCH, fixed] builtin-clone: create remotes/origin/HEAD symref, if guessed","fromName":"Johan Herland","fromEmail":"johan@herland.net","sentAt":"2008-03-03T17:41:46Z","receivedAt":"2008-03-03T17:41:46Z","isPatch":true,"sender":{"key":"johan@herland.net","avatar":"https://avatars.githubusercontent.com/u/547031?v=4"},"body":"On Monday 03 March 2008, Kristian Høgsberg wrote:\n> Oh and I'm wondering if\n> \n> strbuf_initf(&real_ref, \"%s/%s\", branch_top, head);\n> \n> would be a worthwhile addition to the strbuf API...\n\n+1. This is about the first thing I started looking for in strbuf.h when I first looked at strbufs...\n\n\n...Johan\n\n-- \nJohan Herland, \nwww.herland.net\n"},{"id":"70817","messageId":"alpine.LNX.1.00.0803031318000.19665@iabervon.org","threadId":"12315","inReplyTo":"200803031004.16568.johan@herland.net","subject":"Re: [PATCH] Add test for cloning with \"--reference\" repo being a subset of source repo","fromName":"Daniel Barkalow","fromEmail":"barkalow@iabervon.org","sentAt":"2008-03-03T18:21:33Z","receivedAt":"2008-03-03T18:21:33Z","isPatch":true,"sender":{"key":"barkalow@iabervon.org","avatar":"https://avatars.githubusercontent.com/u/55364219?v=4"},"body":"On Mon, 3 Mar 2008, Johan Herland wrote:\n\n> Not sure what's going on here, yet, but I thought I'd give you a heads up.\n\nI figured it out, and pushed out a fix; it was doing everything correctly, \nbut it wrote to the alternates files after the library had read that file, \nso it then didn't notice that it actually had the objects that are in the \nsecond alternate repository.\n\n\t-Daniel\n*This .sig left intentionally blank*\n"},{"id":"70835","messageId":"alpine.LSU.1.00.0803031955020.22527@racer.site","threadId":"12315","inReplyTo":"20080303170942.GB23210@artemis.madism.org","subject":"Re: [PATCH, fixed] builtin-clone: create remotes/origin/HEAD symref, if guessed","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2008-03-03T19:55:42Z","receivedAt":"2008-03-03T19:55:42Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Mon, 3 Mar 2008, Pierre Habouzit wrote:\n\n> On Mon, Mar 03, 2008 at 05:05:13PM +0000, Kristian Høgsberg wrote:\n> > Oh and I'm wondering if\n> > \n> > strbuf_initf(&real_ref, \"%s/%s\", branch_top, head);\n> > \n> > would be a worthwhile addition to the strbuf API...\n> \n> I don't think so, unless there are 1289 places in git that would \n> benefit from the shortcut it gives, but I really doubt it.\n\nI think the proper question is: how many places in Git would benefit from \nstrbuf_initf()?\n\nWell, I stated already that I like it.\n\nCiao,\nDscho\n"},{"id":"70875","messageId":"200803040402.57993.johan@herland.net","threadId":"12315","inReplyTo":"alpine.LNX.1.00.0803031318000.19665@iabervon.org","subject":"Re: [PATCH] Add test for cloning with \"--reference\" repo being a subset of source repo","fromName":"Johan Herland","fromEmail":"johan@herland.net","sentAt":"2008-03-04T03:02:57Z","receivedAt":"2008-03-04T03:02:57Z","isPatch":true,"sender":{"key":"johan@herland.net","avatar":"https://avatars.githubusercontent.com/u/547031?v=4"},"body":"On Monday 03 March 2008, Daniel Barkalow wrote:\n> On Mon, 3 Mar 2008, Johan Herland wrote:\n> \n> > Not sure what's going on here, yet, but I thought I'd give you a heads up.\n> \n> I figured it out, and pushed out a fix; it was doing everything correctly, \n> but it wrote to the alternates files after the library had read that file, \n> so it then didn't notice that it actually had the objects that are in the \n> second alternate repository.\n\nThanks. After looking a bit more at the original test repo where I found\nthis issue, I discovered another, similar bug. This one seems ugly; brace\nyourself:\n\nIn some cases (I'm not exactly sure of all the preconditions) when\ncloning with \"--reference\", it seems git tries to access a loose object\nin the \"--reference\" repo instead of in the cloned repo, even if that\nobject is already present in the cloned repo and _missing_ in the\n\"--reference\" repo. The symptom is this error message:\n error: Trying to write ref $ref with nonexistant object $sha1\n\nAfter playing around with this in gdb, it seems the problem is all the\nway down in sha1_file_name() (sha1_file.c). This function is responsible\nfor generating the loose object filename for a given $sha1. It keeps a\nstatic char *base which is initially set to the object directory name,\nand then calls fill_sha1_path() to copy the rest of the object filename\ninto the following bytes. On subsequent calls, only the fill_sha1_path()\npart is done, thereby reusing the base from the previous invocation.\n\nWhat I observe is that this base is not reset after accessing loose\nobjects in the \"--reference\" repo. Thus, later when accessing objects in\nthe cloned repo, sha1_file_name() generates incorrect filenames (pointing\nto the \"--reference\" repo instead of the cloned repo).\n\nOf course, this often goes undetected since the \"--reference\" repo often\nhave the same loose objects as the clone.\n\nUnfortunately (from a builtin git-clone's POV) this seems to be\nsymptomatic of a deeper problem in this part of the code: Using\nfunction-static variables as caches only works as far as the cache\nis in sync with reality. Especially when switching between multiple\nrepositories within the same process, it seems that several of these\nvariables are left with invalid data in them. This needs to be fixed,\nif not only for now, then at least as part of the libification effort.\n\nI'm not sure what is the best way of fixing this issue; my initial guess\nis to move these function-static variables out to file-level, and make\nsure they're properly reset whenever the appropriate context is changed\n(typically when set_git_dir() is called, I guess).\n\nHere are the function-static variables I immediately found in sha1_file.c\n(there may be more, both in sha1_file.c and in other files):\n- sha1_file_name(): static char *base\n- sha1_pack_name(): static char *base\n- sha1_pack_index_name(): static char *base\n- find_pack_entry(): static struct packed_git *last_found\n (not sure about this one)\n\nI will follow up this email with two patches, one adding the failing test,\nand one providing a simple fix for that specific test (although very much\ninsufficient as a fix for the actual issue described above).\n\n\n...Johan\n\n-- \nJohan Herland, \nwww.herland.net\n"},{"id":"70876","messageId":"200803040404.17133.johan@herland.net","threadId":"12315","inReplyTo":"200803040402.57993.johan@herland.net","subject":"[PATCH 1/2] Add test illustrating issues with sha1_file_name() and switching repos","fromName":"Johan Herland","fromEmail":"johan@herland.net","sentAt":"2008-03-04T03:04:16Z","receivedAt":"2008-03-04T03:04:16Z","isPatch":true,"sender":{"key":"johan@herland.net","avatar":"https://avatars.githubusercontent.com/u/547031?v=4"},"body":"This test fails with the current iteration of builtin-clone.\n\nAfter builtin-clone.c have finished processing the \"--reference\" option,\nit switches (i.e. calls set_git_dir()) to the cloned repo. However, when\nupdating refs in the cloned repo, git is unable to find the (loose) objects\nthey point at due to the underlying plumbing generating incorrect filenames\nfor these loose objects (referring to non-existing files in the\n\"--reference\" repo instead of existing files in the cloned repo).\n\nSigned-off-by: Johan Herland \n---\n t/t5700-clone-reference.sh | 21 +++++++++++++++++++++\n 1 files changed, 21 insertions(+), 0 deletions(-)\n\ndiff --git a/t/t5700-clone-reference.sh b/t/t5700-clone-reference.sh\nindex d318780..40826ac 100755\n--- a/t/t5700-clone-reference.sh\n+++ b/t/t5700-clone-reference.sh\n@@ -118,4 +118,25 @@ test_expect_success 'cloning with reference being subset of source (-l -s)' \\\n \n cd \"$base_dir\"\n \n+test_expect_success 'preparing alternate repository #1' \\\n+'test_create_repo F && cd F &&\n+echo first > file1 &&\n+git add file1 &&\n+git commit -m initial'\n+\n+cd \"$base_dir\"\n+\n+test_expect_success 'cloning alternate repo #2 and adding changes to repo #1' \\\n+'git clone F G && cd F &&\n+echo second > file2 &&\n+git add file2 &&\n+git commit -m addition'\n+\n+cd \"$base_dir\"\n+\n+test_expect_failure 'cloning alternate repo #1, using #2 as reference' \\\n+'git clone --reference G F H'\n+\n+cd \"$base_dir\"\n+\n test_done\n-- \n1.5.4.3.328.gcaed\n\n\n"},{"id":"70877","messageId":"200803040405.04537.johan@herland.net","threadId":"12315","inReplyTo":"200803040402.57993.johan@herland.net","subject":"[PATCH 2/2] Overly simplistic fix for issue with sha1_file_name() and switching repos","fromName":"Johan Herland","fromEmail":"johan@herland.net","sentAt":"2008-03-04T03:05:04Z","receivedAt":"2008-03-04T03:05:04Z","isPatch":true,"sender":{"key":"johan@herland.net","avatar":"https://avatars.githubusercontent.com/u/547031?v=4"},"body":"This is not a final fix, just an illustration of how synchronizing the\n\"char *base\" holding the object directory does in fact fix the test\nadded by the previous patch. A real fix will explicitly reset \"base\"\nwhen needed, without comparing with get_object_directory() on every\ninvocation. A real fix will also have to deal with the other similar\nissue in this (and possibly other) file(s).\n\nSigned-off-by: Johan Herland \n---\n sha1_file.c | 5 +++--\n t/t5700-clone-reference.sh | 2 +-\n 2 files changed, 4 insertions(+), 3 deletions(-)\n\ndiff --git a/sha1_file.c b/sha1_file.c\nindex 4ce4d9d..909226e 100644\n--- a/sha1_file.c\n+++ b/sha1_file.c\n@@ -161,9 +161,10 @@ char *sha1_file_name(const unsigned char *sha1)\n {\n \tstatic char *name, *base;\n \n-\tif (!base) {\n-\t\tconst char *sha1_file_directory = get_object_directory();\n+\tconst char *sha1_file_directory = get_object_directory();\n+\tif (!base || prefixcmp(base, sha1_file_directory) != 0) {\n \t\tint len = strlen(sha1_file_directory);\n+\t\tfree(base);\n \t\tbase = xmalloc(len + 60);\n \t\tmemcpy(base, sha1_file_directory, len);\n \t\tmemset(base+len, 0, 60);\ndiff --git a/t/t5700-clone-reference.sh b/t/t5700-clone-reference.sh\nindex 40826ac..0c42d9f 100755\n--- a/t/t5700-clone-reference.sh\n+++ b/t/t5700-clone-reference.sh\n@@ -134,7 +134,7 @@ git commit -m addition'\n \n cd \"$base_dir\"\n \n-test_expect_failure 'cloning alternate repo #1, using #2 as reference' \\\n+test_expect_success 'cloning alternate repo #1, using #2 as reference' \\\n 'git clone --reference G F H'\n \n cd \"$base_dir\"\n-- \n1.5.4.3.328.gcaed\n\n"},{"id":"71009","messageId":"alpine.LNX.1.00.0803041801320.19665@iabervon.org","threadId":"12315","inReplyTo":"200803040402.57993.johan@herland.net","subject":"Re: [PATCH] Add test for cloning with \"--reference\" repo being a subset of source repo","fromName":"Daniel Barkalow","fromEmail":"barkalow@iabervon.org","sentAt":"2008-03-04T23:10:25Z","receivedAt":"2008-03-04T23:10:25Z","isPatch":true,"sender":{"key":"barkalow@iabervon.org","avatar":"https://avatars.githubusercontent.com/u/55364219?v=4"},"body":"On Tue, 4 Mar 2008, Johan Herland wrote:\n\n> On Monday 03 March 2008, Daniel Barkalow wrote:\n> > On Mon, 3 Mar 2008, Johan Herland wrote:\n> > \n> > > Not sure what's going on here, yet, but I thought I'd give you a heads up.\n> > \n> > I figured it out, and pushed out a fix; it was doing everything correctly, \n> > but it wrote to the alternates files after the library had read that file, \n> > so it then didn't notice that it actually had the objects that are in the \n> > second alternate repository.\n> \n> Thanks. After looking a bit more at the original test repo where I found\n> this issue, I discovered another, similar bug. This one seems ugly; brace\n> yourself:\n> \n> In some cases (I'm not exactly sure of all the preconditions) when\n> cloning with \"--reference\", it seems git tries to access a loose object\n> in the \"--reference\" repo instead of in the cloned repo, even if that\n> object is already present in the cloned repo and _missing_ in the\n> \"--reference\" repo. The symptom is this error message:\n> error: Trying to write ref $ref with nonexistant object $sha1\n> \n> After playing around with this in gdb, it seems the problem is all the\n> way down in sha1_file_name() (sha1_file.c). This function is responsible\n> for generating the loose object filename for a given $sha1. It keeps a\n> static char *base which is initially set to the object directory name,\n> and then calls fill_sha1_path() to copy the rest of the object filename\n> into the following bytes. On subsequent calls, only the fill_sha1_path()\n> part is done, thereby reusing the base from the previous invocation.\n>\n> What I observe is that this base is not reset after accessing loose\n> objects in the \"--reference\" repo. Thus, later when accessing objects in\n> the cloned repo, sha1_file_name() generates incorrect filenames (pointing\n> to the \"--reference\" repo instead of the cloned repo).\n> \n> Of course, this often goes undetected since the \"--reference\" repo often\n> have the same loose objects as the clone.\n> \n> Unfortunately (from a builtin git-clone's POV) this seems to be\n> symptomatic of a deeper problem in this part of the code: Using\n> function-static variables as caches only works as far as the cache\n> is in sync with reality. Especially when switching between multiple\n> repositories within the same process, it seems that several of these\n> variables are left with invalid data in them. This needs to be fixed,\n> if not only for now, then at least as part of the libification effort.\n> \n> I'm not sure what is the best way of fixing this issue; my initial guess\n> is to move these function-static variables out to file-level, and make\n> sure they're properly reset whenever the appropriate context is changed\n> (typically when set_git_dir() is called, I guess).\n\nI think we should be able to avoid setting git_dir to anything other than \nthe repo we're creating, which would avoid this problem for the present, \nalthough, as you say, it would be good to be able to switch around as \ninstructed for libification purposes eventually.\n\n\t-Daniel\n*This .sig left intentionally blank*\n"},{"id":"71015","messageId":"alpine.LNX.1.00.0803041922090.19665@iabervon.org","threadId":"12315","inReplyTo":"alpine.LNX.1.00.0803041801320.19665@iabervon.org","subject":"Re: [PATCH] Add test for cloning with \"--reference\" repo being a subset of source repo","fromName":"Daniel Barkalow","fromEmail":"barkalow@iabervon.org","sentAt":"2008-03-05T00:24:30Z","receivedAt":"2008-03-05T00:24:30Z","isPatch":true,"sender":{"key":"barkalow@iabervon.org","avatar":"https://avatars.githubusercontent.com/u/55364219?v=4"},"body":"On Tue, 4 Mar 2008, Daniel Barkalow wrote:\n\n> I think we should be able to avoid setting git_dir to anything other than \n> the repo we're creating, which would avoid this problem for the present, \n> although, as you say, it would be good to be able to switch around as \n> instructed for libification purposes eventually.\n\nOkay, stuff pushed out to not use git_dir to access the reference repo, \nand an additional test that requires that we actually note that we have \nthe refs in the reference repository (because otherwise we could pass all \nthe tests by just making --reference useless, but that's obviously no \ngood).\n\n\t-Daniel\n*This .sig left intentionally blank*\n"},{"id":"71151","messageId":"200803060056.27288.johan@herland.net","threadId":"12315","inReplyTo":"alpine.LNX.1.00.0803041922090.19665@iabervon.org","subject":"Re: [PATCH] Add test for cloning with \"--reference\" repo being a subset of source repo","fromName":"Johan Herland","fromEmail":"johan@herland.net","sentAt":"2008-03-05T23:56:27Z","receivedAt":"2008-03-05T23:56:27Z","isPatch":true,"sender":{"key":"johan@herland.net","avatar":"https://avatars.githubusercontent.com/u/547031?v=4"},"body":"On Wednesday 05 March 2008, Daniel Barkalow wrote:\n> On Tue, 4 Mar 2008, Daniel Barkalow wrote:\n> > I think we should be able to avoid setting git_dir to anything other than \n> > the repo we're creating, which would avoid this problem for the present, \n> > although, as you say, it would be good to be able to switch around as \n> > instructed for libification purposes eventually.\n> \n> Okay, stuff pushed out to not use git_dir to access the reference repo, \n> and an additional test that requires that we actually note that we have \n> the refs in the reference repository (because otherwise we could pass all \n> the tests by just making --reference useless, but that's obviously no \n> good).\n\nThanks. Nice work. The testsuite passes, and it all looks good from here. :)\n\n\n...Johan\n\n-- \nJohan Herland, \nwww.herland.net\n"}]}