{"thread":{"id":"12708","subject":"[PATCH/RFC] fast-import: allow \"reset\" without \"from\" to delete a branch","startedAt":"2008-03-15T14:59:49Z","lastAt":"2008-03-16T21:17:11Z","messageCount":4,"participants":["Eyvind Bernhardsen","Shawn O. Pearce","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"72186","messageId":"7AFA021C-062D-4FC2-85EB-1DD6C054BEA4@orakel.ntnu.no","threadId":"12708","inReplyTo":null,"subject":"[PATCH/RFC] fast-import: allow \"reset\" without \"from\" to delete a branch","fromName":"Eyvind Bernhardsen","fromEmail":"eyvind-git@orakel.ntnu.no","sentAt":"2008-03-15T14:59:49Z","receivedAt":"2008-03-15T14:59:49Z","isPatch":true,"sender":{"key":"eyvind.bernhardsen@gmail.com","avatar":"https://avatars.githubusercontent.com/u/106762?v=4"},"body":"Resetting a branch without \"from\" and not making any further commits\nto it currently causes fast-import to fail with an error message.\n\nThis patch prevents the error, allowing \"reset\" to be used to delete\na branch.\n\nSigned-off-by: Eyvind Bernhardsen \n---\nSince commit c3b0dec (\"Be more careful about updating refs\"), git fast- \nimport has given the following error message on every import from \ncvs2svn:\n\n\terror: Trying to write ref refs/heads/TAG.FIXUP with nonexistant \nobject 0000000000000000000000000000000000000000\n\terror: Unable to update refs/heads/TAG.FIXUP\n\nThe imported repository is fine, but the error message finally bugged \nme enough to figure out what was going on, and the explanation is \nsimple. If a branch is reset in fast-import, and no further commits \nare made on that branch, the final dump_branches() call in fast- \nimport.c fails.\n\ncvs2svn creates a TAG.FIXUP branch for every tag and then resets it \nafter the tag has been set. The intent is that TAG.FIXUP should be \ndeleted, and this patch makes that work without error (the branch is \nactually deleted even without this patch).\n\nIt's a small change and the test suite passes, but I'm not sure if \nusing reset to delete a branch is desired behaviour, so I would \nappreciate it if someone who actually knows what they are doing could \ntake a look at it :)\n\n fast-import.c | 5 +++--\n 1 files changed, 3 insertions(+), 2 deletions(-)\n\ndiff --git a/fast-import.c b/fast-import.c\nindex 655913d..989ba94 100644\n--- a/fast-import.c\n+++ b/fast-import.c\n@@ -1539,8 +1539,9 @@ static int update_branch(struct branch *b)\n \t\t\treturn -1;\n \t\t}\n \t}\n-\tif (write_ref_sha1(lock, b->sha1, msg) < 0)\n-\t\treturn error(\"Unable to update %s\", b->name);\n+\tif (!is_null_sha1(b->sha1))\n+\t\tif (write_ref_sha1(lock, b->sha1, msg) < 0)\n+\t\t\treturn error(\"Unable to update %s\", b->name);\n \treturn 0;\n }\n\n-- \n1.5.4.4.555.ga98c.dirty\n"},{"id":"72205","messageId":"20080316041240.GH8410@spearce.org","threadId":"12708","inReplyTo":"7AFA021C-062D-4FC2-85EB-1DD6C054BEA4@orakel.ntnu.no","subject":"Re: [PATCH/RFC] fast-import: allow \"reset\" without \"from\" to delete a branch","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2008-03-16T04:12:40Z","receivedAt":"2008-03-16T04:12:40Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"Eyvind Bernhardsen wrote:\n> It's a small change and the test suite passes, but I'm not sure if \n> using reset to delete a branch is desired behaviour, so I would \n> appreciate it if someone who actually knows what they are doing could \n> take a look at it :)\n\nI think this is a slightly better patch, as it avoids creating a\nlock file around the ref if we aren't going to actually alter it.\n\nAt present fast-import does not allow an application to delete a\nbranch that existed when fast-import started, but if the branch\nwas strictly transient within the fast-import process (like the\ncvs2svn TAG.FIXUP) then there is no problem.\n\nIs this patch acceptable? Note it is from you, I carried in your\ncommit message, SBO, etc.\n \n--8>--\nFrom: Eyvind Bernhardsen \nSubject: [PATCH] fast-import: allow \"reset\" without \"from\" to delete temporary branch\n\nResetting a branch without \"from\" and not making any further commits\nto it currently causes fast-import to fail with an error message.\n\nThis patch prevents the error, allowing \"reset\" to be used to delete\na branch.\n\nSigned-off-by: Eyvind Bernhardsen \nSigned-off-by: Shawn O. Pearce \n---\n fast-import.c | 2 ++\n 1 files changed, 2 insertions(+), 0 deletions(-)\n\ndiff --git a/fast-import.c b/fast-import.c\nindex 655913d..73e5439 100644\n--- a/fast-import.c\n+++ b/fast-import.c\n@@ -1516,6 +1516,8 @@ static int update_branch(struct branch *b)\n \tstruct ref_lock *lock;\n \tunsigned char old_sha1[20];\n \n+\tif (is_null_sha1(b->sha1))\n+\t\treturn 0;\n \tif (read_ref(b->name, old_sha1))\n \t\thashclr(old_sha1);\n \tlock = lock_any_ref_for_update(b->name, old_sha1, 0);\n-- \n1.5.4.4.640.g8ae62\n\n-- \nShawn.\n"},{"id":"72232","messageId":"283B81B0-4493-41DC-A575-F72910B1EFFA@orakel.ntnu.no","threadId":"12708","inReplyTo":"20080316041240.GH8410@spearce.org","subject":"[PATCH] fast-import: Allow \"reset\" to delete a new branch without error","fromName":"Eyvind Bernhardsen","fromEmail":"eyvind-git@orakel.ntnu.no","sentAt":"2008-03-16T19:49:09Z","receivedAt":"2008-03-16T19:49:09Z","isPatch":true,"sender":{"key":"eyvind.bernhardsen@gmail.com","avatar":"https://avatars.githubusercontent.com/u/106762?v=4"},"body":"Creating a branch in fast-import and then resetting it without making\nany further commits to it currently causes an error message at the\nend of the import.\n\nThis error is triggered by cvs2svn's git backend, which uses a\ntemporary fixup branch when it creates tags, because the fixup branch\nis reset after each tag.\n\nThis patch prevents the error, allowing \"reset\" to be used to delete\ntemporary branches.\n\nSigned-off-by: Eyvind Bernhardsen \n---\nOn 16. mars. 2008, at 05.12, Shawn O. Pearce wrote:\n\n> I think this is a slightly better patch, as it avoids creating a\n> lock file around the ref if we aren't going to actually alter it.\n\n\nYes, that's a much better patch, and since you pointed out that \nexisting branches won't be deleted, here it is again with a better \ncommit message. Thanks!\n\n fast-import.c | 2 ++\n 1 files changed, 2 insertions(+), 0 deletions(-)\n\ndiff --git a/fast-import.c b/fast-import.c\nindex 655913d..73e5439 100644\n--- a/fast-import.c\n+++ b/fast-import.c\n@@ -1516,6 +1516,8 @@ static int update_branch(struct branch *b)\n \tstruct ref_lock *lock;\n \tunsigned char old_sha1[20];\n\n+\tif (is_null_sha1(b->sha1))\n+\t\treturn 0;\n \tif (read_ref(b->name, old_sha1))\n \t\thashclr(old_sha1);\n \tlock = lock_any_ref_for_update(b->name, old_sha1, 0);\n-- \n1.5.4.4.608.gc20d.dirty\n"},{"id":"72241","messageId":"7vfxuqid2g.fsf@gitster.siamese.dyndns.org","threadId":"12708","inReplyTo":"283B81B0-4493-41DC-A575-F72910B1EFFA@orakel.ntnu.no","subject":"Re: [PATCH] fast-import: Allow \"reset\" to delete a new branch without error","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-03-16T21:17:11Z","receivedAt":"2008-03-16T21:17:11Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Eyvind Bernhardsen writes:\n\n> On 16. mars. 2008, at 05.12, Shawn O. Pearce wrote:\n>\n>> I think this is a slightly better patch, as it avoids creating a\n>> lock file around the ref if we aren't going to actually alter it.\n>\n> Yes, that's a much better patch, and since you pointed out that\n> existing branches won't be deleted, here it is again with a better\n> commit message. Thanks!\n\nI'll add Acked-by from Shawn and apply. Thanks, both.\n"}]}