From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andrew Jones Date: Mon, 29 Apr 2024 10:52:02 +0200 Subject: [PATCH 1/3] Makefile: clean auto-generated *.c files properly In-Reply-To: <8f9ec7cd-0346-4f9e-919c-9b6e8f8a77c8@codethink.co.uk> References: <20240401213438.590209-1-ivan.orlov0322@gmail.com> <20240401213438.590209-2-ivan.orlov0322@gmail.com> <20240422-57f6d7d8e00b7b11cf364cb1@orel> <420f589f-78d2-4f16-a0ae-850488b5279c@gmail.com> <4ebdca94-8f4a-452f-9607-0029676b12b2@codethink.co.uk> <88d05469-2fd1-40df-b8bf-0fb6652ea76d@gmail.com> <8f9ec7cd-0346-4f9e-919c-9b6e8f8a77c8@codethink.co.uk> Message-ID: <20240429-8f8b7bfd81689a43c0b68be4@orel> List-Id: To: opensbi@lists.infradead.org MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit 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 > > > > > > > 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 > 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 > --- > 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