Skip to content

Reset after git bisect in script #16589

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jan 9, 2023
Merged

Reset after git bisect in script #16589

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions project/scripts/bisect.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -112,3 +112,4 @@ class CommitBisect(validationScriptPath: String):
s"git bisect bad $fistBadHash".!
s"git bisect good $lastGoodHash".!
Seq("git", "bisect", "run", "sh", "-c", bisectRunScript).!
s"git bisect reset".!
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From my experience it seems quite convenient not to reset actually. If the script works correctly you are already checked out at the commit which introduced the regression and you can immediately play with the codebase to minimize further or try to find a fix. On the other hand, if something went wrong with bisecting itself, you can try to bisect further manually with git bisect good/bad. Probably it would be good to get some feedback from other people using the bisect script to make its usage even more convenient.
Maybe we should make reset optional under a flag then?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From my experience I got more problems by not resetting. It is also really simple to checkout the first falling commit because the hash is printed at the end of the bisection.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, trying to fix the issue on the failing commit is not always useful because the code might have changed after that commit.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And what about the use case of continuing to bisect manually? And could you give some examples of the problems you had because of not resetting?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And what about the use case of continuing to bisect manually?

What does it mean to continue the bisect manually after the bisection ends?

And could you give some examples of the problems you had because of not resetting?

  • Starting to fix the issue and then realised that I am not on the main branch anymore. In that situation, it is likely that the fix will conflict with master
  • Starting a new bisect script with another failing example. Either from the issue (such as minimized or unminimized) or from a related issue
  • Starting the next bisection for a different issue

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One example of manual continuation is deadlock in the bsp, I had problems like that in a mixed Java/Scala sources which requires to use bsp server. In such case resetting the bisect would require re-trying all commits again, which can again deadlock.
In my case I was able to finish bisect by manually killing the bsp server, marking the commits manually as good and bad as well as manually passing the command to bisect

Copy link
Contributor

@prolativ prolativ Jan 9, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What does it mean to continue the bisect manually after the bisection ends?

In some cases you can manually mark commits as good/bad and skip running the evaluation command. I have never used it myself but @WojciechMazur seems to have some experience with that. But maybe that's not a very common issue

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In some cases you can manually mark commits as good/bad and skip running the evaluation command. I have never used it myself but @WojciechMazur seems to have some experience with that. But maybe that's not a very common issue

If you do that you do not need this script. You can just use git bisect run directly with the test script.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok, this sounds reasonable