search

clearpathrobotics / robot_upstart

ROS package of helper functions related to bringing up roslaunch on system startup.
BSD 3-Clause "New" or "Revised" License
196 stars 96 forks source link

Added option to install under systemd rather than upstart #41

Closed ncasaril closed 8 years ago

ncasaril commented 8 years ago

Sorry for the "hackyness" - I wanted a quick fix to allow installation under systemd. Should perhaps be under a kinetic branch as well.

This links to issue #37

Tested under ubuntu 16.04 on x86_64 and armv7 with Kinetic. Feedback appreciated :)

ncasaril commented 8 years ago

I'm not entirely satisfied that my solution actually enables start at boot time. Leave it with me and I'll run some more test and verify and/or fix the startup behaviour.

On Tuesday, 1 November 2016, Mike Purvis notifications@github.com wrote:

@mikepurvis commented on this pull request.

In src/robot_upstart/install_script.py https://github.com/clearpathrobotics/robot_upstart/pull/41#pullrequestreview-6604994 :

@@ -91,6 +94,10 @@ def main(): if args.augment: j.generate_system_files = False

  • j.install()
  • provider=providers.Upstart
  • if args.systemd:
  • provider=providers.Systemd
  • j.install(Provider=provider)

Ideally we would detect the init system here and just do the right thing, but having looked into it previously, I acknowledge that detecting it reliably can be tricky. Other possibility for an interface would be --provider systemd, but I'm not super attached to that; the flag is fine.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/clearpathrobotics/robot_upstart/pull/41#pullrequestreview-6604994, or mute the thread https://github.com/notifications/unsubscribe-auth/ANoYjLr9uUpOrcZzOV5tVtCmUQiM1TX6ks5q5zIPgaJpZM4KlyiV .

ncasaril commented 8 years ago

The final change where I added a post_install function to the providers should probably either be creating the required symlink:

/etc/systemd/system/multi-user.target.wants/(job_name).service -> /lib/systemd/system/(job_name).service

or we need to have the functionality to also run commands as sudo through a similar method as the mutate_files script.

mikepurvis commented 8 years ago

Note that #43 is making the mutate_files script able to create symlinks.

mikepurvis commented 8 years ago

This looks great, thanks for taking a stab at the detection thing too. Let me know if you want to look in the symlink thing. Either way, if this is ready for merge, can I trouble you to rebase?

ncasaril commented 8 years ago

I'll have a look at the symlink possibility as this would remove the need for the post function altogether. And then I'll rebase as well. :)

On Thursday, 10 November 2016, Mike Purvis notifications@github.com wrote:

This looks great, thanks for taking a stab at the detection thing too. Let me know if you want to look in the symlink thing. Either way, if this is ready for merge, can I trouble you to rebase?

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/clearpathrobotics/robot_upstart/pull/41#issuecomment-259446007, or mute the thread https://github.com/notifications/unsubscribe-auth/ANoYjHQvoeppOusgTTxTQ5Dq6C_n5hs4ks5q8eqIgaJpZM4KlyiV .

ncasaril commented 8 years ago

I actually merged in the latest changes from your jade-devel branch instead of rebasing - was that a mistake? Sorry but I'm very familiar with the rebase part of git. Would you mind pointing me in the right direction?

mikepurvis commented 8 years ago

No worries, that'll do!

mikepurvis commented 8 years ago

For future reference: https://git-scm.com/book/en/v2/Git-Branching-Rebasing