diff mbox

package/libgles: postpone the check for a missing GLES provider

Message ID 1386702439-10093-2-git-send-email-yann.morin.1998@free.fr
State Superseded
Headers show

Commit Message

Yann E. MORIN Dec. 10, 2013, 7:07 p.m. UTC 
From: "Yann E. MORIN" <yann.morin.1998@free.fr>

Because some GLES providers may be in BR2_EXTERNAL, $(LIBGLES_DEPENDENCIES)
might be empty hwen we test it.

So, we can't rely on it to define LIBGLES_CONFIGURE_CMDS, and we must
postpone the check until later, ie. at runtime.

Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
---
 package/opengl/libgles/libgles.mk | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

Comments

Arnout Vandecappelle Dec. 11, 2013, 10:46 a.m. UTC | #1 
On 10/12/13 20:07, Yann E. MORIN wrote:
> From: "Yann E. MORIN" <yann.morin.1998@free.fr>
>
> Because some GLES providers may be in BR2_EXTERNAL, $(LIBGLES_DEPENDENCIES)
> might be empty hwen we test it.
>
> So, we can't rely on it to define LIBGLES_CONFIGURE_CMDS, and we must
> postpone the check until later, ie. at runtime.
>
> Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
> ---
>   package/opengl/libgles/libgles.mk | 13 +++++++++----
>   1 file changed, 9 insertions(+), 4 deletions(-)
>
> diff --git a/package/opengl/libgles/libgles.mk b/package/opengl/libgles/libgles.mk
> index ec157ac..c2e1acf 100644
> --- a/package/opengl/libgles/libgles.mk
> +++ b/package/opengl/libgles/libgles.mk
> @@ -22,11 +22,16 @@ ifeq ($(BR2_PACKAGE_GPU_VIV_BIN_MX6Q),y)
>   LIBGLES_DEPENDENCIES += gpu-viv-bin-mx6q
>   endif
>
> -ifeq ($(LIBGLES_DEPENDENCIES),)
> +# Because some GLES providers may be in BR2_EXTERNAL,
> +# $(LIBGLES_DEPENDENCIES) might be empty right here.
> +# So, we can't rely on it to define LIBGLES_CONFIGURE_CMDS
> +# right now, and we must postpone the check until later,
> +# ie. at runtime.
>   define LIBGLES_CONFIGURE_CMDS
> -	echo "No libGLES implementation selected. Configuration error."
> -	exit 1
> +	if [ -z "$${LIBGLES_DEPENDENCIES}" ]; then \

  This should be "$(LIBGLES_DEPENDENCIES)", but otherwise it looks like 
it would work.

  Regards,
  Arnout

> +		echo "No libGLES implementation selected. Configuration error."; \
> +		exit 1; \
> +	fi
>   endef
> -endif
>
>   $(eval $(generic-package))
>
Yann E. MORIN Dec. 11, 2013, 12:25 p.m. UTC | #2 
Arnout, All,

On Wednesday 11 December 2013 11:46:59 Arnout Vandecappelle wrote:
> On 10/12/13 20:07, Yann E. MORIN wrote:
> > From: "Yann E. MORIN" <yann.morin.1998@free.fr>
> >
> > Because some GLES providers may be in BR2_EXTERNAL, $(LIBGLES_DEPENDENCIES)
> > might be empty hwen we test it.
> >
> > So, we can't rely on it to define LIBGLES_CONFIGURE_CMDS, and we must
> > postpone the check until later, ie. at runtime.
> >
> > Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
> > ---
> >   package/opengl/libgles/libgles.mk | 13 +++++++++----
> >   1 file changed, 9 insertions(+), 4 deletions(-)
> >
> > diff --git a/package/opengl/libgles/libgles.mk b/package/opengl/libgles/libgles.mk
> > index ec157ac..c2e1acf 100644
> > --- a/package/opengl/libgles/libgles.mk
> > +++ b/package/opengl/libgles/libgles.mk
> > @@ -22,11 +22,16 @@ ifeq ($(BR2_PACKAGE_GPU_VIV_BIN_MX6Q),y)
> >   LIBGLES_DEPENDENCIES += gpu-viv-bin-mx6q
> >   endif
> >
> > -ifeq ($(LIBGLES_DEPENDENCIES),)
> > +# Because some GLES providers may be in BR2_EXTERNAL,
> > +# $(LIBGLES_DEPENDENCIES) might be empty right here.
> > +# So, we can't rely on it to define LIBGLES_CONFIGURE_CMDS
> > +# right now, and we must postpone the check until later,
> > +# ie. at runtime.
> >   define LIBGLES_CONFIGURE_CMDS
> > -	echo "No libGLES implementation selected. Configuration error."
> > -	exit 1
> > +	if [ -z "$${LIBGLES_DEPENDENCIES}" ]; then \
> 
>   This should be "$(LIBGLES_DEPENDENCIES)", but otherwise it looks like 
> it would work.

I was afraid using a make variable here would be expanded too early
again, that's why I postponed its expansion into the shell command
itself.

But it does not work either, since the variable is not exported.

I'm waiting for feedback from David to confirm either or both
solutions work...

Regards,
Yann E. MORIN.
David Corvoysier Dec. 11, 2013, 1:03 p.m. UTC | #3 
Guys,

The first solution did not work (as yann pointed out, the variable is 
not exported), but the second does.
Who's in for a patch (me ?)

David


Le 11/12/2013 13:25, Yann E. MORIN a écrit :
> Arnout, All,
>
> On Wednesday 11 December 2013 11:46:59 Arnout Vandecappelle wrote:
>> On 10/12/13 20:07, Yann E. MORIN wrote:
>>> From: "Yann E. MORIN" <yann.morin.1998@free.fr>
>>>
>>> Because some GLES providers may be in BR2_EXTERNAL, $(LIBGLES_DEPENDENCIES)
>>> might be empty hwen we test it.
>>>
>>> So, we can't rely on it to define LIBGLES_CONFIGURE_CMDS, and we must
>>> postpone the check until later, ie. at runtime.
>>>
>>> Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
>>> ---
>>>    package/opengl/libgles/libgles.mk | 13 +++++++++----
>>>    1 file changed, 9 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/package/opengl/libgles/libgles.mk b/package/opengl/libgles/libgles.mk
>>> index ec157ac..c2e1acf 100644
>>> --- a/package/opengl/libgles/libgles.mk
>>> +++ b/package/opengl/libgles/libgles.mk
>>> @@ -22,11 +22,16 @@ ifeq ($(BR2_PACKAGE_GPU_VIV_BIN_MX6Q),y)
>>>    LIBGLES_DEPENDENCIES += gpu-viv-bin-mx6q
>>>    endif
>>>
>>> -ifeq ($(LIBGLES_DEPENDENCIES),)
>>> +# Because some GLES providers may be in BR2_EXTERNAL,
>>> +# $(LIBGLES_DEPENDENCIES) might be empty right here.
>>> +# So, we can't rely on it to define LIBGLES_CONFIGURE_CMDS
>>> +# right now, and we must postpone the check until later,
>>> +# ie. at runtime.
>>>    define LIBGLES_CONFIGURE_CMDS
>>> -	echo "No libGLES implementation selected. Configuration error."
>>> -	exit 1
>>> +	if [ -z "$${LIBGLES_DEPENDENCIES}" ]; then \
>>    This should be "$(LIBGLES_DEPENDENCIES)", but otherwise it looks like
>> it would work.
> I was afraid using a make variable here would be expanded too early
> again, that's why I postponed its expansion into the shell command
> itself.
>
> But it does not work either, since the variable is not exported.
>
> I'm waiting for feedback from David to confirm either or both
> solutions work...
>
> Regards,
> Yann E. MORIN.
>
David Corvoysier Dec. 11, 2013, 2:05 p.m. UTC | #4 
Sorry, I did another try from a clean environment and the second 
solution didn't work either: LIBGLES_DEPENDENCIES has the right 
(modified) value when it the LIBGLES_CONFIGURE_CMDS is evaluated, but 
not when the dependencies themselves are evaluated (I think it happens 
in pkg-generic.mk).

So, the build doesn't fail on the LIBGLES_CONFIGURE_CMDS test, but if 
the xx_userland package has not been installed beforehand, it is not 
installed automatically when libgles is built.

David

Le 11/12/2013 14:03, David Corvoysier a écrit :
> Guys,
>
> The first solution did not work (as yann pointed out, the variable is 
> not exported), but the second does.
> Who's in for a patch (me ?)
>
> David
>
Arnout Vandecappelle Dec. 12, 2013, 10 p.m. UTC | #5 
On 11/12/13 15:05, David Corvoysier wrote:
> Sorry, I did another try from a clean environment and the second solution
> didn't work either: LIBGLES_DEPENDENCIES has the right (modified) value
> when it the LIBGLES_CONFIGURE_CMDS is evaluated, but not when the
> dependencies themselves are evaluated (I think it happens in
> pkg-generic.mk).

  Yes that's right. In this case, there's no workaround AFAICS...


  Thomas, do you remember what was the reason to include external.mk 
after package/*/*.mk rather than before? If there is no specific reason 
for it, we could move it.

  I also feel that the way the opengl stuff is handled now is a bit 
hacky, but I don't have any better ideas.


  Regards,
  Arnout


> So, the build doesn't fail on the LIBGLES_CONFIGURE_CMDS test, but if the
> xx_userland package has not been installed beforehand, it is not
> installed automatically when libgles is built.
>
> David
>
> Le 11/12/2013 14:03, David Corvoysier a écrit :
>> Guys,
>>
>> The first solution did not work (as yann pointed out, the variable is
>> not exported), but the second does.
>> Who's in for a patch (me ?)
>>
>> David
>>
> _______________________________________________
> buildroot mailing list
> buildroot@busybox.net
> http://lists.busybox.net/mailman/listinfo/buildroot
>
>
Thomas Petazzoni Dec. 12, 2013, 10:13 p.m. UTC | #6 
Dear Arnout Vandecappelle,

On Thu, 12 Dec 2013 23:00:18 +0100, Arnout Vandecappelle wrote:

>   Thomas, do you remember what was the reason to include external.mk 
> after package/*/*.mk rather than before? If there is no specific reason 
> for it, we could move it.

No, I don't see a particular reason. It was merely based on the idea
that BR2_EXTERNAL "adds" more packages to the set of packages provided
by Buildroot, so it felt logical to include the BR2_EXTERNAL stuff
*after* the BR packages. But I don't see anything that prevents the
BR2_EXTERNAL stuff from being moved before the Buildroot packages
inclusions. And Yann has already submitted a patch that does this.

>   I also feel that the way the opengl stuff is handled now is a bit 
> hacky, but I don't have any better ideas.

Huh? Can you be more specific in what you find hacky? The way the
OpenGL stuff is handled today is kind of becoming the norm to handle
virtual packages, so it'd be good to understand what you think is hacky
in this implementation.

Thanks!

Thomas
Arnout Vandecappelle Dec. 12, 2013, 11:08 p.m. UTC | #7 
On 12/12/13 23:13, Thomas Petazzoni wrote:
>> >   I also feel that the way the opengl stuff is handled now is a bit
>> >hacky, but I don't have any better ideas.
> Huh? Can you be more specific in what you find hacky? The way the
> OpenGL stuff is handled today is kind of becoming the norm to handle
> virtual packages, so it'd be good to understand what you think is hacky
> in this implementation.

  It's hacky because you have double binding between opengl/libgles and 
the gles suppliers: opengl/libgles/libgles.mk enumerates all possible 
suppliers, and the suppliers select BR2_PACKAGE_HAS_OPENGL_ES.


  Regards,
  Arnout
Thomas Petazzoni Dec. 17, 2013, 6:11 a.m. UTC | #8 
Dear Arnout Vandecappelle,

On Fri, 13 Dec 2013 00:08:46 +0100, Arnout Vandecappelle wrote:

> > Huh? Can you be more specific in what you find hacky? The way the
> > OpenGL stuff is handled today is kind of becoming the norm to handle
> > virtual packages, so it'd be good to understand what you think is
> > hacky in this implementation.
> 
>   It's hacky because you have double binding between opengl/libgles
> and the gles suppliers: opengl/libgles/libgles.mk enumerates all
> possible suppliers, and the suppliers select
> BR2_PACKAGE_HAS_OPENGL_ES.

Ok, I understand. Even though I don't think it's really a major issue, I
believe there are two possible directions to fix this:

 1. Since the .mk part is centralized in opengl/libgles, but the
    Config.in is not (spread in each OpenGL implementation doing the
    select BR2_PACKAGE_HAS_OPENGL_ES), we can centralize the Config.in
    logic by removing the "select BR2_PACKAGE_HAS_OPENGL_ES" in each
    OpenGL implementation, and define BR2_PACKAGE_HAS_OPENGL_EL as
    something like:

config BR2_PACKAGE_HAS_OPENGL_ES
	bool
	default y if BR2_PACKAGE_RPI_FIRMWARE
	default y if BR2_PACKAGE_THIS_OTHER_OPENGL_IMPLEMENTATION
	default y if BR2_PACKAGE_...

 2. Or, we can take the opposite route by pushing the currently
    centralized libgles.mk logic that adds each OpenGL implementation
    in LIBGLES_DEPENDENCIES down into each OpenGL implementation .mk
    file. But that requires a late evaluation of $(generic-package), so
    that all OpenGL implementations can be registered in
    LIBGLES_DEPENDENCIES before the generic-package macro of libgles.mk
    is evaluated. This would require something like Yann's patch.

Best regards,

Thomas
Yann E. MORIN Dec. 17, 2013, 7:58 a.m. UTC | #9 
Thomas, All,

On Tuesday 17 December 2013 07:11:00 Thomas Petazzoni wrote:
> On Fri, 13 Dec 2013 00:08:46 +0100, Arnout Vandecappelle wrote:
> > > Huh? Can you be more specific in what you find hacky? The way the
> > > OpenGL stuff is handled today is kind of becoming the norm to handle
> > > virtual packages, so it'd be good to understand what you think is
> > > hacky in this implementation.
> > 
> >   It's hacky because you have double binding between opengl/libgles
> > and the gles suppliers: opengl/libgles/libgles.mk enumerates all
> > possible suppliers, and the suppliers select
> > BR2_PACKAGE_HAS_OPENGL_ES.
> 
> Ok, I understand. Even though I don't think it's really a major issue, I
> believe there are two possible directions to fix this:
> 
>  1. Since the .mk part is centralized in opengl/libgles, but the
>     Config.in is not (spread in each OpenGL implementation doing the
>     select BR2_PACKAGE_HAS_OPENGL_ES), we can centralize the Config.in
>     logic by removing the "select BR2_PACKAGE_HAS_OPENGL_ES" in each
>     OpenGL implementation, and define BR2_PACKAGE_HAS_OPENGL_EL as
>     something like:
> 
> config BR2_PACKAGE_HAS_OPENGL_ES
> 	bool
> 	default y if BR2_PACKAGE_RPI_FIRMWARE
> 	default y if BR2_PACKAGE_THIS_OTHER_OPENGL_IMPLEMENTATION
> 	default y if BR2_PACKAGE_...

With this first proposal, it becomes a bit more complex to
implement providers in BR2_EXTERNAL.

>  2. Or, we can take the opposite route by pushing the currently
>     centralized libgles.mk logic that adds each OpenGL implementation
>     in LIBGLES_DEPENDENCIES down into each OpenGL implementation .mk
>     file. But that requires a late evaluation of $(generic-package), so
>     that all OpenGL implementations can be registered in
>     LIBGLES_DEPENDENCIES before the generic-package macro of libgles.mk
>     is evaluated. This would require something like Yann's patch.

Needless to say I would highly prefer this second solution.

Regards,
Yann E. MORIN.
Thomas Petazzoni Dec. 17, 2013, 9:04 a.m. UTC | #10 
Dear Yann E. MORIN,

On Tue, 17 Dec 2013 08:58:13 +0100, Yann E. MORIN wrote:

> >  1. Since the .mk part is centralized in opengl/libgles, but the
> >     Config.in is not (spread in each OpenGL implementation doing the
> >     select BR2_PACKAGE_HAS_OPENGL_ES), we can centralize the
> > Config.in logic by removing the "select BR2_PACKAGE_HAS_OPENGL_ES"
> > in each OpenGL implementation, and define BR2_PACKAGE_HAS_OPENGL_EL
> > as something like:
> > 
> > config BR2_PACKAGE_HAS_OPENGL_ES
> > 	bool
> > 	default y if BR2_PACKAGE_RPI_FIRMWARE
> > 	default y if BR2_PACKAGE_THIS_OTHER_OPENGL_IMPLEMENTATION
> > 	default y if BR2_PACKAGE_...
> 
> With this first proposal, it becomes a bit more complex to
> implement providers in BR2_EXTERNAL.

Ah, true.

> >  2. Or, we can take the opposite route by pushing the currently
> >     centralized libgles.mk logic that adds each OpenGL
> > implementation in LIBGLES_DEPENDENCIES down into each OpenGL
> > implementation .mk file. But that requires a late evaluation of
> > $(generic-package), so that all OpenGL implementations can be
> > registered in LIBGLES_DEPENDENCIES before the generic-package macro
> > of libgles.mk is evaluated. This would require something like
> > Yann's patch.
> 
> Needless to say I would highly prefer this second solution.

Right. In principle, I have nothing against this solution. It's just
that I am not sure to fully grasp the consequences of the change you're
proposing. I'm a bit worried about "weird" consequences that we may not
be thinking of at this time. But maybe we should simply apply the
patch, and see if it causes problems for some specific use cases.

Best regards,

Thomas
Yann E. MORIN Dec. 17, 2013, 10:07 p.m. UTC | #11 
Thomas, All,

On 2013-12-17 10:04 +0100, Thomas Petazzoni spake thusly:
> On Tue, 17 Dec 2013 08:58:13 +0100, Yann E. MORIN wrote:
[--SNIP--]
> > >  2. Or, we can take the opposite route by pushing the currently
> > >     centralized libgles.mk logic that adds each OpenGL
> > > implementation in LIBGLES_DEPENDENCIES down into each OpenGL
> > > implementation .mk file. But that requires a late evaluation of
> > > $(generic-package), so that all OpenGL implementations can be
> > > registered in LIBGLES_DEPENDENCIES before the generic-package macro
> > > of libgles.mk is evaluated. This would require something like
> > > Yann's patch.
> > 
> > Needless to say I would highly prefer this second solution.
> 
> Right. In principle, I have nothing against this solution. It's just
> that I am not sure to fully grasp the consequences of the change you're
> proposing. I'm a bit worried about "weird" consequences that we may not
> be thinking of at this time. But maybe we should simply apply the
> patch, and see if it causes problems for some specific use cases.

OK, so we should apply it soon, since the holidays season is approaching
pretty fast now, and it would be nive to be able to react quickly in
case anything goes south...

Peter, care to have a look at it and comment (or apply it), please? ;-)
    http://patchwork.ozlabs.org/patch/301874/

Regards,
Yann E. MORIN.
Arnout Vandecappelle Dec. 17, 2013, 10:20 p.m. UTC | #12 
On 17/12/13 10:04, Thomas Petazzoni wrote:
> Dear Yann E. MORIN,
>
> On Tue, 17 Dec 2013 08:58:13 +0100, Yann E. MORIN wrote:
>
>>>   1. Since the .mk part is centralized in opengl/libgles, but the
>>>      Config.in is not (spread in each OpenGL implementation doing the
>>>      select BR2_PACKAGE_HAS_OPENGL_ES), we can centralize the
>>> Config.in logic by removing the "select BR2_PACKAGE_HAS_OPENGL_ES"
>>> in each OpenGL implementation, and define BR2_PACKAGE_HAS_OPENGL_EL
>>> as something like:
>>>
>>> config BR2_PACKAGE_HAS_OPENGL_ES
>>> 	bool
>>> 	default y if BR2_PACKAGE_RPI_FIRMWARE
>>> 	default y if BR2_PACKAGE_THIS_OTHER_OPENGL_IMPLEMENTATION
>>> 	default y if BR2_PACKAGE_...
>>
>> With this first proposal, it becomes a bit more complex to
>> implement providers in BR2_EXTERNAL.
>
> Ah, true.

  Also it feels inconvenient to me that the virtual package should "know" 
about all its providers.


>
>>>   2. Or, we can take the opposite route by pushing the currently
>>>      centralized libgles.mk logic that adds each OpenGL
>>> implementation in LIBGLES_DEPENDENCIES down into each OpenGL
>>> implementation .mk file. But that requires a late evaluation of
>>> $(generic-package), so that all OpenGL implementations can be
>>> registered in LIBGLES_DEPENDENCIES before the generic-package macro
>>> of libgles.mk is evaluated. This would require something like
>>> Yann's patch.
>>
>> Needless to say I would highly prefer this second solution.
>
> Right. In principle, I have nothing against this solution. It's just
> that I am not sure to fully grasp the consequences of the change you're
> proposing. I'm a bit worried about "weird" consequences that we may not
> be thinking of at this time. But maybe we should simply apply the
> patch, and see if it causes problems for some specific use cases.

  I'm also a bit afraid of the consequences. It also makes make 
processing, which is already difficult to understand, even more obfuscated.


  Here's a wild idea...

In rpi-userland/Config.in:

if BR2_PACKAGE_RPI_USERLAND
config BR2_PACKAGE_LIBEGL_PROVIDER
	string
	default "rpi-userland"
endif


In opengl/libegl/libegl.mk:

LIBEGL_DEPENDENCIES = $(call qstrip,$(BR2PACKAGE_LIBEGL_PROVIDER))


  It's still hackish of course, because:

- rpi-userland/Config.in defines a symbol "belonging" to the libegl package;

- only one provider can be defined, Kconfig will scream if it's defined 
twice;

- it may not work at all :-).


  Regards,
  Arnout
Yann E. MORIN Dec. 17, 2013, 10:35 p.m. UTC | #13 
Arnout, All,

On 2013-12-17 23:20 +0100, Arnout Vandecappelle spake thusly:
> On 17/12/13 10:04, Thomas Petazzoni wrote:
> >Dear Yann E. MORIN,
> >
> >On Tue, 17 Dec 2013 08:58:13 +0100, Yann E. MORIN wrote:
> >
> >>>  1. Since the .mk part is centralized in opengl/libgles, but the
> >>>     Config.in is not (spread in each OpenGL implementation doing the
> >>>     select BR2_PACKAGE_HAS_OPENGL_ES), we can centralize the
> >>>Config.in logic by removing the "select BR2_PACKAGE_HAS_OPENGL_ES"
> >>>in each OpenGL implementation, and define BR2_PACKAGE_HAS_OPENGL_EL
> >>>as something like:
> >>>
> >>>config BR2_PACKAGE_HAS_OPENGL_ES
> >>>	bool
> >>>	default y if BR2_PACKAGE_RPI_FIRMWARE
> >>>	default y if BR2_PACKAGE_THIS_OTHER_OPENGL_IMPLEMENTATION
> >>>	default y if BR2_PACKAGE_...
> >>
> >>With this first proposal, it becomes a bit more complex to
> >>implement providers in BR2_EXTERNAL.
> >
> >Ah, true.
> 
>  Also it feels inconvenient to me that the virtual package should "know"
> about all its providers.

Agreed.

> >>>  2. Or, we can take the opposite route by pushing the currently
> >>>     centralized libgles.mk logic that adds each OpenGL
> >>>implementation in LIBGLES_DEPENDENCIES down into each OpenGL
> >>>implementation .mk file. But that requires a late evaluation of
> >>>$(generic-package), so that all OpenGL implementations can be
> >>>registered in LIBGLES_DEPENDENCIES before the generic-package macro
> >>>of libgles.mk is evaluated. This would require something like
> >>>Yann's patch.
> >>
> >>Needless to say I would highly prefer this second solution.
> >
> >Right. In principle, I have nothing against this solution. It's just
> >that I am not sure to fully grasp the consequences of the change you're
> >proposing. I'm a bit worried about "weird" consequences that we may not
> >be thinking of at this time. But maybe we should simply apply the
> >patch, and see if it causes problems for some specific use cases.
> 
>  I'm also a bit afraid of the consequences. It also makes make processing,
> which is already difficult to understand, even more obfuscated.
> 
> 
>  Here's a wild idea...
> 
> In rpi-userland/Config.in:
> 
> if BR2_PACKAGE_RPI_USERLAND
> config BR2_PACKAGE_LIBEGL_PROVIDER
> 	string
> 	default "rpi-userland"
> endif
> 
> In opengl/libegl/libegl.mk:
> 
> LIBEGL_DEPENDENCIES = $(call qstrip,$(BR2PACKAGE_LIBEGL_PROVIDER))
> 

That's about what I am experimenting right now! :-p

But I've done it slightly differently:

package/opengl/libegl/Config.in:
    config BR2_LIBEGL_PROVIDER
        string

package/rpi-userland/Config.in:
    config BR2_LIBEGL_PROVIDER
        default "rpi-userland" if BR2_PACKAGE_RPI_USERLAND

And the same .mk fragment you suggested for libegl.

My solution is a little bit more compact, and since it does not use a
package-named variable, we can say that packages do not step on
one-another's feet. Yet, a bit hackish, I have to concede...

> 
>  It's still hackish of course, because:
> 
> - rpi-userland/Config.in defines a symbol "belonging" to the libegl package;
> 
> - only one provider can be defined, Kconfig will scream if it's defined
> twice;

Is it even valid to have two providers of the same functioanlity? What
would happen: what libEGL.so would be used? Probably the last one
installed, ie. the one from the alphabetically-last provider.

> - it may not work at all :-).

I'll tell you when I'm done with my checks... ;-p

Regards,
Yann E. MORIN.
Arnout Vandecappelle Dec. 19, 2013, 4:58 p.m. UTC | #14 
On 17/12/13 23:35, Yann E. MORIN wrote:
> Arnout, All,
>
> On 2013-12-17 23:20 +0100, Arnout Vandecappelle spake thusly:
[snip]
>>   Here's a wild idea...
>>
>> In rpi-userland/Config.in:
>>
>> if BR2_PACKAGE_RPI_USERLAND
>> config BR2_PACKAGE_LIBEGL_PROVIDER
>> 	string
>> 	default "rpi-userland"
>> endif
>>
>> In opengl/libegl/libegl.mk:
>>
>> LIBEGL_DEPENDENCIES = $(call qstrip,$(BR2PACKAGE_LIBEGL_PROVIDER))
>>
>
> That's about what I am experimenting right now! :-p
>
> But I've done it slightly differently:
>
> package/opengl/libegl/Config.in:
>      config BR2_LIBEGL_PROVIDER
>          string
>
> package/rpi-userland/Config.in:
>      config BR2_LIBEGL_PROVIDER
>          default "rpi-userland" if BR2_PACKAGE_RPI_USERLAND
>
> And the same .mk fragment you suggested for libegl.

  Nice! I'm all for it! I didn't know it was allowed in Kconfig to define 
the same symbol twice. But indeed, we already do that in arch/


> My solution is a little bit more compact, and since it does not use a
> package-named variable, we can say that packages do not step on
> one-another's feet. Yet, a bit hackish, I have to concede...
>
>>
>>   It's still hackish of course, because:
>>
>> - rpi-userland/Config.in defines a symbol "belonging" to the libegl package;
>>
>> - only one provider can be defined, Kconfig will scream if it's defined
>> twice;
>
> Is it even valid to have two providers of the same functioanlity? What
> would happen: what libEGL.so would be used? Probably the last one
> installed, ie. the one from the alphabetically-last provider.

  Completely true. With your construct, if somehow two providers are 
selected, Kconfig will probably just give you the first-sourced provider.


  Regards,
  Arnout

>
>> - it may not work at all :-).
>
> I'll tell you when I'm done with my checks... ;-p
>
> Regards,
> Yann E. MORIN.
>
Yann E. MORIN Dec. 19, 2013, 8:43 p.m. UTC | #15 
Arnout, All,

On 2013-12-19 17:58 +0100, Arnout Vandecappelle spake thusly:
> On 17/12/13 23:35, Yann E. MORIN wrote:
> >Arnout, All,
> >
> >On 2013-12-17 23:20 +0100, Arnout Vandecappelle spake thusly:
> [snip]
> >>  Here's a wild idea...
> >>
> >>In rpi-userland/Config.in:
> >>
> >>if BR2_PACKAGE_RPI_USERLAND
> >>config BR2_PACKAGE_LIBEGL_PROVIDER
> >>	string
> >>	default "rpi-userland"
> >>endif
> >>
> >>In opengl/libegl/libegl.mk:
> >>
> >>LIBEGL_DEPENDENCIES = $(call qstrip,$(BR2PACKAGE_LIBEGL_PROVIDER))
> >>
> >
> >That's about what I am experimenting right now! :-p
> >
> >But I've done it slightly differently:
> >
> >package/opengl/libegl/Config.in:
> >     config BR2_LIBEGL_PROVIDER
> >         string
> >
> >package/rpi-userland/Config.in:
> >     config BR2_LIBEGL_PROVIDER
> >         default "rpi-userland" if BR2_PACKAGE_RPI_USERLAND
> >
> >And the same .mk fragment you suggested for libegl.
> 
>  Nice! I'm all for it! I didn't know it was allowed in Kconfig to define the
> same symbol twice. But indeed, we already do that in arch/

In fact, you can define a symbol only once: the moment you give it a
type is when the symbol is actually 'defined'.

You can however use the symbol in different places, to set a default
value or other properties (depends, selecti, options...)

There are a few properties of a symbol that can only be set once:
  - the type  (obviously)
  - the prompt, even if it is conditional
  - the help text

> >My solution is a little bit more compact, and since it does not use a
> >package-named variable, we can say that packages do not step on
> >one-another's feet. Yet, a bit hackish, I have to concede...
> >
> >>
> >>  It's still hackish of course, because:
> >>
> >>- rpi-userland/Config.in defines a symbol "belonging" to the libegl package;
> >>
> >>- only one provider can be defined, Kconfig will scream if it's defined
> >>twice;
> >
> >Is it even valid to have two providers of the same functioanlity? What
> >would happen: what libEGL.so would be used? Probably the last one
> >installed, ie. the one from the alphabetically-last provider.
> 
>  Completely true. With your construct, if somehow two providers are
> selected, Kconfig will probably just give you the first-sourced provider.

No, we can't know which one is used: the symbols are stored in buckets,
indexed by the hash of each symbol.

So, there is no (simple) way to know what symbols takes precedence, since
we can't easily predict what bucket they'll end up, so what symbol will
be hit first/last.

Regards,
Yann E. MORIN.
diff mbox

Patch

diff --git a/package/opengl/libgles/libgles.mk b/package/opengl/libgles/libgles.mk
index ec157ac..c2e1acf 100644
--- a/package/opengl/libgles/libgles.mk
+++ b/package/opengl/libgles/libgles.mk
@@ -22,11 +22,16 @@  ifeq ($(BR2_PACKAGE_GPU_VIV_BIN_MX6Q),y)
 LIBGLES_DEPENDENCIES += gpu-viv-bin-mx6q
 endif
 
-ifeq ($(LIBGLES_DEPENDENCIES),)
+# Because some GLES providers may be in BR2_EXTERNAL,
+# $(LIBGLES_DEPENDENCIES) might be empty right here.
+# So, we can't rely on it to define LIBGLES_CONFIGURE_CMDS
+# right now, and we must postpone the check until later,
+# ie. at runtime.
 define LIBGLES_CONFIGURE_CMDS
-	echo "No libGLES implementation selected. Configuration error."
-	exit 1
+	if [ -z "$${LIBGLES_DEPENDENCIES}" ]; then \
+		echo "No libGLES implementation selected. Configuration error."; \
+		exit 1; \
+	fi
 endef
-endif
 
 $(eval $(generic-package))