| Message ID | 20240401213438.590209-2-ivan.orlov0322@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | CArray improvements | expand |
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@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. > > Signed-off-by: Ivan Orlov <ivan.orlov0322@gmail.com> > --- > Makefile | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/Makefile b/Makefile > index 680c19a..4519277 100644 > --- a/Makefile > +++ b/Makefile > @@ -684,6 +684,8 @@ clean: > $(CMD_PREFIX)find $(build_dir) -type f -name "*.bin" -exec rm -rf {} + > $(if $(V), @echo " RM $(build_dir)/*.dtb") > $(CMD_PREFIX)find $(build_dir) -type f -name "*.dtb" -exec rm -rf {} + > + $(if $(V), @echo " RM $(build_dir)/*.c") > + $(CMD_PREFIX)find $(build_dir) -type f -name "*.c" -exec rm -rf {} + > > # Rule for "make distclean" > .PHONY: distclean > -- > 2.34.1 > Reviewed-by: Andrew Jones <ajones@ventanamicro.com>
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@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?
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@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? How about using find to look for any .carray files and then work out what the build filename to clean is from them?
On Tue, Apr 23, 2024 at 03:58:26PM +0100, 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@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? The filename change idea crossed my mind, but, when I saw we currently don't generate any other types of C files, I didn't think it'd be worth the churn [yet]. Thanks, drew
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@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
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@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, it would probably be easier to look through build for any .c files that contain the word Generated if the change to add a comment to the header is added...
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@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
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@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
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@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: From 7e1f02ebdd168a329a9a393f3a25c74d6b5f837d Mon Sep 17 00:00:00 2001 From: Ben Dooks <ben.dooks@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@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) {} + $(if $(V), @echo " RM $(build_dir)/*.a") $(CMD_PREFIX)find $(build_dir) -type f -name "*.a" -exec rm -rf {} + $(if $(V), @echo " RM $(build_dir)/*.elf") diff --git a/scripts/rm_carray.sh b/scripts/rm_carray.sh new file mode 100755 index 0000000..4648115 --- /dev/null +++ b/scripts/rm_carray.sh @@ -0,0 +1,48 @@ +#!/usr/bin/env bash + +function usage() +{ + echo "Usage:" + echo "$0 [src-dir] [build-dir] <carray files>" + exit 1; +} + +for i in "$*"; do echo $i; echo "..."; done + +SRC_DIR=$1 +shift +BUILD_DIR=$1 +shift + +if [ -z "${SRC_DIR}" ]; then + echo "No source directory specified" + usage +fi + +if [ -z "${BUILD_DIR}" ]; then + echo "No build directory specified" + usage +fi + +if [ "$#" -lt 1 ]; then + echo "No carray files to process" + usage +fi + +while [ "$#" -ge 1 ]; do + FILE=$1 + shift + + DIR_NAME=$(dirname $FILE | sed -e "s@{BUILD_DIR}/@@g") + FILE_NAME=`grep NAME $FILE | cut -d " " -f 2 | sed -e "s/$$/.c/g"` + NAME=${BUILD_DIR}/${DIR_NAME}/${FILE_NAME} + + echo ${NAME} + if [ -e ${NAME} ]; then + rm ${NAME} + if [ $? -ne 0 ]; then + echo "Failed to remove ${NAME}" + exit 1 + fi + fi +done
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@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@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@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
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@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@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@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?
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@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 Either Ivan or I will try and get a look at this, not sure how much will end up getting changed? > >> >> From 7e1f02ebdd168a329a9a393f3a25c74d6b5f837d Mon Sep 17 00:00:00 2001 >> From: Ben Dooks <ben.dooks@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@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 >
diff --git a/Makefile b/Makefile index 680c19a..4519277 100644 --- a/Makefile +++ b/Makefile @@ -684,6 +684,8 @@ clean: $(CMD_PREFIX)find $(build_dir) -type f -name "*.bin" -exec rm -rf {} + $(if $(V), @echo " RM $(build_dir)/*.dtb") $(CMD_PREFIX)find $(build_dir) -type f -name "*.dtb" -exec rm -rf {} + + $(if $(V), @echo " RM $(build_dir)/*.c") + $(CMD_PREFIX)find $(build_dir) -type f -name "*.c" -exec rm -rf {} + # Rule for "make distclean" .PHONY: distclean
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@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. Signed-off-by: Ivan Orlov <ivan.orlov0322@gmail.com> --- Makefile | 2 ++ 1 file changed, 2 insertions(+)