Patchwork systemd: Add systemd package to PACKAGE var

login
register
mail settings
Submitter Khem Raj
Date Feb. 12, 2013, 8:22 a.m.
Message ID <1360657337-7154-1-git-send-email-raj.khem@gmail.com>
Download mbox | patch
Permalink /patch/44501/
State New
Headers show

Comments

Khem Raj - Feb. 12, 2013, 8:22 a.m.
If someone defines SYSTEMD_PACKAGES to be different
then ${PN} then we need to make sure that they get
added to PACKAGES variable

Signed-off-by: Khem Raj <raj.khem@gmail.com>
---
 meta/classes/systemd.bbclass |    7 +++++++
 1 file changed, 7 insertions(+)
Ross Burton - Feb. 12, 2013, 9:08 a.m.
On Tuesday, 12 February 2013 at 08:22, Khem Raj wrote:
> If someone defines SYSTEMD_PACKAGES to be different
> then ${PN} then we need to make sure that they get
> added to PACKAGES variable

The only case it won't already be in PACKAGES is if you're creating a package which contains just the service file, which as I've said before isn't recommended - package the service files along with the binaries that they are executing.

Or is there another use-case I'm missing?

Ross
Anders Darander - Feb. 12, 2013, 3:01 p.m.
* Ross Burton <ross.burton@intel.com> [130212 10:09]:

> On Tuesday, 12 February 2013 at 08:22, Khem Raj wrote:
> > If someone defines SYSTEMD_PACKAGES to be different
> > then ${PN} then we need to make sure that they get
> > added to PACKAGES variable

> The only case it won't already be in PACKAGES is if you're creating a package which contains just the service file, which as I've said before isn't recommended - package the service files along with the binaries that they are executing.

> Or is there another use-case I'm missing?

Well, there is always the possibillity that the install is split into
multiple packages, with binaries also in some sub-packages. I can't
recall right now if we have such packages, where the binaries in the
sub-packages should be started by init, though.

Still, I'd say that it might be a valid use case for some applications.

Cheers,
Anders
Ross Burton - Feb. 12, 2013, 3:06 p.m.
On Tuesday, 12 February 2013 at 15:01, Anders Darander wrote:
> > Or is there another use-case I'm missing?
> 
> Well, there is always the possibillity that the install is split into
> multiple packages, with binaries also in some sub-packages. I can't
> recall right now if we have such packages, where the binaries in the
> sub-packages should be started by init, though.

And those sub-packages are obviously already in PACKAGES, so this isn't required again.  I'm just trying to keep the amount of magic in the class to a minimum unless it's required.

Ross
Anders Darander - Feb. 12, 2013, 4:49 p.m.
* Ross Burton <ross.burton@intel.com> [130212 16:07]:

> On Tuesday, 12 February 2013 at 15:01, Anders Darander wrote:
> > > Or is there another use-case I'm missing?

> > Well, there is always the possibillity that the install is split into
> > multiple packages, with binaries also in some sub-packages. I can't
> > recall right now if we have such packages, where the binaries in the
> > sub-packages should be started by init, though.

> And those sub-packages are obviously already in PACKAGES, so this
> isn't required again.  I'm just trying to keep the amount of magic in
> the class to a minimum unless it's required.

Ah, of course they are...

I completely mis-read the patch... As the patch only was concerned with
adding packages to PACKAGES, I completely agree with you. Lets get rid
of that extra magic.

Cheers,
Anders
Khem Raj - Feb. 12, 2013, 5:35 p.m.
On Tue, Feb 12, 2013 at 1:08 AM, Ross Burton <ross.burton@intel.com> wrote:
> On Tuesday, 12 February 2013 at 08:22, Khem Raj wrote:
>> If someone defines SYSTEMD_PACKAGES to be different
>> then ${PN} then we need to make sure that they get
>> added to PACKAGES variable
>
> The only case it won't already be in PACKAGES is if you're creating a package which contains just the service file, which as I've said before isn't recommended - package the service files along with the binaries that they are executing.
>
> Or is there another use-case I'm missing?


Thinking about it from different perspective, I agree that probably
adding an extra package is not right and having unitfiles as part of
package proper is fine. but we should definitely have a check where if
SYSTEMD_PACKAGES dont exist in PACKAGES then it should error out or
warn about it.

>
> Ross
Ross Burton - Feb. 12, 2013, 9:06 p.m.
On 12 February 2013 17:35, Khem Raj <raj.khem@gmail.com> wrote:
> Thinking about it from different perspective, I agree that probably
> adding an extra package is not right and having unitfiles as part of
> package proper is fine. but we should definitely have a check where if
> SYSTEMD_PACKAGES dont exist in PACKAGES then it should error out or
> warn about it.

Yes, throwing an error/warning does make sense, as the alternative
would be files mysteriously disappearing.  Can you send a patch for
that?

Ross
Khem Raj - Feb. 12, 2013, 10:42 p.m.
On Tue, Feb 12, 2013 at 1:06 PM, Burton, Ross <ross.burton@intel.com> wrote:
>
> Yes, throwing an error/warning does make sense, as the alternative
> would be files mysteriously disappearing.  Can you send a patch for
> that?

I have something for test.

Patch

diff --git a/meta/classes/systemd.bbclass b/meta/classes/systemd.bbclass
index 32cc5c2..672f304 100644
--- a/meta/classes/systemd.bbclass
+++ b/meta/classes/systemd.bbclass
@@ -46,6 +46,12 @@  def systemd_populate_packages(d):
             val = (d.getVar(var, True) or "").strip()
         return val
 
+    # prepend systemd-packages not already included
+    def systemd_create_package(pkg_systemd):
+        packages = d.getVar('PACKAGES', True)
+        if not pkg_systemd in packages:
+            d.appendVar('PACKAGES', " " + pkg_systemd)
+
 
     # Add a runtime dependency on systemd to pkg
     def systemd_add_rdepends(pkg):
@@ -144,6 +150,7 @@  def systemd_populate_packages(d):
     # Run all modifications once when creating package
     if os.path.exists(d.getVar("D", True)):
         for pkg in d.getVar('SYSTEMD_PACKAGES', True).split():
+            systemd_create_package(pkg)
             if d.getVar('SYSTEMD_SERVICE_' + pkg, True):
                 systemd_generate_package_scripts(pkg)
                 systemd_add_rdepends(pkg)