[PATCH 1/3] Makefile: clean auto-generated *.c files properly

[Ben Dooks]

On 29/04/2024 09:52, Andrew Jones wrote:
> On Fri, Apr 26, 2024 at 05:25:12PM GMT, Ben Dooks wrote:
>> On 23/04/2024 16:41, Ivan Orlov wrote:
>>> On 4/23/24 16:26, Ben Dooks wrote:
>>>> On 23/04/2024 16:20, Ben Dooks wrote:
>>>>> On 23/04/2024 15:58, Ivan Orlov wrote:
>>>>>> On 4/22/24 16:19, Andrew Jones wrote:
>>>>>>> On Mon, Apr 01, 2024 at 10:34:36PM +0100, Ivan Orlov wrote:
>>>>>>>> Currently, `make clean` doesn't remove auto-generated .c files in the
>>>>>>>> `build/` directory. It means that we don't have a reliable way of
>>>>>>>> regenerating these files except from removing the `build/` directory
>>>>>>>> manually.
>>>>>>>>
>>>>>>>> Update the `clean` target in order to remove these files as well.
>>>>>>>>
>>>>>>>> In the discussion of the "[PATCH v2 3/5] Makefile: clean '.c' files
>>>>>>>> generated by carray", Andrew Jones
>>>>>>>> <ajones at ventanamicro.com> suggested
>>>>>>>> placing the auto-generated .c files into the
>>>>>>>> `build/generated/` folder.
>>>>>>>> However, I believe it may not be necessary as in
>>>>>>>> fact all of the files
>>>>>>>> in `build/` are auto-generated.
>>>>>>>
>>>>>>> Since the Makefile enforces that the build dir is not the same as the
>>>>>>> source dir and the only C files we currently generate
>>>>>>> are carray files,
>>>>>>> then OK. I still think it would be nice to be more specific about what
>>>>>>> we clean, though.
>>>>>>>
>>>>>>
>>>>>> Hi Andrew,
>>>>>>
>>>>>> Thank you very much for the review!
>>>>>>
>>>>>> I see a few approaches how we could make the
>>>>>> CArray-generated files cleaning more clear. I believe we
>>>>>> could either put all of the CArray-generated files into a
>>>>>> subdirectory of `build/` (as you suggested) or add a suffix
>>>>>> to a filename of an auto-generated .c file (for instance,
>>>>>> sbi_unit_tests.carray -> sbi_unit_tests_carray.c, and the
>>>>>> pattern for `make clean` would be like "rm -rf
>>>>>> build/*_carray.c").
>>>>>>
>>>>>> The former would probably need significant update of the
>>>>>> Makefile. The latter, on the other hand, seems more flaky...
>>>>>> What do you think of that?
>>>>>
>>>>> I did a quick shell command to find all the object basenames, via:
>>>>>
>>>>> $ find . -type f -name "*.carray" | xargs grep NAME | cut -d ' ' -f 2
>>>>> sbi_unit_tests
>>>>> sbi_ecall_exts
>>>>> fdt_irqchip_drivers
>>>>> fdt_timer_drivers
>>>>> fdt_serial_drivers
>>>>> fdt_i2c_adapter_drivers
>>>>> fdt_ipi_drivers
>>>>> fdt_gpio_drivers
>>>>> fdt_regmap_drivers
>>>>> fdt_reset_drivers
>>>>> platform_override_modules
>>>>>
>>>>> so doing:
>>>>>
>>>>> $ find . -type f -name "*.carray" | xargs grep NAME | cut -d ' '
>>>>> -f 2 | sed  's/$/.o/g'  | xargs -n1 find build -name
>>>>> build/lib/sbi/sbi_ecall_exts.o
>>>>> build/platform/generic/lib/utils/irqchip/fdt_irqchip_drivers.o
>>>>> build/platform/generic/lib/utils/timer/fdt_timer_drivers.o
>>>>> build/platform/generic/lib/utils/serial/fdt_serial_drivers.o
>>>>> build/platform/generic/lib/utils/i2c/fdt_i2c_adapter_drivers.o
>>>>> build/platform/generic/lib/utils/ipi/fdt_ipi_drivers.o
>>>>> build/platform/generic/lib/utils/gpio/fdt_gpio_drivers.o
>>>>> build/platform/generic/lib/utils/regmap/fdt_regmap_drivers.o
>>>>> build/platform/generic/lib/utils/reset/fdt_reset_drivers.o
>>>>> build/platform/generic/platform_override_modules.o
>>>>>
>>>>> finds all the carray build files
>>>>
>>>> of course, i meant  sed -e 's/$/.c/g' to find the .c files not their
>>>> outputs which would have been removed anyway
>>>>
>>>
>>> Sounds like an another solution, thanks! I'm not sure if it should be
>>> done now, though... There is a chance that it could make the Makefile
>>> less readable. But it is definitely more clean than removing the
>>> entirety of build/.c files
>>
>> I made this patch to try and go through all the .carray generated
>> files in build and remove them. I'll submit it if people agree that
>> it is a reasonable idea:
> 
> I prefer changing the name of the generated C file to include 'carray'
> and then just using RM *.carray.c
> 
>>
>>  From 7e1f02ebdd168a329a9a393f3a25c74d6b5f837d Mon Sep 17 00:00:00 2001
>> From: Ben Dooks <ben.dooks at codethink.co.uk>
>> Date: Tue, 23 Apr 2024 17:57:42 +0100
>> Subject: [PATCH] make: remove carray generated files via new script
>>
>> Create a script to find the .carray generated files and allow them to
>> be removed during make clean.
>>
>> Signed-off-by: Ben Dooks <ben.dooks at codethink.co.uk>
>> ---
>>   Makefile             |  3 +++
>>   scripts/rm_carray.sh | 48 ++++++++++++++++++++++++++++++++++++++++++++
>>   2 files changed, 51 insertions(+)
>>   create mode 100755 scripts/rm_carray.sh
>>
>> diff --git a/Makefile b/Makefile
>> index 7df39b4..7f70934 100644
>> --- a/Makefile
>> +++ b/Makefile
>> @@ -687,6 +687,9 @@ clean:
>>   	$(CMD_PREFIX)mkdir -p $(build_dir)
>>   	$(if $(V), @echo " RM        $(build_dir)/*.o")
>>   	$(CMD_PREFIX)find $(build_dir) -type f -name "*.o" -exec rm -rf {} +
>> +	$(if $(V), @echo " RM        $(build_dir)/*.o (carray)")
>> +	$(CMD_PREFIX)find  $(src_dir) -type f -name "*.carray" -exec
>> $(src_dir)/scripts/rm_carray.sh  $(src_dir) $(platform_build_dir) {} +
>> +	$(CMD_PREFIX)find  $(platform_src_dir) -type f -name "*.carray" -exec
>> $(src_dir)/scripts/rm_carray.sh  $(platform_src_dir) $(platform_build_dir)
> 
> The script could have the find embedded in it, allowing it to be run
> standalone.
> 
> Thanks,
> drew

At this point should we just get the patch for "scripts/carray.sh: Add 
comment to generated files" merged and debate the best way to deal with
the cleaning?

-- 
Ben Dooks				http://www.codethink.co.uk/
Senior Engineer				Codethink - Providing Genius

https://www.codethink.co.uk/privacy.html