Summary: | net-misc/isatapd - add systemd support | ||
---|---|---|---|
Product: | Gentoo Linux | Reporter: | Yichao Zhou <broken.zhou> |
Component: | Current packages | Assignee: | Michael Weber (RETIRED) <xmw> |
Status: | RESOLVED FIXED | ||
Severity: | normal | CC: | bugs, systemd |
Priority: | Normal | ||
Version: | unspecified | ||
Hardware: | All | ||
OS: | Linux | ||
URL: | https://github.com/shlusiak/isatapd/issues/2 | ||
Whiteboard: | |||
Package list: | Runtime testing required: | --- | |
Bug Depends on: | |||
Bug Blocks: | 448882 | ||
Attachments: |
/etc/systemd/system/isatapd.service.d/00gentoo
/usr/lib64/systemd/system/isatapd.service |
Description
Yichao Zhou
2013-08-27 16:25:10 UTC
upstream bug report: https://github.com/shlusiak/isatapd/issues/2 regarding EnvironmentFile, it's preferred to use another file inside /etc/systemd instead of conf.d as explained at: https://wiki.gentoo.org/wiki/Systemd/Ebuild_policy If you can update patch to apply to that, would be nice :D I use conf.d because I'm using ntpd's unit file as template. It is using conf.d. Is that meaning ntpd should also change its service file? I found it hard to port OpenRC script to s systemd because I do not have a shell environment in unit file. What is the suitable location to store a sh script to configure something such as command-line options? Or I should just patch the target executable file? For this case, I would have a gentoo.conf file under /etc/systemd similar to current conf.d file. Service file would then simply have a ExecStart with that --interval and other options inheritting them from $ISATAP_ variables. Reading current conf.d file I would try to minimize it to support what people should really set and, then, if you doubt about how to replace: [ -n "${ISATAP_INTERVAL}" ] && DAEMON_OPTS="${DAEMON_OPTS} --interval ${ISATAP_INTERVAL} " from init.d file I would simply change config file to *set* vars to the default values (instead of getting defaults when unset) For example: # Interval in seconds to check for DNS changes. Set to 0 to disable. # Default: 3600 ISATAP_CHECK_DNS="3600" -> notice the line is uncommented and, then, ExecStart command would be called with that ISATAP_CHECK_DNS value (that would be default one in this case) (In reply to Pacho Ramos from comment #4) > For this case, I would have a gentoo.conf file under /etc/systemd similar to > current conf.d file. Service file would then simply have a ExecStart with > that --interval and other options inheritting them from $ISATAP_ variables. > > Reading current conf.d file I would try to minimize it to support what > people should really set and, then, if you doubt about how to replace: > [ -n "${ISATAP_INTERVAL}" ] && DAEMON_OPTS="${DAEMON_OPTS} > --interval ${ISATAP_INTERVAL} " > > from init.d file I would simply change config file to *set* vars to the > default values (instead of getting defaults when unset) > > For example: > # Interval in seconds to check for DNS changes. Set to 0 to disable. > # Default: 3600 > ISATAP_CHECK_DNS="3600" -> notice the line is uncommented > > and, then, ExecStart command would be called with that ISATAP_CHECK_DNS > value (that would be default one in this case) I tried this way last night. Unfortunately, I did not success. One reason is that some option has default parameter "auto", you cannot set it explicitly. "--link auto" will give you an error message. A script is necessary without doing patch. Of course, I can simply delete these options. I just wonder what is the standard way to "add a script". But it seems that it hasn't. Another is that unit file in "net-misc/ntp" is using "conf.d". Should I submit a bug report? (In reply to Yichao Zhou from comment #5) [...] > I tried this way last night. Unfortunately, I did not success. One reason > is that some option has default parameter "auto", you cannot set it > explicitly. "--link auto" will give you an error message. A script is > necessary without doing patch. > If that is the only failing option, I would simply drop it > Of course, I can simply delete these options. I just wonder what is the > standard way to "add a script". But it seems that it hasn't. > There is a way that would be to add a helper (like x11-misc/virtualgl), but it's over complicated for this case and I wouldn't use it. > Another is that unit file in "net-misc/ntp" is using "conf.d". Should I > submit a bug report? We are aware of the problem and, since we are in a transition period, I think it's not needed :/ (we don't have time and resources to do all at the same time -> add new unit files and migrate available ones to new locations. Will anyway need to review that one as it has a problem with ntp-client unit file). On the other hand, if that kind of conf.d -> /etc/systemd bugs comes with patches, they are always welcome of course :) Is any other option apart of --link affected by that? If not, could you provide a service file with comment #4 approach? Thanks See https://github.com/shlusiak/isatapd/issues/2. Maybe we need to wait sometime? (In reply to Yichao Zhou from comment #8) > See https://github.com/shlusiak/isatapd/issues/2. > > Maybe we need to wait sometime? Please don't get me wrong, I was simply reviewing (and committing when possible) all blockers of the tracker bug and came across this one again ;) (In reply to Pacho Ramos from comment #9) > (In reply to Yichao Zhou from comment #8) > > See https://github.com/shlusiak/isatapd/issues/2. > > > > Maybe we need to wait sometime? > > Please don't get me wrong, I was simply reviewing (and committing when > possible) all blockers of the tracker bug and came across this one again ;) ...... Sorry for my bad English. I think what I want to say is "wait some time". I have just realized that "wait sometime" has another meaning. Sorry for that. Which environment file should I use? I read wiki and I cannot have such directory/file like /etc/systemd/system/foo.service.d/gentoo.conf. $ grep EnvironmentFile -R ntpd.service:EnvironmentFile=/etc/conf.d/ntpd saslauthd.service:EnvironmentFile=/etc/conf.d/saslauthd isatapd.service:EnvironmentFile=/etc/conf.d/isatapd smartd.service:EnvironmentFile=-/etc/sysconfig/smartmontools sntp.service:EnvironmentFile=/etc/conf.d/sntp ntp-client.service:EnvironmentFile=/etc/conf.d/ntp-client It contains no good example. (In reply to Yichao Zhou from comment #11) > Which environment file should I use? > > I read wiki and I cannot have such directory/file like > /etc/systemd/system/foo.service.d/gentoo.conf. It should be: /etc/systemd/system/isatapd.service.d/gentoo.conf And that should contain the vars you are going to use in unit file (In reply to Pacho Ramos from comment #12) > (In reply to Yichao Zhou from comment #11) > > Which environment file should I use? > > > > I read wiki and I cannot have such directory/file like > > /etc/systemd/system/foo.service.d/gentoo.conf. > > It should be: > /etc/systemd/system/isatapd.service.d/gentoo.conf > > And that should contain the vars you are going to use in unit file Emmm... Are there some packages which actually uses this style that I can refer to? On my system, there does not exists such directory like xxx.services.d. $ grep -r EnvironmentFile * -> Looks like there isn't any yet :/, I only see the old style conf.d way and some of them using a general config file in /etc (like courier-imap) (In reply to Pacho Ramos from comment #14) > $ grep -r EnvironmentFile * > > -> Looks like there isn't any yet :/, I only see the old style conf.d way > and some of them using a general config file in /etc (like courier-imap) I just do not understand why this is so strange.. /etc/systemd/system/isatapd.service.d/gentoo.conf Why it is called gentoo.conf? Why we need a whole directory for this single file? Is this file the default EnvironmentFile? (In reply to Yichao Zhou from comment #15) > (In reply to Pacho Ramos from comment #14) > > $ grep -r EnvironmentFile * > > > > -> Looks like there isn't any yet :/, I only see the old style conf.d way > > and some of them using a general config file in /etc (like courier-imap) > > I just do not understand why this is so strange.. > > /etc/systemd/system/isatapd.service.d/gentoo.conf > > Why it is called gentoo.conf? It's per our policy, to use "gentoo.conf" for our settings and let any other names for users wanting to play with config under that location > Why we need a whole directory for this > single file? Because upstream wants to use this dir :/ > Is this file the default EnvironmentFile? I think we still need to specify it explictely in unit file > Because upstream wants to use this dir :/ Are you sure? Could you give a reference? Fedora suggests put environment files at /etc/sysconfig: http://fedoraproject.org/wiki/Packaging:Systemd#EnvironmentFiles_and_support_for_.2Fetc.2Fsysconfig_files If fedora even not follows that, I suspect if such rule exists. I'm afraid that we misunderstood something. See https://wiki.archlinux.org/index.php/Systemd, section "Editing provided unit files" for some information. But I have not found a official explanation about xxx.service.d under systemd. > I think we still need to specify it explictely in unit file This is not very intuitive. We we have some .d directories, such as profile.d, env.d, all the files in that directory will be sourced. But for /etc/systemd/system/isatapd.service.d/gentoo.conf, only gentoo.conf will be used. This is very confusing for me. I think /etc/conf.d is much more reasonable since it provide a uniform way to configure both systemd and openrc. Using /etc/systemd/system/isatapd.service.d/gentoo.conf is just too strange to me and may be incorrect. And there is even no ebuilds follows this rule. Anyway, I hope this policy could be changed. (In reply to Yichao Zhou from comment #15) > (In reply to Pacho Ramos from comment #14) > > $ grep -r EnvironmentFile * > > > > -> Looks like there isn't any yet :/, I only see the old style conf.d way > > and some of them using a general config file in /etc (like courier-imap) > > Why it is called gentoo.conf? Because it's Gentoo-specific. > Why we need a whole directory for this single file? That's the design of '.d' directories. Directory with possibly multiple files inside. > Is this file the default EnvironmentFile? It's not an EnvironmentFile. It's unit override file. (In reply to Michał Górny from comment #18) > (In reply to Yichao Zhou from comment #15) > > (In reply to Pacho Ramos from comment #14) > > > $ grep -r EnvironmentFile * > > > > > > -> Looks like there isn't any yet :/, I only see the old style conf.d way > > > and some of them using a general config file in /etc (like courier-imap) > > > > Why it is called gentoo.conf? > > Because it's Gentoo-specific. > > > Why we need a whole directory for this single file? > > That's the design of '.d' directories. Directory with possibly multiple > files inside. > > > Is this file the default EnvironmentFile? > > It's not an EnvironmentFile. It's unit override file. Then where should I place the EnvironmentFile? You don't use that thing. Simple as that. > You don't use that thing. Simple as that.
Why I don't need a environment file? Then how do I write configuration.
(In reply to Yichao Zhou from comment #21) > > You don't use that thing. Simple as that. > > Why I don't need a environment file? Then how do I write configuration. Environment files are *not* for configuration. As the name says, they set *environment* variables for the program. That is, everything that's in EnvironmentFile= gets *exported*. That said, you *don't* write configuration. You can only provide an example snippet of what user may want to override via 00gentoo.conf file. Please wait till we commit the relevant stuff and then we'll prepare a few examples. The idea will be to install a /etc/systemd/systemd/foo.service.d/00gentoo.conf with something like: Environment=FOO= Latest ntp ebuild will show you how to deal with conf files and units version bump? +*isatapd-0.9.7 (28 Sep 2013) + + 28 Sep 2013; Pacho Ramos <pacho@gentoo.org> +files/isatapd.service, + +isatapd-0.9.7.ebuild: + Version bump, add unit file (#482690 by Yichao Zhou) + Today I upgrade my isatapd and found the systemd file which gentoo ships doesn't work. I update the systemd configuration files so that it should pass the QA now. Created attachment 371020 [details]
/etc/systemd/system/isatapd.service.d/00gentoo
Created attachment 371022 [details]
/usr/lib64/systemd/system/isatapd.service
This looks to me more like an "enhancement" over our current unit file (that is pretty similar to Arch one and works ok for them without needing to play with all the options). I would open a new bug report to discuss this new suggested changes No. The systemd file that gentoo ships now doesn't work at all. In the original bug, I propose to add EnvironmentFile=/etc/conf.d/isatapd ExecStart=/usr/sbin/isatapd ${DAEMON_OPTS} ${ISATAP_ROUTERS} However, in the portage version, "EnvironmentFile" and " ${DAEMON_OPTS} ${ISATAP_ROUTERS}" is removed for unknown reason, which makes it doesn't work: isatapd need to know which router it uses. Any updates? The old systemd file for isatapd really does not work at all. As explained in: https://wiki.gentoo.org/wiki/Systemd/Ebuild_policy#Unit_file_guidelines In general we don't want to share conf.d files with openRC neither allow people to pass random options to the executables. I would add a /etc/systemd/system/foo.service.d/gentoo.conf to let people set ISATAP_ROUTERS :/ (In reply to Pacho Ramos from comment #33) > As explained in: > https://wiki.gentoo.org/wiki/Systemd/Ebuild_policy#Unit_file_guidelines > > In general we don't want to share conf.d files with openRC neither allow > people to pass random options to the executables. I would add a > /etc/systemd/system/foo.service.d/gentoo.conf to let people set > ISATAP_ROUTERS :/ Hi Pacho Ramos, I have added two new files in my previous comments, which I think implemented what you want and work on my computer. Would you please do a check for them? Thanks. +*isatapd-0.9.7-r1 (30 Mar 2014) + + 30 Mar 2014; Pacho Ramos <pacho@gentoo.org> +files/isatapd.service-r1, + +files/isatapd.service.conf, +isatapd-0.9.7-r1.ebuild: + Fix systemd support (#482690 by Yichao Zhou) + |