Use sqlite #124
Draft
Use sqlite #124
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
I'm not following the motivation here. Based on the commit messages, I can see some ideas about potential features that would come out of this, but I think the folks who are or might use Lorri are looking for two things: GC root setups and flakes and I'd like to prioritize getting those done. |
Yeah, I feel like flakes are kind of important; but I personally don’t use them yet, so I don’t have a huge motivation to work on them. I’ll of course review any changes to them.
The motivation is that I want lorri to keep around info between runs, more than the current GC roots. And a small database file is the easiest setup for that. In particular, adding timings to previous evaluation and build times of files, so that lorri can estimate how long a change is gonna take to build. |
It’s just a better environment that way … sigh
Instead of having complex generic types internally that we map over, just create the json in a macro. Removes `ProjectAdded` from `Reason`, because it was not used anywhere. Changes the daemon format, so make sure to run the daemon with this new lorri version or you get serialization errors.
This new function makes it easy to race two threads, instead of syncing on shared state. This also simplifies the duration code, since all we are doing is waiting on timeouts and the stop channel.
Instead of using serde, defining a clear API boundary (preventing breakage on refactors).
We only pass the `cas` through the `Project` constructor, never actually using it in the project `impl`!. This means we don’t need to burden `Project` with the cas, since it’s only used by the builder in the end. Insead, put it on the build loop for now. Funnily enough, `StartUserShell` does not need the project init anymore, cause it only wanted the `cas` from it.
Collect errors and continue deleting when json is not on.
This is super crappy at the moment, but we want to ideally save most of the data in the database instead of indirectly in the filesystem. This gives us better leverage for storing other fun things, like which files we last watched for a project, or how long changes to the files took.
These two structs are only used for preventing automatic cleaning of these resources, so their content is never used. Newer rust will warn about that for structs as well.
Yay, information hiding!
The 5ms difference would sometimes trigger this channel first, so let’s give it 20ms difference.
We’ve only really used `shell` for the first few months, then expected people to switch to `direnv`. So we only have real support for `bash` and `zsh`. We should note that. By not shadowing `project` in the main function, I uncovered that we missed out on using the nested logger for `Info`.
What an amazing piece of code. Apparently it wasn’t even worth the work to remove all `dbg!` macros before submitting it upstream. Get rid of adding `stderr` to the checked output. I have a very trigger-happy finger that was very close to simply deleting this whole file without a second thought, but I suppose having some kind of integration test for this is at least a little useful.
Every thing related to roots should be in that module.
Instead of manually listing the files and doing some ad-hoc reading of files to list every gc dir, use our existing project setup to fetch this information. This switches around the logic a bit, because I couldn’t be bothered to keep it all the same. Especially around the liveness of nix files. Basically there might be a nix_file symlink file, but there might also not. And if it’s there, there might be a nix_file behind it or not. Same for the timestamp and the gc root symlink.
We create a function which consumes the project and deletes its GC root.
Previously it would say “0 days” for anything that’s younger than one day, now it will use the smallest unit that’s not 0.
In addition to updating the backlink to nix_file, we also update the gc_roots table in our database to contain the same info. The next step is to read everything from the database instead of from the symlinks. Currently, the gc_roots table is deleted at every start (to make changes to the schema easier), after everything works through the database, we have to set up a one-time migration (and write a test for it …) and clean up the symlinks.
This was a lot more complicated than I expected, because you cannot share a rusqlite `Connection` object across unwrap boundaries (you can share it across threads, but our `Async` does automatic unwrapping …). So what I do here is that whenever we want to clone a `Project`, I create something I call “skeleton”, which is a clone of the project without the connection object (just the sqlite file path), then on the other side of the boundary I turn the skeleton back into a Project by connecting to the database again. I think this is quite an okay solution.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
release.nix(seerelease.nixfor instructions)