bisect: allow `git bisect` to run from subdirectory

From: Sangeeta Jain <[email protected]>

As git rebase was never prevented to run from subdirectory we shouldn't prevent git bisect to run from subdirectories.

This commit removes the
restriction on git bisect to run only from top level directory thereby allowing it to run from any subdirectory.

Signed-off-by Sangeeta Jain <[email protected]>

---
[Outreachy] bisect: allow git bisect to run from subdirectory

As git rebase was never prevented to run from subdirectory we shouldn't prevent git bisect to run from subdirectories. This commit removes the restriction on git bisect to run only from top level directory thereby allowing it to run from any subdirectory.

Signed-off-by Sangeeta Jain [email protected] [[email protected]]

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-765%2Fsangu09%2Fbisect_fix-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-765/sangu09/bisect_fix-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/765

  git-bisect.sh               | 1 +
  t/t6030-bisect-porcelain.sh | 7 +++++++
  2 files changed, 8 insertions(+)

git-bisect.sh +0 -0

index ea7e684ebb..9cd0fa0483 100755

git-bisect.sh +0 -0

  _x40='[0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f]'

t/t6030-bisect-porcelain.sh +7 -0

index aa226381be..6b68cc01d0 100755
@@ -49,6 +49,13 @@ test_expect_success 'bisect starts with only one bad' '
  	git bisect next
  '
 
+test_expect_success 'bisect runs in a subdirectory' '
+    mkdir -p subdir &&
+    git -C subdir bisect start &&
+    git -C subdir bisect good &&
+    git -C subdir bisect reset
+'
+
  test_expect_success 'bisect does not start with only one good' '
  	git bisect reset &&
  	git bisect start &&

base-commit: 69986e19ffcfb9af674ae5180689ab7bbf92ed28

Hi Sangeeta

It's great to see you tackling another patch

Quoted 2 lines · Sangeeta via GitGitGadget

From: Sangeeta Jain <[email protected]>

Quoted 2 lines

As git rebase was never prevented to run from subdirectory we shouldn't prevent git bisect to run from subdirectories.

I'm not sure it's relevant to bisect whether or not rebase can be run from a subdirectory. What is important is that we want all commands to be able to be run from a subdirectory unless there is a good reason not to (and there isn't for bisect)

I'm not sure that that's the case: Junio pointed out a while[1] ago that we'd have to answer the question of "what happens if I'm in a subdirectory that goes away during some point of the bisection?". I think that you could probably find an answer to that question, but the fact that there isn't an obvious one seems to indicate that we're going down the wrong path.

I agree that it would be nice to run bisect from any directory, but it may not be as easy as I'd hope.

Hi Sangeeta

It's great to see you tackling another patch

Quoted 4 lines · Sangeeta via GitGitGadget

From: Sangeeta Jain <[email protected]>

As git rebase was never prevented to run from subdirectory we shouldn't prevent git bisect to run from subdirectories.

I'm not sure it's relevant to bisect whether or not rebase can be run from a subdirectory.

What is important is that we want all commands to be able to be run from a subdirectory unless there is a good reason not to (and there isn't for bisect)

I'd recommend adding [Outreachy] to the beginning of the first line of the commit message as well.

Quoted 7 lines

git-bisect.sh +0 -0

index ea7e684ebb..9cd0fa0483 100755
@@ -32,6 +32,7 @@ git bisect run <cmd>...
  Please use "git help bisect" to get the full man page.'
 

git-bisect.sh git bisect run <cmd>... · line 36

Quoted 3 lines

git-bisect.sh +1 -0

  OPTIONS_SPEC=
+SUBDIRECTORY_OK=Yes
  . git-sh-setup

git bisect run takes an optional script that is run to determine if the current commit is good or bad. If the script is given with a relative path then we need to make sure it is invoked from the subdirectory that git bisect run was started from. As far as I can see git-bisect.sh does not change directory but it would be good to have a test for git bisect run <cmd> from a subdirectory.

Quoted 9 lines · Taylor Blau

I'm not sure that that's the case: Junio pointed out a while[1] ago that we'd have to answer the question of "what happens if I'm in a subdirectory that goes away during some point of the bisection?". I think that you could probably find an answer to that question, but the fact that there isn't an obvious one seems to indicate that we're going down the wrong path.

I agree that it would be nice to run bisect from any directory, but it may not be as easy as I'd hope.

True.

I would not mind all that much a single "git checkout ancient" that makes the $cwd go away and confuse the user. But a bisect session would jump around versions randomly (eh, logarithmically?) and you'd end up switching out of a version in a non-existing $cwd to another version that has the directory (created internally by mkdir(2)), and I'm fairly certain that your phantom $cwd that is not connected to any other filesystem entity and the directory that should be at the same path in the newly checked-out version are different filesystem entities. I'd rather not have to think about the interaction between git and the system after that point.

Hi Junio,

Quoted 14 lines · Junio C Hamano

Quoted 9 lines · Taylor Blau

I'm not sure that that's the case: Junio pointed out a while[1] ago that we'd have to answer the question of "what happens if I'm in a subdirectory that goes away during some point of the bisection?". I think that you could probably find an answer to that question, but the fact that there isn't an obvious one seems to indicate that we're going down the wrong path.

I agree that it would be nice to run bisect from any directory, but it may not be as easy as I'd hope.

True.

Quoted 10 lines

I would not mind all that much a single "git checkout ancient" that makes the $cwd go away and confuse the user. But a bisect session would jump around versions randomly (eh, logarithmically?) and you'd end up switching out of a version in a non-existing $cwd to another version that has the directory (created internally by mkdir(2)), and I'm fairly certain that your phantom $cwd that is not connected to any other filesystem entity and the directory that should be at the same path in the newly checked-out version are different filesystem entities. I'd rather not have to think about the interaction between git and the system after that point.

By that token, we should also prevent git rebase from running in a subdirectory, but we don't.

Besides, this only becomes an issue when the directory becomes _empty_ (including untracked files) because we don't remove it otherwise.

I am actually more worried about bisecting between revisions that replace the current subdirectory by a symlink or something.

But again, this is pretty much precisely the kind of scenario that we _already_ allow running into with git rebase. So I see little point refusing git bisect users to run in a subdirectory.

I know that _I_ often grumble after git bisect start fails, then try to pull out the last remains of my patience and insert -C .. or -C ../.. between git and bisect and _still_ get what I want, all while shaking my imaginary fist at git bisect for forcing me to type those extra keystrokes.

Ciao,
Dscho

Hi Phillip,

Quoted 10 lines · Phillip Wood

Hi Sangeeta

It's great to see you tackling another patch

Quoted 4 lines · Sangeeta via GitGitGadget

From: Sangeeta Jain <[email protected]>

As git rebase was never prevented to run from subdirectory we shouldn't prevent git bisect to run from subdirectories.

Quoted 2 lines

I'm not sure it's relevant to bisect whether or not rebase can be run from a subdirectory.

The relevance is this: _iff_ we want to prevent git bisect from running in a subdirectory under the premise that that subdirectory might need to be removed, then why don't we prevent git rebase from running in a subdirectory when that command is equally likely to remove that subdirectory?

Quoted 3 lines

What is important is that we want all commands to be able to be run from a subdirectory unless there is a good reason not to (and there isn't for bisect)

There is a difference in quality here, though. If you run, say, git fetch in a subdirectory, or git commit or git show, there is not the same worry that that subdirectory will go away as with git bisect, git rebase, git switch, git pull and other commands.

Quoted 2 lines

I'd recommend adding [Outreachy] to the beginning of the first line of the commit message as well.

I am opposed to that. The fact that this comes in via Outreachy is interesting at the moment, for reviewers, but not for posterity (i.e. in the commit that will make it into the commit history).

Quoted 7 lines

Quoted 7 lines

git-bisect.sh +0 -0

index ea7e684ebb..9cd0fa0483 100755
@@ -32,6 +32,7 @@ git bisect run <cmd>...
   Please use "git help bisect" to get the full man page.'
 

Quoted 10 lines

git-bisect.sh git bisect run <cmd>... · line 36

Quoted 3 lines

git-bisect.sh +1 -0

   OPTIONS_SPEC=
+SUBDIRECTORY_OK=Yes
   . git-sh-setup

git bisect run takes an optional script that is run to determine if the current commit is good or bad. If the script is given with a relative path then we need to make sure it is invoked from the subdirectory that git bisect run was started from. As far as I can see git-bisect.sh does not change directory but it would be good to have a test for git bisect run <cmd> from a subdirectory.

That's a very good point. We should first add a regression test for exactly that use case, and then make sure that it passes.

Ciao,
Dscho

I think both bisect and rebase should be documented as running commands from the repository root as this is what rebase does and it gets around the missing directory problem.

I'm not sure rebase is doing the right thing with a relative path though. My feeling is it would be less suprising to resolve relative paths to the directory where the bisect/rebase is started and store the absolute path. The script may disappear while rebasing but that can happen now if the user points us to a script in a directory that disappears while we're rebasing

Quoted 6 lines

The relevance is this: _iff_ we want to prevent git bisect from running
in a subdirectory under the premise that that subdirectory might need to be removed, then why don't we prevent git rebase from running in a subdirectory when that command is equally likely to remove that subdirectory?

That's a good point, I'd completely missed the analogy with rebase --exec

If a step in the rebase sequence makes a directory disappear (or turns a directory into a file), and the command given by -x is in the directory (it is immaterial if it is given as relative or full pathname from the command line), hopefully the step of the rebase sequence that would lose the directory would error out, in order to prevent an untracked but not ignored file from getting clobbered.

Even before speculating such an "advanced" mode of operation, do we know that rebasing a history that makes a directory disappear and reappear work?

Quoted 10 lines · Phillip Wood

I think both bisect and rebase should be documented as running commands from the repository root as this is what rebase does and it gets around the missing directory problem.

I'm not sure rebase is doing the right thing with a relative path though. My feeling is it would be less suprising to resolve relative paths to the directory where the bisect/rebase is started and store the absolute path. The script may disappear while rebasing but that can happen now if the user points us to a script in a directory that disappears while we're rebasing

If a step in the rebase sequence makes a directory disappear (or turns a directory into a file), and the command given by -x is in the directory (it is immaterial if it is given as relative or full pathname from the command line), hopefully the step of the rebase sequence that would lose the directory would error out, in order to prevent an untracked but not ignored file from getting clobbered.

Even before speculating such an "advanced" mode of operation, do we know that rebasing a history that makes a directory disappear and reappear work?

For example, if there is a history like this:

- commit #0: an empty tree
- commit #1: adds a file D/F
- commit #2: moves the file D/F to F (i.e. the toplevel)
- commit #3: moves the file F to D (i.e. D becomes a file)
- commit #4: moves the file D to E
- commit #5: moves the file E to D/E (i.e. D becomes a directory again)

does it do what expect it to do if we replay the history c0..c5 on top of a comit that records an empty tree if we start the rebase in an empty directory D?

Here is what I tried in an empty directory, and the last "ls -la" shows an empty directory, even if you try "ls -la D" from a separate shell after everything is done, you'd see a file D/E there. If a platform exists that does not allow removing a directory that is the $cwd of any process, I would not be surprised if the whole thing failed in a mysterious (to the end user) way.

#!/bin/sh
test -d .git && exit ;# safety
rm -fr D E F
git init

git commit --allow-empty -m 'an empty tree'
git tag commit0

mkdir D && >D/F && git add D/F
git commit -m 'add a file D/F'

git mv D/F F && git commit -m 'move D/F to F'
rm -rf D

git mv F D && git commit -m 'move F to D'

git mv D E && git commit -m 'move D to E'

mkdir D && git mv E D/E && git commit -m 'move E to D/E'

git tag commit5

echo history made

git checkout --orphan rebuilt
git rm -r -f .
git commit --allow-empty -m 'another empty tree'

mkdir D
cd D

git rebase --onto HEAD commit0 commit5^0
ls -la

Yes, I agree. We need to make some changes in git rebase to make it work from the subdirectory, but that doesn't mean that we should completely restrict it from running in the subdirectory, and the same follows for git bisect.

Quoted 7 lines · Phillip Wood

Quoted 6 lines

The relevance is this: _iff_ we want to prevent git bisect from running
in a subdirectory under the premise that that subdirectory might need to be removed, then why don't we prevent git rebase from running in a subdirectory when that command is equally likely to remove that subdirectory?

Quoted 1 line

That's a good point, I'd completely missed the analogy with rebase --exec

I concur. Perhaps we should make rebase a bit more careful.