{"thread":{"id":"51773","subject":"[PATCH 0/1] checkout: add simple check for 'git checkout -b'","startedAt":"2019-08-29T17:01:35Z","lastAt":"2019-08-30T17:18:50Z","messageCount":11,"participants":["Derrick Stolee via GitGitGadget","Elijah Newren","Phillip Wood","Derrick Stolee","Pratyush Yadav","Taylor Blau","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":1},"messages":[{"id":"381521","messageId":"pull.325.git.gitgitgadget@gmail.com","threadId":"51773","inReplyTo":null,"subject":"[PATCH 0/1] checkout: add simple check for 'git checkout -b'","fromName":"Derrick Stolee via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-08-29T17:01:31Z","receivedAt":"2019-08-29T17:01:35Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"This series replaces \"[RFC] Revert/delay performance regression in 'git\ncheckout -b'\" [1].\n\nThe community response to that series seems to be:\n\n 1. 'git switch' is still experimental, so don't warn that users should\n adopt it right away.\n \n \n 2. 'git checkout -b ' should do the fast thing, and we should check the\n arguments directly.\n \n \n\nThis series makes 'git checkout -b ' do the same thing as 'git switch -c '.\n\nThanks, -Stolee\n\n[1] https://public-inbox.org/git/pull.317.git.gitgitgadget@gmail.com/\n\nDerrick Stolee (1):\n checkout: add simple check for 'git checkout -b'\n\n builtin/checkout.c | 9 +++++++++\n 1 file changed, 9 insertions(+)\n\n\nbase-commit: 5fa0f5238b0cd46cfe7f6fa76c3f526ea98148d9\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-325%2Fderrickstolee%2Fcheckout-b-fast-again-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-325/derrickstolee/checkout-b-fast-again-v1\nPull-Request: https://github.com/gitgitgadget/git/pull/325\n-- \ngitgitgadget\n"},{"id":"381522","messageId":"dcf5c60c69d8275a557ffe3d3ae30911d2140162.1567098090.git.gitgitgadget@gmail.com","threadId":"51773","inReplyTo":"pull.325.git.gitgitgadget@gmail.com","subject":"[PATCH 1/1] checkout: add simple check for 'git checkout -b'","fromName":"Derrick Stolee via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-08-29T17:01:32Z","receivedAt":"2019-08-29T17:01:36Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"From: Derrick Stolee \n\nThe 'git switch' command was created to separate half of the\nbehavior of 'git checkout'. It specifically has the mode to\ndo nothing with the index and working directory if the user\nonly specifies to create a new branch and change HEAD to that\nbranch. This is also the behavior most users expect from\n'git checkout -b', but for historical reasons it also performs\nan index update by scanning the working directory. This can be\nslow for even moderately-sized repos.\n\nA performance fix for 'git checkout -b' was introduced by\nfa655d8411 (checkout: optimize \"git checkout -b \"\n2018-08-16). That change includes details about the config\nsetting checkout.optimizeNewBranch when the sparse-checkout\nfeature is required. The way this change detected if this\nbehavior change is safe was through the skip_merge_working_tree()\nmethod. This method was complex and needed to be updated\nas new options were introduced.\n\nThis behavior was essentially reverted by 65f099b (\"switch:\nno worktree status unless real branch switch happens\"\n2019-03-29). Instead, two members of the checkout_opts struct\nwere used to distinguish between 'git checkout' and 'git switch':\n\n * switch_branch_doing_nothing_is_ok\n * only_merge_on_switching_branches\n\nThese settings have opposite values depending on if we start\nin cmd_checkout or cmd_switch.\n\nThe message for 64f099b includes \"Users of big repos are\nencouraged to move to switch.\" Making this change while\n'git switch' is still experimental is too aggressive.\n\nCreate a happy medium between these two options by making\n'git checkout -b ' behave just like 'git switch',\nbut only if we read exactly those arguments. This must\nbe done in cmd_checkout to avoid the arguments being\nconsumed by the option parsing logic.\n\nThis differs from the previous change by fa644d8 in that\nthe config option checkout.optimizeNewBranch remains\ndeleted. This means that 'git checkout -b' will ignore\nthe index merge even if we have a sparse-checkout file.\nWhile this is a behavior change for 'git checkout -b',\nit matches the behavior of 'git switch -c'.\n\nSigned-off-by: Derrick Stolee \n---\n builtin/checkout.c | 9 +++++++++\n 1 file changed, 9 insertions(+)\n\ndiff --git a/builtin/checkout.c b/builtin/checkout.c\nindex 6123f732a2..116200cf90 100644\n--- a/builtin/checkout.c\n+++ b/builtin/checkout.c\n@@ -1713,6 +1713,15 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)\n \topts.overlay_mode = -1;\n \topts.checkout_index = -2; /* default on */\n \topts.checkout_worktree = -2; /* default on */\n+\t\n+\tif (argc == 3 && !strcmp(argv[1], \"-b\")) {\n+\t\t/*\n+\t\t * User ran 'git checkout -b ' and expects\n+\t\t * the same behavior as 'git switch -c '.\n+\t\t */\n+\t\topts.switch_branch_doing_nothing_is_ok = 0;\n+\t\topts.only_merge_on_switching_branches = 1;\n+\t}\n \n \toptions = parse_options_dup(checkout_options);\n \toptions = add_common_options(&opts, options);\n-- \ngitgitgadget\n"},{"id":"381524","messageId":"CABPp-BEgEOr8f7TeD-oqN6A0MtZMDoeuy4SLdCXn--unRwGHwA@mail.gmail.com","threadId":"51773","inReplyTo":"dcf5c60c69d8275a557ffe3d3ae30911d2140162.1567098090.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 1/1] checkout: add simple check for 'git checkout -b'","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2019-08-29T17:25:03Z","receivedAt":"2019-08-29T17:25:18Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"On Thu, Aug 29, 2019 at 10:04 AM Derrick Stolee via GitGitGadget\n wrote:\n>\n> From: Derrick Stolee \n>\n> The 'git switch' command was created to separate half of the\n> behavior of 'git checkout'. It specifically has the mode to\n> do nothing with the index and working directory if the user\n> only specifies to create a new branch and change HEAD to that\n> branch. This is also the behavior most users expect from\n> 'git checkout -b', but for historical reasons it also performs\n> an index update by scanning the working directory. This can be\n> slow for even moderately-sized repos.\n>\n> A performance fix for 'git checkout -b' was introduced by\n> fa655d8411 (checkout: optimize \"git checkout -b \"\n> 2018-08-16). That change includes details about the config\n> setting checkout.optimizeNewBranch when the sparse-checkout\n> feature is required. The way this change detected if this\n> behavior change is safe was through the skip_merge_working_tree()\n> method. This method was complex and needed to be updated\n> as new options were introduced.\n>\n> This behavior was essentially reverted by 65f099b (\"switch:\n> no worktree status unless real branch switch happens\"\n> 2019-03-29). Instead, two members of the checkout_opts struct\n> were used to distinguish between 'git checkout' and 'git switch':\n>\n> * switch_branch_doing_nothing_is_ok\n> * only_merge_on_switching_branches\n>\n> These settings have opposite values depending on if we start\n> in cmd_checkout or cmd_switch.\n>\n> The message for 64f099b includes \"Users of big repos are\n> encouraged to move to switch.\" Making this change while\n> 'git switch' is still experimental is too aggressive.\n>\n> Create a happy medium between these two options by making\n> 'git checkout -b ' behave just like 'git switch',\n> but only if we read exactly those arguments. This must\n> be done in cmd_checkout to avoid the arguments being\n> consumed by the option parsing logic.\n>\n> This differs from the previous change by fa644d8 in that\n> the config option checkout.optimizeNewBranch remains\n> deleted. This means that 'git checkout -b' will ignore\n> the index merge even if we have a sparse-checkout file.\n> While this is a behavior change for 'git checkout -b',\n> it matches the behavior of 'git switch -c'.\n>\n> Signed-off-by: Derrick Stolee \n> ---\n> builtin/checkout.c | 9 +++++++++\n> 1 file changed, 9 insertions(+)\n>\n> diff --git a/builtin/checkout.c b/builtin/checkout.c\n> index 6123f732a2..116200cf90 100644\n> --- a/builtin/checkout.c\n> +++ b/builtin/checkout.c\n> @@ -1713,6 +1713,15 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)\n> opts.overlay_mode = -1;\n> opts.checkout_index = -2; /* default on */\n> opts.checkout_worktree = -2; /* default on */\n> +\n> + if (argc == 3 && !strcmp(argv[1], \"-b\")) {\n> + /*\n> + * User ran 'git checkout -b ' and expects\n> + * the same behavior as 'git switch -c '.\n> + */\n> + opts.switch_branch_doing_nothing_is_ok = 0;\n> + opts.only_merge_on_switching_branches = 1;\n> + }\n>\n> options = parse_options_dup(checkout_options);\n> options = add_common_options(&opts, options);\n> --\n> gitgitgadget\n\nNice! Thanks for doing this; a small and localized performance hack\nis much nicer than a big and non-localized one. I also appreciate the\ndetailed history in the commit message.\n\nJust for fun, I tested on linux (with a relatively fast SSD) using a\nsimple git-bomb repo with 10M index entries but a sparse checkout of\njust one file. 'git switch -c' takes approximately 0.004s before or\nafter this patch. 'git checkout -b' before this patch:\n\n$ time git checkout -b newbranch1\nSwitched to a new branch 'newbranch1'\n\nreal 0m13.533s\nuser 0m9.824s\nsys 0m2.828s\n\n\nAfter this patch:\n\n$ time git checkout -b newbranch2\nSwitched to a new branch 'newbranch2'\n\nreal 0m0.003s\nuser 0m0.000s\nsys 0m0.000s\n\n\nAnyway, looks good to me.\n"},{"id":"381535","messageId":"7e406b35-9ff6-348f-bdee-7540381f000e@gmail.com","threadId":"51773","inReplyTo":"dcf5c60c69d8275a557ffe3d3ae30911d2140162.1567098090.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 1/1] checkout: add simple check for 'git checkout -b'","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2019-08-29T18:54:08Z","receivedAt":"2019-08-29T18:54:13Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"Hi Stolee\n\nOn 29/08/2019 18:01, Derrick Stolee via GitGitGadget wrote:\n> From: Derrick Stolee \n> \n> The 'git switch' command was created to separate half of the\n> behavior of 'git checkout'. It specifically has the mode to\n> do nothing with the index and working directory if the user\n> only specifies to create a new branch and change HEAD to that\n> branch. This is also the behavior most users expect from\n> 'git checkout -b', but for historical reasons it also performs\n> an index update by scanning the working directory. This can be\n> slow for even moderately-sized repos.\n> \n> A performance fix for 'git checkout -b' was introduced by\n> fa655d8411 (checkout: optimize \"git checkout -b \"\n> 2018-08-16). That change includes details about the config\n> setting checkout.optimizeNewBranch when the sparse-checkout\n> feature is required. The way this change detected if this\n> behavior change is safe was through the skip_merge_working_tree()\n> method. This method was complex and needed to be updated\n> as new options were introduced.\n> \n> This behavior was essentially reverted by 65f099b (\"switch:\n> no worktree status unless real branch switch happens\"\n> 2019-03-29). Instead, two members of the checkout_opts struct\n> were used to distinguish between 'git checkout' and 'git switch':\n> \n> * switch_branch_doing_nothing_is_ok\n> * only_merge_on_switching_branches\n> \n> These settings have opposite values depending on if we start\n> in cmd_checkout or cmd_switch.\n> \n> The message for 64f099b includes \"Users of big repos are\n> encouraged to move to switch.\" Making this change while\n> 'git switch' is still experimental is too aggressive.\n> \n> Create a happy medium between these two options by making\n> 'git checkout -b ' behave just like 'git switch',\n> but only if we read exactly those arguments. This must\n> be done in cmd_checkout to avoid the arguments being\n> consumed by the option parsing logic.\n> \n> This differs from the previous change by fa644d8 in that\n> the config option checkout.optimizeNewBranch remains\n> deleted. This means that 'git checkout -b' will ignore\n> the index merge even if we have a sparse-checkout file.\n> While this is a behavior change for 'git checkout -b',\n> it matches the behavior of 'git switch -c'.\n> \n> Signed-off-by: Derrick Stolee \n> ---\n> builtin/checkout.c | 9 +++++++++\n> 1 file changed, 9 insertions(+)\n> \n> diff --git a/builtin/checkout.c b/builtin/checkout.c\n> index 6123f732a2..116200cf90 100644\n> --- a/builtin/checkout.c\n> +++ b/builtin/checkout.c\n> @@ -1713,6 +1713,15 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)\n> \topts.overlay_mode = -1;\n> \topts.checkout_index = -2; /* default on */\n> \topts.checkout_worktree = -2; /* default on */\n> +\t\n> +\tif (argc == 3 && !strcmp(argv[1], \"-b\")) {\n> +\t\t/*\n> +\t\t * User ran 'git checkout -b ' and expects\n\nWhat if the user ran 'git checkout -b'? Then argc == 2.\n\nBest Wishes\n\nPhillip\n\n> +\t\t * the same behavior as 'git switch -c '.\n> +\t\t */\n> +\t\topts.switch_branch_doing_nothing_is_ok = 0;\n> +\t\topts.only_merge_on_switching_branches = 1;\n> +\t}\n> \n> \toptions = parse_options_dup(checkout_options);\n> \toptions = add_common_options(&opts, options);\n> \n"},{"id":"381539","messageId":"206819d0-8066-e8e8-8967-8a1db05eea26@gmail.com","threadId":"51773","inReplyTo":"7e406b35-9ff6-348f-bdee-7540381f000e@gmail.com","subject":"Re: [PATCH 1/1] checkout: add simple check for 'git checkout -b'","fromName":"Derrick Stolee","fromEmail":"stolee@gmail.com","sentAt":"2019-08-29T20:07:38Z","receivedAt":"2019-08-29T20:07:43Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 8/29/2019 2:54 PM, Phillip Wood wrote:\n> Hi Stolee\n> \n> On 29/08/2019 18:01, Derrick Stolee via GitGitGadget wrote:\n>> +   \n>> +    if (argc == 3 && !strcmp(argv[1], \"-b\")) {\n>> +        /*\n>> +         * User ran 'git checkout -b ' and expects\n> \n> What if the user ran 'git checkout -b'? Then argc == 2.\n\nGood catch. I'm tempted to say \"don't do that\" to keep this\nsimple. They won't have incorrect results, just slower than\nthe \"with space\" option.\n\nHowever, if there is enough interest in correcting the \"-b\"\ncase, then I can make another attempt at this.\n\n-Stolee\n"},{"id":"381541","messageId":"20190829203037.vzxuaxpsghbdc6vl@yadavpratyush.com","threadId":"51773","inReplyTo":"206819d0-8066-e8e8-8967-8a1db05eea26@gmail.com","subject":"Re: [PATCH 1/1] checkout: add simple check for 'git checkout -b'","fromName":"Pratyush Yadav","fromEmail":"me@yadavpratyush.com","sentAt":"2019-08-29T20:30:37Z","receivedAt":"2019-08-29T20:30:43Z","isPatch":true,"sender":{"key":"me@yadavpratyush.com","avatar":"https://avatars.githubusercontent.com/u/8817931?v=4"},"body":"On 29/08/19 04:07PM, Derrick Stolee wrote:\n> On 8/29/2019 2:54 PM, Phillip Wood wrote:\n> > Hi Stolee\n> > \n> > On 29/08/2019 18:01, Derrick Stolee via GitGitGadget wrote:\n> >> +   \n> >> +    if (argc == 3 && !strcmp(argv[1], \"-b\")) {\n> >> +        /*\n> >> +         * User ran 'git checkout -b ' and expects\n> > \n> > What if the user ran 'git checkout -b'? Then argc == 2.\n> \n> Good catch. I'm tempted to say \"don't do that\" to keep this\n> simple. They won't have incorrect results, just slower than\n> the \"with space\" option.\n> \n> However, if there is enough interest in correcting the \"-b\"\n> case, then I can make another attempt at this.\n \nYou can probably do this with:\n\n !strncmp(argv[1], \"-b\", 2)\n\nThe difference is so little, might as well do it IMO.\n\n-- \nRegards,\nPratyush Yadav\n"},{"id":"381543","messageId":"20190829214014.vijdotp4g65a5mk3@yadavpratyush.com","threadId":"51773","inReplyTo":"20190829203037.vzxuaxpsghbdc6vl@yadavpratyush.com","subject":"Re: [PATCH 1/1] checkout: add simple check for 'git checkout -b'","fromName":"Pratyush Yadav","fromEmail":"me@yadavpratyush.com","sentAt":"2019-08-29T21:40:14Z","receivedAt":"2019-08-29T21:40:20Z","isPatch":true,"sender":{"key":"me@yadavpratyush.com","avatar":"https://avatars.githubusercontent.com/u/8817931?v=4"},"body":"On 30/08/19 02:00AM, Pratyush Yadav wrote:\n> On 29/08/19 04:07PM, Derrick Stolee wrote:\n> > On 8/29/2019 2:54 PM, Phillip Wood wrote:\n> > > Hi Stolee\n> > > \n> > > On 29/08/2019 18:01, Derrick Stolee via GitGitGadget wrote:\n> > >> +   \n> > >> +    if (argc == 3 && !strcmp(argv[1], \"-b\")) {\n> > >> +        /*\n> > >> +         * User ran 'git checkout -b ' and expects\n> > > \n> > > What if the user ran 'git checkout -b'? Then argc == 2.\n> > \n> > Good catch. I'm tempted to say \"don't do that\" to keep this\n> > simple. They won't have incorrect results, just slower than\n> > the \"with space\" option.\n> > \n> > However, if there is enough interest in correcting the \"-b\"\n> > case, then I can make another attempt at this.\n> \n> You can probably do this with:\n> \n> !strncmp(argv[1], \"-b\", 2)\n> \n> The difference is so little, might as well do it IMO.\n \nActually, that is not correct. I took a quick look before writing this \nand missed the fact that argc == 3 is the bigger problem.\n\nThinking a little more about this, you can mix other options with \ncheckout -b, like --track. You can also specify .\n\nNow I don't know enough about this optimization you are doing to know \nwhether we need to optimize when these options are given, but at least \nfor --track I don't see any reason not to.\n\nSo maybe you are better off using something like getopt() (warning: \ngetopt modifies the input string so you probably want to duplicate it) \nif you want to support all cases. Though for this simple case you can \nprobably get away by just directly scanning the argv list for \"-b\" \n(using strncmp instead of strcmp to account for \"-b)\n\n-- \nRegards,\nPratyush Yadav\n"},{"id":"381557","messageId":"CABPp-BF_uBTKT_5YmoMNoToiujuMdQia-OfxOPAJPrhT6jPbdA@mail.gmail.com","threadId":"51773","inReplyTo":"20190829214014.vijdotp4g65a5mk3@yadavpratyush.com","subject":"Re: [PATCH 1/1] checkout: add simple check for 'git checkout -b'","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2019-08-30T00:19:44Z","receivedAt":"2019-08-30T00:19:58Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"On Thu, Aug 29, 2019 at 2:42 PM Pratyush Yadav wrote:\n>\n> On 30/08/19 02:00AM, Pratyush Yadav wrote:\n> > On 29/08/19 04:07PM, Derrick Stolee wrote:\n> > > On 8/29/2019 2:54 PM, Phillip Wood wrote:\n> > > > Hi Stolee\n> > > >\n> > > > On 29/08/2019 18:01, Derrick Stolee via GitGitGadget wrote:\n> > > >> +\n> > > >> + if (argc == 3 && !strcmp(argv[1], \"-b\")) {\n> > > >> + /*\n> > > >> + * User ran 'git checkout -b ' and expects\n> > > >\n> > > > What if the user ran 'git checkout -b'? Then argc == 2.\n> > >\n> > > Good catch. I'm tempted to say \"don't do that\" to keep this\n> > > simple. They won't have incorrect results, just slower than\n> > > the \"with space\" option.\n> > >\n> > > However, if there is enough interest in correcting the \"-b\"\n> > > case, then I can make another attempt at this.\n> >\n> > You can probably do this with:\n> >\n> > !strncmp(argv[1], \"-b\", 2)\n> >\n> > The difference is so little, might as well do it IMO.\n>\n> Actually, that is not correct. I took a quick look before writing this\n> and missed the fact that argc == 3 is the bigger problem.\n>\n> Thinking a little more about this, you can mix other options with\n> checkout -b, like --track. You can also specify .\n>\n> Now I don't know enough about this optimization you are doing to know\n> whether we need to optimize when these options are given, but at least\n> for --track I don't see any reason not to.\n>\n> So maybe you are better off using something like getopt() (warning:\n> getopt modifies the input string so you probably want to duplicate it)\n> if you want to support all cases. Though for this simple case you can\n> probably get away by just directly scanning the argv list for \"-b\"\n> (using strncmp instead of strcmp to account for \"-b)\n\nNO. This would be unsafe to use if is specified. I\nthink either -f or -m together with -b make no sense unless\n is specified, but if they do make sense separately, I'm\nguessing this hack should not be used with those flags. And\nadditional flags may appear in the future that should not be used\ntogether with this hack.\n\nPersonally, although I understand the desire to support any possible\ncases in general, *this is a performance hack*. As such, it should be\nas simple and localized as possible. I don't think supporting\nold-style stuck flags (-b$BRANCH) is worth complicating this. I'm\neven leery of adding support for --track (do any users of huge repos\nuse -b with --track? Does anyone at all use --track anymore? I'm not\nsure I've ever seen any user use that flag in the last 10 years other\nthan myself.) Besides, in the *worst* possible case, the command the\nuser specifies works just fine...it just takes a little longer. My\nopinion is that Stolee's patch is perfect as-is and should not be\ngeneralized at all.\n\nJust my $0.02,\nElijah\n"},{"id":"381559","messageId":"20190830004339.GA34082@syl.lan","threadId":"51773","inReplyTo":"CABPp-BF_uBTKT_5YmoMNoToiujuMdQia-OfxOPAJPrhT6jPbdA@mail.gmail.com","subject":"Re: [PATCH 1/1] checkout: add simple check for 'git checkout -b'","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2019-08-30T00:43:39Z","receivedAt":"2019-08-30T00:43:43Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"Hi Elijah,\n\nOn Thu, Aug 29, 2019 at 05:19:44PM -0700, Elijah Newren wrote:\n> On Thu, Aug 29, 2019 at 2:42 PM Pratyush Yadav wrote:\n> >\n> > On 30/08/19 02:00AM, Pratyush Yadav wrote:\n> > > On 29/08/19 04:07PM, Derrick Stolee wrote:\n> > > > On 8/29/2019 2:54 PM, Phillip Wood wrote:\n> > > > > Hi Stolee\n> > > > >\n> > > > > On 29/08/2019 18:01, Derrick Stolee via GitGitGadget wrote:\n> > > > >> +\n> > > > >> + if (argc == 3 && !strcmp(argv[1], \"-b\")) {\n> > > > >> + /*\n> > > > >> + * User ran 'git checkout -b ' and expects\n> > > > >\n> > > > > What if the user ran 'git checkout -b'? Then argc == 2.\n> > > >\n> > > > Good catch. I'm tempted to say \"don't do that\" to keep this\n> > > > simple. They won't have incorrect results, just slower than\n> > > > the \"with space\" option.\n> > > >\n> > > > However, if there is enough interest in correcting the \"-b\"\n> > > > case, then I can make another attempt at this.\n> > >\n> > > You can probably do this with:\n> > >\n> > > !strncmp(argv[1], \"-b\", 2)\n> > >\n> > > The difference is so little, might as well do it IMO.\n> >\n> > Actually, that is not correct. I took a quick look before writing this\n> > and missed the fact that argc == 3 is the bigger problem.\n> >\n> > Thinking a little more about this, you can mix other options with\n> > checkout -b, like --track. You can also specify .\n> >\n> > Now I don't know enough about this optimization you are doing to know\n> > whether we need to optimize when these options are given, but at least\n> > for --track I don't see any reason not to.\n> >\n> > So maybe you are better off using something like getopt() (warning:\n> > getopt modifies the input string so you probably want to duplicate it)\n> > if you want to support all cases. Though for this simple case you can\n> > probably get away by just directly scanning the argv list for \"-b\"\n> > (using strncmp instead of strcmp to account for \"-b)\n>\n> NO. This would be unsafe to use if is specified. I\n> think either -f or -m together with -b make no sense unless\n> is specified, but if they do make sense separately, I'm\n> guessing this hack should not be used with those flags. And\n> additional flags may appear in the future that should not be used\n> together with this hack.\n>\n> Personally, although I understand the desire to support any possible\n> cases in general, *this is a performance hack*. As such, it should be\n> as simple and localized as possible. I don't think supporting\n> old-style stuck flags (-b$BRANCH) is worth complicating this. I'm\n> even leery of adding support for --track (do any users of huge repos\n> use -b with --track? Does anyone at all use --track anymore? I'm not\n> sure I've ever seen any user use that flag in the last 10 years other\n> than myself.) Besides, in the *worst* possible case, the command the\n> user specifies works just fine...it just takes a little longer. My\n> opinion is that Stolee's patch is perfect as-is and should not be\n> generalized at all.\n\nI wholeheartedly agree with this, and pledge my $.02 towards it as well.\nNow with a combined total of $.04, I think that this patch is ready for\nqueueing as-is.\n\n> Just my $0.02,\n> Elijah\n\nThanks,\nTaylor\n"},{"id":"381592","messageId":"8bf76fcf-539c-6fea-170e-61fc81822ef0@gmail.com","threadId":"51773","inReplyTo":"20190830004339.GA34082@syl.lan","subject":"Re: [PATCH 1/1] checkout: add simple check for 'git checkout -b'","fromName":"Derrick Stolee","fromEmail":"stolee@gmail.com","sentAt":"2019-08-30T16:56:44Z","receivedAt":"2019-08-30T16:56:54Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 8/29/2019 8:43 PM, Taylor Blau wrote:\n> Hi Elijah,\n> \n> On Thu, Aug 29, 2019 at 05:19:44PM -0700, Elijah Newren wrote:\n>> Personally, although I understand the desire to support any possible\n>> cases in general, *this is a performance hack*. As such, it should be\n>> as simple and localized as possible. I don't think supporting\n>> old-style stuck flags (-b$BRANCH) is worth complicating this. I'm\n>> even leery of adding support for --track (do any users of huge repos\n>> use -b with --track? Does anyone at all use --track anymore? I'm not\n>> sure I've ever seen any user use that flag in the last 10 years other\n>> than myself.) Besides, in the *worst* possible case, the command the\n>> user specifies works just fine...it just takes a little longer. My\n>> opinion is that Stolee's patch is perfect as-is and should not be\n>> generalized at all.\n> \n> I wholeheartedly agree with this, and pledge my $.02 towards it as well.\n> Now with a combined total of $.04, I think that this patch is ready for\n> queueing as-is.\n\nThanks, both!\n\n"},{"id":"381594","messageId":"xmqqzhjqy3ke.fsf@gitster-ct.c.googlers.com","threadId":"51773","inReplyTo":"20190830004339.GA34082@syl.lan","subject":"Re: [PATCH 1/1] checkout: add simple check for 'git checkout -b'","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-08-30T17:18:41Z","receivedAt":"2019-08-30T17:18:50Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Taylor Blau writes:\n\n> I wholeheartedly agree with this, and pledge my $.02 towards it as well.\n> Now with a combined total of $.04, I think that this patch is ready for\n> queueing as-is.\n\n;-)\n"}]}