{"thread":{"id":"8063","subject":"[PATCH 1/3] Move remote parsing into a library file out of builtin-push.","startedAt":"2007-05-10T02:04:19Z","lastAt":"2007-05-10T16:40:44Z","messageCount":10,"participants":["Daniel Barkalow","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":3},"messages":[{"id":"41697","messageId":"Pine.LNX.4.64.0705092203130.18541@iabervon.org","threadId":"8063","inReplyTo":null,"subject":"[PATCH 1/3] Move remote parsing into a library file out of builtin-push.","fromName":"Daniel Barkalow","fromEmail":"barkalow@iabervon.org","sentAt":"2007-05-10T02:04:19Z","receivedAt":"2007-05-10T02:04:19Z","isPatch":true,"sender":{"key":"barkalow@iabervon.org","avatar":"https://avatars.githubusercontent.com/u/55364219?v=4"},"body":"The new parser is different from the one in builtin-push in two ways:\nthe default is to use the current branch's remote, if there is one,\nbefore \"origin\"; and config is used in preference to remotes.\n\nSigned-off-by: Daniel Barkalow <barkalow@iabervon.org>\n---\n Makefile       |    5 +-\n builtin-push.c |  190 ++++++-----------------------------------------------\n remote.c       |  201 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++\n remote.h       |   18 +++++\n 4 files changed, 242 insertions(+), 172 deletions(-)\n create mode 100644 remote.c\n create mode 100644 remote.h\n\ndiff --git a/Makefile b/Makefile\nindex e0a1308..dd64b7d 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -288,7 +288,8 @@ LIB_H = \\\n \tdiff.h object.h pack.h pkt-line.h quote.h refs.h list-objects.h sideband.h \\\n \trun-command.h strbuf.h tag.h tree.h git-compat-util.h revision.h \\\n \ttree-walk.h log-tree.h dir.h path-list.h unpack-trees.h builtin.h \\\n-\tutf8.h reflog-walk.h patch-ids.h attr.h decorate.h progress.h mailmap.h\n+\tutf8.h reflog-walk.h patch-ids.h attr.h decorate.h progress.h \\\n+\tmailmap.h remote.h\n \n DIFF_OBJS = \\\n \tdiff.o diff-lib.o diffcore-break.o diffcore-order.o \\\n@@ -310,7 +311,7 @@ LIB_OBJS = \\\n \twrite_or_die.o trace.o list-objects.o grep.o match-trees.o \\\n \talloc.o merge-file.o path-list.o help.o unpack-trees.o $(DIFF_OBJS) \\\n \tcolor.o wt-status.o archive-zip.o archive-tar.o shallow.o utf8.o \\\n-\tconvert.o attr.o decorate.o progress.o mailmap.o\n+\tconvert.o attr.o decorate.o progress.o mailmap.o remote.o\n \n BUILTIN_OBJS = \\\n \tbuiltin-add.o \\\ndiff --git a/builtin-push.c b/builtin-push.c\nindex cb78401..0e602f3 100644\n--- a/builtin-push.c\n+++ b/builtin-push.c\n@@ -5,17 +5,13 @@\n #include \"refs.h\"\n #include \"run-command.h\"\n #include \"builtin.h\"\n-\n-#define MAX_URI (16)\n+#include \"remote.h\"\n \n static const char push_usage[] = \"git-push [--all] [--tags] [--receive-pack=<git-receive-pack>] [--repo=all] [-f | --force] [-v] [<repository> <refspec>...]\";\n \n static int all, tags, force, thin = 1, verbose;\n static const char *receivepack;\n \n-#define BUF_SIZE (2084)\n-static char buffer[BUF_SIZE];\n-\n static const char **refspec;\n static int refspec_nr;\n \n@@ -137,175 +133,29 @@ static void set_refspecs(const char **refs, int nr)\n \texpand_refspecs();\n }\n \n-static int get_remotes_uri(const char *repo, const char *uri[MAX_URI])\n-{\n-\tint n = 0;\n-\tFILE *f = fopen(git_path(\"remotes/%s\", repo), \"r\");\n-\tint has_explicit_refspec = refspec_nr || all || tags;\n-\n-\tif (!f)\n-\t\treturn -1;\n-\twhile (fgets(buffer, BUF_SIZE, f)) {\n-\t\tint is_refspec;\n-\t\tchar *s, *p;\n-\n-\t\tif (!prefixcmp(buffer, \"URL:\")) {\n-\t\t\tis_refspec = 0;\n-\t\t\ts = buffer + 4;\n-\t\t} else if (!prefixcmp(buffer, \"Push:\")) {\n-\t\t\tis_refspec = 1;\n-\t\t\ts = buffer + 5;\n-\t\t} else\n-\t\t\tcontinue;\n-\n-\t\t/* Remove whitespace at the head.. */\n-\t\twhile (isspace(*s))\n-\t\t\ts++;\n-\t\tif (!*s)\n-\t\t\tcontinue;\n-\n-\t\t/* ..and at the end */\n-\t\tp = s + strlen(s);\n-\t\twhile (isspace(p[-1]))\n-\t\t\t*--p = 0;\n-\n-\t\tif (!is_refspec) {\n-\t\t\tif (n < MAX_URI)\n-\t\t\t\turi[n++] = xstrdup(s);\n-\t\t\telse\n-\t\t\t\terror(\"more than %d URL's specified, ignoring the rest\", MAX_URI);\n-\t\t}\n-\t\telse if (is_refspec && !has_explicit_refspec) {\n-\t\t\tif (!wildcard_ref(s))\n-\t\t\t\tadd_refspec(xstrdup(s));\n-\t\t}\n-\t}\n-\tfclose(f);\n-\tif (!n)\n-\t\tdie(\"remote '%s' has no URL\", repo);\n-\treturn n;\n-}\n-\n-static const char **config_uri;\n-static const char *config_repo;\n-static int config_repo_len;\n-static int config_current_uri;\n-static int config_get_refspecs;\n-static int config_get_receivepack;\n-\n-static int get_remote_config(const char* key, const char* value)\n-{\n-\tif (!prefixcmp(key, \"remote.\") &&\n-\t    !strncmp(key + 7, config_repo, config_repo_len)) {\n-\t\tif (!strcmp(key + 7 + config_repo_len, \".url\")) {\n-\t\t\tif (config_current_uri < MAX_URI)\n-\t\t\t\tconfig_uri[config_current_uri++] = xstrdup(value);\n-\t\t\telse\n-\t\t\t\terror(\"more than %d URL's specified, ignoring the rest\", MAX_URI);\n-\t\t}\n-\t\telse if (config_get_refspecs &&\n-\t\t\t !strcmp(key + 7 + config_repo_len, \".push\")) {\n-\t\t\tif (!wildcard_ref(value))\n-\t\t\t\tadd_refspec(xstrdup(value));\n-\t\t}\n-\t\telse if (config_get_receivepack &&\n-\t\t\t !strcmp(key + 7 + config_repo_len, \".receivepack\")) {\n-\t\t\tif (!receivepack) {\n-\t\t\t\tchar *rp = xmalloc(strlen(value) + 16);\n-\t\t\t\tsprintf(rp, \"--receive-pack=%s\", value);\n-\t\t\t\treceivepack = rp;\n-\t\t\t} else\n-\t\t\t\terror(\"more than one receivepack given, using the first\");\n-\t\t}\n-\t}\n-\treturn 0;\n-}\n-\n-static int get_config_remotes_uri(const char *repo, const char *uri[MAX_URI])\n-{\n-\tconfig_repo_len = strlen(repo);\n-\tconfig_repo = repo;\n-\tconfig_current_uri = 0;\n-\tconfig_uri = uri;\n-\tconfig_get_refspecs = !(refspec_nr || all || tags);\n-\tconfig_get_receivepack = (receivepack == NULL);\n-\n-\tgit_config(get_remote_config);\n-\treturn config_current_uri;\n-}\n-\n-static int get_branches_uri(const char *repo, const char *uri[MAX_URI])\n-{\n-\tconst char *slash = strchr(repo, '/');\n-\tint n = slash ? slash - repo : 1000;\n-\tFILE *f = fopen(git_path(\"branches/%.*s\", n, repo), \"r\");\n-\tchar *s, *p;\n-\tint len;\n-\n-\tif (!f)\n-\t\treturn 0;\n-\ts = fgets(buffer, BUF_SIZE, f);\n-\tfclose(f);\n-\tif (!s)\n-\t\treturn 0;\n-\twhile (isspace(*s))\n-\t\ts++;\n-\tif (!*s)\n-\t\treturn 0;\n-\tp = s + strlen(s);\n-\twhile (isspace(p[-1]))\n-\t\t*--p = 0;\n-\tlen = p - s;\n-\tif (slash)\n-\t\tlen += strlen(slash);\n-\tp = xmalloc(len + 1);\n-\tstrcpy(p, s);\n-\tif (slash)\n-\t\tstrcat(p, slash);\n-\turi[0] = p;\n-\treturn 1;\n-}\n-\n-/*\n- * Read remotes and branches file, fill the push target URI\n- * list.  If there is no command line refspecs, read Push: lines\n- * to set up the *refspec list as well.\n- * return the number of push target URIs\n- */\n-static int read_config(const char *repo, const char *uri[MAX_URI])\n-{\n-\tint n;\n-\n-\tif (*repo != '/') {\n-\t\tn = get_remotes_uri(repo, uri);\n-\t\tif (n > 0)\n-\t\t\treturn n;\n-\n-\t\tn = get_config_remotes_uri(repo, uri);\n-\t\tif (n > 0)\n-\t\t\treturn n;\n-\n-\t\tn = get_branches_uri(repo, uri);\n-\t\tif (n > 0)\n-\t\t\treturn n;\n-\t}\n-\n-\turi[0] = repo;\n-\treturn 1;\n-}\n-\n static int do_push(const char *repo)\n {\n-\tconst char *uri[MAX_URI];\n-\tint i, n, errs;\n+\tint i, errs;\n \tint common_argc;\n \tconst char **argv;\n \tint argc;\n+\tstruct remote *remote = remote_get(repo);\n \n-\tn = read_config(repo, uri);\n-\tif (n <= 0)\n+\tif (!remote)\n \t\tdie(\"bad repository '%s'\", repo);\n \n+\tif (remote->receivepack) {\n+\t\tchar *rp = xmalloc(strlen(remote->receivepack) + 16);\n+\t\tsprintf(rp, \"--receive-pack=%s\", remote->receivepack);\n+\t\treceivepack = rp;\n+\t}\n+\tif (!refspec && !all && !tags && remote->push_refspec_nr) {\n+\t\tfor (i = 0; i < remote->push_refspec_nr; i++) {\n+\t\t\tif (!wildcard_ref(remote->push_refspec[i]))\n+\t\t\t\tadd_refspec(remote->push_refspec[i]);\n+\t\t}\n+\t}\n+\n \targv = xmalloc((refspec_nr + 10) * sizeof(char *));\n \targv[0] = \"dummy-send-pack\";\n \targc = 1;\n@@ -318,12 +168,12 @@ static int do_push(const char *repo)\n \tcommon_argc = argc;\n \n \terrs = 0;\n-\tfor (i = 0; i < n; i++) {\n+\tfor (i = 0; i < remote->uri_nr; i++) {\n \t\tint err;\n \t\tint dest_argc = common_argc;\n \t\tint dest_refspec_nr = refspec_nr;\n \t\tconst char **dest_refspec = refspec;\n-\t\tconst char *dest = uri[i];\n+\t\tconst char *dest = remote->uri[i];\n \t\tconst char *sender = \"send-pack\";\n \t\tif (!prefixcmp(dest, \"http://\") ||\n \t\t    !prefixcmp(dest, \"https://\"))\n@@ -341,7 +191,7 @@ static int do_push(const char *repo)\n \t\tif (!err)\n \t\t\tcontinue;\n \n-\t\terror(\"failed to push to '%s'\", uri[i]);\n+\t\terror(\"failed to push to '%s'\", remote->uri[i]);\n \t\tswitch (err) {\n \t\tcase -ERR_RUN_COMMAND_FORK:\n \t\t\terror(\"unable to fork for %s\", sender);\n@@ -362,7 +212,7 @@ static int do_push(const char *repo)\n int cmd_push(int argc, const char **argv, const char *prefix)\n {\n \tint i;\n-\tconst char *repo = \"origin\";\t/* default repository */\n+\tconst char *repo = NULL;\t/* default repository */\n \n \tfor (i = 1; i < argc; i++) {\n \t\tconst char *arg = argv[i];\ndiff --git a/remote.c b/remote.c\nnew file mode 100644\nindex 0000000..32a0acf\n--- /dev/null\n+++ b/remote.c\n@@ -0,0 +1,201 @@\n+#include \"cache.h\"\n+#include \"remote.h\"\n+#include \"refs.h\"\n+\n+static struct remote **remotes;\n+static int allocated_remotes;\n+\n+#define BUF_SIZE (2084)\n+static char buffer[BUF_SIZE];\n+\n+static void add_push_refspec(struct remote *remote, const char *ref)\n+{\n+\tint nr = remote->push_refspec_nr + 1;\n+\tremote->push_refspec =\n+\t\txrealloc(remote->push_refspec, nr * sizeof(char *));\n+\tremote->push_refspec[nr-1] = ref;\n+\tremote->push_refspec_nr = nr;\n+}\n+\n+static void add_uri(struct remote *remote, const char *uri)\n+{\n+\tint nr = remote->uri_nr + 1;\n+\tremote->uri =\n+\t\txrealloc(remote->uri, nr * sizeof(char *));\n+\tremote->uri[nr-1] = uri;\n+\tremote->uri_nr = nr;\n+}\n+\n+static struct remote *make_remote(const char *name, int len)\n+{\n+\tint i, empty = -1;\n+\n+\tfor (i = 0; i < allocated_remotes; i++) {\n+\t\tif (!remotes[i]) {\n+\t\t\tif (empty < 0)\n+\t\t\t\tempty = i;\n+\t\t} else {\n+\t\t\tif (len ? (!strncmp(name, remotes[i]->name, len) &&\n+\t\t\t\t   !remotes[i]->name[len]) :\n+\t\t\t    !strcmp(name, remotes[i]->name))\n+\t\t\t\treturn remotes[i];\n+\t\t}\n+\t}\n+\n+\tif (empty < 0) {\n+\t\tempty = allocated_remotes;\n+\t\tallocated_remotes += allocated_remotes ? allocated_remotes : 1;\n+\t\tremotes = xrealloc(remotes,\n+\t\t\t\t   sizeof(*remotes) * allocated_remotes);\n+\t\tmemset(remotes + empty, 0,\n+\t\t       (allocated_remotes - empty) * sizeof(*remotes));\n+\t}\n+\tremotes[empty] = xcalloc(1, sizeof(struct remote));\n+\tif (len)\n+\t\tremotes[empty]->name = xstrndup(name, len);\n+\telse\n+\t\tremotes[empty]->name = xstrdup(name);\n+\treturn remotes[empty];\n+}\n+\n+static void read_remotes_file(struct remote *remote)\n+{\n+\tFILE *f = fopen(git_path(\"remotes/%s\", remote->name), \"r\");\n+\n+\tif (!f)\n+\t\treturn;\n+\twhile (fgets(buffer, BUF_SIZE, f)) {\n+\t\tint value_list;\n+\t\tchar *s, *p;\n+\n+\t\tif (!prefixcmp(buffer, \"URL:\")) {\n+\t\t\tvalue_list = 0;\n+\t\t\ts = buffer + 4;\n+\t\t} else if (!prefixcmp(buffer, \"Push:\")) {\n+\t\t\tvalue_list = 1;\n+\t\t\ts = buffer + 5;\n+\t\t} else\n+\t\t\tcontinue;\n+\n+\t\twhile (isspace(*s))\n+\t\t\ts++;\n+\t\tif (!*s)\n+\t\t\tcontinue;\n+\n+\t\tp = s + strlen(s);\n+\t\twhile (isspace(p[-1]))\n+\t\t\t*--p = 0;\n+\n+\t\tswitch (value_list) {\n+\t\tcase 0:\n+\t\t\tadd_uri(remote, xstrdup(s));\n+\t\t\tbreak;\n+\t\tcase 1:\n+\t\t\tadd_push_refspec(remote, xstrdup(s));\n+\t\t\tbreak;\n+\t\t}\n+\t}\n+}\n+\n+static void read_branches_file(struct remote *remote)\n+{\n+\tconst char *slash = strchr(remote->name, '/');\n+\tint n = slash ? slash - remote->name : 1000;\n+\tFILE *f = fopen(git_path(\"branches/%.*s\", n, remote->name), \"r\");\n+\tchar *s, *p;\n+\tint len;\n+\n+\tif (!f)\n+\t\treturn;\n+\ts = fgets(buffer, BUF_SIZE, f);\n+\tfclose(f);\n+\tif (!s)\n+\t\treturn;\n+\twhile (isspace(*s))\n+\t\ts++;\n+\tif (!*s)\n+\t\treturn;\n+\tp = s + strlen(s);\n+\twhile (isspace(p[-1]))\n+\t\t*--p = 0;\n+\tlen = p - s;\n+\tif (slash)\n+\t\tlen += strlen(slash);\n+\tp = xmalloc(len + 1);\n+\tstrcpy(p, s);\n+\tif (slash)\n+\t\tstrcat(p, slash);\n+\tadd_uri(remote, p);\n+}\n+\n+static char *default_remote_name = NULL;\n+static const char *current_branch = NULL;\n+static int current_branch_len = 0;\n+\n+static int handle_config(const char *key, const char *value)\n+{\n+\tconst char *name;\n+\tconst char *subkey;\n+\tstruct remote *remote;\n+\tif (!prefixcmp(key, \"branch.\") && current_branch &&\n+\t    !strncmp(key + 7, current_branch, current_branch_len) &&\n+\t    !strcmp(key + 7 + current_branch_len, \".remote\")) {\n+\t\tfree(default_remote_name);\n+\t\tdefault_remote_name = xstrdup(value);\n+\t}\n+\tif (prefixcmp(key,  \"remote.\"))\n+\t\treturn 0;\n+\tname = key + 7;\n+\tsubkey = strrchr(name, '.');\n+\tif (!subkey)\n+\t\treturn error(\"Config with no key for remote %s\", name);\n+\tremote = make_remote(name, subkey - name);\n+\tif (!strcmp(subkey, \".url\")) {\n+\t\tadd_uri(remote, xstrdup(value));\n+\t} else if (!strcmp(subkey, \".push\")) {\n+\t\tadd_push_refspec(remote, xstrdup(value));\n+\t} else if (!strcmp(subkey, \".receivepack\")) {\n+\t\tif (!remote->receivepack)\n+\t\t\tremote->receivepack = xstrdup(value);\n+\t\telse\n+\t\t\terror(\"more than one receivepack given, using the first\");\n+\t}\n+\treturn 0;\n+}\n+\n+static void read_config(void)\n+{\n+\tunsigned char sha1[20];\n+\tconst char *head_ref;\n+\tint flag;\n+\tif (default_remote_name) // did this already\n+\t\treturn;\n+\tdefault_remote_name = xstrdup(\"origin\");\n+\tcurrent_branch = NULL;\n+\thead_ref = resolve_ref(\"HEAD\", sha1, 0, &flag);\n+\tif (head_ref && (flag & REF_ISSYMREF) &&\n+\t    !prefixcmp(head_ref, \"refs/heads/\")) {\n+\t\tcurrent_branch = head_ref + strlen(\"refs/heads/\");\n+\t\tcurrent_branch_len = strlen(current_branch);\n+\t}\n+\tgit_config(handle_config);\n+}\n+\n+struct remote *remote_get(const char *name)\n+{\n+\tstruct remote *ret;\n+\n+\tread_config();\n+\tif (!name)\n+\t\tname = default_remote_name;\n+\tret = make_remote(name, 0);\n+\tif (*name == '/')\n+\t\tadd_uri(ret, name);\n+\tif (!ret->uri)\n+\t\tread_remotes_file(ret);\n+\tif (!ret->uri)\n+\t\tread_branches_file(ret);\n+\tif (!ret->uri)\n+\t\treturn NULL;\n+\treturn ret;\n+}\ndiff --git a/remote.h b/remote.h\nnew file mode 100644\nindex 0000000..73747a8\n--- /dev/null\n+++ b/remote.h\n@@ -0,0 +1,18 @@\n+#ifndef REMOTE_H\n+#define REMOTE_H\n+\n+struct remote {\n+\tconst char *name;\n+\n+\tconst char **uri;\n+\tint uri_nr;\n+\n+\tconst char **push_refspec;\n+\tint push_refspec_nr;\n+\n+\tconst char *receivepack;\n+};\n+\n+struct remote *remote_get(const char *name);\n+\n+#endif\n-- \n1.5.2.rc2.22.g9bca\n"},{"id":"41707","messageId":"7vhcqlma1l.fsf@assigned-by-dhcp.cox.net","threadId":"8063","inReplyTo":"Pine.LNX.4.64.0705092203130.18541@iabervon.org","subject":"Re: [PATCH 1/3] Move remote parsing into a library file out of builtin-push.","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2007-05-10T07:07:18Z","receivedAt":"2007-05-10T07:07:18Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"This seems to break t9400, with \"fatal: bad repository 'gitcvs.git\",\nupon \"git push\".\n\n: gitster t/db/remote; sh t9400-git-cvsserver-server.sh -i -v\n* expecting success: cvs -Q co -d cvswork master &&\n   test \"$(echo $(grep -v ^D cvswork/CVS/Entries|cut -d/ -f2,3,5))\" = \"empty/1.1/\"\ncvs checkout: Updating cvswork\nU cvswork/empty\n*   ok 1: basic checkout\n\n* expecting success: echo testfile1 >testfile1 &&\n   git add testfile1 &&\n   git commit -q -m \"Add testfile1\" &&\n   git push gitcvs.git >/dev/null &&\n   cd cvswork &&\n   cvs -Q update &&\n   test \"$(echo $(grep testfile1 CVS/Entries|cut -d/ -f2,3,5))\" = \"testfile1/1.1/\" &&\n   diff -q testfile1 ../testfile1\nfatal: bad repository 'gitcvs.git'\n* FAIL 2: cvs update (create new file)\n\techo testfile1 >testfile1 &&\n\t   git add testfile1 &&\n\t   git commit -q -m \"Add testfile1\" &&\n\t   git push gitcvs.git >/dev/null &&\n\t   cd cvswork &&\n\t   cvs -Q update &&\n\t   test \"$(echo $(grep testfile1 CVS/Entries|cut -d/ -f2,3,5))\" = \"testfile1/1.1/\" &&\n\t   diff -q testfile1 ../testfile1\n: gitster t/db/remote; \n"},{"id":"41713","messageId":"Pine.LNX.4.64.0705100328260.18541@iabervon.org","threadId":"8063","inReplyTo":"7vhcqlma1l.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH 1/3] Move remote parsing into a library file out of builtin-push.","fromName":"Daniel Barkalow","fromEmail":"barkalow@iabervon.org","sentAt":"2007-05-10T07:45:33Z","receivedAt":"2007-05-10T07:45:33Z","isPatch":true,"sender":{"key":"barkalow@iabervon.org","avatar":"https://avatars.githubusercontent.com/u/55364219?v=4"},"body":"On Thu, 10 May 2007, Junio C Hamano wrote:\n\n> This seems to break t9400, with \"fatal: bad repository 'gitcvs.git\",\n> upon \"git push\".\n> \n> : gitster t/db/remote; sh t9400-git-cvsserver-server.sh -i -v\n> * expecting success: cvs -Q co -d cvswork master &&\n>    test \"$(echo $(grep -v ^D cvswork/CVS/Entries|cut -d/ -f2,3,5))\" = \"empty/1.1/\"\n> cvs checkout: Updating cvswork\n> U cvswork/empty\n> *   ok 1: basic checkout\n> \n> * expecting success: echo testfile1 >testfile1 &&\n>    git add testfile1 &&\n>    git commit -q -m \"Add testfile1\" &&\n>    git push gitcvs.git >/dev/null &&\n\nThe man page doesn't think this is valid, since it only claims absolute \npaths to work for local repositories. (Current builtin-push seems to be \nsloppy; it has the die() that triggers in my version, but its \nread_config() always returns a value that doesn't trigger it, suggesting \nthat there's some sort of bug.)\n\nFollowing patch fixes t9400 to use absolute paths (assuming that's what it \nexpects \"git push gitcvs.git\" to mean).\n\n=== cut here ===\nLocal repository names for git-push are only documented with absolute \npaths, even though current git-push is lenient. Use an absolute path in \nt9400-git-cvsserver-server.sh.\n\nSigned-off-by: Daniel Barkalow <barkalow@iabervon.org>\n---\n\ndiff --git a/t/t9400-git-cvsserver-server.sh b/t/t9400-git-cvsserver-server.sh\nindex f17be6b..178b9b4 100755\n--- a/t/t9400-git-cvsserver-server.sh\n+++ b/t/t9400-git-cvsserver-server.sh\n@@ -45,7 +45,7 @@ test_expect_success 'cvs update (create new file)' \\\n   'echo testfile1 >testfile1 &&\n    git add testfile1 &&\n    git commit -q -m \"Add testfile1\" &&\n-   git push gitcvs.git >/dev/null &&\n+   git push $(pwd)/gitcvs.git >/dev/null &&\n    cd cvswork &&\n    cvs -Q update &&\n    test \"$(echo $(grep testfile1 CVS/Entries|cut -d/ -f2,3,5))\" = \"testfile1/1.1/\" &&\n@@ -56,7 +56,7 @@ test_expect_success 'cvs update (update existing file)' \\\n   'echo line 2 >>testfile1 &&\n    git add testfile1 &&\n    git commit -q -m \"Append to testfile1\" &&\n-   git push gitcvs.git >/dev/null &&\n+   git push $(pwd)/gitcvs.git >/dev/null &&\n    cd cvswork &&\n    cvs -Q update &&\n    test \"$(echo $(grep testfile1 CVS/Entries|cut -d/ -f2,3,5))\" = \"testfile1/1.2/\" &&\n@@ -69,7 +69,7 @@ test_expect_failure \"cvs update w/o -d doesn't create subdir (TODO)\" \\\n    echo >test/empty &&\n    git add test &&\n    git commit -q -m \"Single Subdirectory\" &&\n-   git push gitcvs.git >/dev/null &&\n+   git push $(pwd)/gitcvs.git >/dev/null &&\n    cd cvswork &&\n    cvs -Q update &&\n    test ! -d test'\n@@ -82,7 +82,7 @@ test_expect_success 'cvs update (subdirectories)' \\\n       git add $dir;\n    done) &&\n    git commit -q -m \"deep sub directory structure\" &&\n-   git push gitcvs.git >/dev/null &&\n+   git push $(pwd)/gitcvs.git >/dev/null &&\n    cd cvswork &&\n    cvs -Q update -d &&\n    (for dir in A A/B A/B/C A/D E; do\n@@ -100,7 +100,7 @@ cd \"$WORKDIR\"\n test_expect_success 'cvs update (delete file)' \\\n   'git rm testfile1 &&\n    git commit -q -m \"Remove testfile1\" &&\n-   git push gitcvs.git >/dev/null &&\n+   git push $(pwd)/gitcvs.git >/dev/null &&\n    cd cvswork &&\n    cvs -Q update &&\n    test -z \"$(grep testfile1 CVS/Entries)\" &&\n@@ -111,7 +111,7 @@ test_expect_success 'cvs update (re-add deleted file)' \\\n   'echo readded testfile >testfile1 &&\n    git add testfile1 &&\n    git commit -q -m \"Re-Add testfile1\" &&\n-   git push gitcvs.git >/dev/null &&\n+   git push $(pwd)/gitcvs.git >/dev/null &&\n    cd cvswork &&\n    cvs -Q update &&\n    test \"$(echo $(grep testfile1 CVS/Entries|cut -d/ -f2,3,5))\" = \"testfile1/1.4/\" &&\n"},{"id":"41715","messageId":"7vmz0dktdf.fsf@assigned-by-dhcp.cox.net","threadId":"8063","inReplyTo":"Pine.LNX.4.64.0705100328260.18541@iabervon.org","subject":"Re: [PATCH 1/3] Move remote parsing into a library file out of builtin-push.","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2007-05-10T07:52:44Z","receivedAt":"2007-05-10T07:52:44Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Daniel Barkalow <barkalow@iabervon.org> writes:\n\n> On Thu, 10 May 2007, Junio C Hamano wrote:\n>\n>> This seems to break t9400, with \"fatal: bad repository 'gitcvs.git\",\n>> upon \"git push\".\n>> \n>> : gitster t/db/remote; sh t9400-git-cvsserver-server.sh -i -v\n>> * expecting success: cvs -Q co -d cvswork master &&\n>>    test \"$(echo $(grep -v ^D cvswork/CVS/Entries|cut -d/ -f2,3,5))\" = \"empty/1.1/\"\n>> cvs checkout: Updating cvswork\n>> U cvswork/empty\n>> *   ok 1: basic checkout\n>> \n>> * expecting success: echo testfile1 >testfile1 &&\n>>    git add testfile1 &&\n>>    git commit -q -m \"Add testfile1\" &&\n>>    git push gitcvs.git >/dev/null &&\n>\n> The man page doesn't think this is valid, since it only claims absolute \n> paths to work for local repositories.\n\nDoes it?  I suspect we need to fix the manpage then, as it is\nfairly common to do \n\n\t$ git fetch ../next-door-neighbour\n\nand expect the opposite to work as well.\n\nAnd I think it does today.\n"},{"id":"41718","messageId":"Pine.LNX.4.64.0705100355560.18541@iabervon.org","threadId":"8063","inReplyTo":"7vmz0dktdf.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH 1/3] Move remote parsing into a library file out of builtin-push.","fromName":"Daniel Barkalow","fromEmail":"barkalow@iabervon.org","sentAt":"2007-05-10T08:04:10Z","receivedAt":"2007-05-10T08:04:10Z","isPatch":true,"sender":{"key":"barkalow@iabervon.org","avatar":"https://avatars.githubusercontent.com/u/55364219?v=4"},"body":"On Thu, 10 May 2007, Junio C Hamano wrote:\n\n> Daniel Barkalow <barkalow@iabervon.org> writes:\n> \n> > On Thu, 10 May 2007, Junio C Hamano wrote:\n> >\n> >> This seems to break t9400, with \"fatal: bad repository 'gitcvs.git\",\n> >> upon \"git push\".\n> >> \n> >> : gitster t/db/remote; sh t9400-git-cvsserver-server.sh -i -v\n> >> * expecting success: cvs -Q co -d cvswork master &&\n> >>    test \"$(echo $(grep -v ^D cvswork/CVS/Entries|cut -d/ -f2,3,5))\" = \"empty/1.1/\"\n> >> cvs checkout: Updating cvswork\n> >> U cvswork/empty\n> >> *   ok 1: basic checkout\n> >> \n> >> * expecting success: echo testfile1 >testfile1 &&\n> >>    git add testfile1 &&\n> >>    git commit -q -m \"Add testfile1\" &&\n> >>    git push gitcvs.git >/dev/null &&\n> >\n> > The man page doesn't think this is valid, since it only claims absolute \n> > paths to work for local repositories.\n> \n> Does it?  I suspect we need to fix the manpage then, as it is\n> fairly common to do \n> \n> \t$ git fetch ../next-door-neighbour\n> \n> and expect the opposite to work as well.\n> \n> And I think it does today.\n\nHmm, and I guess URIs on the command line work the same way. How about \nrequiring a '/' somewhere in a repository argument in order to treat it as \na repository instead of a remote name? Then \"../next-door-neighbour\" would \nwork, \"./gitcvs.git\" would work (in the odd case where you actually have a \nbare repository sitting in your working directory), but we'd avoid the \ncurrent default of pushing to a bare repository in \"./origin/\" if nothing \nat all is configured.\n\n\t-Daniel\n*This .sig left intentionally blank*\n"},{"id":"41720","messageId":"7virb1ks1v.fsf@assigned-by-dhcp.cox.net","threadId":"8063","inReplyTo":"Pine.LNX.4.64.0705100355560.18541@iabervon.org","subject":"Re: [PATCH 1/3] Move remote parsing into a library file out of builtin-push.","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2007-05-10T08:21:16Z","receivedAt":"2007-05-10T08:21:16Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Daniel Barkalow <barkalow@iabervon.org> writes:\n\n>> And I think it does today.\n>\n> Hmm, and I guess URIs on the command line work the same way. How about \n> requiring a '/' somewhere in a repository argument in order to treat it as \n> a repository instead of a remote name? Then \"../next-door-neighbour\" would \n> work, \"./gitcvs.git\" would work (in the odd case where you actually have a \n> bare repository sitting in your working directory), but we'd avoid the \n> current default of pushing to a bare repository in \"./origin/\" if nothing \n> at all is configured.\n\nWhen I wrote the message you are responding to, I thought this\nwas a regression from the current behaviour, which (IIRC--it's\ngetting late and I am tired to double check) essentially says if\nthe token is a name of the directory, the target repository is a\nlocal one, but \"we'd avoid...\" part seems to suggest that you\nactually did this deliberately as a fix to some problem in the\ncurrent behaviour.  I am not however sure what it exactly is.\nCould you care to elaborate the part after \"we'd avoid...\" to\nclarify what the problem is, please?\n"},{"id":"41722","messageId":"Pine.LNX.4.64.0705100421490.18541@iabervon.org","threadId":"8063","inReplyTo":"7virb1ks1v.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH 1/3] Move remote parsing into a library file out of builtin-push.","fromName":"Daniel Barkalow","fromEmail":"barkalow@iabervon.org","sentAt":"2007-05-10T08:33:40Z","receivedAt":"2007-05-10T08:33:40Z","isPatch":true,"sender":{"key":"barkalow@iabervon.org","avatar":"https://avatars.githubusercontent.com/u/55364219?v=4"},"body":"On Thu, 10 May 2007, Junio C Hamano wrote:\n\n> Daniel Barkalow <barkalow@iabervon.org> writes:\n> \n> >> And I think it does today.\n> >\n> > Hmm, and I guess URIs on the command line work the same way. How about \n> > requiring a '/' somewhere in a repository argument in order to treat it as \n> > a repository instead of a remote name? Then \"../next-door-neighbour\" would \n> > work, \"./gitcvs.git\" would work (in the odd case where you actually have a \n> > bare repository sitting in your working directory), but we'd avoid the \n> > current default of pushing to a bare repository in \"./origin/\" if nothing \n> > at all is configured.\n> \n> When I wrote the message you are responding to, I thought this\n> was a regression from the current behaviour, which (IIRC--it's\n> getting late and I am tired to double check) essentially says if\n> the token is a name of the directory, the target repository is a\n> local one, but \"we'd avoid...\" part seems to suggest that you\n> actually did this deliberately as a fix to some problem in the\n> current behaviour.  I am not however sure what it exactly is.\n> Could you care to elaborate the part after \"we'd avoid...\" to\n> clarify what the problem is, please?\n\nThe problem, in general, is that, if the remote name you specify (or \n\"origin\" if you don't specify any) is not configured as a remote, it is \ntreated as a filename in the current directory for a local push. E.g.:\n\n$ git init\n$ git push\nfatal: 'origin': unable to chdir or not a git archive\nfatal: The remote end hung up unexpectedly\n\nIt's actually trying to push to ./origin/, which is totally nuts as a \ndefault repository to push to. Similarly, if you typo an actual remote \nname. Furthermore, builtin-push.c has an error message for the situation \nwhere the repository specification is wrong, suggesting that there is \nsome invalid repository specification, but it isn't reachable. And it \ncarefully prevents remote names from starting with a '/', suggestion that \nthat is the distinguishing characteristic between directly-specified \nrepository URIs and configured remotes (which can't really be right, of \ncourse).\n\nI think the right answer is to say that configured remotes cannot contain \nslashes, and directly-specified URIs must contain slashes, and it'll all \nbe clear.\n\n\t-Daniel\n*This .sig left intentionally blank*\n"},{"id":"41723","messageId":"7v7irhkreh.fsf@assigned-by-dhcp.cox.net","threadId":"8063","inReplyTo":"Pine.LNX.4.64.0705100355560.18541@iabervon.org","subject":"Re: [PATCH 1/3] Move remote parsing into a library file out of builtin-push.","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2007-05-10T08:35:18Z","receivedAt":"2007-05-10T08:35:18Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Daniel Barkalow <barkalow@iabervon.org> writes:\n\n> ..., \"./gitcvs.git\" would work (in the odd case where you actually have a \n> bare repository sitting in your working directory), ...\n\nThis is a bit of tangent, because it does not change the issue\nwe are discussing, and I suspect you already know this.\n\nBut to avoid future confusion by people on the list who read\nthis in the archive...\n\n\t\"foo.git\" does _NOT_ mean the directory is a bare\n\trepository.  It is perfectly normal to have \"foo.git/\"\n\tthat has a working tree whose repository data lives in\n\t\"foo.git/.git\".\n\nI _think_ (meaning, I haven't polled the userbase) requiring '/'\nmay not break people's existing setup too badly, while it _is_ a\nregression in the sense that we suddenly start disallowing\nsomething we allowed for a long time, perhaps without no\napparent good reason.\n\nI hope you'll correct me about \"without no apparent good reason\"\npart, which was what I was asking in my previous message.  With\na good rationale, it is much easier to sell a change that is a\nregression in the strictest sense but is unlikely to hurt people\nin practice.\n"},{"id":"41724","messageId":"7v3b25kr0t.fsf@assigned-by-dhcp.cox.net","threadId":"8063","inReplyTo":"Pine.LNX.4.64.0705100421490.18541@iabervon.org","subject":"Re: [PATCH 1/3] Move remote parsing into a library file out of builtin-push.","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2007-05-10T08:43:30Z","receivedAt":"2007-05-10T08:43:30Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Daniel Barkalow <barkalow@iabervon.org> writes:\n\n> On Thu, 10 May 2007, Junio C Hamano wrote:\n>\n>> Daniel Barkalow <barkalow@iabervon.org> writes:\n>> \n>> >> And I think it does today.\n>> >\n>> > Hmm, and I guess URIs on the command line work the same way. How about \n>> > requiring a '/' somewhere in a repository argument in order to treat it as \n>> > a repository instead of a remote name? Then \"../next-door-neighbour\" would \n>> > work, \"./gitcvs.git\" would work (in the odd case where you actually have a \n>> > bare repository sitting in your working directory), but we'd avoid the \n>> > current default of pushing to a bare repository in \"./origin/\" if nothing \n>> > at all is configured.\n>> \n>> When I wrote the message you are responding to, I thought this\n>> was a regression from the current behaviour, which (IIRC--it's\n>> getting late and I am tired to double check) essentially says if\n>> the token is a name of the directory, the target repository is a\n>> local one, but \"we'd avoid...\" part seems to suggest that you\n>> actually did this deliberately as a fix to some problem in the\n>> current behaviour.  I am not however sure what it exactly is.\n>> Could you care to elaborate the part after \"we'd avoid...\" to\n>> clarify what the problem is, please?\n>\n> The problem, in general, is that, if the remote name you specify (or \n> \"origin\" if you don't specify any) is not configured as a remote, it is \n> treated as a filename in the current directory for a local push. E.g.:\n>\n> $ git init\n> $ git push\n> fatal: 'origin': unable to chdir or not a git archive\n> fatal: The remote end hung up unexpectedly\n\nAhh.  You were trying to give it a better error message.\n\nI think I lied in the previous message.  I said we try to see if\nit is a local directory name before using that name, but we do\nnot do it, and leave the error detection to the lower level on\nthe other side (push spawns send-pack which in turn spawns\nreceive-pack) instead.\n\nPerhaps an alternative is to see if the name is configured as a\nremote (if so, we obviously use it), and if not do stat() to see\nif it is a directory (if so, use it as a local repository).\nThen we do not have to impose new restriction of slash at all,\nalthough it might complicate the code a bit more.\n"},{"id":"41751","messageId":"Pine.LNX.4.64.0705101150020.18541@iabervon.org","threadId":"8063","inReplyTo":"7v3b25kr0t.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH 1/3] Move remote parsing into a library file out of builtin-push.","fromName":"Daniel Barkalow","fromEmail":"barkalow@iabervon.org","sentAt":"2007-05-10T16:40:44Z","receivedAt":"2007-05-10T16:40:44Z","isPatch":true,"sender":{"key":"barkalow@iabervon.org","avatar":"https://avatars.githubusercontent.com/u/55364219?v=4"},"body":"On Thu, 10 May 2007, Junio C Hamano wrote:\n\n> Daniel Barkalow <barkalow@iabervon.org> writes:\n> \n> > On Thu, 10 May 2007, Junio C Hamano wrote:\n> >\n> >> Daniel Barkalow <barkalow@iabervon.org> writes:\n> >> \n> >> >> And I think it does today.\n> >> >\n> >> > Hmm, and I guess URIs on the command line work the same way. How about \n> >> > requiring a '/' somewhere in a repository argument in order to treat it as \n> >> > a repository instead of a remote name? Then \"../next-door-neighbour\" would \n> >> > work, \"./gitcvs.git\" would work (in the odd case where you actually have a \n> >> > bare repository sitting in your working directory), but we'd avoid the \n> >> > current default of pushing to a bare repository in \"./origin/\" if nothing \n> >> > at all is configured.\n> >> \n> >> When I wrote the message you are responding to, I thought this\n> >> was a regression from the current behaviour, which (IIRC--it's\n> >> getting late and I am tired to double check) essentially says if\n> >> the token is a name of the directory, the target repository is a\n> >> local one, but \"we'd avoid...\" part seems to suggest that you\n> >> actually did this deliberately as a fix to some problem in the\n> >> current behaviour.  I am not however sure what it exactly is.\n> >> Could you care to elaborate the part after \"we'd avoid...\" to\n> >> clarify what the problem is, please?\n> >\n> > The problem, in general, is that, if the remote name you specify (or \n> > \"origin\" if you don't specify any) is not configured as a remote, it is \n> > treated as a filename in the current directory for a local push. E.g.:\n> >\n> > $ git init\n> > $ git push\n> > fatal: 'origin': unable to chdir or not a git archive\n> > fatal: The remote end hung up unexpectedly\n> \n> Ahh.  You were trying to give it a better error message.\n> \n> I think I lied in the previous message.  I said we try to see if\n> it is a local directory name before using that name, but we do\n> not do it, and leave the error detection to the lower level on\n> the other side (push spawns send-pack which in turn spawns\n> receive-pack) instead.\n> \n> Perhaps an alternative is to see if the name is configured as a\n> remote (if so, we obviously use it), and if not do stat() to see\n> if it is a directory (if so, use it as a local repository).\n> Then we do not have to impose new restriction of slash at all,\n> although it might complicate the code a bit more.\n\nThe problem I see with allowing paths without slashes is if you've got a \nsubproject with a name similar to a remote name, and type the wrong one \n(particularly due to tab-completion), or if you've got a remote name you \nuse in other projects that matches a subproject in a project where you \naren't using that remote name. I think that a repository in your working \ndirectory is unlikely to be something you actually want to push to or \nfetch from (and if this is actually what you want, ./directory is the \nusual unixy thing for saying, no really, I want a relative path in the \ncurrent directory, and would work here; and it would be good practice \nanyway, so that you don't get tripped up if you create a remote \nconfiguration with that name later). Obviously, ../something has a slash; \nit should also take ':' to mean a URI (which I didn't realize last night), \nso that \"person@machine:directory.git\" is a URI.\n\nI think, as a general rule, that it would be cleanest to distinguish \nlexically between repository names that indicate configured remotes and \nrepository names that are URIs, particularly if we only break usage that \npeople only do for writing git regression tests.\n\n\t-Daniel\n*This .sig left intentionally blank*\n"}]}