Skip to content

various improvements to the nix files #228

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

Merged
merged 13 commits into from
Jul 29, 2025

Conversation

dtomvan
Copy link
Contributor

@dtomvan dtomvan commented Jul 28, 2025

This PR complies with the DCO; https://developercertificate.org/

Summary:

  • Added u2c
  • Added partyfuse
  • Support adding any (python) packages to the copyparty package
  • Cosmetic changes, because everything was unformatted spaghetti...
  • Add formatter
  • Add extra checks

I haven't looked at the NixOS module as much, but I think I'll make some improvements to that one later.

dtomvan added 13 commits July 28, 2025 13:28
I wanted to use
https://github.com/9001/copyparty/blob/hovudstraum/bin/hooks/notify.py
but that wasn't really possible without this under the nix package.
This allows contributors to format their nix changes with the `nix fmt`
command.
One downside of the way the nix ecosystem works is that MacFUSE needs to
be installed manually. Luckily the script tells you that already!
Because sometimes an import might be missing, and if it is an optional
then you'll only figure out that it's broken if you set the flag.
Or `nix flake check` will refuse to run the copyparty-full check
@chinponya
Copy link
Contributor

It's good to have an actual nixpkgs contributor look through these.

All changes seem like an improvement to me without breaking anything, so it should be safe to merge this @9001.

It seems like everyone who touched nix in this repo chose to use a different formatter, but since you've included yours in the flake, I'd say we have a winner.

@9001 9001 merged commit 4915b14 into 9001:hovudstraum Jul 29, 2025
@9001
Copy link
Owner

9001 commented Jul 29, 2025

sweet, was waiting for you to get a chance to peep it :>

thanks a lot for the contribution!

@dtomvan dtomvan deleted the dtomvan/push-zoxpmrrlpzvv branch July 29, 2025 00:17
@sodiboo
Copy link

sodiboo commented Jul 29, 2025

It seems like everyone who touched nix in this repo chose to use a different formatter, but since you've included yours in the flake, I'd say we have a winner.

Yeah. It was kinda the wild west for a while. But nixfmt 1.0.0 is The Nix Formatter now; and was only recently released. (see: reformatting nixpkgs).

@dtomvan
Copy link
Contributor Author

dtomvan commented Jul 29, 2025

Thanks for merging! Now I can use hooks with additional dependencies as well!

Okay, so what I'm still interested in:

  • Making the module "nixpkgs ready" to get upstreamed into there
  • Packaging this from source, without the dockerfile. I might need a little help with this one, because there's a bunch of dependencies and patches that get pulled which I can't really grok

@chinponya
Copy link
Contributor

  • Packaging this from source, without the dockerfile. I might need a little help with this one, because there's a bunch of dependencies and patches that get pulled which I can't really grok

It looks like someone beat you to it, but maybe you are interested in collaborating there, in case you find it insufficiently good.
This version uses release source archive, which is actually different from what's on the repo - it contains bundled frontend dependencies (same as the pypi release), sidestepping all of the cursed stuff happening inside dockerfiles.

Strictly speaking that's not a real "built from source" derivation, but it may be a reasonable compromise. Some web dependencies such as prism do not have (at least in the past) a way to create a bundle without using their website (though for that we made our own bundler in the previous attempt to package it from source).

@dtomvan
Copy link
Contributor Author

dtomvan commented Jul 29, 2025

It looks like someone beat you to it, but maybe you are interested in collaborating there, in case you find it insufficiently good.

Yeah, I saw. Very nice! The PR looks fine. I understand it's not entirely "pure" but it's better than "yeah let's just grab the sfx and call it a day" There are a couple packages that are pulled in the dockerfile that could be replaced with preexisting nixpkgs packages or custom derivations, but there's not a lot of juice to squeeze there.

I think there's a much simpler way to improve reproducibility for the web deps and it's ADD --checksum in the dockerfile. That doesn't require wget either, and improves reproducibility not only for the nix package but also for the regular sfx.

luzpaz pushed a commit to luzpaz/copyparty that referenced this pull request Jul 30, 2025
* nix: allow passing extra packages in PATH

* nix: allow passing extra python packages

I wanted to use
https://github.com/9001/copyparty/blob/hovudstraum/bin/hooks/notify.py
but that wasn't really possible without this under the nix package.

* nix: format all nix files with nixfmt

* nix: reduce redundancy in the package

For readability

* nix: remove unused pyftpdlib import

* nix: put makeWrapper into the correct inputs

* nix: fill out all of meta

* nix: set formatter in flake for nix files

This allows contributors to format their nix changes with the `nix fmt`
command.

* nix: add u2c

* nix: add partyfuse

One downside of the way the nix ecosystem works is that MacFUSE needs to
be installed manually. Luckily the script tells you that already!

* nix: add missing cfssl import

* nix: add flake check that makes sure it builds with all flags

Because sometimes an import might be missing, and if it is an optional
then you'll only figure out that it's broken if you set the flag.

* nix: use correct overlay argument names

Or `nix flake check` will refuse to run the copyparty-full check
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