{"thread":{"id":"34136","subject":"[PATCH 0/2] Rebasing pull with autostash","startedAt":"2013-06-14T08:56:16Z","lastAt":"2013-06-14T18:26:11Z","messageCount":14,"participants":["Ramkumar Ramachandra","Matthieu Moy","Phil Hord","John Keeping","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"220789","messageId":"1371200178-9927-1-git-send-email-artagnon@gmail.com","threadId":"34136","inReplyTo":null,"subject":"[PATCH 0/2] Rebasing pull with autostash","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2013-06-14T08:56:16Z","receivedAt":"2013-06-14T08:56:16Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"It's trivial to support the feature :)\n\nRamkumar Ramachandra (2):\n pull: respect rebase.autostash\n pull: clarify the large && { ... } form\n\n git-pull.sh | 7 +++++--\n t/t5520-pull.sh | 11 +++++++++++\n 2 files changed, 16 insertions(+), 2 deletions(-)\n\n-- \n1.8.3.1.379.gb74074e.dirty\n"},{"id":"220791","messageId":"1371200178-9927-2-git-send-email-artagnon@gmail.com","threadId":"34136","inReplyTo":"1371200178-9927-1-git-send-email-artagnon@gmail.com","subject":"[PATCH 1/2] pull: respect rebase.autostash","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2013-06-14T08:56:17Z","receivedAt":"2013-06-14T08:56:17Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"If a rebasing pull is requested, pull unconditionally runs\nrequire_clean_worktree() resulting in:\n\n # dirty worktree or index\n $ git pull\n Cannot pull with rebase: Your index contains uncommitted changes.\n Please commit or stash them.\n\nIt does this to inform the user early on that a rebase cannot be run on\na dirty worktree, and that a stash is required. However,\nrr/rebase-autostash lifts this limitation on rebase by providing a way\nto automatically stash using the rebase.autostash configuration\nvariable. Read this variable in pull, and take advantage of this\nfeature.\n\nSigned-off-by: Ramkumar Ramachandra \n---\n git-pull.sh | 2 ++\n t/t5520-pull.sh | 11 +++++++++++\n 2 files changed, 13 insertions(+)\n\ndiff --git a/git-pull.sh b/git-pull.sh\nindex 638aabb..fb01763 100755\n--- a/git-pull.sh\n+++ b/git-pull.sh\n@@ -44,6 +44,7 @@ merge_args= edit=\n curr_branch=$(git symbolic-ref -q HEAD)\n curr_branch_short=\"${curr_branch#refs/heads/}\"\n rebase=$(git config --bool branch.$curr_branch_short.rebase)\n+autostash=$(git config --bool rebase.autostash)\n if test -z \"$rebase\"\n then\n \trebase=$(git config --bool pull.rebase)\n@@ -203,6 +204,7 @@ test true = \"$rebase\" && {\n \t\t\tdie \"$(gettext \"updating an unborn branch with changes added to the index\")\"\n \t\tfi\n \telse\n+\t\ttest true = \"$autostash\" ||\n \t\trequire_clean_work_tree \"pull with rebase\" \"Please commit or stash them.\"\n \tfi\n \toldremoteref= &&\ndiff --git a/t/t5520-pull.sh b/t/t5520-pull.sh\nindex 6af6c63..08afd25 100755\n--- a/t/t5520-pull.sh\n+++ b/t/t5520-pull.sh\n@@ -102,6 +102,17 @@ test_expect_success 'pull.rebase' '\n \ttest new = $(git show HEAD:file2)\n '\n \n+test_expect_success 'pull --rebase respects rebase.autostash' '\n+\tgit reset --hard before-rebase &&\n+\ttest_config pull.rebase true &&\n+\ttest_config rebase.autostash true &&\n+\techo dirty >>file2\n+\tgit pull . copy &&\n+\ttest $(git rev-parse HEAD^) = $(git rev-parse copy) &&\n+\ttest new = $(git show HEAD:file2) &&\n+\tgrep dirty file2\n+'\n+\n test_expect_success 'branch.to-rebase.rebase' '\n \tgit reset --hard before-rebase &&\n \ttest_config branch.to-rebase.rebase true &&\n-- \n1.8.3.1.379.gb74074e.dirty\n"},{"id":"220790","messageId":"1371200178-9927-3-git-send-email-artagnon@gmail.com","threadId":"34136","inReplyTo":"1371200178-9927-1-git-send-email-artagnon@gmail.com","subject":"[PATCH 2/2] pull: clarify the large && { ... } form","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2013-06-14T08:56:18Z","receivedAt":"2013-06-14T08:56:18Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Remove the large && { ... } form, as the block can be confused with a\nfunction block. Use a simple if-condition instead. No functional\nchanges.\n\nSigned-off-by: Ramkumar Ramachandra \n---\n git-pull.sh | 5 +++--\n 1 file changed, 3 insertions(+), 2 deletions(-)\n\ndiff --git a/git-pull.sh b/git-pull.sh\nindex fb01763..babf009 100755\n--- a/git-pull.sh\n+++ b/git-pull.sh\n@@ -195,7 +195,7 @@ error_on_no_merge_candidates () {\n \texit 1\n }\n \n-test true = \"$rebase\" && {\n+if test true = \"$rebase\"; then\n \tif ! git rev-parse -q --verify HEAD >/dev/null\n \tthen\n \t\t# On an unborn branch\n@@ -220,7 +220,8 @@ test true = \"$rebase\" && {\n \t\t\tbreak\n \t\tfi\n \tdone\n-}\n+fi\n+\n orig_head=$(git rev-parse -q --verify HEAD)\n git fetch $verbosity $progress $dry_run $recurse_submodules --update-head-ok \"$@\" || exit 1\n test -z \"$dry_run\" || exit 0\n-- \n1.8.3.1.379.gb74074e.dirty\n"},{"id":"220792","messageId":"vpqr4g5kqht.fsf@anie.imag.fr","threadId":"34136","inReplyTo":"1371200178-9927-3-git-send-email-artagnon@gmail.com","subject":"Re: [PATCH 2/2] pull: clarify the large && { ... } form","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2013-06-14T09:09:50Z","receivedAt":"2013-06-14T09:09:50Z","isPatch":true,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Ramkumar Ramachandra writes:\n\n> -test true = \"$rebase\" && {\n> +if test true = \"$rebase\"; then\n\nUse a newline, not a ';'.\n\nSee CodingGuideLines:\n\n - Do not write control structures on a single line with semicolon.\n \"then\" should be on the next line for if statements, and \"do\"\n should be on the next line for \"while\" and \"for\".\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"220793","messageId":"vpqfvwlkqb5.fsf@anie.imag.fr","threadId":"34136","inReplyTo":"1371200178-9927-2-git-send-email-artagnon@gmail.com","subject":"Re: [PATCH 1/2] pull: respect rebase.autostash","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2013-06-14T09:13:50Z","receivedAt":"2013-06-14T09:13:50Z","isPatch":true,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Ramkumar Ramachandra writes:\n\n> --- a/git-pull.sh\n> +++ b/git-pull.sh\n> @@ -44,6 +44,7 @@ merge_args= edit=\n> curr_branch=$(git symbolic-ref -q HEAD)\n> curr_branch_short=\"${curr_branch#refs/heads/}\"\n> rebase=$(git config --bool branch.$curr_branch_short.rebase)\n> +autostash=$(git config --bool rebase.autostash)\n> if test -z \"$rebase\"\n> then\n> \trebase=$(git config --bool pull.rebase)\n> @@ -203,6 +204,7 @@ test true = \"$rebase\" && {\n> \t\t\tdie \"$(gettext \"updating an unborn branch with changes added to the index\")\"\n> \t\tfi\n> \telse\n> +\t\ttest true = \"$autostash\" ||\n> \t\trequire_clean_work_tree \"pull with rebase\" \"Please commit or stash them.\"\n\nTrivial, indeed!\n\nIt would be nice to have an --autostash command-line option too, and the\nerror message in \"require_clean_work_tree\" could suggest using it. That\nwould make the feature easily discoverable.\n\nPerhaps this patch could mention \"pull --rebase\" in the doc, like (config.txt)\n\n- \tends. This means that you can run rebase on a dirty worktree.\n+ \tends. This means that you can run rebase or `git pull --rebase` on a dirty worktree.\n\n(or perhaps it's obvious enough and not needed)\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"220802","messageId":"CALkWK0n4QXsRNQxiSfeoPRdwp6kva8V08oG0h9Ld7TpBTgKDKA@mail.gmail.com","threadId":"34136","inReplyTo":"vpqfvwlkqb5.fsf@anie.imag.fr","subject":"Re: [PATCH 1/2] pull: respect rebase.autostash","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2013-06-14T10:41:58Z","receivedAt":"2013-06-14T10:41:58Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Matthieu Moy wrote:\n> It would be nice to have an --autostash command-line option too,\n\nI thought it would be a bit ugly, since it's already overloaded with\noptions to pass to merge.\n\n> and the\n> error message in \"require_clean_work_tree\" could suggest using it. That\n> would make the feature easily discoverable.\n\nCannot pull with rebase: You have unstaged changes.\nPlease commit or stash them. See also: rebase.autostash in git-config(1).\n\n> Perhaps this patch could mention \"pull --rebase\" in the doc, like (config.txt)\n\n--- a/Documentation/git-pull.txt\n+++ b/Documentation/git-pull.txt\n@@ -112,5 +112,6 @@ include::merge-options.txt[]\n See `pull.rebase`, `branch..rebase` and `branch.autosetuprebase` in\n linkgit:git-config[1] if you want to make `git pull` always use\n-`--rebase` instead of merging.\n+`--rebase` instead of merging. See `rebase.autostash` in\n+linkgit:git-config[1] if you want to be able to run on a dirty worktree.\n +\n\nThanks.\n"},{"id":"220803","messageId":"CALkWK0=_K8-drmpkEGkWE3h0fvm=RsQPu3gaDY6Nhkyg2mRHDg@mail.gmail.com","threadId":"34136","inReplyTo":"vpqr4g5kqht.fsf@anie.imag.fr","subject":"Re: [PATCH 2/2] pull: clarify the large && { ... } form","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2013-06-14T10:47:21Z","receivedAt":"2013-06-14T10:47:21Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Matthieu Moy wrote:\n> Use a newline, not a ';'.\n\nThanks.\n"},{"id":"220805","messageId":"CABURp0rFtp0O0RTMmZUxmsHLu15Swg6SSex-H3NvsnR0QkRvvw@mail.gmail.com","threadId":"34136","inReplyTo":"CALkWK0n4QXsRNQxiSfeoPRdwp6kva8V08oG0h9Ld7TpBTgKDKA@mail.gmail.com","subject":"Re: [PATCH 1/2] pull: respect rebase.autostash","fromName":"Phil Hord","fromEmail":"phil.hord@gmail.com","sentAt":"2013-06-14T11:59:16Z","receivedAt":"2013-06-14T11:59:16Z","isPatch":true,"sender":{"key":"phil.hord@gmail.com","avatar":"https://avatars.githubusercontent.com/u/123908?v=4"},"body":"On Fri, Jun 14, 2013 at 6:41 AM, Ramkumar Ramachandra\n wrote:\n> Matthieu Moy wrote:\n>> It would be nice to have an --autostash command-line option too,\n>\n> I thought it would be a bit ugly, since it's already overloaded with\n> options to pass to merge.\n\nEventually I think a switch will be needed. At least there is\n git -c rebase.autostash=false\n\nBut we will probably need to also have --no-autostash for scripts and users.\n\n\nPhil\n"},{"id":"220806","messageId":"CABURp0pVzQ2rVB-B2L+uCMtPV7QFPSgkZnHitCsrPCrR9LJOVA@mail.gmail.com","threadId":"34136","inReplyTo":"1371200178-9927-2-git-send-email-artagnon@gmail.com","subject":"Re: [PATCH 1/2] pull: respect rebase.autostash","fromName":"Phil Hord","fromEmail":"phil.hord@gmail.com","sentAt":"2013-06-14T12:12:56Z","receivedAt":"2013-06-14T12:12:56Z","isPatch":true,"sender":{"key":"phil.hord@gmail.com","avatar":"https://avatars.githubusercontent.com/u/123908?v=4"},"body":"On Fri, Jun 14, 2013 at 4:56 AM, Ramkumar Ramachandra\n wrote:\n> If a rebasing pull is requested, pull unconditionally runs\n> require_clean_worktree() resulting in:\n>\n> # dirty worktree or index\n> $ git pull\n> Cannot pull with rebase: Your index contains uncommitted changes.\n> Please commit or stash them.\n>\n> It does this to inform the user early on that a rebase cannot be run on\n> a dirty worktree, and that a stash is required. However,\n> rr/rebase-autostash lifts this limitation on rebase by providing a way\n> to automatically stash using the rebase.autostash configuration\n> variable. Read this variable in pull, and take advantage of this\n> feature.\n\nThis commit message does not tell me what this commit does. It mostly\ndescribes the current situation. Then it refers to something called\n\"rr/rebase-autostash\" which will lose meaning in the future when this\ncommit is no longer current on the list. A better way to refer to\nthis commit is to say \"this commit\". However, even this is not the\nnorm for this project. The norm here is to avoid such noise by\nspeaking in the imperative mood. That is, do not tell me what this\ncommit does; instead, tell the code what to do. See\nDocumentation/SubmittingPatches:\n\n Describe your changes in imperative mood, e.g. \"make xyzzy do frotz\"\n instead of \"[This patch] makes xyzzy do frotz\" or \"[I] changed xyzzy\n to do frotz\", as if you are giving orders to the codebase to change\n its behaviour. Try to make sure your explanation can be understood\n without external resources. Instead of giving a URL to a mailing list\n archive, summarize the relevant points of the discussion.\n\n\n\n>\n> Signed-off-by: Ramkumar Ramachandra \n> ---\n> git-pull.sh | 2 ++\n> t/t5520-pull.sh | 11 +++++++++++\n> 2 files changed, 13 insertions(+)\n>\n> diff --git a/git-pull.sh b/git-pull.sh\n> index 638aabb..fb01763 100755\n> --- a/git-pull.sh\n> +++ b/git-pull.sh\n> @@ -44,6 +44,7 @@ merge_args= edit=\n> curr_branch=$(git symbolic-ref -q HEAD)\n> curr_branch_short=\"${curr_branch#refs/heads/}\"\n> rebase=$(git config --bool branch.$curr_branch_short.rebase)\n> +autostash=$(git config --bool rebase.autostash)\n> if test -z \"$rebase\"\n> then\n> rebase=$(git config --bool pull.rebase)\n> @@ -203,6 +204,7 @@ test true = \"$rebase\" && {\n> die \"$(gettext \"updating an unborn branch with changes added to the index\")\"\n> fi\n> else\n> + test true = \"$autostash\" ||\n> require_clean_work_tree \"pull with rebase\" \"Please commit or stash them.\"\n> fi\n\nThis commit does not seem useful on its own. All it does is prevent\nthe safety check for a clean work tree when the autostash flag is set.\n I understand that this is necessary for the rest of the change to be\nuseful, but I do not see any reason for it to be split into two\ncommits like this. I think it would be more understandable if this\nwere squashed together with the rest of the change, both now for\nreviews and in the future when someone tries to understand this change\nin retrospect.\n\nIn particular, the commit message suggests that this commit will\nperform the autostash if this variable is set, but it does not do that\nyet. I think if you squash these two together it will be more concise\nand understandable.\n\nThanks,\nPhil\n"},{"id":"220807","messageId":"CABURp0pHUPnRJuGM-8zUhM9bGh5rj6r5692Nhc-1G+K5SAunWA@mail.gmail.com","threadId":"34136","inReplyTo":"CABURp0pVzQ2rVB-B2L+uCMtPV7QFPSgkZnHitCsrPCrR9LJOVA@mail.gmail.com","subject":"Re: [PATCH 1/2] pull: respect rebase.autostash","fromName":"Phil Hord","fromEmail":"phil.hord@gmail.com","sentAt":"2013-06-14T12:21:32Z","receivedAt":"2013-06-14T12:21:32Z","isPatch":true,"sender":{"key":"phil.hord@gmail.com","avatar":"https://avatars.githubusercontent.com/u/123908?v=4"},"body":"On Fri, Jun 14, 2013 at 8:12 AM, Phil Hord wrote:\n> On Fri, Jun 14, 2013 at 4:56 AM, Ramkumar Ramachandra\n> wrote:\n>> If a rebasing pull is requested, pull unconditionally runs\n>> require_clean_worktree() resulting in:\n>>\n>> # dirty worktree or index\n>> $ git pull\n>> Cannot pull with rebase: Your index contains uncommitted changes.\n>> Please commit or stash them.\n>>\n>> It does this to inform the user early on that a rebase cannot be run on\n>> a dirty worktree, and that a stash is required. However,\n>> rr/rebase-autostash lifts this limitation on rebase by providing a way\n>> to automatically stash using the rebase.autostash configuration\n>> variable. Read this variable in pull, and take advantage of this\n>> feature.\n>\n> This commit message does not tell me what this commit does. It mostly\n> describes the current situation. Then it refers to something called\n> \"rr/rebase-autostash\" which will lose meaning in the future when this\n> commit is no longer current on the list. A better way to refer to\n> this commit is to say \"this commit\". However, even this is not the\n> norm for this project. The norm here is to avoid such noise by\n> speaking in the imperative mood. That is, do not tell me what this\n> commit does; instead, tell the code what to do. See\n> Documentation/SubmittingPatches:\n>\n> Describe your changes in imperative mood, e.g. \"make xyzzy do frotz\"\n> instead of \"[This patch] makes xyzzy do frotz\" or \"[I] changed xyzzy\n> to do frotz\", as if you are giving orders to the codebase to change\n> its behaviour. Try to make sure your explanation can be understood\n> without external resources. Instead of giving a URL to a mailing list\n> archive, summarize the relevant points of the discussion.\n>\n>\n>\n>>\n>> Signed-off-by: Ramkumar Ramachandra \n>> ---\n>> git-pull.sh | 2 ++\n>> t/t5520-pull.sh | 11 +++++++++++\n>> 2 files changed, 13 insertions(+)\n>>\n>> diff --git a/git-pull.sh b/git-pull.sh\n>> index 638aabb..fb01763 100755\n>> --- a/git-pull.sh\n>> +++ b/git-pull.sh\n>> @@ -44,6 +44,7 @@ merge_args= edit=\n>> curr_branch=$(git symbolic-ref -q HEAD)\n>> curr_branch_short=\"${curr_branch#refs/heads/}\"\n>> rebase=$(git config --bool branch.$curr_branch_short.rebase)\n>> +autostash=$(git config --bool rebase.autostash)\n>> if test -z \"$rebase\"\n>> then\n>> rebase=$(git config --bool pull.rebase)\n>> @@ -203,6 +204,7 @@ test true = \"$rebase\" && {\n>> die \"$(gettext \"updating an unborn branch with changes added to the index\")\"\n>> fi\n>> else\n>> + test true = \"$autostash\" ||\n>> require_clean_work_tree \"pull with rebase\" \"Please commit or stash them.\"\n>> fi\n>\n> This commit does not seem useful on its own. All it does is prevent\n> the safety check for a clean work tree when the autostash flag is set.\n> I understand that this is necessary for the rest of the change to be\n> useful, but I do not see any reason for it to be split into two\n> commits like this. I think it would be more understandable if this\n> were squashed together with the rest of the change, both now for\n> reviews and in the future when someone tries to understand this change\n> in retrospect.\n>\n> In particular, the commit message suggests that this commit will\n> perform the autostash if this variable is set, but it does not do that\n> yet. I think if you squash these two together it will be more concise\n> and understandable.\n>\n> Thanks,\n> Phil\n\nHaving said all that, I see now that 2/2 in this series is really\nunrelated to this commit and is not the rest of the autostash\nimplementation, so I am more confused than before. Was the rest of\n'autostash' already implemented in some other series? I guess that's\nwhat you meant by \"rr/rebase-autostash\" which is NOT \"this commit\"\nafter all. I am sorry for my confusion, though a clearer commit\nmessage would have helped me in the first place.\n\nPhil\n"},{"id":"220809","messageId":"20130614122950.GE23890@serenity.lan","threadId":"34136","inReplyTo":"CABURp0pVzQ2rVB-B2L+uCMtPV7QFPSgkZnHitCsrPCrR9LJOVA@mail.gmail.com","subject":"Re: [PATCH 1/2] pull: respect rebase.autostash","fromName":"John Keeping","fromEmail":"john@keeping.me.uk","sentAt":"2013-06-14T12:29:50Z","receivedAt":"2013-06-14T12:29:50Z","isPatch":true,"sender":{"key":"john@keeping.me.uk","avatar":"https://avatars.githubusercontent.com/u/1702081?v=4"},"body":"On Fri, Jun 14, 2013 at 08:12:56AM -0400, Phil Hord wrote:\n> On Fri, Jun 14, 2013 at 4:56 AM, Ramkumar Ramachandra\n> wrote:\n> > If a rebasing pull is requested, pull unconditionally runs\n> > require_clean_worktree() resulting in:\n> >\n> > # dirty worktree or index\n> > $ git pull\n> > Cannot pull with rebase: Your index contains uncommitted changes.\n> > Please commit or stash them.\n> >\n> > It does this to inform the user early on that a rebase cannot be run on\n> > a dirty worktree, and that a stash is required. However,\n> > rr/rebase-autostash lifts this limitation on rebase by providing a way\n> > to automatically stash using the rebase.autostash configuration\n> > variable. Read this variable in pull, and take advantage of this\n> > feature.\n> \n> This commit message does not tell me what this commit does. It mostly\n> describes the current situation. Then it refers to something called\n> \"rr/rebase-autostash\" which will lose meaning in the future when this\n> commit is no longer current on the list. A better way to refer to\n> this commit is to say \"this commit\". However, even this is not the\n> norm for this project. The norm here is to avoid such noise by\n> speaking in the imperative mood. That is, do not tell me what this\n> commit does; instead, tell the code what to do. See\n> Documentation/SubmittingPatches:\n\nIt seems to me that Ram's message is already in the imperative. The\nonly (slight) issue is that rr/rebase-autostash will become hard to find\nonce Junio cleans up feature branches that have graduated. Since that\nbranch has graduated to master, it would be clearer to refer to commit\n5879477 (rebase: implement --[no-]autostash and rebase.autostash,\n2013-05-12). Is something like this clearer?\n\n \"git pull\" currently cannot be used with the \"autostash\" feature\n added to \"git rebase\" by commit 5879477 (rebase: implement\n --[no-]autostash and rebase.autostash, 2013-05-12) because it\n unconditionally calls requre_clean_worktree early on, which results\n in:\n\n # dirty worktree or index\n $ git pull\n Cannot pull with rebase: Your index contains uncommitted changes.\n Please commit or stash them.\n\n Remove this restriction by skipping the call to\n require_clean_worktree if the \"rebase.autostash\" configuration\n variable is set.\n"},{"id":"220811","messageId":"CABURp0rPF3WSm9dQe70DBOKtJDtUTL_BdpQOv1h-nwwcCZpD9g@mail.gmail.com","threadId":"34136","inReplyTo":"20130614122950.GE23890@serenity.lan","subject":"Re: [PATCH 1/2] pull: respect rebase.autostash","fromName":"Phil Hord","fromEmail":"phil.hord@gmail.com","sentAt":"2013-06-14T12:41:20Z","receivedAt":"2013-06-14T12:41:20Z","isPatch":true,"sender":{"key":"phil.hord@gmail.com","avatar":"https://avatars.githubusercontent.com/u/123908?v=4"},"body":"On Fri, Jun 14, 2013 at 8:29 AM, John Keeping wrote:\n> On Fri, Jun 14, 2013 at 08:12:56AM -0400, Phil Hord wrote:\n>> On Fri, Jun 14, 2013 at 4:56 AM, Ramkumar Ramachandra\n>> wrote:\n>> > If a rebasing pull is requested, pull unconditionally runs\n>> > require_clean_worktree() resulting in:\n>> >\n>> > # dirty worktree or index\n>> > $ git pull\n>> > Cannot pull with rebase: Your index contains uncommitted changes.\n>> > Please commit or stash them.\n>> >\n>> > It does this to inform the user early on that a rebase cannot be run on\n>> > a dirty worktree, and that a stash is required. However,\n>> > rr/rebase-autostash lifts this limitation on rebase by providing a way\n>> > to automatically stash using the rebase.autostash configuration\n>> > variable. Read this variable in pull, and take advantage of this\n>> > feature.\n>>\n>> This commit message does not tell me what this commit does. It mostly\n>> describes the current situation. Then it refers to something called\n>> \"rr/rebase-autostash\" which will lose meaning in the future when this\n>> commit is no longer current on the list. A better way to refer to\n>> this commit is to say \"this commit\". However, even this is not the\n>> norm for this project. The norm here is to avoid such noise by\n>> speaking in the imperative mood. That is, do not tell me what this\n>> commit does; instead, tell the code what to do. See\n>> Documentation/SubmittingPatches:\n>\n> It seems to me that Ram's message is already in the imperative. The\n> only (slight) issue is that rr/rebase-autostash will become hard to find\n> once Junio cleans up feature branches that have graduated. Since that\n> branch has graduated to master, it would be clearer to refer to commit\n> 5879477 (rebase: implement --[no-]autostash and rebase.autostash,\n> 2013-05-12). Is something like this clearer?\n>\n> \"git pull\" currently cannot be used with the \"autostash\" feature\n> added to \"git rebase\" by commit 5879477 (rebase: implement\n> --[no-]autostash and rebase.autostash, 2013-05-12) because it\n> unconditionally calls requre_clean_worktree early on, which results\n> in:\n>\n> # dirty worktree or index\n> $ git pull\n> Cannot pull with rebase: Your index contains uncommitted changes.\n> Please commit or stash them.\n>\n> Remove this restriction by skipping the call to\n> require_clean_worktree if the \"rebase.autostash\" configuration\n> variable is set.\n\n\nYes, thanks. I was mislead by my poor understanding all the players\ninvolved. This disambiguates things nicely.\n\nPhil\n"},{"id":"220813","messageId":"CALkWK0mbmxahhxqw10vt6Xmc02+ZhmM5XACSMfSfcCyTOs0RBg@mail.gmail.com","threadId":"34136","inReplyTo":"20130614122950.GE23890@serenity.lan","subject":"Re: [PATCH 1/2] pull: respect rebase.autostash","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2013-06-14T12:46:23Z","receivedAt":"2013-06-14T12:46:23Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"John Keeping wrote:\n> Is something like this clearer?\n> [...]\n\nYes, thanks.\n"},{"id":"220902","messageId":"7vk3lwo8fw.fsf@alter.siamese.dyndns.org","threadId":"34136","inReplyTo":"vpqfvwlkqb5.fsf@anie.imag.fr","subject":"Re: [PATCH 1/2] pull: respect rebase.autostash","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-06-14T18:26:11Z","receivedAt":"2013-06-14T18:26:11Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Matthieu Moy writes:\n\n> Ramkumar Ramachandra writes:\n>\n>> --- a/git-pull.sh\n>> +++ b/git-pull.sh\n>> @@ -44,6 +44,7 @@ merge_args= edit=\n>> curr_branch=$(git symbolic-ref -q HEAD)\n>> curr_branch_short=\"${curr_branch#refs/heads/}\"\n>> rebase=$(git config --bool branch.$curr_branch_short.rebase)\n>> +autostash=$(git config --bool rebase.autostash)\n>> if test -z \"$rebase\"\n>> then\n>> \trebase=$(git config --bool pull.rebase)\n>> @@ -203,6 +204,7 @@ test true = \"$rebase\" && {\n>> \t\t\tdie \"$(gettext \"updating an unborn branch with changes added to the index\")\"\n>> \t\tfi\n>> \telse\n>> +\t\ttest true = \"$autostash\" ||\n>> \t\trequire_clean_work_tree \"pull with rebase\" \"Please commit or stash them.\"\n>\n> Trivial, indeed!\n>\n> It would be nice to have an --autostash command-line option too, and the\n> error message in \"require_clean_work_tree\" could suggest using it. That\n> would make the feature easily discoverable.\n\nI would actually suggest doing this the other way around. --autostash\noption, followed by configuration if needed.\n\nI actually have this slight suspicion that rebase.autostash and\npull.autostash may want to be separate variables. Attempting to\nrebase locally when your working tree is in half-baked state is bad\nenough but may be warranted in some workflows, but doing so when\nintegrating with other people's work is a practice in a different\nleague. It would be quite sensible for somebody to want to allow the\nformer (i.e. allows autostash with rebase.autostash=true for local\nrebase) while retaining the safety of the latter (i.e. stop pulling\nwith pull.autostash=false).\n\n> Perhaps this patch could mention \"pull --rebase\" in the doc, like (config.txt)\n>\n> - \tends. This means that you can run rebase on a dirty worktree.\n> + \tends. This means that you can run rebase or `git pull --rebase` on a dirty worktree.\n>\n> (or perhaps it's obvious enough and not needed)\n"}]}