Rename `LocalStore` to `SQLiteStore`

NixOS / nix

Nix, the purely functional package manager

https://nixos.org/

GNU Lesser General Public License v2.1

12.2k stars 1.47k forks source link

Rename `LocalStore` to `SQLiteStore` #9552

Open Ericson2314 opened 9 months ago

Ericson2314 commented 9 months ago

I think the purpose of that class is to implement "SQLite-ness", not "local-ness". "local-ness" is almost a non-concept: for example, building should work with any LocalFSStore. But even ignoring that and assuming "local-ness" still means something, the SQLite implementation is hardly the only such possible "local store", a PostgreSQL store or JSON-metadata-in-files store (https://github.com/NixOS/nix/pull/9551/files) are just in local.

In light of these things, I think we should rename LocalStore to SQLiteStore.

(https://github.com/NixOS/nix/pull/9551 will introduce a MixLocalStore for a few odds and ends that are shared between local stores, and I imagine any other implementations of non-SQLite local stores would want to do the same.)

Such a rename imposes a lot of churn, but I think it would be easy enough to back-port it to 2.19 and 2.18 (it's just some big seds, after all). Then it at least won't get in the way of other backports.

edolstra commented 9 months ago

I'm not necessarily opposed to this, but it should ideally be motivated by a real user need. Are there people who need/want a Postgres or JSON store, and if so, what for?

Ericson2314 commented 9 months ago

I am pretty sure Replit wants a non-SQLite option for the lower store of a layered store, though they probably also prefer narino to JSON. I should let @ryantm speak for them not me.
I personally want to use a PostgreSQL store for build farms, but I would probably use https://github.com/haskell-nix/hnix-store for that so you could argue this doesn't have as much bearing on this code base.

thufschmitt commented 9 months ago

Just my 2 cts here: the LocalStore name might not be future-proof in light of the potential for storing the metadata in other formats, but SQLiteStore is just differently misleading because it's bypassing the fact that this is actually a local store.

Ericson2314 commented 9 months ago

LocalSQLiteStore? I am fine with other names as long as it's clear that this isn't the only possible local store. I do think we need some sort of name new before an out of tree JsonMetaStore (name also pending :)) can happen, because I have a MixLocalStore for that, and MixLocalStore together with LocalStore is too confusing!

nixos-discourse commented 9 months ago

This issue has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/2023-12-18-nix-team-meeting-minutes-113/37050/1