Skip to contentSkip to conversation

package/xbmc: Fix broken variable name

PATCH New Bernd Kuhls 3 participants 9 messages Aug 27, 2014 – Aug 28, 2014

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 package/xbmc: Fix broken variable name

BR2_PACKAGE_XBMC_DBUS is provided by package/xbmc/Config.in

---
 package/xbmc/xbmc.mk |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
package/xbmc/xbmc.mk +1 -1
index bdd634e..0bb6724 100644
@@ -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
 
Signature · 1 line
1.7.10.4
#1 lore raw

PATCH 1/1 package/xbmc: Fix broken variable name

Bernd, All,

On 2014-08-27 22:41 +0200, Bernd Kuhls spake thusly:

Quoted 12 lines

BR2_PACKAGE_XBMC_DBUS is provided by package/xbmc/Config.in

---
 package/xbmc/xbmc.mk |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
package/xbmc/xbmc.mk +0 -0
index bdd634e..0bb6724 100644

package/xbmc/xbmc.mk XBMC_CONF_ENV += INCLUDES="-I$(STAGING_DIR)/usr/include/interface/vcos/pthreads · line 69

Quoted 7 lines
package/xbmc/xbmc.mk +1 -1
@@ -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.

Signature · 6 lines
.-----------------.--------------------.------------------.--------------------.
|  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.  |
'------------------------------^-------^------------------^--------------------'
#2 lore raw

PATCH 1/1 package/xbmc: Fix broken variable name

"Yann E. MORIN" <[email protected]> wrote in news:20140827204942.GA21009 at free.fr:

Quoted 1 line

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

Hi,

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

Regards, Bernd

#3 lore raw

PATCH 1/1 package/xbmc: Fix broken variable name

In reply to Yann E. MORIN
Quoted 28 lines
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line

"Yann" == Yann E MORIN <[email protected]> writes:

Bernd, All,
On 2014-08-27 22:41 +0200, Bernd Kuhls spake thusly:

Quoted 17 lines

BR2_PACKAGE_XBMC_DBUS is provided by package/xbmc/Config.in

---
package/xbmc/xbmc.mk | 2 +-

1 file changed, 1 insertion(+), 1 deletion(-)
package/xbmc/xbmc.mk +0 -0
index bdd634e..0bb6724 100644
@@ -64,7 +64,7 @@ XBMC_CONF_ENV += INCLUDES="-I$(STAGING_DIR)/usr/include/interface/vcos/pthreads

LIBS="-lvcos -lvchostif"
endif

package/xbmc/xbmc.mk +1 -1
-ifeq ($(BR2_PACKAGE_DBUS),y)
+ifeq ($(BR2_PACKAGE_XBMC_DBUS),y)
Quoted 2 lines

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.

Quoted 1 line

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.

Signature · 1 line
Bye, Peter Korsgaard
#4 lore raw

PATCH 1/1 package/xbmc: Fix broken variable name

Peter, All,

On 2014-08-27 23:32 +0200, Peter Korsgaard spake thusly:

Quoted 28 lines
Quoted 27 lines
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line

"Yann" == Yann E MORIN <[email protected]> writes:

Bernd, All,
On 2014-08-27 22:41 +0200, Bernd Kuhls spake thusly:

Quoted 17 lines

BR2_PACKAGE_XBMC_DBUS is provided by package/xbmc/Config.in

---
package/xbmc/xbmc.mk | 2 +-

1 file changed, 1 insertion(+), 1 deletion(-)
package/xbmc/xbmc.mk +0 -0
index bdd634e..0bb6724 100644
@@ -64,7 +64,7 @@ XBMC_CONF_ENV += INCLUDES="-I$(STAGING_DIR)/usr/include/interface/vcos/pthreads

LIBS="-lvcos -lvchostif"
endif

package/xbmc/xbmc.mk +1 -1
-ifeq ($(BR2_PACKAGE_DBUS),y)
+ifeq ($(BR2_PACKAGE_XBMC_DBUS),y)
Quoted 2 lines

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.

Quoted 4 lines
Quoted 1 line

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.

Signature · 6 lines
.-----------------.--------------------.------------------.--------------------.
|  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.  |
'------------------------------^-------^------------------^--------------------'
#5 lore raw

PATCH 1/1 package/xbmc: Fix broken variable name

Hi,

"Yann E. MORIN" <[email protected]> wrote in news:20140827213842.GB21009 at free.fr:

Quoted 4 lines
Quoted 2 lines

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.

Quoted 3 lines

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

#6 lore raw

PATCH 1/1 package/xbmc: Fix broken variable name

Quoted 6 lines
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line

"Bernd" == Bernd Kuhls <[email protected]> writes:

Hi,
"Yann E. MORIN" <[email protected]> wrote in news:20140827213842.GB21009 at free.fr:

Quoted 6 lines
Quoted 4 lines
Quoted 2 lines

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).

Quoted 3 lines

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.

Signature · 1 line
Bye, Peter Korsgaard
#7 lore raw

PATCH 1/1 package/xbmc: Fix broken variable name

In reply to Bernd Kuhls

Bernd, Peter, All,

On 2014-08-28 07:38 +0200, Bernd Kuhls spake thusly:

Quoted 6 lines

"Yann E. MORIN" <[email protected]> wrote in news:20140827213842.GB21009 at free.fr:

Quoted 3 lines
Quoted 2 lines

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

Quoted 6 lines
Quoted 1 line

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.)

Quoted 5 lines
Quoted 3 lines

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.

Signature · 6 lines
.-----------------.--------------------.------------------.--------------------.
|  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.  |
'------------------------------^-------^------------------^--------------------'
#8 lore raw

PATCH 1/1 package/xbmc: Fix broken variable name

In reply to Yann E. MORIN
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line

"Yann" == Yann E MORIN <[email protected]> writes:

Hi,

Quoted 8 lines
Quoted 2 lines

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.

Quoted 6 lines

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.

Signature · 1 line
Bye, Peter Korsgaard