Skip to content

nixos/firefox-syncserver: add PostgreSQL backend support#501943

Open
Titaniumtown wants to merge 4 commits into
NixOS:masterfrom
Titaniumtown:firefox-syncserver/postgresql-support
Open

nixos/firefox-syncserver: add PostgreSQL backend support#501943
Titaniumtown wants to merge 4 commits into
NixOS:masterfrom
Titaniumtown:firefox-syncserver/postgresql-support

Conversation

@Titaniumtown

@Titaniumtown Titaniumtown commented Mar 21, 2026

Copy link
Copy Markdown
Contributor

Added support to the firefox-syncserver module for PostgreSQL databases. The upstream project has support for it, but the module did not. I also modified the actual firefox-syncserver package to allow the PostgreSQL feature flag to be set by changing the target database backend.

I needed to bump the version of the underlying package as the version currently shipping doesn't contain PostgreSQL support.

One concern I have is about the feature flag handling, should we just compile for both mysql and PostgreSQL? Right now I have it so it's either-or and it's determined by a feature flag.

LLMs used: Claude Opus 4.6 via opencode

Things done

  • Built on platform:
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • Tested, as applicable:
  • Ran nixpkgs-review on this PR. See nixpkgs-review usage.
  • Tested basic functionality of all binary files, usually in ./result/bin/.
  • Nixpkgs Release Notes
    • Package update: when the change is major or breaking.
  • NixOS Release Notes
    • Module addition: when adding a new NixOS module.
    • Module update: when the change is significant.
  • Fits CONTRIBUTING.md, pkgs/README.md, maintainers/README.md and other READMEs.

@nixpkgs-ci nixpkgs-ci Bot added 8.has: package (update) This PR updates a package to a newer version 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 9.needs: reviewer This PR currently has no reviewers requested and needs attention. 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: changelog This PR adds or changes release notes 8.has: module (update) This PR changes an existing module in `nixos/` 8.has: documentation This PR adds or changes documentation labels Mar 21, 2026
@Titaniumtown Titaniumtown force-pushed the firefox-syncserver/postgresql-support branch from e4d6d84 to 8af1c5f Compare March 21, 2026 13:26
@nixpkgs-ci nixpkgs-ci Bot requested a review from GetPsyched March 21, 2026 13:31
@nixpkgs-ci nixpkgs-ci Bot removed the 9.needs: reviewer This PR currently has no reviewers requested and needs attention. label Mar 21, 2026
@mweinelt mweinelt mentioned this pull request Apr 8, 2026
Open
13 tasks
ntninja
ntninja suggested changes Apr 8, 2026
View reviewed changes

@ntninja ntninja left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since my independently developed version #507460 of this change was deemed a duplicate of this, I gave your version a review with things that are better in my version (some is better in yours).

should we just compile for both mysql and PostgreSQL?

That is not possible, the source code is written in such a way that if you compile for both you’ll get a symbol clash.

For keeping having the PostgreSQL feature on SyncServer not enable MySQL on TokenServer, I have a small patch:
https://github.com/ntninja/nixpkgs/blob/8f8c170c8628ca864d41eb80078147f828cc37db/pkgs/by-name/sy/syncstorage-rs/01-syncserver-tokenserver-postgres-db.patch

Upstream is mostly happy with it, other than it breaking their CI script assumptions, will update soon hopefully:
mozilla-services/syncstorage-rs#2194

Comment thread pkgs/by-name/sy/syncstorage-rs/package.nix Outdated
Comment thread pkgs/by-name/sy/syncstorage-rs/package.nix
Comment thread nixos/modules/services/networking/firefox-syncserver.nix
Comment thread nixos/modules/services/networking/firefox-syncserver.nix Outdated
Comment thread nixos/modules/services/networking/firefox-syncserver.nix Outdated
Comment thread nixos/modules/services/networking/firefox-syncserver.nix Outdated
mweinelt
mweinelt reviewed Apr 8, 2026
View reviewed changes
Comment thread nixos/doc/manual/release-notes/rl-2511.section.md
Comment thread pkgs/by-name/sy/syncstorage-rs/package.nix
Comment thread nixos/modules/services/networking/firefox-syncserver.nix Outdated
Comment thread nixos/modules/services/networking/firefox-syncserver.nix Outdated
@Titaniumtown

Titaniumtown commented Apr 9, 2026

Copy link
Copy Markdown
Contributor Author

Thanks for all the feedback @mweinelt and @ntninja, I will try and work through this today :)

@Titaniumtown Titaniumtown force-pushed the firefox-syncserver/postgresql-support branch from b2cc6cd to 9f03b92 Compare April 9, 2026 20:19
@mweinelt mweinelt added this to the 26.05 milestone Apr 28, 2026
@mweinelt mweinelt self-assigned this Apr 28, 2026
@Titaniumtown

Titaniumtown commented Apr 28, 2026

Copy link
Copy Markdown
Contributor Author

Thank you for your further review @mweinelt, I'm on it!

@Titaniumtown Titaniumtown force-pushed the firefox-syncserver/postgresql-support branch 3 times, most recently from 19bcaba to 05aa52d Compare April 28, 2026 18:49
@Titaniumtown Titaniumtown force-pushed the firefox-syncserver/postgresql-support branch from 05aa52d to 67d5e80 Compare June 1, 2026 02:29
@Titaniumtown

Titaniumtown commented Jun 1, 2026

Copy link
Copy Markdown
Contributor Author

Rebased.

@raj-magesh raj-magesh mentioned this pull request Jun 19, 2026
Closed
mweinelt
mweinelt requested changes Jul 5, 2026
View reviewed changes
Comment thread nixos/modules/services/networking/firefox-syncserver.nix Outdated
Comment thread nixos/modules/services/networking/firefox-syncserver.nix Outdated
Comment thread nixos/modules/services/networking/firefox-syncserver.nix Outdated
Comment thread nixos/modules/services/networking/firefox-syncserver.nix Outdated
Comment thread nixos/modules/services/networking/firefox-syncserver.nix Outdated
Comment thread pkgs/by-name/sy/syncstorage-rs/package.nix Outdated
@mweinelt

mweinelt commented Jul 5, 2026

Copy link
Copy Markdown
Member

I'm terribly sorry for the long wait. I want postgres support so much, but couldn't find the motivation to get to this PR quickly.

@Titaniumtown

Titaniumtown commented Jul 5, 2026

Copy link
Copy Markdown
Contributor Author

@mweinelt No need to apologize. Thanks for the reviews, I will get to them soon!

@Titaniumtown Titaniumtown force-pushed the firefox-syncserver/postgresql-support branch from 67d5e80 to 5fb2afd Compare July 5, 2026 23:19
mweinelt
mweinelt reviewed Jul 5, 2026
View reviewed changes
Comment thread nixos/modules/services/networking/firefox-syncserver.nix Outdated
@mweinelt

mweinelt commented Jul 5, 2026

Copy link
Copy Markdown
Member

LLMs used: Claude Opus 4.6 via opencode

Per https://github.com/NixOS/nixpkgs/blob/master/CONTRIBUTING.md#transparency this requires an Assisted-By trailer on the relevant commits.

@Titaniumtown

Titaniumtown commented Jul 5, 2026

Copy link
Copy Markdown
Contributor Author

LLMs used: Claude Opus 4.6 via opencode

Per master/CONTRIBUTING.md#transparency this requires an Assisted-By trailer on the relevant commits.

Honestly I have no clue which commits I used that on. It was a very long time ago. Should I add it to each commit just to be safe?

@Titaniumtown Titaniumtown force-pushed the firefox-syncserver/postgresql-support branch from 5fb2afd to 26c629d Compare July 5, 2026 23:50
@Titaniumtown

Titaniumtown commented Jul 5, 2026

Copy link
Copy Markdown
Contributor Author

crap, I think I messed up the git history!

@Titaniumtown Titaniumtown force-pushed the firefox-syncserver/postgresql-support branch from 26c629d to f3f5ac1 Compare July 5, 2026 23:51
@Titaniumtown

Titaniumtown commented Jul 5, 2026

Copy link
Copy Markdown
Contributor Author

fixed

weriomat
weriomat reviewed Jul 6, 2026
View reviewed changes

@weriomat weriomat left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for your work, this look promising

Comment thread nixos/tests/firefox-syncserver.nix Outdated
@Titaniumtown Titaniumtown force-pushed the firefox-syncserver/postgresql-support branch from f3f5ac1 to fcc613a Compare July 6, 2026 19:00
@Titaniumtown
syncstorage-rs: 0.21.1-unstable-2026-01-26 -> 0.23.3
521bc60
@Titaniumtown
syncstorage-rs: add dbBackend argument for PostgreSQL support
40063ae
@Titaniumtown
nixos/firefox-syncserver: add PostgreSQL backend support
3533b67
@Titaniumtown
syncstorage-rs: split into syncstorage-rs-{mysql,pgsql}
18d61ba 
Expose both database backends as top-level packages so Hydra builds
and caches each variant. The firefox-syncserver module now picks the
appropriate package based on `services.firefox-syncserver.database.type`
when the user does not supply one explicitly.

The `package` option becomes `nullOr package` with a `null` default;
this avoids requiring a user-supplied custom package to implement the
`dbBackend` override attribute.
@Titaniumtown Titaniumtown force-pushed the firefox-syncserver/postgresql-support branch from fcc613a to 18d61ba Compare July 6, 2026 19:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mweinelt mweinelt mweinelt requested changes
@GetPsyched GetPsyched Awaiting requested review from GetPsyched
+2 more reviewers
@weriomat weriomat weriomat left review comments
@ntninja ntninja ntninja requested changes
Reviewers whose approvals may not affect merge requirements

Requested changes must be addressed to merge this pull request.

Assignees

@mweinelt mweinelt

Labels

6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: changelog This PR adds or changes release notes 8.has: documentation This PR adds or changes documentation 8.has: module (update) This PR changes an existing module in `nixos/` 8.has: package (update) This PR updates a package to a newer version 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux.

Projects

None yet

Milestone

26.05

Development

Successfully merging this pull request may close these issues.

4 participants

@Titaniumtown @mweinelt @ntninja @weriomat