Result of nixpkgs-review pr 104253 run on x86_64-linux 1

Result of nixpkgs-review pr 104253 run on x86_64-darwin 1

/marvin opt-in
/status needs_reviewer

Hi! I'm an experimental bot. My goal is to guide this PR through its stages, hopefully ending with a merge. You can read up on the usage here.

I see there was a merge conflict due to #105368 forcing the update to sqlite-utils 3.0 in b8c6488 without adding the sqlite-fts4 dependency.

I've resolved the conflict, but for some reason there are two failing tests across all python versions, I'm investigating.

looks to be a small flakey difference. I'm okay with just disabling the tests

builder for '/nix/store/x10pnlagpnchjc73wqp38isjq1045pzz-python3.7-sqlite-utils-3.0.drv' failed with exit code 1; last 10 log lines:
          assert 0 == result.exit_code
          size_after_optimize = os.stat(db_path).st_size
  >       assert size_after_optimize < size_before_optimize
  E       assert 1667072 < 1662976

Thanks for taking a look, I agree this PR is pretty confusing at the moment. I was just going to poke it awake earlier today when I saw the merge conflict, and had to run.

Is there a better way I can clarify the confusion through the commit history? Should I revert b8c6488 instead, to keep the changes together and more atomic?

I have a hunch on how to fix the tests, I'll test it as soon as I get back to my pc.

I don't think the faulty commit should be reverted, rather this PR should be renamed to better represent that a dependency is added. The commit message should acknowledge the faulty commit and point out the mistake that was left there - and that now it is fixed.

Thanks @kisik21, I've reworded the commit.

I've also tracked down the cause for the failing tests, its due to an update of sqlite from 3.33 to 3.34. The tests fail at cfc56fb, and pass at the immediately previous e4d17dc (after waiting for 150 packages to rebuild overnight). I've opened an issue upstream (simonw/sqlite-utils#209), and disabled the failing two tests for now.

Testing this tool is straight forward:

$ cp /nix/var/nix/db/db.sqlite /tmp/db.sqlite
$ ./result/bin/sqlite-utils enable-fts --fts4 /tmp/db.sqlite ValidPaths path
$ ./result/bin/sqlite-utils search /tmp/db.sqlite ValidPaths firefox

Also optimize still works, I think it only fails for the contrived example in the tests because the entire DB is set up for full-text searching.

$ find /tmp/db.sqlite -printf '%s'
46256128
$ ./result/bin/sqlite-utils optimize /tmp/db.sqlite
$ find /tmp/db.sqlite -printf '%s'
30429184

I've run your example and I can confirm that this is working. Should we merge this now or wait for an upstream to fix the failing test? I'll leave this decision to whoever will be assigned to merge this, since I'm not yet well-versed in Nixpkgs policies, but if I were to decide, I'd err on the side of merging, as the functionality seems to be working as intended (as far as demonstrated by the remaining enabled tests and the manual test case described here).

/status needs_merger

Reminder: Please review!

Reminder: This Pull Request is awaiting merger. If you are the assigned reviewer with commit permission, please have a look. If you can't, please say so. If the status is not accurate, please change it. If nothing happens, this PR will be put back in the needs_reviewer queue in one day.