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

full systemd support #502

Merged
merged 3 commits into from
Sep 3, 2014
Merged

Conversation

knight-of-ni
Copy link
Member

This method combines ideas from the previous two pull requests. zmpkg will automatically detect that systemd is running, and it will automatically redirect any start/stop/restart commands through systemctl.

@knight-of-ni
Copy link
Member Author

Note that this pull request determines whether or not systemd is running on the fly, without needing to check any special config variables set during build time.

This works properly, even if zmpkg.pl is called (by root) direct from the command line.

This pull request makes policy kit a requirement (regardless of using systemd or not). I checked multiple Linux distros, and they all ship with polkit installed so I don't think this will be a problem. Only the zmsystemctl.pl script gets elevated permission and only when called by the webuser account.

I did make some effort to make sure this pull request works with FreeBSD, but I am still not entirely certain wether polkit is installed by default or if the pull request is looking in the right path for the policy file folders (I had some trouble with my freebsd vm and have not taken the time to fix it).

@PX03AFK I made an attempt to update your spec file for openesuse. Please verify.

@knight-of-ni knight-of-ni added this to the systemd milestone Aug 10, 2014
@PX03AFK
Copy link
Contributor

PX03AFK commented Aug 13, 2014

I found a couple of things which need to be added. In
scripts/CMakeLists.txt you need

configure_file(zmsystemctl.pl "${CMAKE_CURRENT_BINARY_DIR}/zmsystemctl.pl"
COPYONLY)

otherwise zmsystemctl.pl doesn't get into the build structure.

I think you also need to add scripts/zmsystemctl.pl to configure.ac.

Still need to a little more checking for a complete build and am intending
to look at using the generic service files as well.

Like the solution though.


From: Andrew Bauer [mailto:notifications@github.com]
Sent: 10 August 2014 15:01
To: ZoneMinder/ZoneMinder
Cc: David Wilcox
Subject: Re: [ZoneMinder] full systemd support (#502)

Note that this pull request determines whether or not systemd is running on
the fly, without needing to check any special config variables set during
build time.

This pull request makes policy kit a requirement (regardless of using
systemd or not). I checked multiple Linux distros, and they all include this
so I don't think this will be a problem.

I did make some effort to make sure this pull request works with FreeBSD,
but I am still not entirely certain wether polkit is installed by default or
if the pull request is looking in the right path for the policy file folders
(I had some trouble with my freebsd vm and have not taken the time to fix
it).

@PX03AFK https://github.com/PX03AFK I made an attempt to update your spec
file for openesuse. Please verify.

Reply to this email directly or view
#502 (comment)
it on GitHub.
<https://github.com/notifications/beacon/6486570__eyJzY29wZSI6Ik5ld3NpZXM6Qm
VhY29uIiwiZXhwaXJlcyI6MTcyMzI5ODQzMCwiZGF0YSI6eyJpZCI6MzkyODUzNDJ9fQ==--a022
d79d89e4c34faa7c19b7c58adf84705a586b.gif>

@knight-of-ni
Copy link
Member Author

Thanks for the feedback.

Adding zmsystemctl.pl to configure.ac causes autotools to look for a "zmsystemctl.pl.in" and the build then fails becuase there isn't one. ...that's also why I didn't add a similar statement to cmakelists.txt. The rpms I've built off this do contain zmsystemctl.pl, but you may still be right about the cmake change. I'll look into it tonight.

The generic service file came from @barjac 's Maegia zoneminder rpm. It looked like a good starting point, and due to the nature of systemd I think we may be able to get away with the same service file across all linux distros.

@PX03AFK
Copy link
Contributor

PX03AFK commented Aug 19, 2014

The problem is only with zmsystemctl.pl. Even if you renamed it you would
still need to add a line in CMakeLists.txt to produce the .pl file from a
.in, whether in-source or out-of-source. The difference would be that if it
is a .in file then whether it is in or out won't matter. All I have done is
to do a copy of the .pl file, conditional on out-of-source.

I've also discovered that there is a complimentary %cmake_install which
achieves both cd build and make install so slightly tidier. The problem,
from my perspective, is that I can't find any documentation about cmake in
rpmbuild in OpenSuse - hence why it's taken me a little time to work out
what was happening.

The point I was trying to make is that at present it appears that OpenSuse
may be the only OS which defaults rpmbuild to out-of-source but if others go
in that direction we just need to be aware for the future.


From: Andrew Bauer [mailto:notifications@github.com]
Sent: 19 August 2014 08:38
To: ZoneMinder/ZoneMinder
Cc: David Wilcox
Subject: Re: [ZoneMinder] full systemd support (#502)

In the grand scheme of things, this particular pull request aside, does
cmake currently work w/o having to unnessarily move any files around (or do
anything else weird) inside your spec file? By that I mean, anything
unusual. I've opened up two random spec files from Opensuse and a zoneminder
spec file from Maegia ....all three have to "cd build" along with a few
other minor differences so I'd call that normal for an out of source build.

Maybe I misread your intentions, but what I understood was that, there was
some issue with the way cmake handled all the perl scripts for an out of
source build, which caused you to do something extraordinary in your spec
file. If this is the case then this would require us to take a look the
whole cmake process, not just this pull request, and hence the need to
create a new issue.

If this is not the case then you have my apologies as I misunderstood you.
If this is truly just an issue with zmsystemctl.pl and no other files, then
I'll just rename it to zmsystemctl.pl.in and process it like all the other
perl scripts.

Let me know.

Reply to this email directly or view
#502 (comment)
it on GitHub.
<https://github.com/notifications/beacon/6486570__eyJzY29wZSI6Ik5ld3NpZXM6Qm
VhY29uIiwiZXhwaXJlcyI6MTcyNDA3MTA4MCwiZGF0YSI6eyJpZCI6MzkyODUzNDJ9fQ==--61d4
a894bb73cbbe0f2e993db34bc135808b6733.gif>

@knight-of-ni
Copy link
Member Author

I've gone through and deleted some of the comments in this thread simply to make it easier to read.
The issue presented by @PX03AFK was one of maintaining compatibility with an an out-of-source build. commit ad364d0 resolves that.

@@ -123,6 +123,18 @@ if ( $command eq "state" )
$command = "restart";
}

# Check if we are running systemd and if we have been called by the system
Copy link
Member

Choose a reason for hiding this comment

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

Maybe I'm just picky, but I'd rather see this whole block go away, and the below if ( systemdRunning() ) block go into its own sub, which is called from the existing if ( $command blocks. Otherwise looks OK.

Copy link
Member Author

Choose a reason for hiding this comment

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

I totally agree with this, but I think I'd rather do that in a separate pull request. If you look at the entire zmpkg, I'd argue that every single one of the if ( $command blocks should be in its own sub. So if I can get you to agree to compromise, I'd ask you to merge the systemd pull request first, and then I'll open new pull request after I re-write the entire zmpkg.

@kylejohnson
Copy link
Member

This looks OK to merge. I don't see anything which would break existing systems.

kylejohnson added a commit that referenced this pull request Sep 3, 2014
@kylejohnson kylejohnson merged commit 4d579e2 into ZoneMinder:master Sep 3, 2014
@clipo1979
Copy link
Contributor

I've been trying to get ZM running on CentOS 7 with some success but the systemd start up script is still hit and miss, we need more to be included so the install can be as simple as the old sys v scripts.

This might be a dumb reply and once I've put my developer hat fully back on I'm willing to contribute to improve the systemd install process but some initial guidance as to how you have modified ZM to run with systemd is needed.

Step 10, simply copying the default script isnt that helpful....

@knight-of-ni
Copy link
Member Author

See my responses for how to build and configure zoneminder on CentOS 7 in this forum thread:
http://www.zoneminder.com/forums/viewtopic.php?f=32&t=22713

Make sure you don't have the legacy init script under /etc/init.d. That is guaranteed to break things.

@knight-of-ni knight-of-ni deleted the systemd-2 branch December 23, 2016 15:03
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.

6 participants