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

I've made a NixOS flake #1084

Open
Noodlez1232 opened this issue Oct 4, 2024 · 9 comments
Open

I've made a NixOS flake #1084

Noodlez1232 opened this issue Oct 4, 2024 · 9 comments

Comments

@Noodlez1232
Copy link
Contributor

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
Copy link
Owner

jmbannon commented Oct 5, 2024

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
Copy link
Contributor Author

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
Copy link
Owner

jmbannon commented Oct 8, 2024

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
Copy link
Contributor Author

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
Copy link
Owner

jmbannon commented Oct 9, 2024

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

@mitchty
Copy link

mitchty commented Oct 13, 2024

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
Copy link
Owner

@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
Copy link
Contributor Author

Noodlez1232 commented Oct 15, 2024

@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
Copy link

mitchty commented Oct 16, 2024

@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!

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

No branches or pull requests

3 participants