{"thread":{"id":"32435","subject":"[PATCH 1/2] tests: move test_cmp_rev to test-lib-functions","startedAt":"2012-12-21T19:10:10Z","lastAt":"2012-12-24T07:20:10Z","messageCount":11,"participants":["Martin von Zweigbergk","Junio C Hamano","Christian Couder","Philip Oakley"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"205366","messageId":"1356117013-20613-1-git-send-email-martinvonz@gmail.com","threadId":"32435","inReplyTo":null,"subject":"[PATCH 1/2] tests: move test_cmp_rev to test-lib-functions","fromName":"Martin von Zweigbergk","fromEmail":"martinvonz@gmail.com","sentAt":"2012-12-21T19:10:10Z","receivedAt":"2012-12-21T19:10:10Z","isPatch":true,"sender":{"key":"martinvonz@gmail.com","avatar":"https://avatars.githubusercontent.com/u/891642?v=4"},"body":"A function for checking that two given parameters refer to the same\nrevision was defined in several places, so move the definition to\ntest-lib-functions.sh instead.\n\nSigned-off-by: Martin von Zweigbergk <martinvonz@gmail.com>\n---\n t/t1505-rev-parse-last.sh           | 18 +++++-------------\n t/t3404-rebase-interactive.sh       |  6 ------\n t/t3507-cherry-pick-conflict.sh     |  6 ------\n t/t3508-cherry-pick-many-commits.sh |  8 ++------\n t/t3510-cherry-pick-sequence.sh     |  6 ------\n t/t6030-bisect-porcelain.sh         |  4 +---\n t/test-lib-functions.sh             |  7 +++++++\n 7 files changed, 15 insertions(+), 40 deletions(-)\n\ndiff --git a/t/t1505-rev-parse-last.sh b/t/t1505-rev-parse-last.sh\nindex d709ecf..4969edb 100755\n--- a/t/t1505-rev-parse-last.sh\n+++ b/t/t1505-rev-parse-last.sh\n@@ -32,32 +32,24 @@ test_expect_success 'setup' '\n #\n # and 'side' should be the last branch\n \n-test_rev_equivalent () {\n-\n-\tgit rev-parse \"$1\" > expect &&\n-\tgit rev-parse \"$2\" > output &&\n-\ttest_cmp expect output\n-\n-}\n-\n test_expect_success '@{-1} works' '\n-\ttest_rev_equivalent side @{-1}\n+\ttest_cmp_rev side @{-1}\n '\n \n test_expect_success '@{-1}~2 works' '\n-\ttest_rev_equivalent side~2 @{-1}~2\n+\ttest_cmp_rev side~2 @{-1}~2\n '\n \n test_expect_success '@{-1}^2 works' '\n-\ttest_rev_equivalent side^2 @{-1}^2\n+\ttest_cmp_rev side^2 @{-1}^2\n '\n \n test_expect_success '@{-1}@{1} works' '\n-\ttest_rev_equivalent side@{1} @{-1}@{1}\n+\ttest_cmp_rev side@{1} @{-1}@{1}\n '\n \n test_expect_success '@{-2} works' '\n-\ttest_rev_equivalent master @{-2}\n+\ttest_cmp_rev master @{-2}\n '\n \n test_expect_success '@{-3} fails' '\ndiff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh\nindex 32fdc99..8462be1 100755\n--- a/t/t3404-rebase-interactive.sh\n+++ b/t/t3404-rebase-interactive.sh\n@@ -29,12 +29,6 @@ Initial setup:\n \n . \"$TEST_DIRECTORY\"/lib-rebase.sh\n \n-test_cmp_rev () {\n-\tgit rev-parse --verify \"$1\" >expect.rev &&\n-\tgit rev-parse --verify \"$2\" >actual.rev &&\n-\ttest_cmp expect.rev actual.rev\n-}\n-\n set_fake_editor\n \n # WARNING: Modifications to the initial repository can change the SHA ID used\ndiff --git a/t/t3507-cherry-pick-conflict.sh b/t/t3507-cherry-pick-conflict.sh\nindex c82f721..223b984 100755\n--- a/t/t3507-cherry-pick-conflict.sh\n+++ b/t/t3507-cherry-pick-conflict.sh\n@@ -11,12 +11,6 @@ test_description='test cherry-pick and revert with conflicts\n \n . ./test-lib.sh\n \n-test_cmp_rev () {\n-\tgit rev-parse --verify \"$1\" >expect.rev &&\n-\tgit rev-parse --verify \"$2\" >actual.rev &&\n-\ttest_cmp expect.rev actual.rev\n-}\n-\n pristine_detach () {\n \tgit checkout -f \"$1^0\" &&\n \tgit read-tree -u --reset HEAD &&\ndiff --git a/t/t3508-cherry-pick-many-commits.sh b/t/t3508-cherry-pick-many-commits.sh\nindex 340afc7..4e7136b 100755\n--- a/t/t3508-cherry-pick-many-commits.sh\n+++ b/t/t3508-cherry-pick-many-commits.sh\n@@ -5,15 +5,11 @@ test_description='test cherry-picking many commits'\n . ./test-lib.sh\n \n check_head_differs_from() {\n-\thead=$(git rev-parse --verify HEAD) &&\n-\targ=$(git rev-parse --verify \"$1\") &&\n-\ttest \"$head\" != \"$arg\"\n+\t! test_cmp_rev HEAD \"$1\"\n }\n \n check_head_equals() {\n-\thead=$(git rev-parse --verify HEAD) &&\n-\targ=$(git rev-parse --verify \"$1\") &&\n-\ttest \"$head\" = \"$arg\"\n+\ttest_cmp_rev HEAD \"$1\"\n }\n \n test_expect_success setup '\ndiff --git a/t/t3510-cherry-pick-sequence.sh b/t/t3510-cherry-pick-sequence.sh\nindex b5fb527..7b7a89d 100755\n--- a/t/t3510-cherry-pick-sequence.sh\n+++ b/t/t3510-cherry-pick-sequence.sh\n@@ -24,12 +24,6 @@ pristine_detach () {\n \tgit clean -d -f -f -q -x\n }\n \n-test_cmp_rev () {\n-\tgit rev-parse --verify \"$1\" >expect.rev &&\n-\tgit rev-parse --verify \"$2\" >actual.rev &&\n-\ttest_cmp expect.rev actual.rev\n-}\n-\n test_expect_success setup '\n \tgit config advice.detachedhead false &&\n \techo unrelated >unrelated &&\ndiff --git a/t/t6030-bisect-porcelain.sh b/t/t6030-bisect-porcelain.sh\nindex 72e28ee..3e0e15f 100755\n--- a/t/t6030-bisect-porcelain.sh\n+++ b/t/t6030-bisect-porcelain.sh\n@@ -676,9 +676,7 @@ test_expect_success 'bisect fails if tree is broken on trial commit' '\n check_same()\n {\n \techo \"Checking $1 is the same as $2\" &&\n-\tgit rev-parse \"$1\" > expected.same &&\n-\tgit rev-parse \"$2\" > expected.actual &&\n-\ttest_cmp expected.same expected.actual\n+\ttest_cmp_rev \"$1\" \"$2\"\n }\n \n test_expect_success 'bisect: --no-checkout - start commit bad' '\ndiff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh\nindex 22a4f8f..fa62d01 100644\n--- a/t/test-lib-functions.sh\n+++ b/t/test-lib-functions.sh\n@@ -602,6 +602,13 @@ test_cmp() {\n \t$GIT_TEST_CMP \"$@\"\n }\n \n+# Tests that its two parameters refer to the same revision\n+test_cmp_rev () {\n+\tgit rev-parse --verify \"$1\" >expect.rev &&\n+\tgit rev-parse --verify \"$2\" >actual.rev &&\n+\ttest_cmp expect.rev actual.rev\n+}\n+\n # Print a sequence of numbers or letters in increasing order.  This is\n # similar to GNU seq(1), but the latter might not be available\n # everywhere (and does not do letters).  It may be used like:\n-- \n1.8.0.1.240.ge8a1f5a\n"},{"id":"205368","messageId":"1356117013-20613-2-git-send-email-martinvonz@gmail.com","threadId":"32435","inReplyTo":"1356117013-20613-1-git-send-email-martinvonz@gmail.com","subject":"[PATCH 2/2] learn to pick/revert into unborn branch","fromName":"Martin von Zweigbergk","fromEmail":"martinvonz@gmail.com","sentAt":"2012-12-21T19:10:11Z","receivedAt":"2012-12-21T19:10:11Z","isPatch":true,"sender":{"key":"martinvonz@gmail.com","avatar":"https://avatars.githubusercontent.com/u/891642?v=4"},"body":">From the user's point of view, it seems natural to think that\ncherry-picking into an unborn branch should work, so make it work,\nwith or without --ff.\n\nCherry-picking anything other than a commit that only adds files, will\nnaturally result in conflicts. Similarly, revert also works, but will\nresult in conflicts unless the specified revision only deletes files.\n\nSigned-off-by: Martin von Zweigbergk <martinvonz@gmail.com>\n\n---\n\nThe plan is to use this for fixing \"git rebase --root\" as discussed in\nhttp://thread.gmane.org/gmane.comp.version-control.git/205796\n\nIs there a better way of creating an unborn branch than what I do in\nthe test cases?\n\n sequencer.c                   | 19 +++++++++++--------\n t/t3501-revert-cherry-pick.sh |  9 +++++++++\n t/t3506-cherry-pick-ff.sh     |  8 ++++++++\n 3 files changed, 28 insertions(+), 8 deletions(-)\n\ndiff --git a/sequencer.c b/sequencer.c\nindex 2260490..1ac1ceb 100644\n--- a/sequencer.c\n+++ b/sequencer.c\n@@ -186,14 +186,15 @@ static int error_dirty_index(struct replay_opts *opts)\n \treturn -1;\n }\n \n-static int fast_forward_to(const unsigned char *to, const unsigned char *from)\n+static int fast_forward_to(const unsigned char *to, const unsigned char *from,\n+\t\t\t   int unborn)\n {\n \tstruct ref_lock *ref_lock;\n \n \tread_cache();\n \tif (checkout_fast_forward(from, to, 1))\n \t\texit(1); /* the callee should have complained already */\n-\tref_lock = lock_any_ref_for_update(\"HEAD\", from, 0);\n+\tref_lock = lock_any_ref_for_update(\"HEAD\", unborn ? null_sha1 : from, 0);\n \treturn write_ref_sha1(ref_lock, to, \"cherry-pick\");\n }\n \n@@ -390,7 +391,7 @@ static int do_pick_commit(struct commit *commit, struct replay_opts *opts)\n \tstruct commit_message msg = { NULL, NULL, NULL, NULL, NULL };\n \tchar *defmsg = NULL;\n \tstruct strbuf msgbuf = STRBUF_INIT;\n-\tint res;\n+\tint res, unborn = 0;\n \n \tif (opts->no_commit) {\n \t\t/*\n@@ -402,9 +403,10 @@ static int do_pick_commit(struct commit *commit, struct replay_opts *opts)\n \t\tif (write_cache_as_tree(head, 0, NULL))\n \t\t\tdie (_(\"Your index file is unmerged.\"));\n \t} else {\n-\t\tif (get_sha1(\"HEAD\", head))\n-\t\t\treturn error(_(\"You do not have a valid HEAD\"));\n-\t\tif (index_differs_from(\"HEAD\", 0))\n+\t\tunborn = get_sha1(\"HEAD\", head);\n+\t\tif (unborn)\n+\t\t\thashcpy(head, EMPTY_TREE_SHA1_BIN);\n+\t\tif (index_differs_from(unborn ? EMPTY_TREE_SHA1_HEX : \"HEAD\", 0))\n \t\t\treturn error_dirty_index(opts);\n \t}\n \tdiscard_cache();\n@@ -435,8 +437,9 @@ static int do_pick_commit(struct commit *commit, struct replay_opts *opts)\n \telse\n \t\tparent = commit->parents->item;\n \n-\tif (opts->allow_ff && parent && !hashcmp(parent->object.sha1, head))\n-\t\treturn fast_forward_to(commit->object.sha1, head);\n+\tif (opts->allow_ff &&\n+\t    (parent && !hashcmp(parent->object.sha1, head) || !parent && unborn))\n+\t     return fast_forward_to(commit->object.sha1, head, unborn);\n \n \tif (parent && parse_commit(parent) < 0)\n \t\t/* TRANSLATORS: The first %s will be \"revert\" or\ndiff --git a/t/t3501-revert-cherry-pick.sh b/t/t3501-revert-cherry-pick.sh\nindex 34c86e5..6f489e2 100755\n--- a/t/t3501-revert-cherry-pick.sh\n+++ b/t/t3501-revert-cherry-pick.sh\n@@ -100,4 +100,13 @@ test_expect_success 'revert forbidden on dirty working tree' '\n \n '\n \n+test_expect_success 'chery-pick on unborn branch' '\n+\tgit checkout --orphan unborn &&\n+\tgit rm --cached -r . &&\n+\trm -rf * &&\n+\tgit cherry-pick initial &&\n+\tgit diff --quiet initial &&\n+\t! test_cmp_rev initial HEAD\n+'\n+\n test_done\ndiff --git a/t/t3506-cherry-pick-ff.sh b/t/t3506-cherry-pick-ff.sh\nindex 51ca391..373aad6 100755\n--- a/t/t3506-cherry-pick-ff.sh\n+++ b/t/t3506-cherry-pick-ff.sh\n@@ -105,4 +105,12 @@ test_expect_success 'cherry pick a root commit with --ff' '\n \ttest \"$(git rev-parse --verify HEAD)\" = \"1df192cd8bc58a2b275d842cede4d221ad9000d1\"\n '\n \n+test_expect_success 'chery-pick --ff on unborn branch' '\n+\tgit checkout --orphan unborn &&\n+\tgit rm --cached -r . &&\n+\trm -rf * &&\n+\tgit cherry-pick --ff first &&\n+\ttest_cmp_rev first HEAD\n+'\n+\n test_done\n-- \n1.8.0.1.240.ge8a1f5a\n"},{"id":"205440","messageId":"7vr4mhpi0l.fsf@alter.siamese.dyndns.org","threadId":"32435","inReplyTo":"1356117013-20613-2-git-send-email-martinvonz@gmail.com","subject":"Re: [PATCH 2/2] learn to pick/revert into unborn branch","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-12-23T03:24:58Z","receivedAt":"2012-12-23T03:24:58Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Martin von Zweigbergk <martinvonz@gmail.com> writes:\n\n>>From the user's point of view, it seems natural to think that\n> cherry-picking into an unborn branch should work, so make it work,\n> with or without --ff.\n\nI actually am having a hard time imagining how that could ever be\nnatural.\n\nWhen you are on an unborn branch, you may have some files in your\nworking tree, and some of them may even be registered to the index,\nbut the index is merely for your convenience to create your first\ncommit, and as far as the history is concered, it does not matter.\n\nBy definition you do not have any history in such a state.  What\ndoes it even mean to \"cherry-pick\" another commit, especially\nwithout the --no-commit option?  The resulting commit will carry the\nmessage taken from the original commit, but does what it says match\nwhat you have done?\n\nI can understand that it may sometimes make sense to do\n\n  $ git show --diff-filter=A $that_commit | git apply\n\nas a way to further update the uncommitted state you have in the\nworking tree, so I can sort of buy that --no-commit case might make\nsome sense (but if you make a commit after \"cherry-pick --no-commit\",\nyou still get the log message from that commit, which does not\nexplain the other things you have in your working tree) in a limited\nsituation.\n\nIt seems to me that the only case that may make sense is to grab the\ncontents from an existing tree, which might be better served with\n\n  $ git checkout $that_commit -- $these_paths_I_am_interested_in\n\n> Cherry-picking anything other than a commit that only adds files, will\n> naturally result in conflicts. Similarly, revert also works, but will\n> result in conflicts unless the specified revision only deletes files.\n\nYou may be able to make it \"work\" for some definition of \"work\", but\nI am not sure how useful it is.\n\nPuzzled...\n"},{"id":"205441","messageId":"7vlicppg9g.fsf@alter.siamese.dyndns.org","threadId":"32435","inReplyTo":"1356117013-20613-2-git-send-email-martinvonz@gmail.com","subject":"Re: [PATCH 2/2] learn to pick/revert into unborn branch","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-12-23T04:02:51Z","receivedAt":"2012-12-23T04:02:51Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Martin von Zweigbergk <martinvonz@gmail.com> writes:\n\n> @@ -435,8 +437,9 @@ static int do_pick_commit(struct commit *commit, struct replay_opts *opts)\n>  \telse\n>  \t\tparent = commit->parents->item;\n>  \n> -\tif (opts->allow_ff && parent && !hashcmp(parent->object.sha1, head))\n> -\t\treturn fast_forward_to(commit->object.sha1, head);\n> +\tif (opts->allow_ff &&\n> +\t    (parent && !hashcmp(parent->object.sha1, head) || !parent && unborn))\n\nStyle (from GNU); please avoid (A && B || C) and spell the\nprecedence between && and || explicitly, i.e.\n\n\t((A && B) || C)\n\n> +\tgit rm --cached -r . &&\n> +\trm -rf * &&\n\nNot \"git rm -rf .\" and as two separate steps?\n"},{"id":"205443","messageId":"CANiSa6i0-Z=FkPnSJxgT+3ABHTzgOTNNNUb=wHQpm2DKAN_UOw@mail.gmail.com","threadId":"32435","inReplyTo":"7vr4mhpi0l.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 2/2] learn to pick/revert into unborn branch","fromName":"Martin von Zweigbergk","fromEmail":"martinvonz@gmail.com","sentAt":"2012-12-23T06:24:41Z","receivedAt":"2012-12-23T06:24:41Z","isPatch":true,"sender":{"key":"martinvonz@gmail.com","avatar":"https://avatars.githubusercontent.com/u/891642?v=4"},"body":"On Sat, Dec 22, 2012 at 7:24 PM, Junio C Hamano <gitster@pobox.com> wrote:\n> Martin von Zweigbergk <martinvonz@gmail.com> writes:\n>\n>>>From the user's point of view, it seems natural to think that\n>> cherry-picking into an unborn branch should work, so make it work,\n>> with or without --ff.\n>\n> I actually am having a hard time imagining how that could ever be\n> natural.\n\nFair enough. What's natural is of course very subjective. In my\nopinion, whenever possible, operations on an unborn branch should\nbehave exactly as they would on an arbitrary commit whose tree just\nhappens to be empty. Of course, pretty much any operation that needs\nmore than the tree (indirectly) pointed to by HEAD would fail the\n\"whenever possible\" clause. I realize that cherry-pick _does_ need the\ncurrent commit to record the parent of the resulting commit, but that\nalmost seems like an implementation detail, i.e whether we're adding a\nparent or adding no parent (when on unborn branch) to the list of\nparents.\n\nIn the same way, I think \"git reset\" should work on an unborn branch,\neven though there is no commit that we can be \"modifying index and\nworking tree to match\" (from man page). I think many users, like me,\nthink of unborn branches as having an empty tree, rather than being a\nspecial state before history is created. Sure, such thinking is not\ntechnically correct, but it still seems to be some people's intuition\n(including mine).\n\n>> Cherry-picking anything other than a commit that only adds files, will\n>> naturally result in conflicts. Similarly, revert also works, but will\n>> result in conflicts unless the specified revision only deletes files.\n>\n> You may be able to make it \"work\" for some definition of \"work\", but\n> I am not sure how useful it is.\n\nAs for use cases, I didn't consider that much more than that it might\nbe useful for implementing \"git rebase --root\". I haven't implemented\nthat yet, so I can't say for sure that it will work out.\n\nOne use case might be to rewrite history by creating an new unborn\nbranch and picking the initial commit and a subset of other commits.\nAnyway, I didn't implement it because I thought it would be very\nuseful, but mostly because I just thought it should work (for\ncompleteness).\n\nI could resend as part of my rebase series (called mz/rebase-range at\nsome point) once that's done. Then we can discuss another solution in\nthe scope of that series if we don't agree on allowing on cherry-pick\non an unborn branch.\n\nBtw, I have another series, which I'll send after 1.8.1, that teaches\n\"git reset\" to work on an unborn branch (among other things). We might\nwant to decide to support both or neither of the commands on an unborn\nbranch.\n\nOn Sat, Dec 22, 2012 at 7:24 PM, Junio C Hamano <gitster@pobox.com> wrote:\n> Martin von Zweigbergk <martinvonz@gmail.com> writes:\n>\n>>>From the user's point of view, it seems natural to think that\n>> cherry-picking into an unborn branch should work, so make it work,\n>> with or without --ff.\n>\n> I actually am having a hard time imagining how that could ever be\n> natural.\n>\n> When you are on an unborn branch, you may have some files in your\n> working tree, and some of them may even be registered to the index,\n> but the index is merely for your convenience to create your first\n> commit, and as far as the history is concered, it does not matter.\n>\n> By definition you do not have any history in such a state.  What\n> does it even mean to \"cherry-pick\" another commit, especially\n> without the --no-commit option?  The resulting commit will carry the\n> message taken from the original commit, but does what it says match\n> what you have done?\n>\n> I can understand that it may sometimes make sense to do\n>\n>   $ git show --diff-filter=A $that_commit | git apply\n>\n> as a way to further update the uncommitted state you have in the\n> working tree, so I can sort of buy that --no-commit case might make\n> some sense (but if you make a commit after \"cherry-pick --no-commit\",\n> you still get the log message from that commit, which does not\n> explain the other things you have in your working tree) in a limited\n> situation.\n>\n> It seems to me that the only case that may make sense is to grab the\n> contents from an existing tree, which might be better served with\n>\n>   $ git checkout $that_commit -- $these_paths_I_am_interested_in\n>\n>> Cherry-picking anything other than a commit that only adds files, will\n>> naturally result in conflicts. Similarly, revert also works, but will\n>> result in conflicts unless the specified revision only deletes files.\n>\n> You may be able to make it \"work\" for some definition of \"work\", but\n> I am not sure how useful it is.\n>\n> Puzzled...\n>\n"},{"id":"205444","messageId":"CAP8UFD0GsqPSk-WstydjZHXc5WSmDJimfRcx4Mn7Uyw0s3LdpA@mail.gmail.com","threadId":"32435","inReplyTo":"CANiSa6i0-Z=FkPnSJxgT+3ABHTzgOTNNNUb=wHQpm2DKAN_UOw@mail.gmail.com","subject":"Re: [PATCH 2/2] learn to pick/revert into unborn branch","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2012-12-23T07:01:52Z","receivedAt":"2012-12-23T07:01:52Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"On Sun, Dec 23, 2012 at 7:24 AM, Martin von Zweigbergk\n<martinvonz@gmail.com> wrote:\n>\n> As for use cases, I didn't consider that much more than that it might\n> be useful for implementing \"git rebase --root\". I haven't implemented\n> that yet, so I can't say for sure that it will work out.\n>\n> One use case might be to rewrite history by creating an new unborn\n> branch and picking the initial commit and a subset of other commits.\n> Anyway, I didn't implement it because I thought it would be very\n> useful, but mostly because I just thought it should work (for\n> completeness).\n\nI agree that it would be nice if it worked.\n\nThanks,\nChristian.\n"},{"id":"205448","messageId":"7v4njcpof8.fsf@alter.siamese.dyndns.org","threadId":"32435","inReplyTo":"CANiSa6i0-Z=FkPnSJxgT+3ABHTzgOTNNNUb=wHQpm2DKAN_UOw@mail.gmail.com","subject":"Re: [PATCH 2/2] learn to pick/revert into unborn branch","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-12-23T19:18:51Z","receivedAt":"2012-12-23T19:18:51Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Martin von Zweigbergk <martinvonz@gmail.com> writes:\n\n> On Sat, Dec 22, 2012 at 7:24 PM, Junio C Hamano <gitster@pobox.com> wrote:\n>> Martin von Zweigbergk <martinvonz@gmail.com> writes:\n>>\n>>>>From the user's point of view, it seems natural to think that\n>>> cherry-picking into an unborn branch should work, so make it work,\n>>> with or without --ff.\n>>\n>> I actually am having a hard time imagining how that could ever be\n>> natural.\n>\n> Fair enough. What's natural is of course very subjective.  ...\n> happens to be empty. Of course, pretty much any operation that needs\n> more than the tree (indirectly) pointed to by HEAD would fail the\n> \"whenever possible\" clause. I realize that cherry-pick _does_ need the\n> current commit to record the parent of the resulting commit,...\n\nYes, and I do not think it is an implementation detail.\n\nI am not opposed to an \"internal use\" of the cherry-pick machinery to\nimplement a corner case of \"rebase -i\":\n\n    1. Your first commit adds \"Makefile\" and \"hello.c\", to build the\n       \"Hello world\" program.\n\n    2. Your second commmit adds \"goodbye.c\" and modifies \"Makefile\",\n       to add the \"Goodbye world\" program.\n\n    3. You run \"rebase -i --root\" to get this insn sheet:\n\n\tpick Add Makefile and hello.c for \"Hello world\"\n        pick Add goodbye.c for \"Goodbye world\"\n\n       and swap them:\n\n        pick Add goodbye.c for \"Goodbye world\"\n\tpick Add Makefile and hello.c for \"Hello world\"\n\n    4. The first one conflicts, as it wants to add new bits in\n       \"Makefile\" that does not exist.  You edit it and make the\n       result pretend as if \"goodbye.c were the first program you\n       added to the project (i.e. adding the common build\n       infrastructure bits you did not change from the real first\n       commit back to \"Makefile\", but making sure it does not yet\n       mention \"hello.c\").\n\n    5. \"rebase --continue\" will give you conflicts for the second\n       one too, and your resolution is likely to match the tip\n       before you started the whole \"rebase -i\".\n\nIn step 4., you would be internally using the cherry-pick machinery\nto implement the step of \"rebase -i\" sequence.  That is what I would\ncall an implementation detail.  And that is cherry-picking to the\nroot.  It transplants something that used to depend on the entire\nhistory behind it to be the beginning of the history so its log\nneeds to be adjusted, but \"rebase -i\" can choose to always make it\nconflict and force the user to write a correct log message, so it\nwon't expose the fundamental flaw you would add if you allowed the\nend-user facing \"cherry-pick\" to pick something to create a new root\ncommit without interaction.\n\n> In the same way, I think \"git reset\" should work on an unborn branch,\n> even though there is no commit that we can be \"modifying index and\n> working tree to match\" (from man page).\n\nI agree that \"git reset\" without any commit parameter to reset the\nindex and optionally the working tree (with \"--hard\") should reset\nfrom an empty tree when you do not yet have any commit.  If HEAD\npoints at an existing commit, its tree is what you reset the\ncontents from.  If you do not have any commit yet, by definition\nthat tree is an empty tree.\n\nBut I do not think it has anything to do with \"cherry-pick to empty\",\nso I do not agree with \"In the same way\" at all.\n\n> As for use cases, I didn't consider that much more than that it might\n> be useful for implementing \"git rebase --root\". I haven't implemented\n> that yet, so I can't say for sure that it will work out.\n\nI think it makes sense only as an internal implementation detail of\n\"rebase -i --root\".\n\n> One use case might be to rewrite history by creating an new unborn\n> branch and picking the initial commit and a subset of other commits.\n\nIf you mean, in the above sample history, to \"git cherry-pick\" the\ncommit that starts the \"Hello world\" and then do something else on\ntop of the resulting history, how would that be different from\nforking from that existing root commit?\n\n> Anyway, I didn't implement it because I thought it would be very\n> useful, but mostly because I just thought it should work (for\n> completeness).\n\nI would not exactly call X \"complete\" if X works in one way in most\ncases and it works in quite a different way in one other case, only\nbecause it would have to barf if it wanted to work in the same way\nas in most cases, and the different behaviour is chosen only because\n\"X that does something is better than X that stops in an impossible\nsituation and barfs\".\n"},{"id":"205449","messageId":"7vzk14o9sk.fsf@alter.siamese.dyndns.org","threadId":"32435","inReplyTo":"CAP8UFD0GsqPSk-WstydjZHXc5WSmDJimfRcx4Mn7Uyw0s3LdpA@mail.gmail.com","subject":"Re: [PATCH 2/2] learn to pick/revert into unborn branch","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-12-23T19:20:11Z","receivedAt":"2012-12-23T19:20:11Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Christian Couder <christian.couder@gmail.com> writes:\n\n> I agree that it would be nice if it worked.\n\nThat is not saying anything.\n\nYes, it would be nice if everything worked.  But the question in the\nthread is \"with what definition of 'work'?\"\n"},{"id":"205450","messageId":"7vvcbso93f.fsf@alter.siamese.dyndns.org","threadId":"32435","inReplyTo":"7v4njcpof8.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 2/2] learn to pick/revert into unborn branch","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-12-23T19:35:16Z","receivedAt":"2012-12-23T19:35:16Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> Yes, and I do not think it is an implementation detail.\n>\n> I am not opposed to an \"internal use\" of the cherry-pick machinery to\n> implement a corner case of \"rebase -i\":\n> ...\n> In step 4., you would be internally using the cherry-pick machinery\n> to implement the step of \"rebase -i\" sequence.  That is what I would\n> call an implementation detail.  And that is cherry-picking to the\n> root.  It transplants something that used to depend on the entire\n> history behind it ...\n\nJust to add another example, I do not think I would be opposed to\nthe case where you \"edit\" the root commit in the above example,\ni.e. keeping the \"Hello world\" as the root commit, but modifying its\ntree and/or log message. The internal impemenation detail has to\nfirst chery-pick that existing commit on top of a void state before\nit gives the user a chance to tweak the tree and commit the result\nwith a modified log message.  Just like \"commit --amend\" can be used\nto amend the root commit, it logically makes sense the recreated\ncommit records nothing as its parent if done when HEAD is not valid\nyet.\n"},{"id":"205453","messageId":"F74DFEF2E8914E76BDA501CA5CD4605F@PhilipOakley","threadId":"32435","inReplyTo":"7vzk14o9sk.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 2/2] learn to pick/revert into unborn branch","fromName":"Philip Oakley","fromEmail":"philipoakley@iee.org","sentAt":null,"receivedAt":"2012-12-23T20:21:11Z","isPatch":true,"sender":{"key":"philipoakley@iee.email","avatar":"https://avatars.githubusercontent.com/u/914343?v=4"},"body":"From: \"Junio C Hamano\" <gitster@pobox.com> Sent: Sunday, December 23,\n2012 3:24 AM\nSubject: Re: [PATCH 2/2] learn to pick/revert into unborn branch\n> Martin von Zweigbergk <martinvonz@gmail.com> writes:\n>\n>>>From the user's point of view, it seems natural to think that\n>> cherry-picking into an unborn branch should work, so make it work,\n>> with or without --ff.\n>\n> I actually am having a hard time imagining how that could ever be\n> natural.\n>\n> When you are on an unborn branch, you may have some files in your\n> working tree, and some of them may even be registered to the index,\n> but the index is merely for your convenience to create your first\n> commit, and as far as the history is concered, it does not matter.\n>\n> By definition you do not have any history in such a state.  What\n> does it even mean to \"cherry-pick\" another commit, especially\n> without the --no-commit option?  The resulting commit will carry the\n> message taken from the original commit, but does what it says match\n> what you have done?\n\n\nFrom: \"Junio C Hamano\"  Sent: Sunday, December 23, 2012 7:20 PM\nSubject: Re: [PATCH 2/2] learn to pick/revert into unborn branch\n> Christian Couder <christian.couder@gmail.com> writes:\n>\n>> I agree that it would be nice if it worked.\n>\n> That is not saying anything.\n>\n> Yes, it would be nice if everything worked.  But the question in the\n> thread is \"with what definition of 'work'?\"\n> --\n\n>From the dumb user perspective, I would have thought that the first\ncommit to be cherry picked for an unborn branch would be the complete\ncommit, which is then planted as the branch's start commit. We tend to\ntalk of cherry picking commits, though the documentation does say 'the\nchanges introduced', which allows such a (mistaken) user perspective for\nthis particular case.\n\nIt is only in retrospect, and a bit of extra thought, that one could see\nthat the commit's message would not actually describe the new situation\nand should have been edited.\n\nThat doesn't mean that it would be right to allow such an initilisation\nof an unborn branch, it's more an explanation of how the idea may have\ndeveloped.\n\nPhilip\n"},{"id":"205464","messageId":"CANiSa6i5f6wU5R5U43+NpZfOTTX0e_GFzNVxxA412DB4ES4P8w@mail.gmail.com","threadId":"32435","inReplyTo":"7v4njcpof8.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 2/2] learn to pick/revert into unborn branch","fromName":"Martin von Zweigbergk","fromEmail":"martinvonz@gmail.com","sentAt":"2012-12-24T07:20:10Z","receivedAt":"2012-12-24T07:20:10Z","isPatch":true,"sender":{"key":"martinvonz@gmail.com","avatar":"https://avatars.githubusercontent.com/u/891642?v=4"},"body":"On Sun, Dec 23, 2012 at 11:18 AM, Junio C Hamano <gitster@pobox.com> wrote:\n> Martin von Zweigbergk <martinvonz@gmail.com> writes:\n>> On Sat, Dec 22, 2012 at 7:24 PM, Junio C Hamano <gitster@pobox.com> wrote:\n>\n> I am not opposed to an \"internal use\" of the cherry-pick machinery to\n> implement a corner case of \"rebase -i\":\n>....\n>     3. You run \"rebase -i --root\" to get this insn sheet:\n>\n>         pick Add Makefile and hello.c for \"Hello world\"\n>         pick Add goodbye.c for \"Goodbye world\"\n>\n>        and swap them:\n>\n>         pick Add goodbye.c for \"Goodbye world\"\n>         pick Add Makefile and hello.c for \"Hello world\"\n\nRight, and as you point out in your later message, editing the initial\ncommit is another (and more useful) use case. It could of course be\nspecial cased in \"git rebase\", but I think doing it \"git cherry-pick\"\nis the right thing to do. Hopefully git-rebase.sh can then just reset\nto the void state and git-rebase--interactive.sh can just continue\nwithout knowing or caring whether it got started on an unborn branch.\nHmm... I just realized the \"branch\" in \"unborn branch\" really means we\ndon't have \"unborn detached HEAD\", do we? So some more tricks are\nprobably necessary after all. :-(\n\n> [...] It transplants something that used to depend on the entire\n> history behind it to be the beginning of the history so its log\n> needs to be adjusted, but \"rebase -i\" can choose to always make it\n> conflict and force the user to write a correct log message, so it\n> won't expose the fundamental flaw you would add if you allowed the\n> end-user facing \"cherry-pick\" to pick something to create a new root\n> commit without interaction.\n\nIf I understand you correctly, you are suggesting that \"git rebase\"\nshould set the action from \"pick\" to \"edit\" for the first commit in\nthe insn sheet if it is not a root commit. \"git rebase -i --root\"\ndoesn't currently do that, but it certainly could.\n\n> I agree that \"git reset\" without any commit parameter to reset the\n> index and optionally the working tree (with \"--hard\") should reset\n> from an empty tree when you do not yet have any commit.\n\nGood to hear.\n\n> But I do not think it has anything to do with \"cherry-pick to empty\",\n> so I do not agree with \"In the same way\" at all.\n\nSee later comment.\n\n>> One use case might be to rewrite history by creating an new unborn\n>> branch and picking the initial commit and a subset of other commits.\n>\n> If you mean, in the above sample history, to \"git cherry-pick\" the\n> commit that starts the \"Hello world\" and then do something else on\n> top of the resulting history, how would that be different from\n> forking from that existing root commit?\n\nTrue, the result would be the same. The user's thought process might\nbe a little different (\"let me start from scratch\" vs \"let me start\nalmost from scratch\"), but that's a very minor difference that I'm\nsure any user would quickly overcome.\n\n>> Anyway, I didn't implement it because I thought it would be very\n>> useful, but mostly because I just thought it should work (for\n>> completeness).\n>\n> I would not exactly call X \"complete\" if X works in one way in most\n> cases and it works in quite a different way in one other case, only\n> because it would have to barf if it wanted to work in the same way\n> as in most cases, and the different behaviour is chosen only because\n> \"X that does something is better than X that stops in an impossible\n> situation and barfs\".\n\nI agree, of course, but I don't see the behavior as different. When\nthinking about behavior around the root of the history, I imagine that\nall root commits actually have a parent, and that they all have the\nsame parent. I also imagine that on an unborn branch, instead of being\ninvalid, HEAD points to that same \"single root\" commit with an empty\ntree. Despite this model not matching git's, I find that this helps me\nreason about what the behavior of various commands should be.\n\nWith this reasoning, cherry-picking into an unborn branch is no\ndifferent from cherry-picking into any commit with an empty tree\n(which of course would be rare, but not forbidden).\n"}]}