Hello Johannes,

On 26.05.23 07:46, Johannes Zink wrote:
> Hi Ahmad,
> 
> On 5/25/23 20:13, Ahmad Fatoum wrote:
>> include/generated/utsrelease.h is only generated in the prepare make
>> target, which is not a dependency of the scripts make target. Builds of
>> the scripts target, e.g. sandbox $(make hosttools_defconfig; make scripts),
>> may thus fail because the file had not yet been generated:
>>
>> <command-line>: fatal error: /build/source/include/generated/utsrelease.h:
>>     No such file or directory
>>     compilation terminated.
>>
>> That scripts doesn't depend on prepare may be due to scripts/kconfig.
>> The kernel may side step this issue by having tools not needed for
>> prepare in a separate tools/.
>>
> 
> do you have a suggestion how this issue could be solved?
> I think that it is - nevertheless - useful to have a version number in the 
> tooling.

Agreed. I am looking forward to a v2 that doesn't break the build. ;)

> Do you know of a flag that is set when the files have been generated?
> Maybe we can include the file conditionally if and only if it has been 
> generated.

That would mean we accept that the race condition is there and that depending on
timing/parallelism, we either end up with a version or unknown...

> Do you think that alternatively we can require the prepare make target as a 
> dependency for scripts/imx?

AFAICS, scripts/Makefile is called recursively, so it may not be feasible to 
depend on
top-level Makefile's prepare for a target defined within. If everything needed 
for
Kconfig is part of another target (e.g. config or scripts_basic), then maybe 
scripts
could be made to depend on prepare. A worthwhile alternative may be moving 
stuff not
needed for the build, like all USB loaders, into a tools/ directory, but then 
one
needs to take care of dependencies on files in scripts/.

Cheers,
Ahmad

> 
> Johannes
> 
>> Until this is resolved, revert commit 
>> c1b50061f4b33482ae749f9d6d6c92aa5bf6b37a.
>>
>> Cc: Johannes Zink <[email protected]>
>> Signed-off-by: Ahmad Fatoum <[email protected]>
>> ---
>>   scripts/imx/Makefile         |  2 +-
>>   scripts/imx/imx-usb-loader.c | 22 +---------------------
>>   2 files changed, 2 insertions(+), 22 deletions(-)
>>
>> diff --git a/scripts/imx/Makefile b/scripts/imx/Makefile
>> index b3be3886d8eb..dbfa82910a55 100644
>> --- a/scripts/imx/Makefile
>> +++ b/scripts/imx/Makefile
>> @@ -3,7 +3,7 @@
>>   hostprogs-always-$(CONFIG_ARCH_IMX_IMXIMAGE)    += imx-image
>>   hostprogs-always-$(CONFIG_ARCH_IMX_USBLOADER)    += imx-usb-loader
>>   -HOSTCFLAGS_imx-usb-loader.o = `pkg-config --cflags libusb-1.0` -include 
>> $(objtree)/include/generated/utsrelease.h
>> +HOSTCFLAGS_imx-usb-loader.o = `pkg-config --cflags libusb-1.0`
>>   HOSTLDLIBS_imx-usb-loader  = `pkg-config --libs libusb-1.0`
>>     imx-usb-loader-target-userccflags += `$(CROSS_PKG_CONFIG) --cflags 
>> libusb-1.0`
>> diff --git a/scripts/imx/imx-usb-loader.c b/scripts/imx/imx-usb-loader.c
>> index 676f077c2557..839288f753cc 100644
>> --- a/scripts/imx/imx-usb-loader.c
>> +++ b/scripts/imx/imx-usb-loader.c
>> @@ -46,10 +46,6 @@
>>   #define FT_DCD    0xee
>>   #define FT_LOAD_ONLY    0x00
>>   -#ifndef UTS_RELEASE
>> -#define UTS_RELEASE "unknown"
>> -#endif
>> -
>>   /*
>>    * comment from libusb:
>>    * As per the USB 3.0 specs, the current maximum limit for the depth is 7.
>> @@ -1530,15 +1526,9 @@ static void usage(const char *prgname)
>>           "-p <devpath> Specify device path: <bus>-<port>[.<port>]...\n"
>>           "-s           skip DCD included in image\n"
>>           "-v           verbose (give multiple times to increase)\n"
>> -        "--version    display version number\n"
>>           "-h           this help\n", prgname);
>>   }
>>   -static void version(const char *prgname)
>> -{
>> -    fprintf(stderr, "%s %s\n", prgname, UTS_RELEASE);
>> -}
>> -
>>   int main(int argc, char *argv[])
>>   {
>>       libusb_device **devs;
>> @@ -1554,20 +1544,10 @@ int main(int argc, char *argv[])
>>       char *initfile = NULL;
>>       char *devpath = NULL;
>>       char *devtype = NULL;
>> -    int opt_version = 0;
>> -    struct option long_options[] = {
>> -        {"version", no_argument, &opt_version, 1},
>> -        { }
>> -    };
>>         w.do_dcd_once = 1;
>>   -    while ((opt = getopt_long(argc, argv, "cvhd:i:p:s", long_options, 
>> NULL)) != -1) {
>> -        if (opt_version) {
>> -            version(argv[0]);
>> -            exit(EXIT_SUCCESS);
>> -        }
>> -
>> +    while ((opt = getopt(argc, argv, "cvhd:i:p:s")) != -1) {
>>           switch (opt) {
>>           case 'c':
>>               verify = 1;
> 

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |


Reply via email to