[PATCH] init script: source /etc/sysconfig/sssd

[Pavel Březina]

https://fedorahosted.org/sssd/ticket/1959

Is there anyway how to achieve this in systemd? Do we want the path 
configurable?

[Jakub Hrozek]

On Mon, Jun 24, 2013 at 03:34:19PM +0200, Pavel Březina wrote:
> https://fedorahosted.org/sssd/ticket/1959
>
> Is there anyway how to achieve this in systemd?

Yes, see EnvironmentFile
> Do we want the path
> configurable?

Yes, we do.

[Pavel Březina]

On 06/24/2013 03:37 PM, Jakub Hrozek wrote:
> On Mon, Jun 24, 2013 at 03:34:19PM +0200, Pavel Březina wrote:
> > https://fedorahosted.org/sssd/ticket/1959
> >
> > Is there anyway how to achieve this in systemd?
>
> Yes, see EnvironmentFile
> > Do we want the path
> > configurable?
>
> Yes, we do.

Thanks, new patch is attached.

[Stephen Gallagher]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 06/25/2013 05:40 AM, Pavel Březina wrote:
> On 06/24/2013 03:37 PM, Jakub Hrozek wrote:
> > On Mon, Jun 24, 2013 at 03:34:19PM +0200, Pavel Březina wrote:
> > > https://fedorahosted.org/sssd/ticket/1959
> > >
> > > Is there anyway how to achieve this in systemd?
> >
> > Yes, see EnvironmentFile
> > > Do we want the path configurable?
> >
> > Yes, we do.
>
> Thanks, new patch is attached.

Looks correct to me. Pavel informed me off-list that the "-" in the
unit file indicates "use it if it exists".

Ack.
> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v1.4.13 (GNU/Linux)
> Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/
>
> iEYEARECAAYFAlHJlpYACgkQeiVVYja6o6MWOwCfRF4AQa/HmSjpj2Ri+DvjOJPr
> ltkAnjp7sKCr0ntQf0b4gAg0JInqEM6h
> =I/UO
> -----END PGP SIGNATURE-----

[Jakub Hrozek]

On Tue, Jun 25, 2013 at 09:09:42AM -0400, Stephen Gallagher wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
> On 06/25/2013 05:40 AM, Pavel Březina wrote:
> > On 06/24/2013 03:37 PM, Jakub Hrozek wrote:
> > > On Mon, Jun 24, 2013 at 03:34:19PM +0200, Pavel Březina wrote:
> > > > https://fedorahosted.org/sssd/ticket/1959
> > > >
> > > > Is there anyway how to achieve this in systemd?
> > >
> > > Yes, see EnvironmentFile
> > > > Do we want the path configurable?
> > >
> > > Yes, we do.
> >
> > Thanks, new patch is attached.
>
> Looks correct to me. Pavel informed me off-list that the "-" in the
> unit file indicates "use it if it exists".
>
> Ack.

I tested both sysv and init script (just the default configure value,
though) and they seem to be working fine:

# cat /etc/sysconfig/sssd 
FOO=BAR
# cat /proc/$(pidof sssd)/environ
BOOT_IMAGE=/vmlinuz-3.9.5-301.fc19.x86_64SYSFONT=TrueKEYTABLE=usLANG=en_US.UTF-8PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/binFOO=BAR

[Jakub Hrozek]

On Tue, Jun 25, 2013 at 03:31:03PM +0200, Jakub Hrozek wrote:
> On Tue, Jun 25, 2013 at 09:09:42AM -0400, Stephen Gallagher wrote:
> > -----BEGIN PGP SIGNED MESSAGE-----
> > Hash: SHA1
> >
> > On 06/25/2013 05:40 AM, Pavel Březina wrote:
> > > On 06/24/2013 03:37 PM, Jakub Hrozek wrote:
> > > > On Mon, Jun 24, 2013 at 03:34:19PM +0200, Pavel Březina wrote:
> > > > > https://fedorahosted.org/sssd/ticket/1959
> > > > >
> > > > > Is there anyway how to achieve this in systemd?
> > > >
> > > > Yes, see EnvironmentFile
> > > > > Do we want the path configurable?
> > > >
> > > > Yes, we do.
> > >
> > > Thanks, new patch is attached.
> >
> > Looks correct to me. Pavel informed me off-list that the "-" in the
> > unit file indicates "use it if it exists".
> >
> > Ack.
>
> I tested both sysv and init script (just the default configure value,
> though) and they seem to be working fine:
>
> # cat /etc/sysconfig/sssd 
> FOO=BAR
> # cat /proc/$(pidof sssd)/environ
> BOOT_IMAGE=/vmlinuz-3.9.5-301.fc19.x86_64SYSFONT=TrueKEYTABLE=usLANG=en_US.UTF-8PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/binFOO=BAR

Pushed to master

[Jakub Hrozek]

On Tue, Jun 25, 2013 at 05:07:41PM +0200, Jakub Hrozek wrote:
> On Tue, Jun 25, 2013 at 03:31:03PM +0200, Jakub Hrozek wrote:
> > On Tue, Jun 25, 2013 at 09:09:42AM -0400, Stephen Gallagher wrote:
> > > -----BEGIN PGP SIGNED MESSAGE-----
> > > Hash: SHA1
> > >
> > > On 06/25/2013 05:40 AM, Pavel Březina wrote:
> > > > On 06/24/2013 03:37 PM, Jakub Hrozek wrote:
> > > > > On Mon, Jun 24, 2013 at 03:34:19PM +0200, Pavel Březina wrote:
> > > > > > https://fedorahosted.org/sssd/ticket/1959
> > > > > >
> > > > > > Is there anyway how to achieve this in systemd?
> > > > >
> > > > > Yes, see EnvironmentFile
> > > > > > Do we want the path configurable?
> > > > >
> > > > > Yes, we do.
> > > >
> > > > Thanks, new patch is attached.
> > >
> > > Looks correct to me. Pavel informed me off-list that the "-" in the
> > > unit file indicates "use it if it exists".
> > >
> > > Ack.
> >
> > I tested both sysv and init script (just the default configure value,
> > though) and they seem to be working fine:
> >
> > # cat /etc/sysconfig/sssd 
> > FOO=BAR
> > # cat /proc/$(pidof sssd)/environ
> > BOOT_IMAGE=/vmlinuz-3.9.5-301.fc19.x86_64SYSFONT=TrueKEYTABLE=usLANG=en_US.UTF-8PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/binFOO=BAR
>
> Pushed to master

Hi,

attached is a 1.9 backport for review. Cherry picking the patch didn't
go cleanly, so I'd like to get a quick review before pushing the patch.

[Lukas Slebodnik]

On (08/08/13 17:20), Jakub Hrozek wrote:
> On Tue, Jun 25, 2013 at 05:07:41PM +0200, Jakub Hrozek wrote:
> > On Tue, Jun 25, 2013 at 03:31:03PM +0200, Jakub Hrozek wrote:
> > > On Tue, Jun 25, 2013 at 09:09:42AM -0400, Stephen Gallagher wrote:
> > > > -----BEGIN PGP SIGNED MESSAGE-----
> > > > Hash: SHA1
> > > >
> > > > On 06/25/2013 05:40 AM, Pavel Březina wrote:
> > > > > On 06/24/2013 03:37 PM, Jakub Hrozek wrote:
> > > > > > On Mon, Jun 24, 2013 at 03:34:19PM +0200, Pavel Březina wrote:
> > > > > > > https://fedorahosted.org/sssd/ticket/1959
> > > > > > > 
> > > > > > > Is there anyway how to achieve this in systemd?
> > > > > >
> > > > > > Yes, see EnvironmentFile
> > > > > >
> > > > > > > Do we want the path configurable?
> > > > > >
> > > > > > Yes, we do.
> > > > >
> > > > > Thanks, new patch is attached.
> > > >
> > > > Looks correct to me. Pavel informed me off-list that the "-" in the
> > > > unit file indicates "use it if it exists".
> > > >
> > > > Ack.
> > >
> > > I tested both sysv and init script (just the default configure value,
> > > though) and they seem to be working fine:
> > >
> > > # cat /etc/sysconfig/sssd 
> > > FOO=BAR
> > > # cat /proc/$(pidof sssd)/environ
> > > BOOT_IMAGE=/vmlinuz-3.9.5-301.fc19.x86_64SYSFONT=TrueKEYTABLE=usLANG=en_US.UTF-8PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/binFOO=BAR
> >
> > Pushed to master
>
> Hi,
>
> attached is a 1.9 backport for review. Cherry picking the patch didn't
> go cleanly, so I'd like to get a quick review before pushing the patch.

NACK
> From bc3472a58868057b62edf40f22c5338689eda31c Mon Sep 17 00:00:00 2001
> From: =?UTF-8?q?Pavel=20B=C5=99ezina?= 
> Date: Mon, 24 Jun 2013 15:30:04 +0200
> Subject: [PATCH] init script: source /etc/sysconfig/sssd

... snip ...
> diff --git a/src/sysv/sssd b/src/sysv/sssd
> index ca929e9ec62d179fc35d22b915d5ca965d515edd..e9930dd463ae5edcb8f13c7dc47c8e9cac147259 100644
> --- a/src/sysv/sssd
> +++ b/src/sysv/sssd

^^^^^^^^^^
This file is not listed in file configure.ac in macro AC_CONFIG_FILES
and file doesn't have suffix ".in"
> @@ -31,6 +31,10 @@ prog="sssd"
> # Source function library.
> . /etc/init.d/functions
>
> +if [ -f @environment_file@ ]; then
> . @environment_file@

^^^^^^^^^^^^^^^^^^
Substitution will not be made and bash script will be not able to source file
with name "@environment_file@"
> +fi
>
> SSSD=/usr/sbin/sssd

LS

[Jakub Hrozek]

On Thu, Aug 08, 2013 at 05:52:16PM +0200, Lukas Slebodnik wrote:
> On (08/08/13 17:20), Jakub Hrozek wrote:
> > On Tue, Jun 25, 2013 at 05:07:41PM +0200, Jakub Hrozek wrote:
> > > On Tue, Jun 25, 2013 at 03:31:03PM +0200, Jakub Hrozek wrote:
> > > > On Tue, Jun 25, 2013 at 09:09:42AM -0400, Stephen Gallagher wrote:
> > > > > -----BEGIN PGP SIGNED MESSAGE-----
> > > > > Hash: SHA1
> > > > >
> > > > > On 06/25/2013 05:40 AM, Pavel Březina wrote:
> > > > > > On 06/24/2013 03:37 PM, Jakub Hrozek wrote:
> > > > > > > On Mon, Jun 24, 2013 at 03:34:19PM +0200, Pavel Březina wrote:
> > > > > > >> https://fedorahosted.org/sssd/ticket/1959
> > > > > > >> 
> > > > > > >> Is there anyway how to achieve this in systemd?
> > > > > > > 
> > > > > > > Yes, see EnvironmentFile
> > > > > > > 
> > > > > > >> Do we want the path configurable?
> > > > > > > 
> > > > > > > Yes, we do.
> > > > > >
> > > > > > Thanks, new patch is attached.
> > > > >
> > > > > Looks correct to me. Pavel informed me off-list that the "-" in the
> > > > > unit file indicates "use it if it exists".
> > > > >
> > > > > Ack.
> > > >
> > > > I tested both sysv and init script (just the default configure value,
> > > > though) and they seem to be working fine:
> > > >
> > > > # cat /etc/sysconfig/sssd 
> > > > FOO=BAR
> > > > # cat /proc/$(pidof sssd)/environ
> > > > BOOT_IMAGE=/vmlinuz-3.9.5-301.fc19.x86_64SYSFONT=TrueKEYTABLE=usLANG=en_US.UTF-8PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/binFOO=BAR
> > >
> > > Pushed to master
> >
> > Hi,
> >
> > attached is a 1.9 backport for review. Cherry picking the patch didn't
> > go cleanly, so I'd like to get a quick review before pushing the patch.
>
> NACK
> > From bc3472a58868057b62edf40f22c5338689eda31c Mon Sep 17 00:00:00 2001
> > From: =?UTF-8?q?Pavel=20B=C5=99ezina?= 
> > Date: Mon, 24 Jun 2013 15:30:04 +0200
> > Subject: [PATCH] init script: source /etc/sysconfig/sssd
>
> ... snip ...
> > diff --git a/src/sysv/sssd b/src/sysv/sssd
> > index ca929e9ec62d179fc35d22b915d5ca965d515edd..e9930dd463ae5edcb8f13c7dc47c8e9cac147259 100644
> > --- a/src/sysv/sssd
> > +++ b/src/sysv/sssd
>       ^^^^^^^^^^
>
> This file is not listed in file configure.ac in macro AC_CONFIG_FILES
> and file doesn't have suffix ".in"
> > @@ -31,6 +31,10 @@ prog="sssd"
> > # Source function library.
> > . /etc/init.d/functions
> >
> > +if [ -f @environment_file@ ]; then
> > . @environment_file@
>     ^^^^^^^^^^^^^^^^^^
>
> Substitution will not be made and bash script will be not able to source file
> with name "@environment_file@"
> > +fi
> >
> > SSSD=/usr/sbin/sssd
>
> LS

You know, this is why I send even trivial backports for review. Thank
you for the catch, now both patches are attached.

[Lukas Slebodnik]

On (08/08/13 22:06), Jakub Hrozek wrote:
> On Thu, Aug 08, 2013 at 05:52:16PM +0200, Lukas Slebodnik wrote:
> > On (08/08/13 17:20), Jakub Hrozek wrote:
> > > On Tue, Jun 25, 2013 at 05:07:41PM +0200, Jakub Hrozek wrote:
> > > > On Tue, Jun 25, 2013 at 03:31:03PM +0200, Jakub Hrozek wrote:
> > > > > On Tue, Jun 25, 2013 at 09:09:42AM -0400, Stephen Gallagher wrote:
> > > > > > -----BEGIN PGP SIGNED MESSAGE-----
> > > > > > Hash: SHA1
> > > > > >
> > > > > > On 06/25/2013 05:40 AM, Pavel Březina wrote:
> > > > > > > On 06/24/2013 03:37 PM, Jakub Hrozek wrote:
> > > > > > >> On Mon, Jun 24, 2013 at 03:34:19PM +0200, Pavel Březina wrote:
> > > > > > >>> https://fedorahosted.org/sssd/ticket/1959
> > > > > > >>> 
> > > > > > >>> Is there anyway how to achieve this in systemd?
> > > > > > >> 
> > > > > > >> Yes, see EnvironmentFile
> > > > > > >> 
> > > > > > >>> Do we want the path configurable?
> > > > > > >> 
> > > > > > >> Yes, we do.
> > > > > > > 
> > > > > > > Thanks, new patch is attached.
> > > > > > >
> > > > > >
> > > > > > Looks correct to me. Pavel informed me off-list that the "-" in the
> > > > > > unit file indicates "use it if it exists".
> > > > > >
> > > > > > Ack.
> > > > >
> > > > > I tested both sysv and init script (just the default configure value,
> > > > > though) and they seem to be working fine:
> > > > >
> > > > > # cat /etc/sysconfig/sssd 
> > > > > FOO=BAR
> > > > > # cat /proc/$(pidof sssd)/environ
> > > > > BOOT_IMAGE=/vmlinuz-3.9.5-301.fc19.x86_64SYSFONT=TrueKEYTABLE=usLANG=en_US.UTF-8PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/binFOO=BAR
> > > >
> > > > Pushed to master
> > >
> > > Hi,
> > >
> > > attached is a 1.9 backport for review. Cherry picking the patch didn't
> > > go cleanly, so I'd like to get a quick review before pushing the patch.
> >
> > NACK
> > > From bc3472a58868057b62edf40f22c5338689eda31c Mon Sep 17 00:00:00 2001
> > > From: =?UTF-8?q?Pavel=20B=C5=99ezina?= 
> > > Date: Mon, 24 Jun 2013 15:30:04 +0200
> > > Subject: [PATCH] init script: source /etc/sysconfig/sssd
> >
> > ... snip ...
> > > diff --git a/src/sysv/sssd b/src/sysv/sssd
> > > index ca929e9ec62d179fc35d22b915d5ca965d515edd..e9930dd463ae5edcb8f13c7dc47c8e9cac147259 100644
> > > --- a/src/sysv/sssd
> > > +++ b/src/sysv/sssd
> >       ^^^^^^^^^^
> >
> > This file is not listed in file configure.ac in macro AC_CONFIG_FILES
> > and file doesn't have suffix ".in"
> > > @@ -31,6 +31,10 @@ prog="sssd"
> > > # Source function library.
> > > . /etc/init.d/functions
> > >
> > > +if [ -f @environment_file@ ]; then
> > > . @environment_file@
> >     ^^^^^^^^^^^^^^^^^^
> >
> > Substitution will not be made and bash script will be not able to source file
> > with name "@environment_file@"
> > > +fi
> > >
> > > SSSD=/usr/sbin/sssd
> >
> > LS
>
> You know, this is why I send even trivial backports for review. Thank
> you for the catch, now both patches are attached.

I tested fedora19 build and RHEL6 build.
Everything is OK.

ACK to both patches.

LS

[Jakub Hrozek]

On Thu, Aug 08, 2013 at 10:45:29PM +0200, Lukas Slebodnik wrote:
> On (08/08/13 22:06), Jakub Hrozek wrote:
> > On Thu, Aug 08, 2013 at 05:52:16PM +0200, Lukas Slebodnik wrote:
> > > On (08/08/13 17:20), Jakub Hrozek wrote:
> > > > On Tue, Jun 25, 2013 at 05:07:41PM +0200, Jakub Hrozek wrote:
> > > > > On Tue, Jun 25, 2013 at 03:31:03PM +0200, Jakub Hrozek wrote:
> > > > > > On Tue, Jun 25, 2013 at 09:09:42AM -0400, Stephen Gallagher wrote:
> > > > > > > -----BEGIN PGP SIGNED MESSAGE-----
> > > > > > > Hash: SHA1
> > > > > > > 
> > > > > > > On 06/25/2013 05:40 AM, Pavel Březina wrote:
> > > > > > > > On 06/24/2013 03:37 PM, Jakub Hrozek wrote:
> > > > > > > >> On Mon, Jun 24, 2013 at 03:34:19PM +0200, Pavel Březina wrote:
> > > > > > > >>> https://fedorahosted.org/sssd/ticket/1959
> > > > > > > >>> 
> > > > > > > >>> Is there anyway how to achieve this in systemd?
> > > > > > > >> 
> > > > > > > >> Yes, see EnvironmentFile
> > > > > > > >> 
> > > > > > > >>> Do we want the path configurable?
> > > > > > > >> 
> > > > > > > >> Yes, we do.
> > > > > > > > 
> > > > > > > > Thanks, new patch is attached.
> > > > > > > > 
> > > > > > > 
> > > > > > > Looks correct to me. Pavel informed me off-list that the "-" in the
> > > > > > > unit file indicates "use it if it exists".
> > > > > > > 
> > > > > > > Ack.
> > > > > >
> > > > > > I tested both sysv and init script (just the default configure value,
> > > > > > though) and they seem to be working fine:
> > > > > >
> > > > > > # cat /etc/sysconfig/sssd 
> > > > > > FOO=BAR
> > > > > > # cat /proc/$(pidof sssd)/environ
> > > > > > BOOT_IMAGE=/vmlinuz-3.9.5-301.fc19.x86_64SYSFONT=TrueKEYTABLE=usLANG=en_US.UTF-8PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/binFOO=BAR
> > > > >
> > > > > Pushed to master
> > > >
> > > > Hi,
> > > >
> > > > attached is a 1.9 backport for review. Cherry picking the patch didn't
> > > > go cleanly, so I'd like to get a quick review before pushing the patch.
> > >
> > > NACK
> > > > From bc3472a58868057b62edf40f22c5338689eda31c Mon Sep 17 00:00:00 2001
> > > > From: =?UTF-8?q?Pavel=20B=C5=99ezina?= 
> > > > Date: Mon, 24 Jun 2013 15:30:04 +0200
> > > > Subject: [PATCH] init script: source /etc/sysconfig/sssd
> > >
> > > ... snip ...
> > > > diff --git a/src/sysv/sssd b/src/sysv/sssd
> > > > index ca929e9ec62d179fc35d22b915d5ca965d515edd..e9930dd463ae5edcb8f13c7dc47c8e9cac147259 100644
> > > > --- a/src/sysv/sssd
> > > > +++ b/src/sysv/sssd
> > >       ^^^^^^^^^^
> > >
> > > This file is not listed in file configure.ac in macro AC_CONFIG_FILES
> > > and file doesn't have suffix ".in"
> > > > @@ -31,6 +31,10 @@ prog="sssd"
> > > > # Source function library.
> > > > . /etc/init.d/functions
> > > >
> > > > +if [ -f @environment_file@ ]; then
> > > > . @environment_file@
> > >     ^^^^^^^^^^^^^^^^^^
> > >
> > > Substitution will not be made and bash script will be not able to source file
> > > with name "@environment_file@"
> > > > +fi
> > > >
> > > > SSSD=/usr/sbin/sssd
> > >
> > > LS
> >
> > You know, this is why I send even trivial backports for review. Thank
> > you for the catch, now both patches are attached.
>
> I tested fedora19 build and RHEL6 build.
> Everything is OK.
>
> ACK to both patches.
>
> LS

Pushed both to sssd-1-9