Skip to content
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

Add two new sqlite backends, using builtin support and a module [also talk about maintainership transfer] #86

Closed
wants to merge 2 commits into from

Conversation

tarsius
Copy link
Member

@tarsius tarsius commented Jan 12, 2022

A few months ago I wrote a backend that uses the sqlite module by @pekingduck. Since fixing a major bug I have been using it successfully. (I just noticed I already tried to do that four years ago but for unknown reasons abandoned that effort.)

Emacs recently added builtin support for sqlite on the master branch (to be releases in Emacs 29). That implementation is based on the mentioned module (but is part of Emacs itself, no module needed).

Yesterday I refreshed my Emacsql backend that uses the module and written a new one that uses the builtin support. Given that these backends are still very young, this should not be merged immediately, but I would like to learn now whether you would like to merge something like this eventually, or if I have to maintain these libraries separately.

I am going to use backends for a while myself, one for forge and one with epkg. Since I started doing that yesterday, nothing broke so far. ;) If any users see this, I would appreciate it if they could give these backends a try too.

@tarsius
Copy link
Member Author

tarsius commented Jan 12, 2022

Due to how Emacsql my packages use Eieio, packages that want to let the user choose between different SQLite have to add quite a bit of boilerplate:

(defcustom forge-database-connector 'sqlite
  "The database connector used by Forge.

This must be set before `forge' is loaded.  To use an alternative
connector you must install the respective package explicitly.

When `sqlite', then use the `emacsql-sqlite' library that is
being maintained in the same repository as `emacsql' itself.

When `sqlite-builtin', then use the builtin support in Emacs 29.
When `sqlite-module', then use a module provided by the `sqlite3'
package.  These two backends are experimental.
See https://github.com/skeeto/emacsql/pull/86.

When `libsqlite3', then use the `emacsql-libsqlite' library,
which itself uses a module provided by the `sqlite3' package.
This is still experimental and likely to be deprecated in
favor of `sqlite-module'.

When `sqlite3', then use the `emacsql-sqlite3' library, which
uses the official `sqlite3' command-line tool, which I do not
recommended because it is not suitable to be used like this,
but has the advantage that you likely don't need a compiler.
See https://nullprogram.com/blog/2014/02/06/."
  :package-version '(forge . "0.3.0")
  :group 'forge
  :type '(choice (const sqlite)
                 (const sqlite-builtin)
                 (const sqlite-module)
                 (const libsqlite3)
                 (const sqlite3)
                 (symbol :tag "other")))

(declare-function forge-database--eieio-childp "forge-db.el" (obj) t)
(cl-ecase forge-database-connector
  (sqlite
   (defclass forge-database (emacsql-sqlite-connection closql-database)
     ((object-class :initform 'forge-repository))))
  (sqlite-builtin
   (require (quote emacsql-sqlite-builtin))
   (with-no-warnings
     (defclass forge-database (emacsql-sqlite-builtin-connection closql-database)
       ((object-class :initform 'epkg-package)))))
  (sqlite-module
   (require (quote emacsql-sqlite-module))
   (with-no-warnings
     (defclass forge-database (emacsql-sqlite-module-connection closql-database)
       ((object-class :initform 'epkg-package)))))
  (libsqlite3
   (require (quote emacsql-libsqlite3))
   (with-no-warnings
     (defclass forge-database (emacsql-libsqlite3-connection closql-database)
       ((object-class :initform 'forge-repository)))))
  (sqlite3
   (require (quote emacsql-sqlite3))
   (with-no-warnings
     (defclass forge-database (emacsql-sqlite3-connection closql-database)
       ((object-class :initform 'forge-repository))))))

Any ideas how to improve that?

@tarsius
Copy link
Member Author

tarsius commented Jan 12, 2022

@skangas this probably interests you.

@tarsius
Copy link
Member Author

tarsius commented Mar 10, 2022

I am going to release these two packages in a few days unless you tell me you want to merge them here instead.

@skeeto
Copy link
Contributor

skeeto commented Mar 12, 2022 via email

@tarsius
Copy link
Member Author

tarsius commented Mar 15, 2022

Thanks for your trust, but I don't want to become the only/main maintainer for two reasons.

  1. I am trying very hard not to take on new responsibilities because I have taken on too much in the past and the resulting workload is keeping me from working on more interesting things.

  2. I share your doubts about some of the early design decisions. In the long run I plan to move away from Emacsql. Both Emacsql and closql (my weird OO interface on top of that) were the right move at the time, because that made the update from the previous implementation of epkg (which stored objects individually on disk, very slow) straight forward and worthwhile.

Now that Emacs (29) includes basic support for SQLite, I expect more development to happen on that front. The possibility of adding Emacsql to NonGNU Elpa was brought up on emacs-devel. A brief discussion followed about sql dsls and issues that come with that. I welcome that, because I don't want the Emacs developers to just adopt Emacsql and would rather see them learn from its successes and failures, and get inspired by other non-elisp dsls.

But I expect that to progress slowly and am interested in Emacsql not dying in the mean time, so I am willing to help with Emacsql maintenance in a limited capacity.

Maybe we could also interest some of the people who worked on the current SQLite support in Emacs (or showed interest in it) to contribute a little to Emacsql as well. That would also allow them to learn from its example, good and bad. Let's summon a few of them now: @larsmagne, @monnier, @skangas. Hi there ! 👋 (Of course feel free to discuss this on emacs-devel instead.)

Christopher, if you give me commit access, then I would merge my two pull requests (the release thing and the new backends). I had a brief look at some other pull requests and I might soon merge another two or so. There also appear to be some low-hanging fruit among the issues. Of course I would deal with any regressions and create new releases when that becomes appropriate. But it would likely be months until I find the time and motivation to deal with another handful of issues and pull requests. I wouldn't be much more active than you, but at least there would be two of us then.

@tarsius
Copy link
Member Author

tarsius commented Mar 15, 2022

Christopher, the main sqlite discussion thread on emacs-devel begins at https://lists.gnu.org/archive/html/emacs-devel/2021-12/msg00463.html. It's one of those huge ones.

@tarsius
Copy link
Member Author

tarsius commented Sep 14, 2022

@skeeto I am now willing to take over as sole maintainer.

I am still trying to take a break, so I cannot promise to take care of the backlog right away, but I do intend to take care of important issues, such as creating new releases, and maybe some low hanging fruit. I also intend to keep at it for the next few years, but don't intend to implement major new features (aside from these new sqlite backends).

@tarsius
Copy link
Member Author

tarsius commented Sep 15, 2022

I've contacted Christopher by mail and he has made me the new maintainer.

The repository now lives at https://github.com/magit/emacsql/, but I guess you already know that. 😀

I plan to create a release in the coming days. I might merge some minor pull-requests before doing that, but also expect to create another release fairly soon. The main change in that second release will be the addition of these two backends.

@tarsius tarsius changed the title Add two new sqlite backends, using builtin support and a module Add two new sqlite backends, using builtin support and a module [also talk about maintainership transfer] Sep 15, 2022
@emacscollective emacscollective closed this by deleting the head repository Sep 15, 2022
@tarsius tarsius reopened this Sep 15, 2022
@tarsius tarsius mentioned this pull request Sep 15, 2022
@alphapapa
Copy link

@tarsius Thanks for stepping up to maintain this package. And thanks, @skeeto for all your work over the years.

@tarsius
Copy link
Member Author

tarsius commented Oct 2, 2022

Still working on it. In case anyone is wondering.

@tarsius
Copy link
Member Author

tarsius commented Oct 21, 2022

It's basically ready except that I want to improve error reporting in Emacs' sqlite.c.

@tarsius
Copy link
Member Author

tarsius commented Oct 25, 2022

The new backends are now available from the master branch.

Improved error reporting for emacsql-sqlite-builtin should come in a few days, when I merge https://debbugs.gnu.org/cgi/bugreport.cgi?bug=58363 into Emacs.

@tarsius tarsius closed this Oct 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants