libdrm: intel/Android.mk: Filter libdrm_intel library requirements on x86/x86_64

When building AOSP after updating libdrm project to the
freedesktop/master branch, I've seen the following build errors:

external/libdrm/intel/Android.mk: error: libdrm_intel
(SHARED_LIBRARIES android-arm64) missing libpciaccess
(SHARED_LIBRARIES android-arm64) You can set
ALLOW_MISSING_DEPENDENCIES=true in your environment if this is
intentional, but that may defer real problems until later in the
build.

Using ALLOW_MISSING_DEPENDENCIES=true when building allows
things to function properly, but is not ideal.

So basically, while I'm not including the libdrm_intel package
into the build, just the fact that the Android.mk file references
libpciaccess which isn't a repo included in AOSP causes the build
failure.

So it seems we need some sort of conditional filter in the
Android.mk to skip over it if we're not building for intel.

Change-Id: I6cb51c7bb0a7d1a0ab1723b7d3f20aea38988495
Cc: Emil Velikov <emil.l.velikov@gmail.com>
Cc: Chad Versace <chad.versace@linux.intel.com>
Cc: Marissa Wall <marissaw@google.com>
Cc: Sean Paul <seanpaul@google.com>
Cc: Rob Herring <rob.herring@linaro.org>
Cc: Dan Willemsen <dwillemsen@google.com>
Cc: Tomasz Figa <tfiga@google.com>
Cc: Robert Foss <robert.foss@collabora.com>
Signed-off-by: John Stultz <john.stultz@linaro.org>

---
v2: Check for x86_64 as well
---
 intel/Android.mk | 2 ++
 1 file changed, 2 insertions(+)

-- 
2.7.4

On 14 March 2018 at 16:47, John Stultz <john.stultz@linaro.org> wrote:
> When building AOSP after updating libdrm project to the
> freedesktop/master branch, I've seen the following build errors:
>
> external/libdrm/intel/Android.mk: error: libdrm_intel
> (SHARED_LIBRARIES android-arm64) missing libpciaccess
> (SHARED_LIBRARIES android-arm64) You can set
> ALLOW_MISSING_DEPENDENCIES=true in your environment if this is
> intentional, but that may defer real problems until later in the
> build.
>
> Using ALLOW_MISSING_DEPENDENCIES=true when building allows
> things to function properly, but is not ideal.
>
> So basically, while I'm not including the libdrm_intel package
> into the build, just the fact that the Android.mk file references
> libpciaccess which isn't a repo included in AOSP causes the build
> failure.
>
> So it seems we need some sort of conditional filter in the
> Android.mk to skip over it if we're not building for intel.
>
Could swear I asked a few times already, but cannot see an answer.
Why/how does this happen - did you forget to set BOARD_GPU_DRIVERS?

One way to avoid this kind of clutches like is to have meta drivers
like "arm-all" or "x86-all".

Some examples:
 - the Mesa i965/anv drivers will not build for arm
 - the Mesa vc4 (even vc5?) driver has some perf. sensitive arm/thumb assembly
 - building the following combinations is waste of resources -
i915/i965/i915g on !x86, freedreno/etnaviv/imx on !arm

Without something like my earlier suggestion all of the above will
need to be special cased. And more are to come with time :-\

That is, unless I'm loosing my marbles. In which case don't be shy and
let me know, please.

Thanks
Emil

On Wed, Mar 14, 2018 at 10:21 AM, Emil Velikov <emil.l.velikov@gmail.com> wrote:
> On 14 March 2018 at 16:47, John Stultz <john.stultz@linaro.org> wrote:
>> When building AOSP after updating libdrm project to the
>> freedesktop/master branch, I've seen the following build errors:
>>
>> external/libdrm/intel/Android.mk: error: libdrm_intel
>> (SHARED_LIBRARIES android-arm64) missing libpciaccess
>> (SHARED_LIBRARIES android-arm64) You can set
>> ALLOW_MISSING_DEPENDENCIES=true in your environment if this is
>> intentional, but that may defer real problems until later in the
>> build.
>>
>> Using ALLOW_MISSING_DEPENDENCIES=true when building allows
>> things to function properly, but is not ideal.
>>
>> So basically, while I'm not including the libdrm_intel package
>> into the build, just the fact that the Android.mk file references
>> libpciaccess which isn't a repo included in AOSP causes the build
>> failure.
>>
>> So it seems we need some sort of conditional filter in the
>> Android.mk to skip over it if we're not building for intel.
>>
> Could swear I asked a few times already, but cannot see an answer.
> Why/how does this happen - did you forget to set BOARD_GPU_DRIVERS?

Again, this is from the Android build, as the top level Android.mk calls:
include $(call all-makefiles-under,$(LOCAL_PATH))

Which includes all Android.mk files in all the sub directories
(regardless of any BOARD_GPU_DRIVERS value).

The error is that while we don't build the libdrm_intel module, the
android build system still throws a error when any
LOCAL_SHARED_LIBRARIES files aren't present in the larger build
environment.

Since the intel/Android.mk specifies
LOCAL_SHARED_LIBRARIES := \
        libdrm \
        libpciaccess

And in AOSP there is no libpciaccess module, it generates the error.

thanks
-john

On Wed, Mar 14, 2018 at 12:21 PM, Emil Velikov <emil.l.velikov@gmail.com> wrote:
> On 14 March 2018 at 16:47, John Stultz <john.stultz@linaro.org> wrote:
>> When building AOSP after updating libdrm project to the
>> freedesktop/master branch, I've seen the following build errors:
>>
>> external/libdrm/intel/Android.mk: error: libdrm_intel
>> (SHARED_LIBRARIES android-arm64) missing libpciaccess
>> (SHARED_LIBRARIES android-arm64) You can set
>> ALLOW_MISSING_DEPENDENCIES=true in your environment if this is
>> intentional, but that may defer real problems until later in the
>> build.
>>
>> Using ALLOW_MISSING_DEPENDENCIES=true when building allows
>> things to function properly, but is not ideal.
>>
>> So basically, while I'm not including the libdrm_intel package
>> into the build, just the fact that the Android.mk file references
>> libpciaccess which isn't a repo included in AOSP causes the build
>> failure.
>>
>> So it seems we need some sort of conditional filter in the
>> Android.mk to skip over it if we're not building for intel.
>>
> Could swear I asked a few times already, but cannot see an answer.
> Why/how does this happen - did you forget to set BOARD_GPU_DRIVERS?
>
> One way to avoid this kind of clutches like is to have meta drivers
> like "arm-all" or "x86-all".
>
> Some examples:
>  - the Mesa i965/anv drivers will not build for arm

They used to...

>  - the Mesa vc4 (even vc5?) driver has some perf. sensitive arm/thumb assembly
>  - building the following combinations is waste of resources -
> i915/i965/i915g on !x86, freedreno/etnaviv/imx on !arm

I disagree. To use the kernel as an example, it is very valuable to
have your driver code build for x86 allmodconfig even if it is
something that never runs on x86 because lots of people and bots build
that.

If you require having ARM cross compilers and environment setup to
build test "arm" drivers in mesa/libdrm, then you've really cut down
the number of people doing build testing. Just look at Android build
testing. I can count the number of people that do that on one hand.

> Without something like my earlier suggestion all of the above will
> need to be special cased. And more are to come with time :-\
>
> That is, unless I'm loosing my marbles. In which case don't be shy and
> let me know, please.

I think this has been beaten to death and will apply it. It's an easy
revert if someone has an itch to come up with something better.

Rob

On 14 March 2018 at 17:30, John Stultz <john.stultz@linaro.org> wrote:
> On Wed, Mar 14, 2018 at 10:21 AM, Emil Velikov <emil.l.velikov@gmail.com> wrote:
>> On 14 March 2018 at 16:47, John Stultz <john.stultz@linaro.org> wrote:
>>> When building AOSP after updating libdrm project to the
>>> freedesktop/master branch, I've seen the following build errors:
>>>
>>> external/libdrm/intel/Android.mk: error: libdrm_intel
>>> (SHARED_LIBRARIES android-arm64) missing libpciaccess
>>> (SHARED_LIBRARIES android-arm64) You can set
>>> ALLOW_MISSING_DEPENDENCIES=true in your environment if this is
>>> intentional, but that may defer real problems until later in the
>>> build.
>>>
>>> Using ALLOW_MISSING_DEPENDENCIES=true when building allows
>>> things to function properly, but is not ideal.
>>>
>>> So basically, while I'm not including the libdrm_intel package
>>> into the build, just the fact that the Android.mk file references
>>> libpciaccess which isn't a repo included in AOSP causes the build
>>> failure.
>>>
>>> So it seems we need some sort of conditional filter in the
>>> Android.mk to skip over it if we're not building for intel.
>>>
>> Could swear I asked a few times already, but cannot see an answer.
>> Why/how does this happen - did you forget to set BOARD_GPU_DRIVERS?
>
> Again, this is from the Android build, as the top level Android.mk calls:
> include $(call all-makefiles-under,$(LOCAL_PATH))
>
> Which includes all Android.mk files in all the sub directories
> (regardless of any BOARD_GPU_DRIVERS value).
>
> The error is that while we don't build the libdrm_intel module, the
> android build system still throws a error when any
> LOCAL_SHARED_LIBRARIES files aren't present in the larger build
> environment.
>
So Android would evaluate dependencies, even when the object is not
build/required.
That is fairly counter intuitive, hence why I've been stuck in suck a loop.

Fwiw the patch is
Reviewed-by: Emil Velikov <emil.velikov@collabora.com>

Thanks for the explanation and patience!
Emil

On 16 March 2018 at 14:35, Rob Herring <robh@kernel.org> wrote:
> On Wed, Mar 14, 2018 at 12:21 PM, Emil Velikov <emil.l.velikov@gmail.com> wrote:
>> On 14 March 2018 at 16:47, John Stultz <john.stultz@linaro.org> wrote:
>>> When building AOSP after updating libdrm project to the
>>> freedesktop/master branch, I've seen the following build errors:
>>>
>>> external/libdrm/intel/Android.mk: error: libdrm_intel
>>> (SHARED_LIBRARIES android-arm64) missing libpciaccess
>>> (SHARED_LIBRARIES android-arm64) You can set
>>> ALLOW_MISSING_DEPENDENCIES=true in your environment if this is
>>> intentional, but that may defer real problems until later in the
>>> build.
>>>
>>> Using ALLOW_MISSING_DEPENDENCIES=true when building allows
>>> things to function properly, but is not ideal.
>>>
>>> So basically, while I'm not including the libdrm_intel package
>>> into the build, just the fact that the Android.mk file references
>>> libpciaccess which isn't a repo included in AOSP causes the build
>>> failure.
>>>
>>> So it seems we need some sort of conditional filter in the
>>> Android.mk to skip over it if we're not building for intel.
>>>
>> Could swear I asked a few times already, but cannot see an answer.
>> Why/how does this happen - did you forget to set BOARD_GPU_DRIVERS?
>>
>> One way to avoid this kind of clutches like is to have meta drivers
>> like "arm-all" or "x86-all".
>>
>> Some examples:
>>  - the Mesa i965/anv drivers will not build for arm
>
> They used to...
>
There are still

Fair enough. Guess the compiler produced


>>  - the Mesa vc4 (even vc5?) driver has some perf. sensitive arm/thumb assembly
>>  - building the following combinations is waste of resources -
>> i915/i965/i915g on !x86, freedreno/etnaviv/imx on !arm
>
> I disagree. To use the kernel as an example, it is very valuable to
> have your driver code build for x86 allmodconfig even if it is
> something that never runs on x86 because lots of people and bots build
> that.
>
> If you require having ARM cross compilers and environment setup to
> build test "arm" drivers in mesa/libdrm, then you've really cut down
> the number of people doing build testing. Just look at Android build
> testing. I can count the number of people that do that on one hand.
>
FWIW I had suggested the same thing (build test everything) a few
times in the past.
For one reason or another the wider Mesa community did not agree :-\

>> Without something like my earlier suggestion all of the above will
>> need to be special cased. And more are to come with time :-\
>>
>> That is, unless I'm loosing my marbles. In which case don't be shy and
>> let me know, please.
>
> I think this has been beaten to death and will apply it. It's an easy
> revert if someone has an itch to come up with something better.
>
You have to admit though - evaluating dependencies for something that
is not build is fairly counter-intuitive.

Regardless - yes, I had a dull moment.
I deeply appreciate the patience and help.

-Emil

Hey John,

On Wed, Mar 14, 2018 at 5:47 PM, John Stultz <john.stultz@linaro.org> wrote:
> When building AOSP after updating libdrm project to the
> freedesktop/master branch, I've seen the following build errors:
>
> external/libdrm/intel/Android.mk: error: libdrm_intel
> (SHARED_LIBRARIES android-arm64) missing libpciaccess
> (SHARED_LIBRARIES android-arm64) You can set
> ALLOW_MISSING_DEPENDENCIES=true in your environment if this is
> intentional, but that may defer real problems until later in the
> build.
>
> Using ALLOW_MISSING_DEPENDENCIES=true when building allows
> things to function properly, but is not ideal.
>
> So basically, while I'm not including the libdrm_intel package
> into the build, just the fact that the Android.mk file references
> libpciaccess which isn't a repo included in AOSP causes the build
> failure.
>
> So it seems we need some sort of conditional filter in the
> Android.mk to skip over it if we're not building for intel.

I'm afraid this change has snowballed straight into the mesa build where
it's now missing dependencies for i915_dri:

external/mesa3d/src/mesa/drivers/dri/i915/Android.mk: error:
i915_dri (SHARED_LIBRARIES android-arm) missing libdrm_intel

Maybe that one needs the BOARD_GPU_DRIVERS treatment instead..

Thanks,
Stefan

On Tue, Mar 20, 2018 at 6:55 AM, Stefan Schake <stschake@gmail.com> wrote:
> Hey John,
>
> On Wed, Mar 14, 2018 at 5:47 PM, John Stultz <john.stultz@linaro.org> wrote:
>> When building AOSP after updating libdrm project to the
>> freedesktop/master branch, I've seen the following build errors:
>>
>> external/libdrm/intel/Android.mk: error: libdrm_intel
>> (SHARED_LIBRARIES android-arm64) missing libpciaccess
>> (SHARED_LIBRARIES android-arm64) You can set
>> ALLOW_MISSING_DEPENDENCIES=true in your environment if this is
>> intentional, but that may defer real problems until later in the
>> build.
>>
>> Using ALLOW_MISSING_DEPENDENCIES=true when building allows
>> things to function properly, but is not ideal.
>>
>> So basically, while I'm not including the libdrm_intel package
>> into the build, just the fact that the Android.mk file references
>> libpciaccess which isn't a repo included in AOSP causes the build
>> failure.
>>
>> So it seems we need some sort of conditional filter in the
>> Android.mk to skip over it if we're not building for intel.
>
> I'm afraid this change has snowballed straight into the mesa build where
> it's now missing dependencies for i915_dri:
>
> external/mesa3d/src/mesa/drivers/dri/i915/Android.mk: error:
> i915_dri (SHARED_LIBRARIES android-arm) missing libdrm_intel
>
> Maybe that one needs the BOARD_GPU_DRIVERS treatment instead..

Oof. Apologies for missing this, my environment uses the AOSP mesa3d.

I'll pull that down and take a look at what a better solution should be.

Rob: Do you want to revert in the meantime?

Apologies again!
-john

On Tue, Mar 20, 2018 at 6:55 AM, Stefan Schake <stschake@gmail.com> wrote:
> Hey John,
>
> On Wed, Mar 14, 2018 at 5:47 PM, John Stultz <john.stultz@linaro.org> wrote:
>> When building AOSP after updating libdrm project to the
>> freedesktop/master branch, I've seen the following build errors:
>>
>> external/libdrm/intel/Android.mk: error: libdrm_intel
>> (SHARED_LIBRARIES android-arm64) missing libpciaccess
>> (SHARED_LIBRARIES android-arm64) You can set
>> ALLOW_MISSING_DEPENDENCIES=true in your environment if this is
>> intentional, but that may defer real problems until later in the
>> build.
>>
>> Using ALLOW_MISSING_DEPENDENCIES=true when building allows
>> things to function properly, but is not ideal.
>>
>> So basically, while I'm not including the libdrm_intel package
>> into the build, just the fact that the Android.mk file references
>> libpciaccess which isn't a repo included in AOSP causes the build
>> failure.
>>
>> So it seems we need some sort of conditional filter in the
>> Android.mk to skip over it if we're not building for intel.
>
> I'm afraid this change has snowballed straight into the mesa build where
> it's now missing dependencies for i915_dri:
>
> external/mesa3d/src/mesa/drivers/dri/i915/Android.mk: error:
> i915_dri (SHARED_LIBRARIES android-arm) missing libdrm_intel
>
> Maybe that one needs the BOARD_GPU_DRIVERS treatment instead..

So tinkering here, it seems to me just changing the conditionalizing
to skipping over just the libpciaccess addition to
LOCAL_SHARED_LIBRARIES might be a simpler solution.

Or would you see that as too ugly?

thanks
-john

On Tue, Mar 20, 2018 at 2:49 AM, John Stultz <john.stultz@linaro.org> wrote:
> On Tue, Mar 20, 2018 at 6:55 AM, Stefan Schake <stschake@gmail.com> wrote:
>> Hey John,
>>
>> On Wed, Mar 14, 2018 at 5:47 PM, John Stultz <john.stultz@linaro.org> wrote:
>>> When building AOSP after updating libdrm project to the
>>> freedesktop/master branch, I've seen the following build errors:
>>>
>>> external/libdrm/intel/Android.mk: error: libdrm_intel
>>> (SHARED_LIBRARIES android-arm64) missing libpciaccess
>>> (SHARED_LIBRARIES android-arm64) You can set
>>> ALLOW_MISSING_DEPENDENCIES=true in your environment if this is
>>> intentional, but that may defer real problems until later in the
>>> build.
>>>
>>> Using ALLOW_MISSING_DEPENDENCIES=true when building allows
>>> things to function properly, but is not ideal.
>>>
>>> So basically, while I'm not including the libdrm_intel package
>>> into the build, just the fact that the Android.mk file references
>>> libpciaccess which isn't a repo included in AOSP causes the build
>>> failure.
>>>
>>> So it seems we need some sort of conditional filter in the
>>> Android.mk to skip over it if we're not building for intel.
>>
>> I'm afraid this change has snowballed straight into the mesa build where
>> it's now missing dependencies for i915_dri:
>>
>> external/mesa3d/src/mesa/drivers/dri/i915/Android.mk: error:
>> i915_dri (SHARED_LIBRARIES android-arm) missing libdrm_intel
>>
>> Maybe that one needs the BOARD_GPU_DRIVERS treatment instead..
>
> So tinkering here, it seems to me just changing the conditionalizing
> to skipping over just the libpciaccess addition to
> LOCAL_SHARED_LIBRARIES might be a simpler solution.
>
> Or would you see that as too ugly?

I guess my real problem is with libpciaccess itself. Okay, we can't ding
it for not being in AOSP, plenty of upstream Android isn't. But then the
freedesktop/upstream repo of libpciaccess doesn't support Android build,
the copies I found on Android-x86 and intel-ia are outdated and the latter
doesn't seem to have it in its manifest. Beyond that, the only reference is
in intel_bufmgr that if I understood the other discussion correctly is no
longer used in Mesa?

But that's mostly ranting, I don't even have any Intel hardware I use for
Android. So I guess in the spirit of achieving the original goal of making
that annoying error go away, ignoring only libpciaccess is just as well.

Thanks,
Stefan

On 20 March 2018 at 01:49, John Stultz <john.stultz@linaro.org> wrote:
> On Tue, Mar 20, 2018 at 6:55 AM, Stefan Schake <stschake@gmail.com> wrote:
>> Hey John,
>>
>> On Wed, Mar 14, 2018 at 5:47 PM, John Stultz <john.stultz@linaro.org> wrote:
>>> When building AOSP after updating libdrm project to the
>>> freedesktop/master branch, I've seen the following build errors:
>>>
>>> external/libdrm/intel/Android.mk: error: libdrm_intel
>>> (SHARED_LIBRARIES android-arm64) missing libpciaccess
>>> (SHARED_LIBRARIES android-arm64) You can set
>>> ALLOW_MISSING_DEPENDENCIES=true in your environment if this is
>>> intentional, but that may defer real problems until later in the
>>> build.
>>>
>>> Using ALLOW_MISSING_DEPENDENCIES=true when building allows
>>> things to function properly, but is not ideal.
>>>
>>> So basically, while I'm not including the libdrm_intel package
>>> into the build, just the fact that the Android.mk file references
>>> libpciaccess which isn't a repo included in AOSP causes the build
>>> failure.
>>>
>>> So it seems we need some sort of conditional filter in the
>>> Android.mk to skip over it if we're not building for intel.
>>
>> I'm afraid this change has snowballed straight into the mesa build where
>> it's now missing dependencies for i915_dri:
>>
>> external/mesa3d/src/mesa/drivers/dri/i915/Android.mk: error:
>> i915_dri (SHARED_LIBRARIES android-arm) missing libdrm_intel
>>
>> Maybe that one needs the BOARD_GPU_DRIVERS treatment instead..
>
> So tinkering here, it seems to me just changing the conditionalizing
> to skipping over just the libpciaccess addition to
> LOCAL_SHARED_LIBRARIES might be a simpler solution.
>
> Or would you see that as too ugly?
>
Or just remove all the libpciaccess business for Android?

The API that truly depends on the library, was preemptively added with
zero users.
And then the author went MIA :-(

-Emil

On 20 March 2018 at 10:23, Emil Velikov <emil.l.velikov@gmail.com> wrote:
> On 20 March 2018 at 01:49, John Stultz <john.stultz@linaro.org> wrote:
>> On Tue, Mar 20, 2018 at 6:55 AM, Stefan Schake <stschake@gmail.com> wrote:
>>> Hey John,
>>>
>>> On Wed, Mar 14, 2018 at 5:47 PM, John Stultz <john.stultz@linaro.org> wrote:
>>>> When building AOSP after updating libdrm project to the
>>>> freedesktop/master branch, I've seen the following build errors:
>>>>
>>>> external/libdrm/intel/Android.mk: error: libdrm_intel
>>>> (SHARED_LIBRARIES android-arm64) missing libpciaccess
>>>> (SHARED_LIBRARIES android-arm64) You can set
>>>> ALLOW_MISSING_DEPENDENCIES=true in your environment if this is
>>>> intentional, but that may defer real problems until later in the
>>>> build.
>>>>
>>>> Using ALLOW_MISSING_DEPENDENCIES=true when building allows
>>>> things to function properly, but is not ideal.
>>>>
>>>> So basically, while I'm not including the libdrm_intel package
>>>> into the build, just the fact that the Android.mk file references
>>>> libpciaccess which isn't a repo included in AOSP causes the build
>>>> failure.
>>>>
>>>> So it seems we need some sort of conditional filter in the
>>>> Android.mk to skip over it if we're not building for intel.
>>>
>>> I'm afraid this change has snowballed straight into the mesa build where
>>> it's now missing dependencies for i915_dri:
>>>
>>> external/mesa3d/src/mesa/drivers/dri/i915/Android.mk: error:
>>> i915_dri (SHARED_LIBRARIES android-arm) missing libdrm_intel
>>>
>>> Maybe that one needs the BOARD_GPU_DRIVERS treatment instead..
>>
>> So tinkering here, it seems to me just changing the conditionalizing
>> to skipping over just the libpciaccess addition to
>> LOCAL_SHARED_LIBRARIES might be a simpler solution.
>>
>> Or would you see that as too ugly?
>>
> Or just remove all the libpciaccess business for Android?
>
Namely this patch [1] from the Android tree

-Emil

[1] https://android.googlesource.com/platform/external/libdrm/+/f6a1130dffae8de9ddd0c379066daf1df27fc8af