-
-
Notifications
You must be signed in to change notification settings - Fork 14k
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
thelounge: fix build #233511
thelounge: fix build #233511
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Assuming ofborg shows all green, this looks good to me (really want those yarn hooks though and my PR to add a fixup-yarn-lock command independent of yarn2nix...)
I'm going to be a guinea pig for that, pushing in production. |
Production is running with it, without any hiccups. :) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
Waiting on OfBorg. |
Actually, let me try something else... I want to reduce this closure size. |
Before this push, it was just about 400 MiB, now it's 81 MiB. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks good to me too. I'll second Sandro's minor lexicographical ordering nit, but the dontNpmPrune
install hook change looks fine enough for me
(I wish it were reasonable to separate prune/install into separate hooks entirely, but it really sorta has to happen in the middle there, so this is acceptable for now)
In some odd scenarios, `npm prune` either fails, or hangs. I have no idea what could possibly be wrong at the moment, but let's provide an escape hatch for packages that can still use the rest of the install hook's functionality.
Upstream switched to using TypeScript in v4.4.0, which broke the patch. This fixes that issue by migrating to building The Lounge from source, instead of having to patch the minified JavaScript.
Should be good to go now. Waiting on OfBorg. |
Successfully created backport PR for |
It seems that it is not really without hiccups, when upgrading, the service launches, but the message history cannot be read, as it throws : |
I need more details, please -- what system are you running it on? This seemingly can't be reproduced in CI, or on Ryan's end. |
I have the same aarch64-linux my config: https://github.com/shyim/nixcfg/blob/main/systems/shea/services/thelounge.nix |
I am running thelounge on an x86_64 server, the config is at https://git.hubrecht.ovh/hubrecht/nixos/src/branch/master/machines/sd-02/thelounge.nix The issue seems to be that old messages are no longer shown, but new messages still are (I am next to Ryan's guinea pig who confirmed the issue) |
I confirm that my users doesn't see their messages, but the logs are on disk and the SQLite DB is intact. |
Let me look into this, please give me a bit. |
I do have this:
in my systemd logs, FWIW. @winterqt Thank you for looking into it (and I have no worries, I have backups and all.) :) It's annoying that upstream didn't make the error fatal… :/ |
Previously, we never actually built the SQLite binding, causing The Lounge to bail when attempting to load SQLite logs [0]. It wasn't caught before because it wasn't thrown fatally, for whatever reason. Perhaps we should fix this in the future with a patch and/or more robust tests, but for now, let's just fix the issue. [0]: NixOS#233511 (comment)
Previously, we never actually built the SQLite binding, causing The Lounge to bail when attempting to load SQLite logs [0]. It wasn't caught before because it wasn't thrown fatally, for whatever reason. Perhaps we should fix this in the future with a patch and/or more robust tests, but for now, let's just fix the issue. [0]: #233511 (comment) (cherry picked from commit a1cfd90)
Previously, we never actually built the SQLite binding, causing The Lounge to bail when attempting to load SQLite logs [0]. It wasn't caught before because it wasn't thrown fatally, for whatever reason. Perhaps we should fix this in the future with a patch and/or more robust tests, but for now, let's just fix the issue. [0]: #233511 (comment) (cherry picked from commit a1cfd90)
Description of changes
Upstream switched to using TypeScript in v4.4.0, which broke the patch. This fixes that issue by migrating to building The Lounge from source, instead of having to patch the minified JavaScript.
Closes #233630.
Things done
sandbox = true
set innix.conf
? (See Nix manual)nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD"
. Note: all changes have to be committed, also see nixpkgs-review usage./result/bin/
)