{"thread":{"id":"55973","subject":"[PATCH] bisect: allow to run from subdirectories","startedAt":"2021-06-20T21:39:21Z","lastAt":"2021-06-29T02:00:03Z","messageCount":13,"participants":["Roland Hieber","Ævar Arnfjörð Bjarmason","brian m. carlson","Eric Sunshine","Junio C Hamano","Jeff King"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"428004","messageId":"20210620213836.10771-1-rhi@pengutronix.de","threadId":"55973","inReplyTo":null,"subject":"[PATCH] bisect: allow to run from subdirectories","fromName":"Roland Hieber","fromEmail":"rhi@pengutronix.de","sentAt":"2021-06-20T21:38:37Z","receivedAt":"2021-06-20T21:39:21Z","isPatch":true,"sender":{"key":"rhi@pengutronix.de","avatar":"https://avatars.githubusercontent.com/u/192014?v=4"},"body":"Currently, calling 'git bisect' from a directory other than the top\nlevel of a repository only comes up with an error message:\n\n You need to run this command from the toplevel of the working tree.\n\nAfter a glance through the bisect code, there seems to be nothing that\nrelies on the current working directory, and a few hours of bisect usage\nalso didn't turn up any problems. Set the appropriate flag for\ngit-sh-setup to remove the error message.\n\nSigned-off-by: Roland Hieber \n---\n git-bisect.sh | 1 +\n 1 file changed, 1 insertion(+)\n\ndiff --git a/git-bisect.sh b/git-bisect.sh\nindex 6a7afaea8da0..20ba0ee7c18a 100755\n--- a/git-bisect.sh\n+++ b/git-bisect.sh\n@@ -32,6 +32,7 @@ git bisect run ...\n Please use \"git help bisect\" to get the full man page.'\n \n OPTIONS_SPEC=\n+SUBDIRECTORY_OK=1\n . git-sh-setup\n \n _x40='[0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f]'\n-- \n2.29.2\n\n"},{"id":"428007","messageId":"877dio3vg5.fsf@evledraar.gmail.com","threadId":"55973","inReplyTo":"20210620213836.10771-1-rhi@pengutronix.de","subject":"Re: [PATCH] bisect: allow to run from subdirectories","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-06-21T00:35:49Z","receivedAt":"2021-06-21T00:36:33Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Sun, Jun 20 2021, Roland Hieber wrote:\n\n> Currently, calling 'git bisect' from a directory other than the top\n> level of a repository only comes up with an error message:\n>\n> You need to run this command from the toplevel of the working tree.\n>\n> After a glance through the bisect code, there seems to be nothing that\n> relies on the current working directory, and a few hours of bisect usage\n> also didn't turn up any problems. Set the appropriate flag for\n> git-sh-setup to remove the error message.\n>\n> Signed-off-by: Roland Hieber \n> ---\n> git-bisect.sh | 1 +\n> 1 file changed, 1 insertion(+)\n>\n> diff --git a/git-bisect.sh b/git-bisect.sh\n> index 6a7afaea8da0..20ba0ee7c18a 100755\n> --- a/git-bisect.sh\n> +++ b/git-bisect.sh\n> @@ -32,6 +32,7 @@ git bisect run ...\n> Please use \"git help bisect\" to get the full man page.'\n> \n> OPTIONS_SPEC=\n> +SUBDIRECTORY_OK=1\n> . git-sh-setup\n> \n> _x40='[0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f]'\n\nHow does this affect out-of-tree scripts that will be run with \"git\nbisect run\", is the cwd set to the root as they now might expect git to\ncheck, or whatever subdirectory you ran the \"run\" from?\n"},{"id":"428010","messageId":"YM/yqPsRutV2mUbb@camp.crustytoothpaste.net","threadId":"55973","inReplyTo":"877dio3vg5.fsf@evledraar.gmail.com","subject":"Re: [PATCH] bisect: allow to run from subdirectories","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2021-06-21T02:00:08Z","receivedAt":"2021-06-21T02:00:47Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On 2021-06-21 at 00:35:49, Ævar Arnfjörð Bjarmason wrote:\n> \n> On Sun, Jun 20 2021, Roland Hieber wrote:\n> \n> > Currently, calling 'git bisect' from a directory other than the top\n> > level of a repository only comes up with an error message:\n> >\n> > You need to run this command from the toplevel of the working tree.\n> >\n> > After a glance through the bisect code, there seems to be nothing that\n> > relies on the current working directory, and a few hours of bisect usage\n> > also didn't turn up any problems. Set the appropriate flag for\n> > git-sh-setup to remove the error message.\n> >\n> > Signed-off-by: Roland Hieber \n> > ---\n> > git-bisect.sh | 1 +\n> > 1 file changed, 1 insertion(+)\n> >\n> > diff --git a/git-bisect.sh b/git-bisect.sh\n> > index 6a7afaea8da0..20ba0ee7c18a 100755\n> > --- a/git-bisect.sh\n> > +++ b/git-bisect.sh\n> > @@ -32,6 +32,7 @@ git bisect run ...\n> > Please use \"git help bisect\" to get the full man page.'\n> > \n> > OPTIONS_SPEC=\n> > +SUBDIRECTORY_OK=1\n> > . git-sh-setup\n> > \n> > _x40='[0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f]'\n> \n> How does this affect out-of-tree scripts that will be run with \"git\n> bisect run\", is the cwd set to the root as they now might expect git to\n> check, or whatever subdirectory you ran the \"run\" from?\n\nI'm also interested in this, specifically as a patch to the\ndocumentation and a corresponding test (and a commit message\njustification), since folks will rely on whatever behavior we implement\nand we won't want to break it.\n\nWe'd probably also want to add a test at least that the user can invoke\ngit bisect outside of the root of the repository, and maybe that it\nperforms correct results for at least one or two known cases when\ninvoked outside of the root. And I'm also wondering if maybe there are\nother cases that deserve a test along with this change.\n\nAs for the idea itself, I think it's a good one assuming everything\ncontinues to work. It will certainly be more convenient for a lot of\npeople.\n-- \nbrian m. carlson (he/him or they/them)\nToronto, Ontario, CA\n"},{"id":"428012","messageId":"CAPig+cQa3JC-YkGeB-NDy-iP=Mv8BRL6jyXTGXzaqpTLcxj8+w@mail.gmail.com","threadId":"55973","inReplyTo":"YM/yqPsRutV2mUbb@camp.crustytoothpaste.net","subject":"Re: [PATCH] bisect: allow to run from subdirectories","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2021-06-21T02:10:10Z","receivedAt":"2021-06-21T02:10:24Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Sun, Jun 20, 2021 at 10:00 PM brian m. carlson\n wrote:\n> On 2021-06-21 at 00:35:49, Ævar Arnfjörð Bjarmason wrote:\n> > On Sun, Jun 20 2021, Roland Hieber wrote:\n> > > Currently, calling 'git bisect' from a directory other than the top\n> > > level of a repository only comes up with an error message:\n> > >\n> > > You need to run this command from the toplevel of the working tree.\n> >\n> > How does this affect out-of-tree scripts that will be run with \"git\n> > bisect run\", is the cwd set to the root as they now might expect git to\n> > check, or whatever subdirectory you ran the \"run\" from?\n>\n> As for the idea itself, I think it's a good one assuming everything\n> continues to work. It will certainly be more convenient for a lot of\n> people.\n\nThere have been multiple patches sent to the project over the years\nwith the same purpose. One problem, I believe, which has never been\nfully addressed is what happens when the subdirectory from which\ngit-bisect is run gets deleted as part of the bisection.\n\nHere are a couple recent threads triggered by previous such patches\n(but there are probably several more):\n\nhttps://lore.kernel.org/git/pull.765.git.1603271344522.gitgitgadget@gmail.com/\nhttps://lore.kernel.org/git/pull.736.git.git.1584868547682.gitgitgadget@gmail.com/\n"},{"id":"428013","messageId":"xmqqy2b3j317.fsf@gitster.g","threadId":"55973","inReplyTo":"20210620213836.10771-1-rhi@pengutronix.de","subject":"Re: [PATCH] bisect: allow to run from subdirectories","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-06-21T03:43:32Z","receivedAt":"2021-06-21T03:43:57Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Roland Hieber writes:\n\n> Currently, calling 'git bisect' from a directory other than the top\n> level of a repository only comes up with an error message:\n>\n> You need to run this command from the toplevel of the working tree.\n>\n> After a glance through the bisect code, there seems to be nothing that\n> relies on the current working directory, and a few hours of bisect usage\n> also didn't turn up any problems. Set the appropriate flag for\n> git-sh-setup to remove the error message.\n\nTry to find a history in which to run a bisect that\n\n (1) has a directory T in the recent part of the history, and\n\n (2) does not have that directory in the older part of the history.\n Better yet, if the older part of the history has T as a regular\n file, that would be ideal. Even better, if that old regular\n file T was added, then removed, and then recreated, before it\n got turned into a directory.\n\nNow, using the two commits you used to satisfy conditions (1) and\n(2) as \"bad\" and \"good\", and using \"bad - has T as a directory\" and\n\"good - does not have T as a directory\", as the bisection criterion,\ntry to find where \"good\" turns \"bad\"---in other words, find the\ncommit that either creates T as a directory or turns the regular\nfile T into a directory.\n\nPerform that bisect from the subdirectory T. Would that work? I\nsuspect it wouldn't; when trying to check out an old version that\ndoes not have T in the directory, either the checkout would fail\nbecause it cannot remove T which has an active process (i.e. your\nterminal session) in it, or your process sitting in an orphaned\ndirectory (i.e. your \"..\" may still be the original top-level\ndirectory that used to have T subdirectory, but \"cd T\" from the\ntop-level will not reach where you are). All sorts of things can\ngo wrong and that is why we forbid it. Just a single \"cd\" upfront\nwould save the user a lot of headache.\n\nThis does not depend on \"do we have T as a directory?\" being the\nbisection criteria. The important thing is that the current\ndirectory would appear and disappear as the bisection process makes\nyou jump around in the history.\n\nHope this helps understanding the issue.\n"},{"id":"428026","messageId":"20210621093310.eqt3ajic64pgag44@pengutronix.de","threadId":"55973","inReplyTo":"CAPig+cQa3JC-YkGeB-NDy-iP=Mv8BRL6jyXTGXzaqpTLcxj8+w@mail.gmail.com","subject":"Re: [PATCH] bisect: allow to run from subdirectories","fromName":"Roland Hieber","fromEmail":"rhi@pengutronix.de","sentAt":"2021-06-21T09:33:10Z","receivedAt":"2021-06-21T09:33:24Z","isPatch":true,"sender":{"key":"rhi@pengutronix.de","avatar":"https://avatars.githubusercontent.com/u/192014?v=4"},"body":"On Sun, Jun 20, 2021 at 10:10:10PM -0400, Eric Sunshine wrote:\n> On Sun, Jun 20, 2021 at 10:00 PM brian m. carlson\n> wrote:\n> > On 2021-06-21 at 00:35:49, Ævar Arnfjörð Bjarmason wrote:\n> > > On Sun, Jun 20 2021, Roland Hieber wrote:\n> > > > Currently, calling 'git bisect' from a directory other than the top\n> > > > level of a repository only comes up with an error message:\n> > > >\n> > > > You need to run this command from the toplevel of the working tree.\n> > >\n> > > How does this affect out-of-tree scripts that will be run with \"git\n> > > bisect run\", is the cwd set to the root as they now might expect git to\n> > > check, or whatever subdirectory you ran the \"run\" from?\n> >\n> > As for the idea itself, I think it's a good one assuming everything\n> > continues to work. It will certainly be more convenient for a lot of\n> > people.\n> \n> There have been multiple patches sent to the project over the years\n> with the same purpose. One problem, I believe, which has never been\n> fully addressed is what happens when the subdirectory from which\n> git-bisect is run gets deleted as part of the bisection.\n> \n> Here are a couple recent threads triggered by previous such patches\n> (but there are probably several more):\n> \n> https://lore.kernel.org/git/pull.765.git.1603271344522.gitgitgadget@gmail.com/\n> https://lore.kernel.org/git/pull.736.git.git.1584868547682.gitgitgadget@gmail.com/\n\nAh, thanks for explaining the problem. Would a patch that adds a short\nexplanatory comment in git-bisect.sh on the matter help to prevent\npeople sending such patches?\n\n - Roland\n\n-- \nRoland Hieber, Pengutronix e.K. | r.hieber@pengutronix.de |\nSteuerwalder Str. 21 | https://www.pengutronix.de/ |\n31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |\nAmtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |\n"},{"id":"428031","messageId":"87sg1b2xhl.fsf@evledraar.gmail.com","threadId":"55973","inReplyTo":"20210621093310.eqt3ajic64pgag44@pengutronix.de","subject":"Re: [PATCH] bisect: allow to run from subdirectories","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-06-21T12:45:49Z","receivedAt":"2021-06-21T12:50:04Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Mon, Jun 21 2021, Roland Hieber wrote:\n\n> On Sun, Jun 20, 2021 at 10:10:10PM -0400, Eric Sunshine wrote:\n>> On Sun, Jun 20, 2021 at 10:00 PM brian m. carlson\n>> wrote:\n>> > On 2021-06-21 at 00:35:49, Ævar Arnfjörð Bjarmason wrote:\n>> > > On Sun, Jun 20 2021, Roland Hieber wrote:\n>> > > > Currently, calling 'git bisect' from a directory other than the top\n>> > > > level of a repository only comes up with an error message:\n>> > > >\n>> > > > You need to run this command from the toplevel of the working tree.\n>> > >\n>> > > How does this affect out-of-tree scripts that will be run with \"git\n>> > > bisect run\", is the cwd set to the root as they now might expect git to\n>> > > check, or whatever subdirectory you ran the \"run\" from?\n>> >\n>> > As for the idea itself, I think it's a good one assuming everything\n>> > continues to work. It will certainly be more convenient for a lot of\n>> > people.\n>> \n>> There have been multiple patches sent to the project over the years\n>> with the same purpose. One problem, I believe, which has never been\n>> fully addressed is what happens when the subdirectory from which\n>> git-bisect is run gets deleted as part of the bisection.\n>> \n>> Here are a couple recent threads triggered by previous such patches\n>> (but there are probably several more):\n>> \n>> https://lore.kernel.org/git/pull.765.git.1603271344522.gitgitgadget@gmail.com/\n>> https://lore.kernel.org/git/pull.736.git.git.1584868547682.gitgitgadget@gmail.com/\n>\n> Ah, thanks for explaining the problem. Would a patch that adds a short\n> explanatory comment in git-bisect.sh on the matter help to prevent\n> people sending such patches?\n\nHaving skimmed the linked discussions I don't think the consensus is\nthat this shouldn't exist, but that someone who wants it should do some\nresearch on the relevant edge cases, come up with test cases for them,\ndiscuss the trade-offs in a commit message etc.\n\nI for one would welcome such a feature, it's often annoyed me, it should\njust work like \"rebase exec\" in that a \"run\" script should cd to the\nroot, but (as discussed in the linked threads) I don't see why we'd\nprevent it any more than several other commands that already have this\nedge case, but don't explicitly prevent this.\n"},{"id":"428097","messageId":"CAPig+cT9kcuNrwTTRRbZDNWPTb8PVyDVWaUSQvFr0pKj3R2YGQ@mail.gmail.com","threadId":"55973","inReplyTo":"87sg1b2xhl.fsf@evledraar.gmail.com","subject":"Re: [PATCH] bisect: allow to run from subdirectories","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2021-06-21T20:02:37Z","receivedAt":"2021-06-21T20:02:50Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Mon, Jun 21, 2021 at 8:50 AM Ævar Arnfjörð Bjarmason\n wrote:\n> On Mon, Jun 21 2021, Roland Hieber wrote:\n> > On Sun, Jun 20, 2021 at 10:10:10PM -0400, Eric Sunshine wrote:\n> >> There have been multiple patches sent to the project over the years\n> >> with the same purpose. One problem, I believe, which has never been\n> >> fully addressed is what happens when the subdirectory from which\n> >> git-bisect is run gets deleted as part of the bisection.\n> >\n> > Ah, thanks for explaining the problem. Would a patch that adds a short\n> > explanatory comment in git-bisect.sh on the matter help to prevent\n> > people sending such patches?\n>\n> Having skimmed the linked discussions I don't think the consensus is\n> that this shouldn't exist, but that someone who wants it should do some\n> research on the relevant edge cases, come up with test cases for them,\n> discuss the trade-offs in a commit message etc.\n\nBe that as it may, having a comment in the code explaining why it is\ncurrently turned off and what needs to be accomplished before turning\nit on might indeed be a good way to stem the flow of patches which\nmerely flip-the-switch without doing that extra research. Whether or\nnot Junio would accept such a patch documenting the current state of\naffairs is a different question.\n"},{"id":"428147","messageId":"YNEqLV9N88eKLdfo@camp.crustytoothpaste.net","threadId":"55973","inReplyTo":"CAPig+cQa3JC-YkGeB-NDy-iP=Mv8BRL6jyXTGXzaqpTLcxj8+w@mail.gmail.com","subject":"Re: [PATCH] bisect: allow to run from subdirectories","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2021-06-22T00:09:17Z","receivedAt":"2021-06-22T00:09:23Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On 2021-06-21 at 02:10:10, Eric Sunshine wrote:\n> On Sun, Jun 20, 2021 at 10:00 PM brian m. carlson\n> wrote:\n> > On 2021-06-21 at 00:35:49, Ævar Arnfjörð Bjarmason wrote:\n> > > On Sun, Jun 20 2021, Roland Hieber wrote:\n> > > > Currently, calling 'git bisect' from a directory other than the top\n> > > > level of a repository only comes up with an error message:\n> > > >\n> > > > You need to run this command from the toplevel of the working tree.\n> > >\n> > > How does this affect out-of-tree scripts that will be run with \"git\n> > > bisect run\", is the cwd set to the root as they now might expect git to\n> > > check, or whatever subdirectory you ran the \"run\" from?\n> >\n> > As for the idea itself, I think it's a good one assuming everything\n> > continues to work. It will certainly be more convenient for a lot of\n> > people.\n> \n> There have been multiple patches sent to the project over the years\n> with the same purpose. One problem, I believe, which has never been\n> fully addressed is what happens when the subdirectory from which\n> git-bisect is run gets deleted as part of the bisection.\n\nAnd that's the thing I was missing. This did seem a little too simple.\n\nI think there are certainly cases where we know the directory isn't\nchanging; most of the situations in which I've bisected things in Git,\nfor example. But we will, of course, need to specify the behavior when\nthat's not the case, and as Junio said, it probably will just fail in\nunexpected ways, and that wouldn't be a helpful user experience. At the\nvery least, we'd need to document the behavior, and ideally try to make\nit work reasonably gracefully (e.g., by not removing the directory in\nthat case).\n-- \nbrian m. carlson (he/him or they/them)\nToronto, Ontario, CA\n"},{"id":"428202","messageId":"87bl7x3okc.fsf@evledraar.gmail.com","threadId":"55973","inReplyTo":"87sg1b2xhl.fsf@evledraar.gmail.com","subject":"Re: [PATCH] bisect: allow to run from subdirectories","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-06-22T15:27:27Z","receivedAt":"2021-06-22T15:30:17Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Mon, Jun 21 2021, Ævar Arnfjörð Bjarmason wrote:\n\n> On Mon, Jun 21 2021, Roland Hieber wrote:\n>\n>> On Sun, Jun 20, 2021 at 10:10:10PM -0400, Eric Sunshine wrote:\n>>> On Sun, Jun 20, 2021 at 10:00 PM brian m. carlson\n>>> wrote:\n>>> > On 2021-06-21 at 00:35:49, Ævar Arnfjörð Bjarmason wrote:\n>>> > > On Sun, Jun 20 2021, Roland Hieber wrote:\n>>> > > > Currently, calling 'git bisect' from a directory other than the top\n>>> > > > level of a repository only comes up with an error message:\n>>> > > >\n>>> > > > You need to run this command from the toplevel of the working tree.\n>>> > >\n>>> > > How does this affect out-of-tree scripts that will be run with \"git\n>>> > > bisect run\", is the cwd set to the root as they now might expect git to\n>>> > > check, or whatever subdirectory you ran the \"run\" from?\n>>> >\n>>> > As for the idea itself, I think it's a good one assuming everything\n>>> > continues to work. It will certainly be more convenient for a lot of\n>>> > people.\n>>> \n>>> There have been multiple patches sent to the project over the years\n>>> with the same purpose. One problem, I believe, which has never been\n>>> fully addressed is what happens when the subdirectory from which\n>>> git-bisect is run gets deleted as part of the bisection.\n>>> \n>>> Here are a couple recent threads triggered by previous such patches\n>>> (but there are probably several more):\n>>> \n>>> https://lore.kernel.org/git/pull.765.git.1603271344522.gitgitgadget@gmail.com/\n>>> https://lore.kernel.org/git/pull.736.git.git.1584868547682.gitgitgadget@gmail.com/\n>>\n>> Ah, thanks for explaining the problem. Would a patch that adds a short\n>> explanatory comment in git-bisect.sh on the matter help to prevent\n>> people sending such patches?\n>\n> Having skimmed the linked discussions I don't think the consensus is\n> that this shouldn't exist, but that someone who wants it should do some\n> research on the relevant edge cases, come up with test cases for them,\n> discuss the trade-offs in a commit message etc.\n>\n> I for one would welcome such a feature, it's often annoyed me, it should\n> just work like \"rebase exec\" in that a \"run\" script should cd to the\n> root, but (as discussed in the linked threads) I don't see why we'd\n> prevent it any more than several other commands that already have this\n> edge case, but don't explicitly prevent this.\n\nThere's also a related issue: It's not just \"git bisect start\" etc. that\nhave this problem, but also \"git bisect log\". No matter what we do with\nrun etc.\n\nI see no reason for why \"log\" should not locate the relevant log in the\n.git directory, same for \"view\", also \"good\", \"bad\" etc. E.g. one might\nstart a bisect session in one terminal, and be cd'd into the 't/'\ndirectory in another, and then couldn't run \"good/bad\" there.\n\n"},{"id":"428344","messageId":"YNPGb5gvygs++jlv@coredump.intra.peff.net","threadId":"55973","inReplyTo":"xmqqy2b3j317.fsf@gitster.g","subject":"Re: [PATCH] bisect: allow to run from subdirectories","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2021-06-23T23:40:31Z","receivedAt":"2021-06-23T23:40:34Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Jun 21, 2021 at 12:43:32PM +0900, Junio C Hamano wrote:\n\n> Roland Hieber writes:\n> \n> > Currently, calling 'git bisect' from a directory other than the top\n> > level of a repository only comes up with an error message:\n> >\n> > You need to run this command from the toplevel of the working tree.\n> >\n> > After a glance through the bisect code, there seems to be nothing that\n> > relies on the current working directory, and a few hours of bisect usage\n> > also didn't turn up any problems. Set the appropriate flag for\n> > git-sh-setup to remove the error message.\n> \n> Try to find a history in which to run a bisect that\n> \n> (1) has a directory T in the recent part of the history, and\n> \n> (2) does not have that directory in the older part of the history.\n> Better yet, if the older part of the history has T as a regular\n> file, that would be ideal. Even better, if that old regular\n> file T was added, then removed, and then recreated, before it\n> got turned into a directory.\n> \n> Now, using the two commits you used to satisfy conditions (1) and\n> (2) as \"bad\" and \"good\", and using \"bad - has T as a directory\" and\n> \"good - does not have T as a directory\", as the bisection criterion,\n> try to find where \"good\" turns \"bad\"---in other words, find the\n> commit that either creates T as a directory or turns the regular\n> file T into a directory.\n> \n> Perform that bisect from the subdirectory T. Would that work? I\n> suspect it wouldn't; when trying to check out an old version that\n> does not have T in the directory, either the checkout would fail\n> because it cannot remove T which has an active process (i.e. your\n> terminal session) in it, or your process sitting in an orphaned\n> directory (i.e. your \"..\" may still be the original top-level\n> directory that used to have T subdirectory, but \"cd T\" from the\n> top-level will not reach where you are). All sorts of things can\n> go wrong and that is why we forbid it. Just a single \"cd\" upfront\n> would save the user a lot of headache.\n> \n> This does not depend on \"do we have T as a directory?\" being the\n> bisection criteria. The important thing is that the current\n> directory would appear and disappear as the bisection process makes\n> you jump around in the history.\n\nI think that is a good explanation. But I remain somewhat unconvinced\nthat it is that big a problem in practice.\n\nThere are already other cases where checkout may fail (e.g., an\nuntracked build artifact in one commit conflicting with a tracked file\nin another). E.g., this sequence in git.git:\n\n # here we build git-remote-testgit from its .sh counterpart\n git checkout 5afb2ce4cd^\n make\n\n # now imagine our bisect jumps forward; this one drops it from\n # the .gitignore, so its now a precious untracked file\n git checkout 5afb2ce4cd\n make\n\n # now jump backwards. I'm not sure if this can actually happen in\n # a bisection of git.git, since we went forward then back. But\n # possibly you could hit it in parallel lines of history[1], and\n # certainly a similar history which didn't update \".gitignore\" at the\n # same time could run into it.\n git checkout 709a957d9493^\n\nThat last command yields:\n\n error: The following untracked working tree files would be overwritten by checkout:\n\tgit-remote-testgit\n Please move or remove them before you switch branches.\n Aborting\n\nWhich is annoying, but it's pretty clear that you need to remove the\nfile and then re-run your \"bisect good\" or \"bisect bad\" (and if we want\nto make it more clear, then git-bisect could notice that git-checkout\nfailed and add extra advice).\n\nAnd I think any directory typechange shenanigans would end up with a\nsimilar message. It's nice to avoid that ahead of time, but it comes at\nthe cost of bugging the user to preemptively \"cd\" to the toplevel _every\ntime_, even if they are not going to bisect through history where the\ndirectory goes away.\n\nSo while I don't disagree with avoiding the confusing case, it seems\nlike the safety check is overly cautious. (Of course an alternative is\nthat it could actually examine the history to make sure $PWD never goes\naway; I wonder if that would be annoyingly costly or not).\n\nThe more interesting case, I think, is when T is simply removed. If\nthere are untracked files (even ignored build artifacts) in T, then the\ndirectory sticks around anyway. But if not, Git is able to checkout the\noriginal commit and deletes the directory. And then you get:\n\n $ cd builtin\n $ git checkout 81b50f3ce40bfdd66e5d967bf82be001039a9a98^\n $ git status\n fatal: Unable to read current working directory: No such file or directory\n\nThat's potentially more confusing, as things subtly don't work. But as\nshown by the command above, it's not the exclusive domain of bisect.\nPerhaps a poorly written bisect-run script could get confused. But it\nstill doesn't seem to me like it justifies the tradeoff of \"we must\nnever allow bisect from a subdirectory, on the off chance that we might\nrun into this case\".\n\nIf we did want try helping the user in this case, we could also diagnose\nit pretty easily by confirming that $PWD will be in the destination\ncommit before checking it out, and complaining otherwise (which turns it\ninto the earlier case). In fact, regular \"checkout\" could perhaps do\nthat, too, as a courtesy (with \"-f\" overriding).\n\nThere may be other corner cases of interest (I didn't think too hard\nabout weird symlink cases, for example). But overall I think people are\nmore likely to be annoyed by the \"must be at the toplevel\" safety valve\nthan they are by \"checkout failed midway through my bisect\", if only\nbecause the former happens a lot more frequently.\n\n-Peff\n\n[1] I'm sorry not to have produced a real git-bisect example that causes\n \"checkout\" to bail. My subjective recollection is that I have run\n into this problem with git-remote-testgit before during real-world\n use, but I'm not 100% sure it was while bisecting, and not simply\n sight-seeing around history.\n"},{"id":"428658","messageId":"xmqqtulh31nm.fsf@gitster.g","threadId":"55973","inReplyTo":"YNPGb5gvygs++jlv@coredump.intra.peff.net","subject":"Re: [PATCH] bisect: allow to run from subdirectories","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-06-29T01:22:37Z","receivedAt":"2021-06-29T01:22:44Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n>> This does not depend on \"do we have T as a directory?\" being the\n>> bisection criteria. The important thing is that the current\n>> directory would appear and disappear as the bisection process makes\n>> you jump around in the history.\n>\n> I think that is a good explanation. But I remain somewhat unconvinced\n> that it is that big a problem in practice.\n\nIt's just the difference in attitude, I would think. Things like\n\"rebase\" take a more liberal attitude and most of the time things\nwork out OK because removal of a directory is a rare event and\nreplacement of a directory with a non-directory is even rarer, but\nwhen things break there is no provision to help users to know how it\nbroke by diagnosing why the revision cannot be checked out, or why\nthe directory D the user's shell session is sitting in is now\norphaned and different from the directory D the user thinks he is in\nbecause it was removed (while the user's process is in there) and\nthen recreated under the same name, or any of the tricky things.\n\nThe ideal endgame would be to allow operating from subdirectory\n*AND* have provisions for helping users when things go wrong because\nthe starting subdirectory goes away. \"bisect\" works under the more\nconservative philosophy (start strict and forbid operation that we\nknow we didn't spend any effort to avoid taking the user into\ndangerous waters---we can allow it later once we make it safer but\nnot until then).\n\nIt would involve a bit of chicken-and-egg, I would guess. If we\nthink improving Git is more important than avoiding even occasional\nfailures imposed on end-users, then the more liberal approach would\nbe easier to work with---we can allow the command to start from\nsubdirectory even if we do not do anything to help avoid problems,\nlet the users hit a snag and have them report it, which would give\nus some concrete failure mode to work on.\n\n\n\n"},{"id":"428681","messageId":"YNp+oYKd5SSyxMk9@coredump.intra.peff.net","threadId":"55973","inReplyTo":"xmqqtulh31nm.fsf@gitster.g","subject":"Re: [PATCH] bisect: allow to run from subdirectories","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2021-06-29T02:00:01Z","receivedAt":"2021-06-29T02:00:03Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Jun 28, 2021 at 06:22:37PM -0700, Junio C Hamano wrote:\n\n> Jeff King writes:\n> \n> >> This does not depend on \"do we have T as a directory?\" being the\n> >> bisection criteria. The important thing is that the current\n> >> directory would appear and disappear as the bisection process makes\n> >> you jump around in the history.\n> >\n> > I think that is a good explanation. But I remain somewhat unconvinced\n> > that it is that big a problem in practice.\n> \n> It's just the difference in attitude, I would think. Things like\n> \"rebase\" take a more liberal attitude and most of the time things\n> work out OK because removal of a directory is a rare event and\n> replacement of a directory with a non-directory is even rarer, but\n> when things break there is no provision to help users to know how it\n> broke by diagnosing why the revision cannot be checked out, or why\n> the directory D the user's shell session is sitting in is now\n> orphaned and different from the directory D the user thinks he is in\n> because it was removed (while the user's process is in there) and\n> then recreated under the same name, or any of the tricky things.\n> \n> The ideal endgame would be to allow operating from subdirectory\n> *AND* have provisions for helping users when things go wrong because\n> the starting subdirectory goes away. \"bisect\" works under the more\n> conservative philosophy (start strict and forbid operation that we\n> know we didn't spend any effort to avoid taking the user into\n> dangerous waters---we can allow it later once we make it safer but\n> not until then).\n\nYes, I agree with this second paragraph. Just trying to create a\nconstructive path forward, I think I'd be comfortable with a patch\nseries that:\n\n - confirmed that bisect's behavior when checkout fails produces a\n reasonable error message that the user can act on (either from\n checkout itself, or perhaps extra advice from bisect when the\n checkout fails)\n\n - detected the case when the prefix we started from goes away as part\n of the checkout, and turned that into an error (rather than\n orphaning the user's cwd and leading to confusing results). This\n _might_ even be something that regular \"git checkout\" would benefit\n from, too. And I think should not be too expensive to implement (at\n least not after an admittedly moderate amount of thinking on it).\n\n - only then turn on SUBDIRECTORY_OK.\n\n-Peff\n"}]}