{"thread":{"id":"4204","subject":"[PATCH 4/5] Log ref updates made by fetch.","startedAt":"2006-05-19T07:29:26Z","lastAt":"2006-05-19T08:03:14Z","messageCount":3,"participants":["Shawn Pearce","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":5},"messages":[{"id":"20275","messageId":"20060519072926.GE22257@spearce.org","threadId":"4204","inReplyTo":null,"subject":"[PATCH 4/5] Log ref updates made by fetch.","fromName":"Shawn Pearce","fromEmail":"spearce@spearce.org","sentAt":"2006-05-19T07:29:26Z","receivedAt":"2006-05-19T07:29:26Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"If a ref is changed by http-fetch, local-fetch or ssh-fetch\nrecord the change and the remote URL/name in the log for the ref.\nThis requires loading the config file to check logAllRefUpdates.\n\nAlso fixed a bug in the ref lock generation; the log file name was\nnot being produced right due to a bad prefix length.\n\nSigned-off-by: Shawn O. Pearce <spearce@spearce.org>\n\n---\n\n fetch.c       |   17 +++++++++++++++--\n fetch.h       |    3 +++\n http-fetch.c  |    2 ++\n local-fetch.c |    2 ++\n refs.c        |    5 +++--\n ssh-fetch.c   |    2 ++\n 6 files changed, 27 insertions(+), 4 deletions(-)\n\nb929e2a5a8f80c8635cf3c54a6d766902cf87434\ndiff --git a/fetch.c b/fetch.c\nindex 8bdaacb..fd57684 100644\n--- a/fetch.c\n+++ b/fetch.c\n@@ -8,6 +8,7 @@ #include \"blob.h\"\n #include \"refs.h\"\n \n const char *write_ref = NULL;\n+const char *write_ref_log_details = NULL;\n \n const unsigned char *current_ref = NULL;\n \n@@ -206,13 +207,17 @@ int pull(char *target)\n {\n \tstruct ref_lock *lock;\n \tunsigned char sha1[20];\n+\tchar *msg;\n+\tint ret;\n \n \tsave_commit_buffer = 0;\n \ttrack_object_refs = 0;\n \tif (write_ref) {\n \t\tlock = lock_ref_sha1(write_ref, current_ref, 1);\n-\t\tif (!lock)\n+\t\tif (!lock) {\n+\t\t\terror(\"Can't lock ref %s\", write_ref);\n \t\t\treturn -1;\n+\t\t}\n \t}\n \n \tif (!get_recover) {\n@@ -234,7 +239,15 @@ int pull(char *target)\n \t}\n \n \tif (write_ref) {\n-\t\treturn write_ref_sha1(lock, sha1, \"git fetch\");\n+\t\tif (write_ref_log_details) {\n+\t\t\tmsg = xmalloc(strlen(write_ref_log_details) + 12);\n+\t\t\tsprintf(msg, \"fetch from %s\", write_ref_log_details);\n+\t\t} else\n+\t\t\tmsg = NULL;\n+\t\tret = write_ref_sha1(lock, sha1, msg ? msg : \"fetch (unknown)\");\n+\t\tif (msg)\n+\t\t\tfree(msg);\n+\t\treturn ret;\n \t}\n \treturn 0;\n }\ndiff --git a/fetch.h b/fetch.h\nindex 9837a3d..0011548 100644\n--- a/fetch.h\n+++ b/fetch.h\n@@ -25,6 +25,9 @@ extern int fetch_ref(char *ref, unsigned\n /* If set, the ref filename to write the target value to. */\n extern const char *write_ref;\n \n+/* If set additional text will appear in the ref log. */\n+extern const char *write_ref_log_details;\n+\n /* If set, the hash that the current value of write_ref must be. */\n extern const unsigned char *current_ref;\n \ndiff --git a/http-fetch.c b/http-fetch.c\nindex 861644b..cc7bd1f 100644\n--- a/http-fetch.c\n+++ b/http-fetch.c\n@@ -1223,6 +1223,7 @@ int main(int argc, char **argv)\n \tint rc = 0;\n \n \tsetup_git_directory();\n+\tgit_config(git_default_config);\n \n \twhile (arg < argc && argv[arg][0] == '-') {\n \t\tif (argv[arg][1] == 't') {\n@@ -1249,6 +1250,7 @@ int main(int argc, char **argv)\n \t}\n \tcommit_id = argv[arg];\n \turl = argv[arg + 1];\n+\twrite_ref_log_details = url;\n \n \thttp_init();\n \ndiff --git a/local-fetch.c b/local-fetch.c\nindex fa9e697..ffa4887 100644\n--- a/local-fetch.c\n+++ b/local-fetch.c\n@@ -208,6 +208,7 @@ int main(int argc, char **argv)\n \tint arg = 1;\n \n \tsetup_git_directory();\n+\tgit_config(git_default_config);\n \n \twhile (arg < argc && argv[arg][0] == '-') {\n \t\tif (argv[arg][1] == 't')\n@@ -239,6 +240,7 @@ int main(int argc, char **argv)\n \t\tusage(local_pull_usage);\n \tcommit_id = argv[arg];\n \tpath = argv[arg + 1];\n+\twrite_ref_log_details = path;\n \n \tif (pull(commit_id))\n \t\treturn 1;\ndiff --git a/refs.c b/refs.c\nindex 31cf276..d3ddc82 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -142,6 +142,8 @@ static int do_for_each_ref(const char *b\n \t\t\tnamelen = strlen(de->d_name);\n \t\t\tif (namelen > 255)\n \t\t\t\tcontinue;\n+\t\t\tif (namelen>5 && !strcmp(de->d_name+namelen-5,\".lock\"))\n+\t\t\t\tcontinue;\n \t\t\tmemcpy(path + baselen, de->d_name, namelen+1);\n \t\t\tif (stat(git_path(\"%s\", path), &st) < 0)\n \t\t\t\tcontinue;\n@@ -296,7 +298,6 @@ static struct ref_lock* lock_ref_sha1_ba\n \tplen = strlen(path) - plen;\n \tpath = resolve_ref(path, lock->old_sha1, mustexist);\n \tif (!path) {\n-\t\terror(\"Can't read ref %s\", path);\n \t\tunlock_ref(lock);\n \t\treturn NULL;\n \t}\n@@ -326,7 +327,7 @@ struct ref_lock* lock_ref_sha1(const cha\n \tif (check_ref_format(ref))\n \t\treturn NULL;\n \treturn lock_ref_sha1_basic(git_path(\"refs/%s\", ref),\n-\t\tstrlen(ref), old_sha1, mustexist);\n+\t\t5 + strlen(ref), old_sha1, mustexist);\n }\n \n struct ref_lock* lock_any_ref_for_update(const char *ref,\ndiff --git a/ssh-fetch.c b/ssh-fetch.c\nindex 4eb9e04..e3067b8 100644\n--- a/ssh-fetch.c\n+++ b/ssh-fetch.c\n@@ -132,6 +132,7 @@ int main(int argc, char **argv)\n \tif (!prog) prog = \"git-ssh-upload\";\n \n \tsetup_git_directory();\n+\tgit_config(git_default_config);\n \n \twhile (arg < argc && argv[arg][0] == '-') {\n \t\tif (argv[arg][1] == 't') {\n@@ -158,6 +159,7 @@ int main(int argc, char **argv)\n \t}\n \tcommit_id = argv[arg];\n \turl = argv[arg + 1];\n+\twrite_ref_log_details = url;\n \n \tif (setup_connection(&fd_in, &fd_out, prog, url, arg, argv + 1))\n \t\treturn 1;\n-- \n1.3.2.g7278\n"},{"id":"20278","messageId":"7vodxuih2d.fsf@assigned-by-dhcp.cox.net","threadId":"4204","inReplyTo":"20060519072926.GE22257@spearce.org","subject":"Re: [PATCH 4/5] Log ref updates made by fetch.","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2006-05-19T07:56:10Z","receivedAt":"2006-05-19T07:56:10Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Shawn Pearce <spearce@spearce.org> writes:\n\n> diff --git a/refs.c b/refs.c\n> index 31cf276..d3ddc82 100644\n> --- a/refs.c\n> +++ b/refs.c\n> @@ -142,6 +142,8 @@ static int do_for_each_ref(const char *b\n>  \t\t\tnamelen = strlen(de->d_name);\n>  \t\t\tif (namelen > 255)\n>  \t\t\t\tcontinue;\n> +\t\t\tif (namelen>5 && !strcmp(de->d_name+namelen-5,\".lock\"))\n> +\t\t\t\tcontinue;\n>  \t\t\tmemcpy(path + baselen, de->d_name, namelen+1);\n>  \t\t\tif (stat(git_path(\"%s\", path), &st) < 0)\n>  \t\t\t\tcontinue;\n\nNow this got me worried.  Until now I did not realize that we\nare clobbering refnames that ends with \".lock\" if another ref\nwith the name without \".locK\" is updated.  Because we do not\nforbid a name that ends with \".lock\" to be used as a refname,\nthis is an accident waiting to happen.\n\nNot your fault, though.  It was like this ever since the initial\nversion of refs.c was accepted by Linus.\n\nWe could do one of two things: officially forbid any refname\nthat ends with \".lock\", or fix the lockfile naming convention.\n\nNobody should be relying on what the actual lockfile-to-be-\nrenamed-to-become-the-real-file is called.  I suspect we would\nwant to fix refs.c::ref_lock_file_name() to use a name that\nwould never be used as a refname.\n\nWe could make it begin with \".\", so the lock file for the master\n\".git/refs/heads/master\" would become \".git/refs/heads/.master\",\nfor example.  That way, we cannot clobber a valid unrelated ref\n(\".master\" is not a valid ref name), and as an added bonus, you\ndo not even have to have the above hunk.\n\nHmm?\n"},{"id":"20279","messageId":"20060519080314.GG22257@spearce.org","threadId":"4204","inReplyTo":"7vodxuih2d.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH 4/5] Log ref updates made by fetch.","fromName":"Shawn Pearce","fromEmail":"spearce@spearce.org","sentAt":"2006-05-19T08:03:14Z","receivedAt":"2006-05-19T08:03:14Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"Junio C Hamano <junkio@cox.net> wrote:\n> Shawn Pearce <spearce@spearce.org> writes:\n> \n> > diff --git a/refs.c b/refs.c\n> > index 31cf276..d3ddc82 100644\n> > --- a/refs.c\n> > +++ b/refs.c\n> > @@ -142,6 +142,8 @@ static int do_for_each_ref(const char *b\n> >  \t\t\tnamelen = strlen(de->d_name);\n> >  \t\t\tif (namelen > 255)\n> >  \t\t\t\tcontinue;\n> > +\t\t\tif (namelen>5 && !strcmp(de->d_name+namelen-5,\".lock\"))\n> > +\t\t\t\tcontinue;\n> >  \t\t\tmemcpy(path + baselen, de->d_name, namelen+1);\n> >  \t\t\tif (stat(git_path(\"%s\", path), &st) < 0)\n> >  \t\t\t\tcontinue;\n> \n> Now this got me worried.  Until now I did not realize that we\n> are clobbering refnames that ends with \".lock\" if another ref\n> with the name without \".locK\" is updated.  Because we do not\n> forbid a name that ends with \".lock\" to be used as a refname,\n> this is an accident waiting to happen.\n> \n> Not your fault, though.  It was like this ever since the initial\n> version of refs.c was accepted by Linus.\n> \n> We could do one of two things: officially forbid any refname\n> that ends with \".lock\", or fix the lockfile naming convention.\n> \n> Nobody should be relying on what the actual lockfile-to-be-\n> renamed-to-become-the-real-file is called.  I suspect we would\n> want to fix refs.c::ref_lock_file_name() to use a name that\n> would never be used as a refname.\n> \n> We could make it begin with \".\", so the lock file for the master\n> \".git/refs/heads/master\" would become \".git/refs/heads/.master\",\n> for example.  That way, we cannot clobber a valid unrelated ref\n> (\".master\" is not a valid ref name), and as an added bonus, you\n> do not even have to have the above hunk.\n\nNot a bad idea.  I found this bug because fetch.c was trying to\nwork on the bob.lock while it was fetching \"heads/bob\".  Which\ndidn't make sense to me, so I put this skip thing in...\n\nI'd want to get all of the \"lockers\" move to using the common\nlock code before changing the lock name to \".master\" (for example)\nbut yes that's a good idea.\n\n-- \nShawn.\n"}]}