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

self-updating runit file #500

Closed
wants to merge 1 commit into from

Conversation

boredsquirrel
Copy link

This runit script should check for an update, if there is one it installs it, waits a bit and inits the daemon.

A systemd servcice could be useful too.

This runit script should check for an update, if there is one it installs it, waits a bit and inits the daemon.

A systemd servcice could be useful too.
@boredsquirrel
Copy link
Author

currently not working, trying to find the error

@AdnanHodzic
Copy link
Owner

Hi,

Would it make more sense to make #500 & #501 as part of a single PR? Also could you please fill me up on what exactly is the reasoning here?

To check if there's an update when auto-cpufreq is being installed using auto-cpufreq-installer? I would say in most cases folks will have latest changes when they are running --install (as I'll presume they have just cloned the repo).

Similar feature was requested as part of #341, as it would be great to ability to be notified and then install update once there is one.

@boredsquirrel
Copy link
Author

No it should work like that:

  1. The service is started through systemd or runit
  2. First it checks if there is an update, if yes it downloads it
  3. if there was an update it inits the install again, so that its set up
  4. it waits 1 sec to sort things out
  5. it starts everything normally

The testing command needs to be checked if it works. Also for it to run either it needs to find out in what dir it runs and store that variable somewhere, or have it in the homedir (maybe append "auto-cpufreq" to ~/.hidden to hide it).

I would just put it in the .hidden file.

The problem with another solution would be that it needs to be manually stopped, updated, installed and started again. This is some solution, even though probably not the best but the smallest.

I wanted to keep the pulls small, so that you dont need to accept things you dont want.

@AdnanHodzic
Copy link
Owner

Sorry for the delay @trytomakeyouprivate. In meantime, there were changes made to auto-cpufreq-runit file, could you please resolve these conflicts and I'll merge your changes. Thanks!

Copy link
Contributor

@meator meator left a comment

Choose a reason for hiding this comment

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

Hi. I'm packaging auto-cpufreq for Void Linux (a runit distro) and have noticed this PR.

A (runit or otherwise) service shouldn't handle updating the underlying program. I know no service that would do this. This approach has too many drawbacks to list here, but the major one is that neither runit nor systemd are a package manager, they therefore shouldn't handle updates.

This change would make it impossible to include this runit service in a Linux distro package. It is disruptive to the user experience. There are also many ways to install the project, most of which haven't been likely considered by the runit service.

@@ -1,2 +1,14 @@
#!/bin/sh
exec /usr/local/bin/auto-cpufreq --daemon
# test for update
if git -c advice.detachedHead=false pull -v https://github.com/AdnanHodzic/auto-cpufreq.git | grep -q -v '^\s*\(Updating\|Already up-to-date\|From\s\)\s'; then
Copy link
Contributor

Choose a reason for hiding this comment

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

There is no guarantee that git exists. The script should check for dependencies like git and sudo mentioned below and act approprietly.

Where is git pulling to exactly?

exit
else
# install the update
echo "i" | sudo ~/auto-cpufreq/auto-cpufreq-installer && \
Copy link
Contributor

Choose a reason for hiding this comment

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

A runit service is run as root (unless it is a user service, which are rare, not enabled by default, a bit difficult to set up and the least common way services are used in runit).

Using sudo doesn't make sense here, the service is already root. It also adds a dependency on sudo, which doesn't have to be installed. Some people also use doas or other things.

Where is ~? You can't use things like ~ in service scripts without having a deep thought about where, how and by which user will the script be run. I have a suspicion that this script is really tailored to be a user service, because a lot of these things don't make sense otherwise. If that is the case, it should be clearly stated (for example in the line after shebang in a comment).

Why is && \ used everywhere? set +o errexit or something similar should be sufficient.

else
# install the update
echo "i" | sudo ~/auto-cpufreq/auto-cpufreq-installer && \
notify-send -a "auto-cpufreq" "Update installed" && \
Copy link
Contributor

Choose a reason for hiding this comment

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

notify-send doesn't work in (root) services. It shouldn't be run as root and it depends on environment variables that aren't available to runit.

@boredsquirrel
Copy link
Author

hey, yes agree this is not the normal way to put everything in that service. I dont like the monopoly of systemd too (because its a single binary, not because it doesnt work or I dont want a unified Linux userspace, which is 100% needed).

Closing this PR, will look at it again. If you can help make runit work please do so. And true, packaging should be external.

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.

None yet

3 participants