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

porn-vault: init at 0.30.0-rc.11 #355785

Merged
merged 3 commits into from
Nov 20, 2024
Merged

porn-vault: init at 0.30.0-rc.11 #355785

merged 3 commits into from
Nov 20, 2024

Conversation

LuNeder
Copy link
Contributor

@LuNeder LuNeder commented Nov 14, 2024

Adds Porn-Vault and a NixOS Module for it.

This is my first ever nixpkgs PR, so while I did read the contributing guidelines I might have missed something. Please let me know if I need to change anything!

Some notes:

  • I know the startup script is kinda cursed, but it's the cleanest way I was able to make it work with Nix. Please note that:
    • It reliably works and does not break other packages (and I've seen stuff way more cursed than this on nixpkgs lol)
    • If it was simple to patch PV into working without this, I'd make a PR upstream. Tho I have no node or javascript knowledge.
    • I talked to the PV dev on Discord and the next version of PV is planned to remove the need for izzy as well as probably allow setting a different tmp folder (instead of creating one on workdir, which is the reason the startup script is needed). When that arrives the startup script will be able to be removed, so honestly I think we can just leave this like this for now and once that version is released I'll make a new PR updating PV and removing the script if that's ok for y'all.

Also not a problem anymore. Thanks, Scrumplex!! ^-^

- I would love to compile Izzy from source instead of pulling a binary, but I can't get the rustPlatform thing to read the Cargo.toml I provide. It even entirely ignores me if I try to override buildPhase! If anyone can make this work, please let me know!!
- As I mentioned, izzy is to be deprecated and substituted by something inside PV itself on the next Porn-Vault version (no etas for that tho I think), so if compiling is too much work it might be worth it to just leave it pulling the binary for now until the next PV version is released. I was able to compile izzy from source now

Thanks!

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
    • sandbox = relaxed
    • sandbox = true
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 24.11 Release Notes (or backporting 23.11 and 24.05 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

Add a 👍 reaction to pull requests you find important.

@github-actions github-actions bot added 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: documentation This PR adds or changes documentation 8.has: changelog 8.has: module (update) This PR changes an existing module in `nixos/` 8.has: maintainer-list (update) This PR changes `maintainers/maintainer-list.nix` labels Nov 14, 2024
@LuNeder
Copy link
Contributor Author

LuNeder commented Nov 14, 2024

Fixed the formatting, sorry!

@NixOSInfra NixOSInfra added the 12. first-time contribution This PR is the author's first one; please be gentle! label Nov 14, 2024
@justinas
Copy link
Member

I would love to compile Izzy from source instead of pulling a binary, but I can't get the rustPlatform thing to read the Cargo.toml I provide.

Since the upstream does not provide a Cargo.lock, you'd need to generate it and commit it to nixpkgs, supplying it to the derivation via cargoLock.lockFile. nix-init seems to be able to generate a Cargo.lock by default.

The current situation is quite unfortunate, where PV is available for all platforms, but izzy is limited to x86_64-linux because of the prebuilt binary. Commiting a Cargo.lock would be preferred, I think.

Copy link
Member

@justinas justinas left a comment

Choose a reason for hiding this comment

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

Added some general notes on the approach. Not familiar with PV otherwise.

pkgs/by-name/po/porn-vault/porn-vault.sh Outdated Show resolved Hide resolved
pkgs/by-name/po/porn-vault/porn-vault.sh Outdated Show resolved Hide resolved
pkgs/by-name/po/porn-vault/porn-vault.sh Outdated Show resolved Hide resolved
pkgs/by-name/po/porn-vault/package.nix Outdated Show resolved Hide resolved
nixos/modules/services/web-apps/porn-vault.nix Outdated Show resolved Hide resolved
nixos/modules/services/web-apps/porn-vault.nix Outdated Show resolved Hide resolved
@LuNeder
Copy link
Contributor Author

LuNeder commented Nov 14, 2024

I would love to compile Izzy from source instead of pulling a binary, but I can't get the rustPlatform thing to read the Cargo.toml I provide.

Since the upstream does not provide a Cargo.lock, you'd need to generate it and commit it to nixpkgs, supplying it to the derivation via cargoLock.lockFile. nix-init seems to be able to generate a Cargo.lock by default.

The current situation is quite unfortunate, where PV is available for all platforms, but izzy is limited to x86_64-linux because of the prebuilt binary. Commiting a Cargo.lock would be preferred, I think.

I’ll try the lockFile thing, thanks!

@ofborg ofborg bot added 8.has: package (new) This PR adds a new package 11.by: package-maintainer This PR was created by the maintainer of the package it changes 10.rebuild-darwin: 1-10 10.rebuild-darwin: 1 10.rebuild-linux: 1-10 labels Nov 14, 2024
nixos/modules/services/web-apps/porn-vault.nix Outdated Show resolved Hide resolved
pkgs/by-name/po/porn-vault/package.nix Outdated Show resolved Hide resolved
pkgs/by-name/po/porn-vault/package.nix Outdated Show resolved Hide resolved
pkgs/by-name/po/porn-vault/package.nix Outdated Show resolved Hide resolved
pkgs/by-name/po/porn-vault/package.nix Outdated Show resolved Hide resolved
@LuNeder LuNeder force-pushed the porn-vault branch 2 times, most recently from 92566d0 to 4bafd47 Compare November 14, 2024 16:23
@LuNeder
Copy link
Contributor Author

LuNeder commented Nov 14, 2024

I thiiink I’ve answered all of y’all suggestions either by replying or by changing the derivation now

pkgs/by-name/po/porn-vault/package.nix Show resolved Hide resolved
pkgs/by-name/po/porn-vault/package.nix Outdated Show resolved Hide resolved
pkgs/by-name/po/porn-vault/package.nix Outdated Show resolved Hide resolved
pkgs/by-name/po/porn-vault/package.nix Outdated Show resolved Hide resolved
@LuNeder LuNeder force-pushed the porn-vault branch 2 times, most recently from afe3891 to 38733e0 Compare November 16, 2024 02:49
@LuNeder
Copy link
Contributor Author

LuNeder commented Nov 16, 2024

I've attempted to add, on this latest force push, an option to manually specify the configFolder in case someone wanted to configure that separately. It mostly works, except when I specify a pqath in my home. Any other random path seems to work? Anyone knows why's that? I added ReadWritePaths = cfg.configFolder; which should have made that work I think?

@LuNeder
Copy link
Contributor Author

LuNeder commented Nov 16, 2024

Meh, not sure how useful that would be tbh? Maybe I should just remove that...

@LuNeder
Copy link
Contributor Author

LuNeder commented Nov 18, 2024

Apparently I fucked up making a merge commit instead of rebase, but I force pushed so it should have fixed it now?

@github-actions github-actions bot removed 8.has: documentation This PR adds or changes documentation 8.has: changelog labels Nov 18, 2024
Copy link
Member

@getchoo getchoo left a comment

Choose a reason for hiding this comment

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

Everything else looks good!

nixos/modules/services/web-apps/porn-vault/default.nix Outdated Show resolved Hide resolved
@LuNeder
Copy link
Contributor Author

LuNeder commented Nov 20, 2024

Poor ofborg must be super overworked rn, he still hasn’t even started to test the push I added yesterday. Poor little thing.

@LuNeder LuNeder requested a review from getchoo November 20, 2024 12:15
@thiagokokada
Copy link
Contributor

@ofborg eval

@thiagokokada thiagokokada merged commit 778f30c into NixOS:master Nov 20, 2024
29 of 30 checks passed
@LuNeder LuNeder deleted the porn-vault branch November 20, 2024 21:54
@LuNeder LuNeder added the backport release-24.11 Backport PR automatically label Nov 21, 2024
Copy link
Contributor

Successfully created backport PR for release-24.11:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: maintainer-list (update) This PR changes `maintainers/maintainer-list.nix` 8.has: module (new) This PR adds a module in `nixos/` 8.has: module (update) This PR changes an existing module in `nixos/` 8.has: package (new) This PR adds a new package 10.rebuild-darwin: 1-10 10.rebuild-darwin: 1 10.rebuild-linux: 1-10 11.by: package-maintainer This PR was created by the maintainer of the package it changes 12. first-time contribution This PR is the author's first one; please be gentle! backport release-24.11 Backport PR automatically
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants