{"thread":{"id":"13769","subject":"[PATCH v2 2/2] Make old sha1 optional with git update-ref -d","startedAt":"2008-06-02T23:34:42Z","lastAt":"2008-06-03T19:39:23Z","messageCount":6,"participants":["Karl Hasselström","Junio C Hamano"],"isPatch":true,"patchVersion":2,"patchTotal":2},"messages":[{"id":"78435","messageId":"20080602233244.26087.51827.stgit@yoghurt","threadId":"13769","inReplyTo":"20080525193213.GB5674@diana.vm.bytemark.co.uk","subject":"[PATCH v2 0/2] Make old sha1 optional with git update-ref -d","fromName":"Karl Hasselström","fromEmail":"kha@treskal.com","sentAt":"2008-06-02T23:34:42Z","receivedAt":"2008-06-02T23:34:42Z","isPatch":true,"sender":{"key":"kha@treskal.com","avatar":"https://gravatar.com/avatar/f0120c734b5279b345075a28521e1ac66acb20c9913ffe9bf6ae97e53f7f3f13?d=mp&s=160"},"body":"This series makes the old ref value optional for update-ref -d (it's\nalready optional for update-ref without -d).\n\n---\n\nPatch 1/2 updated after comments from Junio.\n\nKarl Hasselström (2):\n Make old sha1 optional with git update-ref -d\n Clean up builtin-update-ref's option parsing\n\n\n Documentation/git-update-ref.txt | 2 +-\n builtin-update-ref.c | 36 ++++++++++++++++++++----------------\n t/t1400-update-ref.sh | 8 ++++++++\n 3 files changed, 29 insertions(+), 17 deletions(-)\n\n-- \nKarl Hasselström, kha@treskal.com\n www.treskal.com/kalle\n"},{"id":"78434","messageId":"20080602233447.26087.14533.stgit@yoghurt","threadId":"13769","inReplyTo":"20080602233244.26087.51827.stgit@yoghurt","subject":"[PATCH v2 1/2] Clean up builtin-update-ref's option parsing","fromName":"Karl Hasselström","fromEmail":"kha@treskal.com","sentAt":"2008-06-02T23:34:48Z","receivedAt":"2008-06-02T23:34:48Z","isPatch":true,"sender":{"key":"kha@treskal.com","avatar":"https://gravatar.com/avatar/f0120c734b5279b345075a28521e1ac66acb20c9913ffe9bf6ae97e53f7f3f13?d=mp&s=160"},"body":"builtin-update-ref's option parsing was somewhat tricky to follow,\nespecially if the -d option was given. This patch cleans it upp a bit,\nat the expense of making it a bit longer.\n\nSigned-off-by: Karl Hasselström \n\n---\n\n builtin-update-ref.c | 34 +++++++++++++++++++---------------\n 1 files changed, 19 insertions(+), 15 deletions(-)\n\n\ndiff --git a/builtin-update-ref.c b/builtin-update-ref.c\nindex 93c1271..cc8bc5c 100644\n--- a/builtin-update-ref.c\n+++ b/builtin-update-ref.c\n@@ -11,7 +11,7 @@ static const char * const git_update_ref_usage[] = {\n \n int cmd_update_ref(int argc, const char **argv, const char *prefix)\n {\n-\tconst char *refname, *value, *oldval, *msg=NULL;\n+\tconst char *refname, *oldval, *msg=NULL;\n \tunsigned char sha1[20], oldsha1[20];\n \tint delete = 0, no_deref = 0;\n \tstruct option options[] = {\n@@ -27,25 +27,29 @@ int cmd_update_ref(int argc, const char **argv, const char *prefix)\n \tif (msg && !*msg)\n \t\tdie(\"Refusing to perform update with empty message.\");\n \n-\tif (argc < 2 || argc > 3)\n-\t\tusage_with_options(git_update_ref_usage, options);\n-\trefname = argv[0];\n-\tvalue = argv[1];\n-\toldval = argv[2];\n-\n-\tif (get_sha1(value, sha1))\n-\t\tdie(\"%s: not a valid SHA1\", value);\n-\n \tif (delete) {\n-\t\tif (oldval)\n+\t\tif (argc != 2)\n+\t\t\tusage_with_options(git_update_ref_usage, options);\n+\t\trefname = argv[0];\n+\t\toldval = argv[1];\n+\t} else {\n+ const char *value;\n+\t\tif (argc < 2 || argc > 3)\n \t\t\tusage_with_options(git_update_ref_usage, options);\n-\t\treturn delete_ref(refname, sha1);\n+\t\trefname = argv[0];\n+\t\tvalue = argv[1];\n+\t\toldval = argv[2];\n+\t\tif (get_sha1(value, sha1))\n+\t\t\tdie(\"%s: not a valid SHA1\", value);\n \t}\n \n-\thashclr(oldsha1);\n+\thashclr(oldsha1); /* all-zero hash in case oldval is the empty string */\n \tif (oldval && *oldval && get_sha1(oldval, oldsha1))\n \t\tdie(\"%s: not a valid old SHA1\", oldval);\n \n-\treturn update_ref(msg, refname, sha1, oldval ? oldsha1 : NULL,\n-\t\t\t no_deref ? REF_NODEREF : 0, DIE_ON_ERR);\n+\tif (delete)\n+\t\treturn delete_ref(refname, oldsha1);\n+\telse\n+\t\treturn update_ref(msg, refname, sha1, oldval ? oldsha1 : NULL,\n+\t\t\t\t no_deref ? REF_NODEREF : 0, DIE_ON_ERR);\n }\n"},{"id":"78433","messageId":"20080602233453.26087.52259.stgit@yoghurt","threadId":"13769","inReplyTo":"20080602233244.26087.51827.stgit@yoghurt","subject":"[PATCH v2 2/2] Make old sha1 optional with git update-ref -d","fromName":"Karl Hasselström","fromEmail":"kha@treskal.com","sentAt":"2008-06-02T23:34:53Z","receivedAt":"2008-06-02T23:34:53Z","isPatch":true,"sender":{"key":"kha@treskal.com","avatar":"https://gravatar.com/avatar/f0120c734b5279b345075a28521e1ac66acb20c9913ffe9bf6ae97e53f7f3f13?d=mp&s=160"},"body":"Giving the old sha1 is already optional when changing a ref, and it's\nquite handy when running update-ref manually. So make it optional for\ndeleting a ref too.\n\nSigned-off-by: Karl Hasselström \n\n---\n\n Documentation/git-update-ref.txt | 2 +-\n builtin-update-ref.c | 6 +++---\n t/t1400-update-ref.sh | 8 ++++++++\n 3 files changed, 12 insertions(+), 4 deletions(-)\n\n\ndiff --git a/Documentation/git-update-ref.txt b/Documentation/git-update-ref.txt\nindex 4dc4759..80b94c3 100644\n--- a/Documentation/git-update-ref.txt\n+++ b/Documentation/git-update-ref.txt\n@@ -7,7 +7,7 @@ git-update-ref - Update the object name stored in a ref safely\n \n SYNOPSIS\n --------\n-'git-update-ref' [-m ] (-d | [--no-deref] [])\n+'git-update-ref' [-m ] (-d [] | [--no-deref] [])\n \n DESCRIPTION\n -----------\ndiff --git a/builtin-update-ref.c b/builtin-update-ref.c\nindex cc8bc5c..9546488 100644\n--- a/builtin-update-ref.c\n+++ b/builtin-update-ref.c\n@@ -4,7 +4,7 @@\n #include \"parse-options.h\"\n \n static const char * const git_update_ref_usage[] = {\n-\t\"git-update-ref [options] -d \",\n+\t\"git-update-ref [options] -d []\",\n \t\"git-update-ref [options] []\",\n \tNULL\n };\n@@ -28,7 +28,7 @@ int cmd_update_ref(int argc, const char **argv, const char *prefix)\n \t\tdie(\"Refusing to perform update with empty message.\");\n \n \tif (delete) {\n-\t\tif (argc != 2)\n+\t\tif (argc < 1 || argc > 2)\n \t\t\tusage_with_options(git_update_ref_usage, options);\n \t\trefname = argv[0];\n \t\toldval = argv[1];\n@@ -48,7 +48,7 @@ int cmd_update_ref(int argc, const char **argv, const char *prefix)\n \t\tdie(\"%s: not a valid old SHA1\", oldval);\n \n \tif (delete)\n-\t\treturn delete_ref(refname, oldsha1);\n+\t\treturn delete_ref(refname, oldval ? oldsha1 : NULL);\n \telse\n \t\treturn update_ref(msg, refname, sha1, oldval ? oldsha1 : NULL,\n \t\t\t\t no_deref ? REF_NODEREF : 0, DIE_ON_ERR);\ndiff --git a/t/t1400-update-ref.sh b/t/t1400-update-ref.sh\nindex b8b7ab4..f387d46 100755\n--- a/t/t1400-update-ref.sh\n+++ b/t/t1400-update-ref.sh\n@@ -42,6 +42,14 @@ test_expect_success \"delete $m\" '\n '\n rm -f .git/$m\n \n+test_expect_success \"delete $m without oldvalue verification\" \"\n+\tgit update-ref $m $A &&\n+\ttest $A = \\$(cat .git/$m) &&\n+\tgit update-ref -d $m &&\n+\t! test -f .git/$m\n+\"\n+rm -f .git/$m\n+\n test_expect_success \\\n \t\"fail to create $n\" \\\n \t\"touch .git/$n_dir\n"},{"id":"78460","messageId":"7vk5h73w2i.fsf@gitster.siamese.dyndns.org","threadId":"13769","inReplyTo":"20080602233453.26087.52259.stgit@yoghurt","subject":"Re: [PATCH v2 2/2] Make old sha1 optional with git update-ref -d","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-06-03T05:51:33Z","receivedAt":"2008-06-03T05:51:33Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Karl Hasselström writes:\n\n> Giving the old sha1 is already optional when changing a ref, and it's\n> quite handy when running update-ref manually. So make it optional for\n> deleting a ref too.\n>\n> Signed-off-by: Karl Hasselström \n\n\"Handy\" is not a very good reason when talking about plumbing command like\nupdate-ref that takes an extra parameter for safety of the last step in\nread, operate, verify-and-update sequence.\n\nAlthough it is not a reason _bad enough_ to make your patch a bad idea,\nperhaps you should rethink the problem at the same time? Whatever you are\ntrying to do cannot be done without manually invoking update-ref directly\nby the end user, perhaps that needs to be addressed?\n"},{"id":"78462","messageId":"20080603064957.GA6701@diana.vm.bytemark.co.uk","threadId":"13769","inReplyTo":"7vk5h73w2i.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH v2 2/2] Make old sha1 optional with git update-ref -d","fromName":"Karl Hasselström","fromEmail":"kha@treskal.com","sentAt":"2008-06-03T06:49:57Z","receivedAt":"2008-06-03T06:49:57Z","isPatch":true,"sender":{"key":"kha@treskal.com","avatar":"https://gravatar.com/avatar/f0120c734b5279b345075a28521e1ac66acb20c9913ffe9bf6ae97e53f7f3f13?d=mp&s=160"},"body":"On 2008-06-02 22:51:33 -0700, Junio C Hamano wrote:\n\n> Karl Hasselström writes:\n>\n> > Giving the old sha1 is already optional when changing a ref, and\n> > it's quite handy when running update-ref manually. So make it\n> > optional for deleting a ref too.\n>\n> \"Handy\" is not a very good reason when talking about plumbing\n> command like update-ref that takes an extra parameter for safety of\n> the last step in read, operate, verify-and-update sequence.\n>\n> Although it is not a reason _bad enough_ to make your patch a bad\n> idea, perhaps you should rethink the problem at the same time?\n> Whatever you are trying to do cannot be done without manually\n> invoking update-ref directly by the end user, perhaps that needs to\n> be addressed?\n\nI need this from time to time when experimenting with StGit -- or,\nmore precisely, when cleaning up afterwards in case it didn't go so\nwell. Packed refs means I can't safely just edit/delete files under\n.git/refs anymore, so I use update-ref instead. And having to use the\nsafety belt every time is just plain tedious. I'd say this is a\nperfect example of when using the plumbing by hand is really what you\nwant.\n\n(In this kind of situation, branch -D often doesn't work since my ref\nisn't under refs/heads, and besides I really don't want any of it's\nsmarts right then -- I want to delete a ref, nothing more.)\n\nBetter suggestions welcome, of course.\n\n-- \nKarl Hasselström, kha@treskal.com\n www.treskal.com/kalle\n"},{"id":"78533","messageId":"7v3anu2tqs.fsf@gitster.siamese.dyndns.org","threadId":"13769","inReplyTo":"20080603064957.GA6701@diana.vm.bytemark.co.uk","subject":"Re: [PATCH v2 2/2] Make old sha1 optional with git update-ref -d","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-06-03T19:39:23Z","receivedAt":"2008-06-03T19:39:23Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Karl Hasselström writes:\n\n> I need this from time to time when experimenting with StGit -- or,\n> more precisely, when cleaning up afterwards in case it didn't go so\n> well. Packed refs means I can't safely just edit/delete files under\n> .git/refs anymore, so I use update-ref instead. And having to use the\n> safety belt every time is just plain tedious. I'd say this is a\n> perfect example of when using the plumbing by hand is really what you\n> want.\n\nOh, I should have guessed, and I am relieved by your answer.\n\nI was worried that in some workflow people may have to use \"update-ref -d\"\nall the time because our Porcelain allows them to create some ref without\ngiving a clean way to remove it (e.g. \"branch\" and \"tag\" allow creation\nand deletion with -d/-D, so they are Ok, but I was worried there might be\nsomething else I overlooked).\n\nManually cleaning up what an experimental version of Porcelain left behind\ndoes not fall into category of \"normal user has to do this all the time\",\nso I do not have to worry.\n\nThanks.\n"}]}