{"thread":{"id":"56923","subject":"[PATCH] pull: should be noop when already-up-to-date","startedAt":"2021-11-17T07:55:54Z","lastAt":"2021-11-18T23:51:02Z","messageCount":2,"participants":["Erwin Villejo via GitGitGadget","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"441410","messageId":"pull.1078.git.1637135750674.gitgitgadget@gmail.com","threadId":"56923","inReplyTo":null,"subject":"[PATCH] pull: should be noop when already-up-to-date","fromName":"Erwin Villejo via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-11-17T07:55:50Z","receivedAt":"2021-11-17T07:55:54Z","isPatch":true,"sender":{"key":"erwinvillejo@gmail.com","avatar":"https://gravatar.com/avatar/5fe0c1efd06f741b2c545bac37596e9acfe74c118a91d0dd9ccf0961d230e97d?d=mp&s=160"},"body":"From: Erwin Villejo <erwin.villejo@gmail.com>\n\nThe already-up-to-date pull bug was fixed for --ff-only but it did not\ninclude the case where --ff or --ff-only are not specified. This updates\nthe --ff-only fix to include the case where --ff or --ff-only are not\nspecified in command line flags or config.\n\nSigned-off-by: Erwin Villejo <erwin.villejo@gmail.com>\n---\n    pull: should be noop when already-up-to-date\n    \n    The already-up-to-date pull bug was fixed for --ff-only but it did not\n    include the case where --ff or --ff-only are not specified. This updates\n    the --ff-only fix to include the case where --ff or --ff-only are not\n    specified in command line flags or config.\n    \n    Signed-off-by: Erwin Villejo erwin.villejo@gmail.com\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-1078%2Ferwinv%2Fmaster-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1078/erwinv/master-v1\nPull-Request: https://github.com/gitgitgadget/git/pull/1078\n\n builtin/pull.c               | 6 ++++--\n t/t7601-merge-pull-config.sh | 6 ++++++\n 2 files changed, 10 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin/pull.c b/builtin/pull.c\nindex 127798ba84e..1cfaf9f3436 100644\n--- a/builtin/pull.c\n+++ b/builtin/pull.c\n@@ -988,6 +988,7 @@ int cmd_pull(int argc, const char **argv, const char *prefix)\n \tstruct object_id rebase_fork_point;\n \tint rebase_unspecified = 0;\n \tint can_ff;\n+\tint divergent;\n \n \tif (!getenv(\"GIT_REFLOG_ACTION\"))\n \t\tset_reflog_message(argc, argv);\n@@ -1102,15 +1103,16 @@ int cmd_pull(int argc, const char **argv, const char *prefix)\n \t}\n \n \tcan_ff = get_can_ff(&orig_head, &merge_heads);\n+\tdivergent = !can_ff && !already_up_to_date(&orig_head, &merge_heads);\n \n \t/* ff-only takes precedence over rebase */\n \tif (opt_ff && !strcmp(opt_ff, \"--ff-only\")) {\n-\t\tif (!can_ff && !already_up_to_date(&orig_head, &merge_heads))\n+\t\tif (divergent)\n \t\t\tdie_ff_impossible();\n \t\topt_rebase = REBASE_FALSE;\n \t}\n \t/* If no action specified and we can't fast forward, then warn. */\n-\tif (!opt_ff && rebase_unspecified && !can_ff) {\n+\tif (!opt_ff && rebase_unspecified && divergent) {\n \t\tshow_advice_pull_non_ff();\n \t\tdie(_(\"Need to specify how to reconcile divergent branches.\"));\n \t}\ndiff --git a/t/t7601-merge-pull-config.sh b/t/t7601-merge-pull-config.sh\nindex 6275641b9c2..bd238d89b0c 100755\n--- a/t/t7601-merge-pull-config.sh\n+++ b/t/t7601-merge-pull-config.sh\n@@ -387,6 +387,12 @@ test_expect_success 'pull prevents non-fast-forward with \"only\" in pull.ff' '\n \ttest_must_fail git pull . c3\n '\n \n+test_expect_success 'already-up-to-date pull succeeds with unspecified pull.ff' '\n+\tgit reset --hard c1 &&\n+\tgit pull . c0 &&\n+\ttest \"$(git rev-parse HEAD)\" = \"$(git rev-parse c1)\"\n+'\n+\n test_expect_success 'already-up-to-date pull succeeds with \"only\" in pull.ff' '\n \tgit reset --hard c1 &&\n \ttest_config pull.ff only &&\n\nbase-commit: cd3e606211bb1cf8bc57f7d76bab98cc17a150bc\n-- \ngitgitgadget\n"},{"id":"441676","messageId":"xmqqbl2huiot.fsf@gitster.g","threadId":"56923","inReplyTo":"pull.1078.git.1637135750674.gitgitgadget@gmail.com","subject":"Re: [PATCH] pull: should be noop when already-up-to-date","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-11-18T23:50:58Z","receivedAt":"2021-11-18T23:51:02Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Erwin Villejo via GitGitGadget\" <gitgitgadget@gmail.com> writes:\n\n> From: Erwin Villejo <erwin.villejo@gmail.com>\n>\n> The already-up-to-date pull bug was fixed for --ff-only but it did not\n> include the case where --ff or --ff-only are not specified. This updates\n> the --ff-only fix to include the case where --ff or --ff-only are not\n> specified in command line flags or config.\n>\n> Signed-off-by: Erwin Villejo <erwin.villejo@gmail.com>\n> ---\n\n> @@ -1102,15 +1103,16 @@ int cmd_pull(int argc, const char **argv, const char *prefix)\n>  \t}\n>  \n>  \tcan_ff = get_can_ff(&orig_head, &merge_heads);\n> +\tdivergent = !can_ff && !already_up_to_date(&orig_head, &merge_heads);\n>  \n>  \t/* ff-only takes precedence over rebase */\n>  \tif (opt_ff && !strcmp(opt_ff, \"--ff-only\")) {\n> -\t\tif (!can_ff && !already_up_to_date(&orig_head, &merge_heads))\n> +\t\tif (divergent)\n>  \t\t\tdie_ff_impossible();\n>  \t\topt_rebase = REBASE_FALSE;\n>  \t}\n>  \t/* If no action specified and we can't fast forward, then warn. */\n> -\tif (!opt_ff && rebase_unspecified && !can_ff) {\n> +\tif (!opt_ff && rebase_unspecified && divergent) {\n>  \t\tshow_advice_pull_non_ff();\n>  \t\tdie(_(\"Need to specify how to reconcile divergent branches.\"));\n>  \t}\n\nLooks correct.  I wonder if there is a simplify these two checks,\nbut let's keep the original code structure for now in this step,\nwhich is about correctness.\n\nWill apply.  Thanks.\n\n> diff --git a/t/t7601-merge-pull-config.sh b/t/t7601-merge-pull-config.sh\n> index 6275641b9c2..bd238d89b0c 100755\n> --- a/t/t7601-merge-pull-config.sh\n> +++ b/t/t7601-merge-pull-config.sh\n> @@ -387,6 +387,12 @@ test_expect_success 'pull prevents non-fast-forward with \"only\" in pull.ff' '\n>  \ttest_must_fail git pull . c3\n>  '\n>  \n> +test_expect_success 'already-up-to-date pull succeeds with unspecified pull.ff' '\n> +\tgit reset --hard c1 &&\n> +\tgit pull . c0 &&\n> +\ttest \"$(git rev-parse HEAD)\" = \"$(git rev-parse c1)\"\n> +'\n> +\n>  test_expect_success 'already-up-to-date pull succeeds with \"only\" in pull.ff' '\n>  \tgit reset --hard c1 &&\n>  \ttest_config pull.ff only &&\n>\n> base-commit: cd3e606211bb1cf8bc57f7d76bab98cc17a150bc\n"}]}