NixOS / nixpkgs

Nix Packages collection & NixOS
MIT License
18k stars 14.01k forks source link

Package request: intel-undervolt #301938

Closed Pandapip1 closed 3 weeks ago

Pandapip1 commented 6 months ago

Project description

intel-undervolt is a tool for undervolting and throttling limits alteration for Intel CPUs.

Metadata


Add a :+1: reaction to issues you find important.

aikooo7 commented 6 months ago

Hey, looking at the source code I found out that this program checks if a configuration file is at /etc. Should I patch the file? To what location?

Pinging some member that probably has more experience, sorry for ping @happysalada.

itsvic-dev commented 6 months ago

Hey, looking at the source code I found out that this program checks if a configuration file is at /etc. Should I patch the file? To what location?

@aikooo7 I think that leaving it as-is is fine, a NixOS module can create the /etc/intel-undervolt.conf file later down the line.

happysalada commented 6 months ago

Right.

The rule of thumb should be that if this a binary that us supposed to be able to operate as standalone, then ideally you package it like it can.

If this is a systemd service anyway, then making the configuration file in the systemd module is fine.

Looking briefly at the readme, it has a non systemd mode apparently.

My personal point of view is that working software is always better than perfect, so if you have something working that is not ideal, you can leave comments where things could be improved so someone can pick it up down the line.

aikooo7 commented 6 months ago

Hey, looking at the source code I found out that this program checks if a configuration file is at /etc. Should I patch the file? To what location?

@aikooo7 I think that leaving it as-is is fine, a NixOS module can create the /etc/intel-undervolt.conf file later down the line.

The file is already made by default and put in /etc

aikooo7 commented 6 months ago

Right.

The rule of thumb should be that if this a binary that us supposed to be able to operate as standalone, then ideally you package it like it can.

If this is a systemd service anyway, then making the configuration file in the systemd module is fine.

Looking briefly at the readme, it has a non systemd mode apparently.

My personal point of view is that working software is always better than perfect, so if you have something working that is not ideal, you can leave comments where things could be improved so someone can pick it up down the line.

I am using the make file which uses the non-systemd, should I package both? Also $out/etc is the best spot? Or just /etc?

happysalada commented 6 months ago

You can only package whichever you find useful. $out/share Is used for stuff that is used outside of the package.

aikooo7 commented 6 months ago

You can only package whichever you find useful.

I normally package what people ask since I don't need any package that is not already packaged.

$out/share Is used for stuff that is used outside of the package.

The thing is I am pretty sure the /etc path is hard coded at config.c, perhaps I should apply a patch?

happysalada commented 6 months ago

Or you can just use substitureinplace if its just a couple pf replacements. Whichever you find the most useful, that or a patch.

I was saying that if you package it, start small with something you find useful, you can always add more onto it later. Its always easier to get smaller PRs merged , so this will be short and swee for you.

itsvic-dev commented 6 months ago

The configuration file is meant to be modified. If you're including an example source file, sure, patch the installation to put it in $out/share. But DO NOT modify the source files to point to $out/, otherwise it will be trying to read from the read-only Nix store.

aikooo7 commented 6 months ago

The configuration file is meant to be modified. If you're including an example source file, sure, patch the installation to put it in $out/share. But DO NOT modify the source files to point to $out/, otherwise it will be trying to read from the read-only Nix store.

Sorry I am new to this but then I should modify the source to point to something like /home/{whoami}/.config/intel-undergolt?

aikooo7 commented 6 months ago

Or you can just use substitureinplace if its just a couple pf replacements. Whichever you find the most useful, that or a patch.

But I should patch the source files to point to /home/{whoami}/.config/intel-undergolt so it not directly pointing to the nix store?

I was saying that if you package it, start small with something you find useful, you can always add more onto it later. Its always easier to get smaller PRs merged , so this will be short and swee for you.

Thanks for the tip, I appreciate your kindness.

happysalada commented 6 months ago

Yes, i think it would make sense to patch it.

There should be a create script as well for the config right ? You just have to make sure that it does the right thing too.

If its seamless for the user for the config to be created and put in his home, then i think its the best behavior possible

aikooo7 commented 6 months ago

Yes, i think it would make sense to patch it.

There should be a create script as well for the config right ? You just have to make sure that it does the right thing too.

If its seamless for the user for the config to be created and put in his home, then i think its the best behavior possible

Hey first of all, sorry for not answering nor doing this, to be honest I completely forgot about it.

Now that I am thinking, perhaps putting in a "global" directory over home or config of user since this is a "system" package?

Also the default config is automatically created so I don't need to worry about that.

darshanCommits commented 2 months ago

Any update on this package?

surfaceflinger commented 2 months ago

fyi we already have https://github.com/georgewhewell/undervolt/ packaged and there's a module under services.undervolt.

darshanCommits commented 2 months ago

These two seems to be different programs but okay, if it works.