[PATCH] Revert "config: tools only: add VIDEO to build bmp_logo"

[Rasmus Villemoes]

On 20/01/2023 16.23, Tom Rini wrote:
> On Fri, Jan 20, 2023 at 08:30:27AM +0000, Peter Robinson wrote:
>> On Thu, Jan 19, 2023 at 2:02 PM Tom Rini <trini at konsulko.com> wrote:
>>>
>>> On Thu, Jan 19, 2023 at 11:52:11AM +0000, Peter Robinson wrote:
>>>> On Thu, Jan 19, 2023 at 11:33 AM Fabio Estevam <festevam at gmail.com> wrote:
>>>>>
>>>>> On Thu, Jan 19, 2023 at 8:18 AM Peter Robinson <pbrobinson at gmail.com> wrote:
>>>>>
>>>>>> Did you read the original thread?
>>>>>
>>>>> I only read your commit log and it is not clear if it is a regression
>>>>> and which commit caused the problem.
>>>>
>>>> I don't remember, this was back in November.
>>>>
>>>>> It lacks a Fixes tag too.
>>>>
>>>> Which are mostly pointless in U-Boot as it's not like the kernel where
>>>> there's stable release cycles.
>>>
>>> They're quite helpful, in order to track when changes are fixing a
>>> problem that was introduced by some other commit.
>>>
>>>>>>> - What is the exact problem you are trying to solve? How can we reproduce it?
>>>>>>
>>>>>> Building that tool with the tools only option.
>>>>>
>>>>> When I try to build U-Boot 2023.01 u-boot-tools-native in OpenEmbedded
>>>>> it works fine.
>>>>
>>>> But does it actually build the bmp_logo tool? It builds fine for me
>>>> too but that tool is no longer built.
>>>>
>>>>> It fails when trying to build it for the target since commit
>>>>> 1cfba53ca46c ("config: tools only: add VIDEO to build
>>>>> bmp_logo").
>>>>
>>>> Where when I build it for Fedora it builds for me and I get the
>>>> bmp_logo which we used to have pre 2023.01
>>>>
>>>>>>> - Is it a regression? What is the commit that caused the problem you
>>>>>>> are trying to solve?
>>>>>
>>>>> What about this part?
>>>>
>>>> I don't remember, see comment above.
>>>
>>> The unanswered question is, why does Fedora ship the bmp_logo tool? I'm
>>> fairly certain it's one of the host tools that's not useful outside of
>>> the build and so shouldn't be packaged.
>>
>> It pre dates my time as maintainer so I actually don't know other than
>> "we've basically shipped it for ever and it's impossible to tell if it
>> has users", we could probably drop it and wait to see if I get bug
>> reports.
> 
> OK, thanks, we'll revert this and see what the fall out, if any, ends up
> being.
> 

So we also hit this problem, not with the target version of u-boot-tools
(which we don't build), but the nativesdk one, with the same symptoms.
We certainly don't use or need the bmp_logo tool, so we're happy with
the revert and will just carry that on top of v2023.01 for now, but
since I spent some time on this I thought I might as well float the fix
I was about to send, before I noticed the revert:

diff --git a/tools/Makefile b/tools/Makefile
index edfa40903d..5c8175cf2a 100644
--- a/tools/Makefile
+++ b/tools/Makefile
@@ -281,8 +281,10 @@ hostprogs-y += printinitialenv
 # Generated LCD/video logo
 LOGO_H = $(objtree)/include/bmp_logo.h
 LOGO_DATA_H = $(objtree)/include/bmp_logo_data.h
+ifeq ($(CROSS_BUILD_TOOLS),)
 LOGO-$(CONFIG_VIDEO_LOGO) += $(LOGO_H)
 LOGO-$(CONFIG_VIDEO_LOGO) += $(LOGO_DATA_H)
+endif

 # Generic logo
 ifeq ($(LOGO_BMP),)

This relies on the Yocto recipe doing (essentially)

      make tools-only_defconfig
      make cross_tools NO_SDL=1

so the top Makefile sets CROSS_BUILD_TOOLS, and thus we avoid adding the
targets that need to invoke the bmp_logo tool (which obviously doesn't
work when built for target, nor when built for nativesdk where the ELF
file ends up with "interpreter
/usr/local/oe-sdk-hardcoded-buildpath/sysroots/x86_64-oesdk-linux/lib/ld-linux-x86-64.so.2"),
while still allowing the bmp_logo tool itself to be built.

Rasmus