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.

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

Conversation

nicolasstucki
Copy link
Contributor

No description provided.

@nicolasstucki nicolasstucki added the fasttrack Simple fix. Reviewer should merge or apply additional changes directly. label Dec 26, 2022
@@ -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

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

@nicolasstucki nicolasstucki merged commit 7337b6f into scala:main Jan 9, 2023
@nicolasstucki nicolasstucki deleted the reset-after-bisect branch January 9, 2023 17:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fasttrack Simple fix. Reviewer should merge or apply additional changes directly.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants