jmbannon / ytdl-sub

Lightweight tool to automate downloading and metadata generation with yt-dlp
https://ytdl-sub.readthedocs.io
GNU General Public License v3.0
1.77k stars 70 forks source link

I've made a NixOS flake #1084

Open Noodlez1232 opened 2 weeks ago

Noodlez1232 commented 2 weeks ago

Hello! I made a NixOS flake containing a module that others can use. I'd like to add it to the documentation as some sort of community/unofficial method of using it. Is there any way that could be done?

jmbannon commented 2 weeks ago

Hi @Noodlez1232 , while I really appreciate the contribution, I'm a bit confused as to what the benefit is of adding NixOS's flake language on top of the ytdl-sub paradigm. I feel adding a new abstraction layer to the official documentation could cause further confusion to an already somewhat complicated app.

Noodlez1232 commented 1 week ago

The idea here isn't to add it in an official manner, but more of an aside of "If you're interested, there's a community-led NixOS flake". Sorry I should've been a bit more clear about that. A footnote, not a big thing.

jmbannon commented 1 week ago

Sure, but I still don't understand the benefit of using flake over yaml. Maybe if it only included the cron part and pointed to a config and subscription file, I could get behind that. But adding another level to defining subscriptions seems redundant

Noodlez1232 commented 1 week ago

Gotcha. Here it's more about having all configuration happen in the Nix language. More of a Nix ecosystem thing than a ytdl-sub thing.

You can set the config file to a YAML file using services.ytdl-sub.extraConfig and services.ytdl-sub.extraSubscriptions using something like builtins.readFile or something like that. I'm also willing to add in an option to just set a simple path instead.

services.ytdl-sub = {
  enable = true;
  timer.enable = true;
  extraConfig = builtins.readFile ./config.yaml;
  extraSubscriptions = builtins.readFile ./subscriptions.yaml;
};

It's just giving people with NixOS a way to run ytdl-sub natively in a NixOS ergonomic way that doesn't require any sort of Docker or container spin-up, since ytdl-sub isn't easy to run natively in NixOS.

jmbannon commented 1 week ago

Got it, I will make a dedicated community additions section and add a link 🙂

mitchty commented 5 days ago

As a fellow nix user this is a great idea to keep overall configuration in one spot without abusing toYaml functions. We could add more checking of inputs as well to make sure the yaml produced is always valid, I'll admit I spent a lot of time digging through docs on what a valid setup looked like.

Probably outside of the scope of this pr but I don't see it running the test suite. https://github.com/mitchty/nixos/blob/master/nix/packages/ytdl-sub.nix#L43-L61

I need to debug what about the recent unit test refactoring is causing md5sum exceptions but it doesn't seem to break anything that I have seen so its low priority in my derivation.

I'd vote for this to all be upstreamed into NixOS/nixpkgs at some point to be honest. I could start a pr for the package to nixpkgs/unstable, the nixos module could be upstreamed separately and if we get it in soon enough get into the nixos 24.11 release.

jmbannon commented 5 days ago

@mitchty the unit test checksums require specific OS and ffmpeg versions (I should write this somewhere...), it's probably not anything you caused.

Yaml validation is built into ytdl-sub so as long as it runs, it should be safe. Just make sure the paths are set correctly

Noodlez1232 commented 3 days ago

@mitchty

I'd vote for this to all be upstreamed into NixOS/nixpkgs at some point to be honest. I could start a pr for the package to nixpkgs/unstable, the nixos module could be upstreamed separately and if we get it in soon enough get into the nixos 24.11 release.

There's a very specific reason I didn't add it to nixpkgs, and it's because it's very tightly tied to the yt-dlp version. If they are not in lockstep, this doesn't build. This is why I did it in a flake instead, since I can lock the version of nixpkgs to include the version of yt-dlp that builds. That being said, if you're willing to front that maintenance burden, I have no qualms. I didn't want to deal with it.

[...] I don't see it running the test suite [...]

Yeah I didn't really bother to learn how to run the test suite. I'm no packaging master, so I haven't really gotten around to figuring out how to run pytest with it.

add more checking of inputs ...

Yeah I wanted to do this as well, and you can see that I did do that for some of them like the configuration.<*>_path options for it. It's actually my next step.

I'd like to note that I did open up a mailing list and all that to contribute to it on SourceHut. No account needed, just an email address to send in patches if you want to.

@jmbannon

Yaml validation is built into ytdl-sub

I was actually wondering if it'd be possible to add a validate subcomand that returns exit code 0 on valid output. This way not just us but other people can use it to make sure their config is working without doing a whole download. I feel like this would've come in handy when I was first using it before I had even started using it with NixOS but just using the docker container.

mitchty commented 2 days ago

@mitchty the unit test checksums require specific OS and ffmpeg versions (I should write this somewhere...), it's probably not anything you caused.

Ah, yeah if you could doc that i can override the ffmpeg derivation used to pin it to the "right" version. I was very confused but when I tested the result it worked so I didn't think too hard on it. Thanks for the explanation!