Bug 793411

could you fill a bug report on https://bugs.freedesktop,org/ since it is an upstream issue ? Thanks.

(In reply to comment #1)
> could you fill a bug report on https://bugs.freedesktop,org/ since it is an
> upstream issue ? Thanks.

Okay, https://bugs.freedesktop.org/show_bug.cgi?id=58083

Heya, I've sent a patch, which is now upstream

http://cgit.freedesktop.org/systemd/systemd/commit/?id=565d91fdf198b88f7c2d72c67cfc6c30341a3596

@frederic: feel free to backport or close it as fixed in next openSUSE release

(In reply to comment #3)
> Heya, I've sent a patch, which is now upstream
> 
> http://cgit.freedesktop.org/systemd/systemd/commit/?id=565d91fdf198b88f7c2d72c67cfc6c30341a3596
> 
> @frederic: feel free to backport or close it as fixed in next openSUSE release

I'll put it on my TODO list for 12.2 + Factory/12.3

patch went into Factory.

I've put it on my 12.2 update package (systemd-44-10.20.1), should be available in one hour maximum: http://download.opensuse.org/repositories/home:/fcrozat:/branches:/openSUSE:/12.2:/Update/openSUSE_12.2_Update/x86_64/

please test.

note that continuation lines are illegal in sysconfig files. fillup will corrupt the file on next package update.

(In reply to comment #6)
> note that continuation lines are illegal in sysconfig files. fillup will
> corrupt the file on next package update.

not an issue for systemd (which only reads file) but worth noting for people.

(In reply to comment #5)
> patch went into Factory.
> 
> I've put it on my 12.2 update package (systemd-44-10.20.1), should be available
> in one hour maximum:
> http://download.opensuse.org/repositories/home:/fcrozat:/branches:/openSUSE:/12.2:/Update/openSUSE_12.2_Update/x86_64/
> 
> please test.

Hi Frederic

something is really wrong with that - afaict, environment variables set with EnvironmentFile are now just substituted as blank.  Whether or not I've used continuation lines in the file.

Did you test only on 12.2 or also on Factory ?

I might have miscorrectly backported the patch.

(In reply to comment #9)
> Did you test only on 12.2 or also on Factory ?
> I might have miscorrectly backported the patch.

Sofar, I only tested your 12.2 update package.

Patch looks correct to me - @per, what is your sysconfig file? The one you have mentioned in commenct#1? Are you sure there are no blank characters after continuation character? Code works only for '\\\n', everything else is not considered as continuation - it's upon an upstream request.

(In reply to comment #8)

> something is really wrong with that - afaict, environment variables set with
> EnvironmentFile are now just substituted as blank.  Whether or not I've used
> continuation lines in the file.

I'm not seeing such behaviour on my test (on 12.2 and Factory), in single or continuated lines.

Could you share your testcase ?

Because I was seeing variables replaced only as blanks, I trimmed my test to roughly this:

/etc/sysconfig/spamd :

KLOP=99

/etc/systemd/system/spamd.service :

ExecStart=/usr/local/bin/kk $KLOP

/usr/local/bin/kk :

#!/bin/sh
echo $@ >/tmp/list
sleep 30

My system is 12.2 + your systemd-44-10.20.1.i586.rpm.

(truncated "ps axfl" display:)

 0:00 /bin/sh /usr/local/bin/kk
 0:00  \_ sleep 30

The $KLOP is clearly being substituted with blanks.

(In reply to comment #13)
> Because I was seeing variables replaced only as blanks, I trimmed my test to
> roughly this:
> 
> /etc/sysconfig/spamd :
> 
> KLOP=99
> 
> /etc/systemd/system/spamd.service :
> 
> ExecStart=/usr/local/bin/kk $KLOP
> 
> /usr/local/bin/kk :
> 
> #!/bin/sh
> echo $@ >/tmp/list
> sleep 30
> 
> My system is 12.2 + your systemd-44-10.20.1.i586.rpm.
> 
> (truncated "ps axfl" display:)
> 
>  0:00 /bin/sh /usr/local/bin/kk
>  0:00  \_ sleep 30
> 
> The $KLOP is clearly being substituted with blanks.

Because it is incorrect use (check the manpage). You should use:

ExecStart=/usr/local/bin/kk ${KLOP}

an easier way to check the environment is :
strings /proc/<PID>/environ

(In reply to comment #14)
> (In reply to comment #13)
> > Because I was seeing variables replaced only as blanks, I trimmed my test to
> > roughly this:
> > 
> > /etc/sysconfig/spamd :
> > 
> > KLOP=99
> > 
> > /etc/systemd/system/spamd.service :
> > 
> > ExecStart=/usr/local/bin/kk $KLOP
> > 
> > /usr/local/bin/kk :
> > 
> > #!/bin/sh
> > echo $@ >/tmp/list
> > sleep 30
> > 
> > My system is 12.2 + your systemd-44-10.20.1.i586.rpm.
> > 
> > (truncated "ps axfl" display:)
> > 
> >  0:00 /bin/sh /usr/local/bin/kk
> >  0:00  \_ sleep 30
> > 
> > The $KLOP is clearly being substituted with blanks.
> 
> Because it is incorrect use (check the manpage). You should use:
> 
> ExecStart=/usr/local/bin/kk ${KLOP}
> 
> an easier way to check the environment is :
> strings /proc/<PID>/environ

My use without {} works with systemd-44-10.8.1.i586.  Besides, I've also tried using {} and (), same result.  FYI, I see a couple of other service unit in /lib/systemd/system using the variable form without {}.  (sshd, ypbind) 

To test which form was the correct one I used:
ExecStart=/usr/local/bin/kk $KLOP ${KLOP} $(KLOP)

If I check /proc/<PID>/environ, none of the variables from my sysconfig file are set.

(In reply to comment #15)

> My use without {} works with systemd-44-10.8.1.i586.  Besides, I've also tried
> using {} and (), same result.  FYI, I see a couple of other service unit in
> /lib/systemd/system using the variable form without {}.  (sshd, ypbind) 

There is a slight difference between $FOO and ${FOO} as explained on man systemd.service

Anyway, please attach both .service and sysconfig files you are using

Created attachment 522657 [details]
/etc/systemd/system/spamd.service from dupont5

Created attachment 522658 [details]
/etc/sysconfig/spamd from dupont5

removing :
#SPAMD_ARGS="-d --socketpath=/var/lib/bulwark/spamd \
#--nouser-config \
#--virtual-config-dir=/var/lib/bulwark/domains/%l/%d \
#--siteconfigpath=/var/lib/bulwark/config \
#--username=bulwark \
#--max-children=25 \
#--timeout-child=600"

fixed the parsing (or uncommenting it too) on 12.2 (I didn't test Factory yet)

Michal, it looks like the commented lines are processed after multiline (I didn't check the code yet).

same issue on Factory

(In reply to comment #19)
> Michal, it looks like the commented lines are processed after multiline (I
> didn't check the code yet).

Yeah, the comment code is below the continuation logic ... will write a fix tomorrow.

Well, the problem was code did not properly delete the auxiliary buffer, so after first commented multi-line definition, all new lines were added to this one, but as it have started by '#', it was just ignored.

Sent a fix to systemd-devel and Frederic.

I confirm the fix works. I've pushed it to my 12.2 test systemd repository (already published) and sr to Base:System for Factory (150752)

This is an autogenerated message for OBS integration:
This bug (793411) was mentioned in
https://build.opensuse.org/request/show/150761 Factory / systemd

Hi, the last and hopefully complete patch has been sent to systemd-devel. It deals with a comments inside multiline definition and as it comes with a decent test, it'd be now 100% correct.

In short - systemd ignores comments inside definitions, and if it does not ends on \\\n, it is considered as end of a definition. Please note that systemd behaves differently than a shell, because it does not take quoting into account.

On the other hand, the most common case

FOO="multi \
line \
definition"

will be handled exactly like a shell.

http://lists.freedesktop.org/archives/systemd-devel/2013-February/008723.html

This is an autogenerated message for OBS integration:
This bug (793411) was mentioned in
https://build.opensuse.org/request/show/155190 Factory / systemd

I think we can close this bug now.

requesting maintenance update for 12.2

 mr 178807 for 12.2

Update started for openSUSE 12.2. See openSUSE:Maintenance:1775. Thanks for your submission.

Update released for openSUSE 12.2. Resolved fixed.

openSUSE-RU-2013:1055-1: An update that has four recommended fixes can now be installed.

Category: recommended (moderate)
Bug References: 793411,796171,804575,808144
CVE References: 
Sources used:
openSUSE 12.2 (src):    systemd-44-10.15.1, systemd-gtk-44-10.15.1