Keeping a project bisectable | Hacker News Reader

Keeping a project bisectable

andrealmeid.com

93 pointspabs384 comments

Bisecting is also useful for end users. They might be able to reproduce bugs on their hardware or software configuration that the developers don’t have access to.

Mozilla has a tool (command line or user-friendly-ish GUI front end) called “mozregression” that Firefox users can use to bisect regressions without compiling Firefox. It downloads and runs Firefox Nightly builds from an archive of years of builds, bisecting good and bad builds until the regression commit is found. An indispensable debugging tool for a very complex application!

https://mozilla.github.io/mozregression/quickstart.html

> Some angry contributor will disable the CI.

I will reject any such contributions and ban anyone who persists with them. I’m not sure what lies below the fold, but if this is a baseline assumption then you’re already fucked.

Like that is the equivalent to a surgeon not washing their hands..

I also do not understand how people end up in this sort of state. To me, absolutely zero new code is going in if builds or tests are failing. Zero. Under any circumstances. I don't care if that failure is a flake. Go demonstrate that it is flakey and disable the test. Don't force a change through red.

But... this approach clearly exists. A while ago I was talking with a coworker about some open source upstreaming they wanted to do. They were struggling to get their code out there because they were fighting against all sorts of broken test suites. They were told that this was simply the norm in the community they were contributing to and the test suites are always broken so don't worry about it. Baffled, they made sure that their new tests passed and sent the code off for review and integration.

So there is perhaps a need for software engineering principles that apply to this situation. Maybe that's wrong. Maybe the solution is like adopting source control - it is a fool's errand to try to handle the case where it isn't present so just tell everybody to adopt it or they aren't serious. But maybe there is a population that needs tools that help them even in a world of fucked integration tests.

Recently git bisect gained got a new capability that was mysteriously missing before: bisect —first-parent. With that, it became possible to bisect also in repos where all commits on feature branches may not be buildable or passing tests, but commits on the development/main/integration branch does.

A feature I’m still missing is to bisect using a new test. That is, I have discovered a bug in the program that there is no test for, so I author a failing test. Now I want to find out when this failure occurred but I can’t bisect because even the previous commit is green as the test didn’t exist. It’s possible to work around by placing the test outside the repo root and bisecting with a script that “authors” the test (e.g copies the file) at each step, but it’s cumbersome and feels like something that could be possible to add to the bisect command, e.g an option to merge a commit (the new test) before testing each step of the bisect.

This might be a good argument for keeping tests as a separate repository. Which would bring a lot more problems of its own, though...

Just put the test in a new untracked file?

That works but only for languages/build-systems where the mere presence of the test is enough to make it run. In others you might have xml project files, test lists etc which makes the addition of a test require a modification of existing files.

The test subcommand of bisect can do anything you want, so it's easy to make a wrapper that does that prep work before starting that actual tests. Just have to remember to pass back the tests' exit code after cleaning up the repository for git.

Good point, though I don't like the idea of a setup where your test file's only executed if you remember to do that personally (if the tooling doesn't already do similar, I'd at least want to generate the list you describe by globbing the relevantly named/located files).

You could just copy that entrypoint into another untracked file, have its list include (or be only - quicker) the new untracked test file, and execute that instead of the normal one while bisecting.

Have you tried duplicating the test temporarily, which should turn the duplicate into an untracked file so you wouldn’t have to copy it at each step?

For the latter I use often use `git stash` for that; stashing off the test case/data, then at each step of the bisect, you can do `git stash apply` (rather than `git stash pop`) to reapply the same changes multiple times.

Before moving on, a quick `git reset —-hard` will get back to a clean flip

I have saved a little script to use with git bisect and a stashed test: https://gist.github.com/progers/998452a15180ae369fee338bb393...

For ruby projects where I could simply put the "new test" in a new file I do this:

1. Create new test (e.g. "spec/reproduce-bug_spec.rb")

2. Ignore it locally: `echo "spec/reproduce_bug_spec.rb" >> .git/info/exclude`

3. Run bisect (something like: `git bisect run rspec spec/reproduce-bug_spec.rb`)

If running the test gets more complex (e.g. installing dependencies as they might change travelling through history), I usually create a wrapper script (and ignore it) to bisect-run-it.

To bisect with a new test, you have to do this:

  $ git bisect [good|bad]

  $ git cherry-pick new-test

  $ ./run-the-test

  $ git reset --hard HEAD^

  $ git bisect [good|bad]

I sometimes use the stash for that: cherry pick the test as a working tree uncommitted change. "git stash" it before every bisect step, then "git pop".

Yeah, since it's more or less a standard cumbersome recipe, it seems it would be a good idea to create a high level git switch for it.

git bisect run --cherry-pick abc123 [....]

to do a regular bisect but at each step cherry picking the commit abc123

I know it's unconventional, but I like to keep my (anything larger than unit) tests in a separate repo so that I'm not running old versions of the tests when I bisect:

- find bug

- write new test to expose it

- bisect commits, running new test each time

- find culprit commit

- stop bisecting, fix bug

- when the test passes, you know bug is fixed.

How do you handle tests against new features that would definitely not pass on commits before the feature is introduced?

Some test frameworks allow you to mark these tests as xfail (expected fail). When you fix the issue, you can remove the xfail in the same commit.

This was my preferred approach for several years. The tests were in a repo separate from the code under test, and each declared its prerequisites, either as a feature or a version pattern.

If a test started failing, I could annotate it as an xfail with an open-ended version pattern. If I noticed that an xfail started passing, making it a upass (I think these were blue in our tests results), I put an upper bound on the xfail version pattern.

The nice thing about this was that I could always run the latest and greatest suite against any version, although it made it a little more complicated to run them as they existed when they were first run.

I'd typically start bisecting only after (or at the changeset) the feature has been introduced. At least with hg (and seemingly git as well) you can tell bisect to use a commit range where one end is good and the other is bad.

If I'm bisecting, I usually already have a single test in mind which exposes the bug I'm looking for, so I'm explicitly running that test only.

In general, I want to store one or more services and a first/last valid commit for each one as metadata for each test so that my framework knows which ones to run based on the environment under test. Haven't added this feature yet though.

My preferred variant of this would be

- write new tests to expose it, but not commit the tests

- bisect commits, running new tests each time

- find culprit commit

- stop bisecting, fix bug

- Look for similar bugs nearby, and ways to improve the new tests, now I know the root problem.

- Figure out the best series of commits for the tests and fixes together, that will be easiest to review by my colleagues/future me/alien archaeologists.

- Send PRs/commit to main.

Until one of your tools makes a backwards incompatible change, and then boom, your whole git history goes down the memory hole. You've got to vendor everything too. Since with a broken commit you can always have git slide back one commit. But it's much harder to spin up a Docker container with the same distro you were using for every part of the bisect.

Nix fixes this. See https://tvl.fyi for experiments with rebuilding goog mono repo infra as nix. Specifically https://code.tvl.fyi - which uses josh to serve the mono repro as micro repos. Presenting “views” which have super fast ci/cd implications.

Cosmopolitan Libc Makefile fixes this. And it does it with plain ordinary GNU Make. https://github.com/jart/cosmopolitan/blob/master/Makefile It even has Landlock LSM based sandboxing for strict dependency checking. https://github.com/jart/cosmopolitan/blob/ab301401c71effb0a0... It's faster too. Also GNU Make is what Google used for their mono repo before they switched to Blaze. They basically cited the reason they switched to Blaze as being because they didn't have Landlock. So now that we do, Make is the obvious choice!

Disclaimer: Google used to pay me to write the coolest features in Bazel like its system for downloading files. https://github.com/bazelbuild/bazel/commit/ed7ced0018dc5c5eb... But I stopped supporting Bazel because they kept breaking git bisect history permantently with backwards incompatible changes.

If you have the version of everything inside Git (e.g. your docker-compose file references Ubuntu 18 or PostgreSQL 13 or whatever), and if your software now depends on a new version you not only update your software but also the versions in the docker-compose file, then it should be possible to spin up docker for the correct version of everything as part of the bisect.

> You've got to vendor everything too

Well I hope people do this

Not necessarily have a downloaded carbon copy of every dependency, but at least enough information to recover it. package-lock.json file for example.

NB. I added a script to backup node_modules of every CI build. Just for paranoia.

Also: Make a project buildable in one step [1]

[1] https://www.joelonsoftware.com/2000/08/09/the-joel-test-12-s...

I personally standardize on a Makefile in the repo root, with a suitable default target. "git clone $x ; cd $x ; make" should always Do The Thing (even if it just calls out to npm or whatever).

Yep:) I feel silly writing a Makefile that runs `go build`, but I still do it:)

Same, but also adding targets for testing which are usable both by developers and CI and prints the tool version before executing it.

Greatly cuts down on the "tests pass on my system, not sure while it failed in CI"

Agreed! Printing tool versions is something I've started doing at my current job and it's so helpful knowing what version of Python or nodejs (or whatever) is running on the ancient TeamCity agents.

At work we use Gitlab. The feature to only allow fast-forward merges to master is awesome.

The result is a linear history, that is much easier to bisect.

It also encourages developers to make small commits, since they are easier to rebase.

Not Gitlab specific.

The main page of a repository configuration in GitHub let's you enable/disable:

- Allow merge commits

- Allow squash merging

- Allow rebase merging

for PRs.

And under branch protection there is a "Require linear history" option.

Hello, I see you stepped on my favourite personal soapbox! :)

https://github.com/isaacs/github/issues/1017

I really, really like semi-linear branching/merging. I.e. always rebase-merging, but with a merge commit.

Reasons, in comparison to Github's "rebase merge" which doesn't produce a merge commit:

1. It makes it clear which commits were part of one PR

2. It makes it clear who did the merge

3. It's okay to not have every commit build. but the one being merged will.

4. Still pretty bisectable. You'll narrow things down at least to the PR that caused an issue, and from there it's usually quite simple.

5. Looks very tidy in gitk & Co

Too bad our microservice architecture lives across 50 repos that all touch the same business functionality.

Whoops.

This actually gives me insight into why one contributor I worked with was crazy about squashing all commits into one when merging into main. I still think that's not good, because you loose the commit message history, and why have history at all if you squash it away. But maybe this was why they preferred it.

I find commit history is important if you have a long running client project with evolving context, so you can figure out why something was changed, not just what.

You can have both. A merge has two parents so one parent is the main branch minus this squashed commit, and the other parent leads to the commits made during development. It's the default behaviour that I've seen everywhere.

git bisect basically refuses to work with merge commits, so using those ruin bisectability.

As well as linear log, cherry-pick and scores of other tools which do not have intelligent merges handling.

> git bisect basically refuses to work with merge commits, so using those ruin bisectability.

I find that quite surprising, given how I see `git bisect` referenced by the Linux community as useful for tracking down the commit that introduced a problem, and Linus uses merge commits (including `octopus merges`) very heavily.

This is one thing that the FAANG workflow gets right. Multiple stacked commits are great as long as each one is a reviewable unit that makes sense on its own, and they don't need to be squashed to merge to main. What you need to avoid are a stack of commits like "added some code today" and "fixed bugs" and "try #3". Learn to amend existing commits instead...

> What you need to avoid are a stack of commits like "added some code today" and "fixed bugs" and "try #3".

That’s what you need to avoid in your mainline. Mandating what developers do in development branches is like having a dress code at work and then complaining about your colleague’s choice of underwear.

Nobody cares about the underwear... until people create a PR explicitly saying "look at my underwear".

And GitHub makes it 1-click easy to make a pull request full of underwear so it happens all the time.

Nobody mandates developers about their private repos and commits, nobody cares about that. This is about what gets merged into master or what gets submitted for review.

Squashing commits loses history, rebasing messy commits pollutes the "first parent" history, merging messy commits is cleaner but can force bisectors to wade through dead ends (which may or may not be useful, and may be worth preserving in a permanent side branch not merged), and rewriting commit histories into atomic commits (what I do most of the time) is difficult work which can take as long as writing the features upfront, and if done wrong can produce non-building commits. Sadly I haven't found a good solution for the workload yet, but this article suggests `git rebase -i main --exec "make"`, which I'll be sure to try when I have a chance.

All my commits build, so I can do a bisect and see the problem is... somewhere in this huge commit? It doesn't seem better to me than "the problem is in one of these two small commits, I don't know which one because they don't build".

The smaller commits are, the more useful they are to bisect.

Large commits are an antipattern that should be reworked, in code review at worst.

There’s no reason to squash commits to achieve this anymore. The —-first-parent option of git bisect makes it so that when you bisect, it won’t select commits from within the middle of a feature branch. It will only pick the commits that were truly part of main. That way you can easily find out which PR broke things.

Git log also has the first parent option and that is really helpful for helping you cut through the noise and see the high level overview of the history without seeing every commit.

Among the options merge commit , rebase and squash merge, rebasing is the problematic one if the commits on the branch are not carefully crafted. Classical merging allows for bisecting and retains historicy.

But IMHO if swash merged lead to a situation that relevant history is lost the PR is too big.

Yes, I consider bisectability the resolution of the "do you want a clean history or do you want a commit history with every little twitch of the developer in it?". A clean history is better, because the latter is not bisectable. On average the latter is always broken. Commits that make it into the "real" branches should only be made when all the tests pass. (Larger projects that can't run all the tests should have a local test suite of all the "fast" or "local" tests, or whatever it is that can be run, for fast testing prior to commit.) Temporary commits on your own branch is fine since you often need their functionality for other local reasons.

However, squashing everything down isn't the only or best answer. One can also create a series of commits, each of which is individually passing all the tests, but isn't one big bang. Of course sometimes you have no choice, some refactorings just fundamentally involve huge atomic changes to the code base, but it's still better to try to break these things down into multiple steps if at all possible so you can come back later with a new test and bisect.

I don't use bisect often, but when I do, it can save me days of effort trying to find problems, so it's worth it to maintain the discipline of keeping the tests passing with all commits. Plus it's already a net benefit; maybe you are a way better programmer than me but I find that my changes that couldn't possibly break anything else in the code base break other things in the code base in modules I wasn't expecting with some frequency. Keeping all the tests passing as much as possible is a critical component in making sure that I make what I think of as "monotonic forward progress". There are things more demoralizing that breaking half the code base every time I make the slightest change and it not being caught until QA or deployment... but making a habit of this is up there for sure. Definitely a great way to add some burnout to your life. I've seen multi-decade projects basically strangle themselves to death this way; nobody was willing to touch any of the core code because it had no testing coverage and every time they tried it broke everything, so they just lived with some real garbage code at the core of it.

One area where I find squashing especially problematic is file renames. If you rename a file in one commit and fix up the references to and from the file in a second, git can follow the rename, but one commit will fail to compile.

If you squash it into one commit on the other side, git will have a very hard time figuring out that the file got renamed and is not a brand new file, as both the rename and the fixup happened at the same time.

The main impediments against git bisect aren't commits that don't build; git bisect has a working solution for that.

The main impediments are external dependencies: the things contributing to the behavior change you're looking for which are not in the git repo you are bisecting:

- components from other git repos it depends on which were changing in parallel at around the time the problem showed up

- platform/toolchain and other dependencies; maybe the problem showed up because of a compiler upgrade.

Plus other things:

- difficult reproducibility of the issue being tested.

- multiple unrelated problems masquerading under the manifestation that you're actually testing for.

Regarding buildable commits; if you want a buildable commit to stay buildable, it has to reference the exact toolchain that was used, which has to be available. A commit that built with GCC 4 may not build with your GCC 12 today.

I've done bisects going back 10, 12 years.

Yeah this sucks and is a failure of current source control tools IMHO.

Bisect is a useful tool. Making commits that don’t compile, don’t pass tests, or don’t run is useful. Squashing commits is useful. Not squashing history is also useful.

There’s no technical reason we can’t have all these things.

> Making commits that don’t compile, don’t pass tests, or don’t run is useful.

I'm not sure I agree with you there on that one.

An example is renaming a method that’s heavily used. It can be nice to have the first commit be the rename, and then the second commit be changing the usages. When you code review that, it makes your job easier. Trivial example but you can work up less trivial ones from there.

It's also useful that PRs come from branches where each of the commits builds. And, these days, to be able to certify that, you need to push each commit one by one (otherwise GitHubCI and GitLabCI at least, will only run a CI job for the single push even if the push included many commits).

To prevent this problem, I wrote this push script: https://github.com/nblockchain/fsx/blob/master/Tools/gitPush...

If not every commit builds, you can use —-first-parent with bisect and bisect PR by PR instead of bisecting all commits.

GP may have meant that this is useful outside of bisecting something in the main branch.

For example, you may want to get your tree in one of the intermittent states and play around with it. Run some tests, try how some object behaves, to better understand the different commits comprising the PR. I do this quite a lot.

> To make sure we keep the bisectability of a project, we just need to make sure that every commit is buildable.

I aspire for this. I don't always succeed; in C++ code, often older commits will rely on transitive stdlib header #includes without including the proper header for what is used. This works, up until a compiler stdlib update will remove these implicit includes, breaking all past versions of your code (and requiring you to manually patch in the include when bisecting in these versions), and requiring you to add the necessary includes moving forward.

This can be hampered by slow build / start times though.

If you are going to bisect you either

1. Write unit tests

2. Run the app.

For 1, the problem is you are bisecting, so your test for HEAD may not compile with earlier versions

For 2. unless it is a nice command line app you can script, you have to wait for it to start up and manually do the steps to reproduce the problem.

Ideally of course if you are always aiming to be bisectable you can mitigate these with fast as possible compile and run times. Probably you would try not to build a monolith in that case.

when you have dozens contributing to unstable CI builds it can be hard as hard as keeping CI builds sane. For a project that is 10+ years old. I try to use git bisect but I have to do it in a few steps because over a range file can have multiple failures and failure you need might be somewhere I between start and some failure that was fixed within the range. Or multiple failures.

debug by bisection is one of those things which I think only occur in some projects. The ones which do atomic commit (or near atomic, at least atomic to the problem intent of the commit) -If you fold 100 updates to random codepoints in one commit, bisection only delivers limited help. How do you unwind the interrelated changes, possibly in one file? So if the style is to stack up edits, save, commit, move on, you will never do this.

it's a fantastic tool, but if you are chasing a kernel bug in an OS, its intensely time consuming to do, without the local clone of the repo to walk back the atomic(ish) changes and my lived reality is the kernel re-make is a lot slower than you want because imputed dependency chains typically re-compile a LOT more than you think.

I would not be surprised if duck debugging and printf() yields the outcome faster than what is (forgive loaded language) blame-finger pointing, sometimes.

It unquestionably works. It is not low-cost. The goal of preserving it is worthy. This is sensible.

One problem that I've had when bisecting is moving back in time towards code where resolved bugs still exist - masking the new bug.

The solution that I found was fixing the bug at the commit that created it, squashing, and then rebasing on top.

But it's far from ideal, and completely non-viable if there's heavy use of the repo from other developers.

And also not ideal if your bugfix accidentally introduces new bugs and you later realise you want to go back to the version before the bugfix, or see what the code change of the bugfix was.

You could keep a zero bug branch and merge back and forth between chronological.

ahh… zero bug branch

    commit 011bf12e41fea384c06873505b25cd44f995926d (HEAD -> main)
    Author: Tom Jakubowski <tom@crystae.net>
    Date:   Sat Aug 6 09:27:05 2022 -0700
    
        initial commit
    
    diff --git a/README b/README
    new file mode 100644
    index 0000000..3b18e51
    --- /dev/null
    +++ b/README
    @@ -0,0 +1 @@
    +hello world

I don’t understand why people waste time manually rebasing pull requests when you can squash merge with one button and get essentially the same result. Not quite as bad as merging a bunch of commits that aren’t bisectable because they didn’t pass CI though.

Rebasing is not that complicated, usually. (if people sending PRs make sure their code is working for each commit)

Yeah but the other problem in pull requests is that if someone makes changes since the last time I reviewed and then they force push I can’t see the difference from the last time I reviewed. Also if they rebase and push multiple commits there’s no guarantee they would all pass CI which means you can’t guarantee it will work with bisecting.

Force push doesn't delete the commit from your computer so you should still be able to diff between the (disjoint) commits using the hash.

If someone else has done the pull request, I review it on GitHub, then they rebase and force push the original commit wouldn’t be on my machine unless I had pulled it earlier correct? Which is something I never do.

If you don't have the ref locally but you have it open in a browser tab, you should† still be able to manually hack the URL to get a diff. Eg https://github.com/git/git/compare/95d1613a9fce4bcfc82d8ba48...

† My company's not on Github Enterprise and I don't have a repo and a second user handy to double check on public Github.

> if someone makes changes since the last time I reviewed and then they force push I can’t see the difference from the last time I reviewed

You need better tools. Even in GitHub you can click on "force pushed" and see the diff.

> there’s no guarantee they would all pass CI

There wasn't any guarantee before, either. If you want that guarantee, make the CI build every commit.

> There wasn't any guarantee before, either. If you want that guarantee, make the CI build every commit.

Guarantee is the wrong word. The point is that if you always squash merge after passing the entire test suite you don’t have a bunch of potential garbage commits in history that you have to wade through when bisecting.

As for building every commit that’s probably a tough sell and a poor use of money for what benefit?

But if it's only gonna find monstrous feature-at-a-time commits, what good is a bisect anyway? I want the culprit commit to be 5 minutes of bad ideas, not 5 minutes of bad ideas mixed with three days of good ones.

Make smaller PRs then. It's a lot slower to bisect if only some of the commits even compile.

If the PR is small enough to naturally fit into a single commit the rebasing vs squashing multiple commits discussion disappears (just please don't merge it!)... there is a single commit.

Many developers make commits that are “wip” with messages to match. Maybe you commit before you go to lunch, or when you go home for the day. These are useless commits and should be rebased or squashed, but many of these same devs won’t do that unless you force them or the tooling does it for them automatically. A minority of devs build a clean history of commits that tells a story, carefully crafted over time (usually manufactured after the fact with liberal use of rebase). For these devs squash is awful. Unfortunately they are truly a minority and the majority ruins things for them.

That's fine. You're checking the difference between the current state of master and the current state of the feature branch, not the difference between the old state of the feature branch and the current state of the feature branch.

First of all, because it makes reviewing the code for reviewers much easier. Reviewing a messy set of random uncurated commits is really suboptimal.

Second, you might have more than one commit you want to merge to master, not just a single commit.

Also rebasing is trivial and very quick once you have done some basic reading about it.

Reply on news.ycombinator.com