[Buildroot] [PATCH] package/xbmc: Fix broken variable name

[Buildroot] [PATCH] package/xbmc: Fix broken variable name

[Bernd Kuhls]

BR2_PACKAGE_XBMC_DBUS is provided by package/xbmc/Config.in

Signed-off-by: Bernd Kuhls <bernd.kuhls@t-online.de>
---
 package/xbmc/xbmc.mk |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/package/xbmc/xbmc.mk b/package/xbmc/xbmc.mk
index bdd634e..0bb6724 100644
--- a/package/xbmc/xbmc.mk
+++ b/package/xbmc/xbmc.mk
@@ -64,7 +64,7 @@ XBMC_CONF_ENV += INCLUDES="-I$(STAGING_DIR)/usr/include/interface/vcos/pthreads
 	LIBS="-lvcos -lvchostif"
 endif
 
-ifeq ($(BR2_PACKAGE_DBUS),y)
+ifeq ($(BR2_PACKAGE_XBMC_DBUS),y)
 XBMC_DEPENDENCIES += dbus
 endif
 
-- 
1.7.10.4

[Buildroot] [PATCH] package/xbmc: Fix broken variable name

[Yann E. MORIN]

Bernd, All,

On 2014-08-27 22:41 +0200, Bernd Kuhls spake thusly:
> BR2_PACKAGE_XBMC_DBUS is provided by package/xbmc/Config.in
> 
> Signed-off-by: Bernd Kuhls <bernd.kuhls@t-online.de>
> ---
>  package/xbmc/xbmc.mk |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/package/xbmc/xbmc.mk b/package/xbmc/xbmc.mk
> index bdd634e..0bb6724 100644
> --- a/package/xbmc/xbmc.mk
> +++ b/package/xbmc/xbmc.mk
> @@ -64,7 +64,7 @@ XBMC_CONF_ENV += INCLUDES="-I$(STAGING_DIR)/usr/include/interface/vcos/pthreads
>  	LIBS="-lvcos -lvchostif"
>  endif
>  
> -ifeq ($(BR2_PACKAGE_DBUS),y)
> +ifeq ($(BR2_PACKAGE_XBMC_DBUS),y)
>  XBMC_DEPENDENCIES += dbus
>  endif

No, because the user can still select the package DBUS directly and
since XBMC has no option to disable DBUS, we still want DBUS to be built
before XBMC.

But granted, this is not obvious from the variable name.

Regards,
Yann E. MORIN.

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'

[Buildroot] [PATCH] package/xbmc: Fix broken variable name

[Bernd Kuhls]

"Yann E. MORIN" <yann.morin.1998@free.fr> wrote in 
news:20140827204942.GA21009 at free.fr:

> But granted, this is not obvious from the variable name.

Hi,

ok, understood. I marked my patch as rejected in patchwork.

Regards, Bernd

[Buildroot] [PATCH] package/xbmc: Fix broken variable name

[Peter Korsgaard]

>>>>> "Yann" == Yann E MORIN <yann.morin.1998@free.fr> writes:

 > Bernd, All,
 > On 2014-08-27 22:41 +0200, Bernd Kuhls spake thusly:
 >> BR2_PACKAGE_XBMC_DBUS is provided by package/xbmc/Config.in
 >> 
 >> Signed-off-by: Bernd Kuhls <bernd.kuhls@t-online.de>
 >> ---
 >> package/xbmc/xbmc.mk |    2 +-
 >> 1 file changed, 1 insertion(+), 1 deletion(-)
 >> 
 >> diff --git a/package/xbmc/xbmc.mk b/package/xbmc/xbmc.mk
 >> index bdd634e..0bb6724 100644
 >> --- a/package/xbmc/xbmc.mk
 >> +++ b/package/xbmc/xbmc.mk
 >> @@ -64,7 +64,7 @@ XBMC_CONF_ENV += INCLUDES="-I$(STAGING_DIR)/usr/include/interface/vcos/pthreads
 >> LIBS="-lvcos -lvchostif"
 >> endif
 >> 
 >> -ifeq ($(BR2_PACKAGE_DBUS),y)
 >> +ifeq ($(BR2_PACKAGE_XBMC_DBUS),y)
 >> XBMC_DEPENDENCIES += dbus
 >> endif

 > No, because the user can still select the package DBUS directly and
 > since XBMC has no option to disable DBUS, we still want DBUS to be built
 > before XBMC.

 > But granted, this is not obvious from the variable name.

Then we should imho get rid of the XBMC_DBUS option if that option
doesn't do anything.

-- 
Bye, Peter Korsgaard

[Buildroot] [PATCH] package/xbmc: Fix broken variable name

[Yann E. MORIN]

Peter, All,

On 2014-08-27 23:32 +0200, Peter Korsgaard spake thusly:
> >>>>> "Yann" == Yann E MORIN <yann.morin.1998@free.fr> writes:
> 
>  > Bernd, All,
>  > On 2014-08-27 22:41 +0200, Bernd Kuhls spake thusly:
>  >> BR2_PACKAGE_XBMC_DBUS is provided by package/xbmc/Config.in
>  >> 
>  >> Signed-off-by: Bernd Kuhls <bernd.kuhls@t-online.de>
>  >> ---
>  >> package/xbmc/xbmc.mk |    2 +-
>  >> 1 file changed, 1 insertion(+), 1 deletion(-)
>  >> 
>  >> diff --git a/package/xbmc/xbmc.mk b/package/xbmc/xbmc.mk
>  >> index bdd634e..0bb6724 100644
>  >> --- a/package/xbmc/xbmc.mk
>  >> +++ b/package/xbmc/xbmc.mk
>  >> @@ -64,7 +64,7 @@ XBMC_CONF_ENV += INCLUDES="-I$(STAGING_DIR)/usr/include/interface/vcos/pthreads
>  >> LIBS="-lvcos -lvchostif"
>  >> endif
>  >> 
>  >> -ifeq ($(BR2_PACKAGE_DBUS),y)
>  >> +ifeq ($(BR2_PACKAGE_XBMC_DBUS),y)
>  >> XBMC_DEPENDENCIES += dbus
>  >> endif
> 
>  > No, because the user can still select the package DBUS directly and
>  > since XBMC has no option to disable DBUS, we still want DBUS to be built
>  > before XBMC.
> 
>  > But granted, this is not obvious from the variable name.
> 
> Then we should imho get rid of the XBMC_DBUS option if that option
> doesn't do anything.

That option does select BR2_PACKAGE_DBUS.

All (most of) the XBMC kconfig options were meant so the user could
directly enable XBMC features without hunting down all needed packages.

I know this is unlike most other packages, since our policy is to enable
a package's option if the providing package is enabled.

But for XBMC, the reasoning was that we would not want such a behaviour,
since some dependencies are not trivial (e.g. shairplay, CEC...), so the
kconfig options to XBMC are here to facilitate the user's life.

Regards,
Yann E. MORIN.

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'

[Buildroot] [PATCH] package/xbmc: Fix broken variable name

[Bernd Kuhls]

Hi,

"Yann E. MORIN" <yann.morin.1998@free.fr> wrote in 
news:20140827213842.GB21009 at free.fr:

>> Then we should imho get rid of the XBMC_DBUS option if that option
>> doesn't do anything.
> 
> That option does select BR2_PACKAGE_DBUS.

should BR2_PACKAGE_DBUS in turn not select BR2_PACKAGE_XBMC_DBUS then?
Otherwise users might think DBUS is disabled for xbmc while it is enabled due 
to BR2_PACKAGE_DBUS. Otoh I could try to provide an xbmc/configure option
--en/disable-dbus upstream.

> But for XBMC, the reasoning was that we would not want such a behaviour,
> since some dependencies are not trivial (e.g. shairplay, CEC...), so the
> kconfig options to XBMC are here to facilitate the user's life.

+1, same should be done for alsa and libva.

Regards, Bernd

[Buildroot] [PATCH] package/xbmc: Fix broken variable name

[Peter Korsgaard]

>>>>> "Yann" == Yann E MORIN <yann.morin.1998@free.fr> writes:

Hi,

 >> Then we should imho get rid of the XBMC_DBUS option if that option
 >> doesn't do anything.

 > That option does select BR2_PACKAGE_DBUS.

 > All (most of) the XBMC kconfig options were meant so the user could
 > directly enable XBMC features without hunting down all needed packages.

 > I know this is unlike most other packages, since our policy is to enable
 > a package's option if the providing package is enabled.

 > But for XBMC, the reasoning was that we would not want such a behaviour,
 > since some dependencies are not trivial (e.g. shairplay, CEC...), so the
 > kconfig options to XBMC are here to facilitate the user's life.

Yes, but it is still not really logical that you get dbus support if you
have dbus (for some other reason?) and _deselect_ BR2_PACKAGE_XBMC_DBUS.

I'm not sure what the best solution is.

-- 
Bye, Peter Korsgaard

[Buildroot] [PATCH] package/xbmc: Fix broken variable name

[Peter Korsgaard]

>>>>> "Bernd" == Bernd Kuhls <bernd.kuhls@t-online.de> writes:

 > Hi,
 > "Yann E. MORIN" <yann.morin.1998@free.fr> wrote in 
 > news:20140827213842.GB21009 at free.fr:

 >>> Then we should imho get rid of the XBMC_DBUS option if that option
 >>> doesn't do anything.
 >> 
 >> That option does select BR2_PACKAGE_DBUS.

 > should BR2_PACKAGE_DBUS in turn not select BR2_PACKAGE_XBMC_DBUS then?

No. The dbus package shouldn't know about xbmc (and kconfig won't allow
circular dependencies).

 > Otherwise users might think DBUS is disabled for xbmc while it is enabled due 
 > to BR2_PACKAGE_DBUS. Otoh I could try to provide an xbmc/configure option
 > --en/disable-dbus upstream.

Indeed. I think an explicit configure option would make most sense.

-- 
Bye, Peter Korsgaard

[Buildroot] [PATCH] package/xbmc: Fix broken variable name

[Yann E. MORIN]

Bernd, Peter, All,

On 2014-08-28 07:38 +0200, Bernd Kuhls spake thusly:
> "Yann E. MORIN" <yann.morin.1998@free.fr> wrote in 
> news:20140827213842.GB21009 at free.fr:
> 
> >> Then we should imho get rid of the XBMC_DBUS option if that option
> >> doesn't do anything.
> > 
> > That option does select BR2_PACKAGE_DBUS.
> 
> should BR2_PACKAGE_DBUS in turn not select BR2_PACKAGE_XBMC_DBUS then?
> Otherwise users might think DBUS is disabled for xbmc while it is enabled due 
> to BR2_PACKAGE_DBUS. Otoh I could try to provide an xbmc/configure option
> --en/disable-dbus upstream.

Well, barring an option in XBMC to explicitly enable/disable DBUS, we
can't do much. What we can do is hide the DBU option in XBMC if DBUS is
already selected (but that's a circular dependency, which kconfig does
not like much.)

> > But for XBMC, the reasoning was that we would not want such a behaviour,
> > since some dependencies are not trivial (e.g. shairplay, CEC...), so the
> > kconfig options to XBMC are here to facilitate the user's life.
> 
> +1, same should be done for alsa and libva.

Yes.

Regards,
Yann E. MORIN.

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'