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

portability issues for EL 7.2 #23

Merged
merged 3 commits into from
Mar 10, 2017
Merged

portability issues for EL 7.2 #23

merged 3 commits into from
Mar 10, 2017

Conversation

brianjmurrell
Copy link
Contributor

Systemd older than 233 doesn't support RuntimeDirectory*
directives and the accepted method for such releases is
to include a file in /usr/lib/tmpfiles.d to create the
[/var]/run/powerman directory.

@garlick
Copy link
Member

garlick commented Mar 8, 2017

Thanks Brian,

systemdtmpdir_ddir = /usr/lib/tmpfiles.d

should be ${prefix}/lib/tmpfiles.d?

@brianjmurrell
Copy link
Contributor Author

should be ${prefix}/lib/tmpfiles.d?

Indeed. PR updated.

@brianjmurrell
Copy link
Contributor Author

I didn't think making a PR for that second patch would actually add it to this PR.

I can try to separate them if desired.

That said, not sure why the check failed. make distcheck works here.

@garlick
Copy link
Member

garlick commented Mar 9, 2017

It's fine with me if these are in one PR.

Also fine if the spec remains a .in.

The travis failure needs attention. Maybe EXTRA_DIST needs to be updated?

@brianjmurrell
Copy link
Contributor Author

Yeah. It was the EXTRA_DIST in the toplevel Makefile.am.

@garlick
Copy link
Member

garlick commented Mar 9, 2017

Sorry I didn't catch this earlier, but should RunttimeDirectory be removed from the unit file? Also should the tmpfiles path be substituted by configure to be ${localstatedir}/run?

@brianjmurrell
Copy link
Contributor Author

I don't think it's supposed to be a problem to have both, particularly if they agree on what they are creating.

But I wonder if we should just remove them for now to be consistent with what the RHEL packagers are doing, which is not adding the RuntimeDirectory* directives but using tmpfiles.d files instead.

I'll push a new patch.

@brianjmurrell
Copy link
Contributor Author

should the tmpfiles path be substituted by configure to be ${localstatedir}/run?

Do you mean the path inside the scripts/tmpfiles.d/powerman file which is currently:

+d /run/powerman   755 daemon daemon

So, on a given EL7 system I am looking at here, there are 7 each files in /usr/lib/tmpfiles.d/ that have (plain) /run as their path and /var/run as their path. So it seems to be pretty evenly divided about where /run is. Maybe the tie breaker is that /usr/lib/tmpfiles.d/systemd.conf itself is using /run and not /var/run.

@garlick
Copy link
Member

garlick commented Mar 9, 2017

I think the problem is that internally powerman uses:

char *rundir = hsprintf("%s/run/powerman", X_LOCALSTATEDIR);

So to match, the tmpdir really should substitute @X_LOCALSTATEDIR@/run/powerman

@garlick
Copy link
Member

garlick commented Mar 9, 2017

Feel free to add my proposed change for byacc from issue #22 to this PR (tagged with "Fixes #22").

Probably should change title of this PR to "Fix EL 7.2 portability issues" so the merge commit ends up with a good message too.

@garlick
Copy link
Member

garlick commented Mar 9, 2017

OK thanks for all this!

If you'll squash "tmpfiles" (b384c7a) with "package: older systemd systems need a tmpfiles.d file" (dc19cea) it will be ready to merge I think.

Systemd older than 233 doesn't support RuntimeDirectory*
directives and the accepted method for such releases is
to include a file in /usr/lib/tmpfiles.d to create the
[/var]/run/powerman directory.
Since there could be different spec files for different distros,
move the one that was provided to examples/powerman_el72.spec to
be clear that it is just an example and that distro packagers could
use as a starting point for their own specs.

That said, try to keep the example functional for EL7.
@brianjmurrell
Copy link
Contributor Author

Oh yes. That squash was supposed to have happened. It failed due to a generated file that was going to be overwritten by the squash, or somesuch. Squashed and the PR updated.

@garlick garlick changed the title package: older systemd systems need a tmpfiles.d file portability issues for EL 7.2 Mar 10, 2017
@garlick garlick merged commit 8adeaa0 into chaos:master Mar 10, 2017
@brianjmurrell
Copy link
Contributor Author

Are these additions significant enough to make a new release? :-)

@garlick
Copy link
Member

garlick commented Mar 10, 2017

I'd kind of like to get #13 and #19 tackled first. (Feel free to pitch in on those if you like :-)

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.

2 participants