[PATCH 0/3] CArray improvements

[PATCH 0/3] CArray improvements

[Ivan Orlov]

Currently, there are a few inconveniences the developer may face when
using CArrays, including:

- Unability to regenerate the carray-related .c files reliably without
removing the `build/` directory
- Confusion caused by source (.c) files in the `build/` directory (the
fact that some file was generated by carray.sh is not obvious)

This patch series fixes them.

Ivan Orlov (3):
  Makefile: clean auto-generated *.c files properly
  scripts/carray.sh: Add comment to generated files
  docs: writing tests: update cleaning instructions

 Makefile              | 2 ++
 docs/writing_tests.md | 4 +---
 scripts/carray.sh     | 1 +
 3 files changed, 4 insertions(+), 3 deletions(-)

-- 
2.34.1

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

[Ivan Orlov]

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(+)

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

[PATCH 2/3] scripts/carray.sh: Add comment to generated files

[Ivan Orlov]

Add a comment about where auto-generated file came from to the carray.sh
output. This should help avoiding confusion for the developers looking
at the build artifacts and finding .c files there.

Signed-off-by: Ivan Orlov <ivan.orlov0322@gmail.com>
---
 scripts/carray.sh | 1 +
 1 file changed, 1 insertion(+)

diff --git a/scripts/carray.sh b/scripts/carray.sh
index 13f9d36..1fa2366 100755
--- a/scripts/carray.sh
+++ b/scripts/carray.sh
@@ -61,6 +61,7 @@ if [ -z "${ARRAY_NAME}" ]; then
 	usage
 fi
 
+printf "// Generated with $(basename $0) from $(basename ${CONFIG_FILE})\n"
 printf "#include <%s>\n\n" "${TYPE_HEADER}"
 
 for VAR in ${VAR_LIST}; do
-- 
2.34.1

[PATCH 3/3] docs: writing tests: update cleaning instructions

[Ivan Orlov]

After the changes introduced by the previous patches are applied, there
is no need of the manual removal of the `build/` directory every time
new test is added. Running `make clean` should be enough to regenerate
the carray-related files.

Update the documentation correspondingly.

Signed-off-by: Ivan Orlov <ivan.orlov0322@gmail.com>
---
 docs/writing_tests.md | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/docs/writing_tests.md b/docs/writing_tests.md
index 0fd15d6..cee4c4d 100644
--- a/docs/writing_tests.md
+++ b/docs/writing_tests.md
@@ -57,9 +57,7 @@ carray-sbi_unit_tests-$(CONFIG_SBIUNIT) += string_test_suite
 libsbi-objs-$(CONFIG_SBIUNIT) += tests/sbi_string_test.o
 ```
 
-If you compiled OpenSBI with CONFIG_SBIUNIT enabled before, you may need to
-manually remove the build folder in order to regenerate the carray files:
-`rm -rf build/`.
+Now, run `make clean` in order to regenerate the carray-related files.
 
 Recompile OpenSBI with the CONFIG_SBIUNIT option enabled and run it in QEMU.
 You will see something like this:
-- 
2.34.1

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

[Andrew Jones]

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>

[PATCH 2/3] scripts/carray.sh: Add comment to generated files

[Andrew Jones]

On Mon, Apr 01, 2024 at 10:34:37PM +0100, Ivan Orlov wrote:
> Add a comment about where auto-generated file came from to the carray.sh
> output. This should help avoiding confusion for the developers looking
> at the build artifacts and finding .c files there.
> 
> Signed-off-by: Ivan Orlov <ivan.orlov0322@gmail.com>
> ---
>  scripts/carray.sh | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/scripts/carray.sh b/scripts/carray.sh
> index 13f9d36..1fa2366 100755
> --- a/scripts/carray.sh
> +++ b/scripts/carray.sh
> @@ -61,6 +61,7 @@ if [ -z "${ARRAY_NAME}" ]; then
>  	usage
>  fi
>  
> +printf "// Generated with $(basename $0) from $(basename ${CONFIG_FILE})\n"
>  printf "#include <%s>\n\n" "${TYPE_HEADER}"
>  
>  for VAR in ${VAR_LIST}; do
> -- 
> 2.34.1
>

Reviewed-by: Andrew Jones <ajones@ventanamicro.com>

[PATCH 3/3] docs: writing tests: update cleaning instructions

[Andrew Jones]

On Mon, Apr 01, 2024 at 10:34:38PM +0100, Ivan Orlov wrote:
> After the changes introduced by the previous patches are applied, there
> is no need of the manual removal of the `build/` directory every time
> new test is added. Running `make clean` should be enough to regenerate
> the carray-related files.
> 
> Update the documentation correspondingly.
> 
> Signed-off-by: Ivan Orlov <ivan.orlov0322@gmail.com>
> ---
>  docs/writing_tests.md | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/docs/writing_tests.md b/docs/writing_tests.md
> index 0fd15d6..cee4c4d 100644
> --- a/docs/writing_tests.md
> +++ b/docs/writing_tests.md
> @@ -57,9 +57,7 @@ carray-sbi_unit_tests-$(CONFIG_SBIUNIT) += string_test_suite
>  libsbi-objs-$(CONFIG_SBIUNIT) += tests/sbi_string_test.o
>  ```
>  
> -If you compiled OpenSBI with CONFIG_SBIUNIT enabled before, you may need to
> -manually remove the build folder in order to regenerate the carray files:
> -`rm -rf build/`.
> +Now, run `make clean` in order to regenerate the carray-related files.
>  
>  Recompile OpenSBI with the CONFIG_SBIUNIT option enabled and run it in QEMU.
>  You will see something like this:
> -- 
> 2.34.1
>

Reviewed-by: Andrew Jones <ajones@ventanamicro.com>

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

[Ivan Orlov]

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?

-- 
Kind regards,
Ivan Orlov

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

[Ben Dooks]

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?

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

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

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

[Andrew Jones]

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

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

[Ben Dooks]

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

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

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

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

[Ben Dooks]

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...


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

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

[PATCH 2/3] scripts/carray.sh: Add comment to generated files

[Ben Dooks]

On 01/04/2024 22:34, Ivan Orlov wrote:
> Add a comment about where auto-generated file came from to the carray.sh
> output. This should help avoiding confusion for the developers looking
> at the build artifacts and finding .c files there.
> 
> Signed-off-by: Ivan Orlov <ivan.orlov0322@gmail.com>
> ---
>   scripts/carray.sh | 1 +
>   1 file changed, 1 insertion(+)
> 
> diff --git a/scripts/carray.sh b/scripts/carray.sh
> index 13f9d36..1fa2366 100755
> --- a/scripts/carray.sh
> +++ b/scripts/carray.sh
> @@ -61,6 +61,7 @@ if [ -z "${ARRAY_NAME}" ]; then
>   	usage
>   fi
>   
> +printf "// Generated with $(basename $0) from $(basename ${CONFIG_FILE})\n"
>   printf "#include <%s>\n\n" "${TYPE_HEADER}"
>   
>   for VAR in ${VAR_LIST}; do

minor nit, "Auto generated" might be a better comment start.

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

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

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

[Ben Dooks]

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

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

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

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

[Ivan Orlov]

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
-- 
Kind regards,
Ivan Orlov

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

[Ben Dooks]

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
-- 
2.37.2.352.g3c44437643



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

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

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

[Andrew Jones]

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

[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@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?

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

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

[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@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
> 

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

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

[PATCH 0/3] CArray improvements

[Anup Patel]

Hi Ivan,

On Tue, Apr 2, 2024 at 3:04?AM Ivan Orlov <ivan.orlov0322@gmail.com> wrote:
>
> Currently, there are a few inconveniences the developer may face when
> using CArrays, including:
>
> - Unability to regenerate the carray-related .c files reliably without
> removing the `build/` directory
> - Confusion caused by source (.c) files in the `build/` directory (the
> fact that some file was generated by carray.sh is not obvious)
>
> This patch series fixes them.
>
> Ivan Orlov (3):
>   Makefile: clean auto-generated *.c files properly
>   scripts/carray.sh: Add comment to generated files
>   docs: writing tests: update cleaning instructions

I assume you (or someone else) will be sending v2 of this series ?

Regards,
Anup

>
>  Makefile              | 2 ++
>  docs/writing_tests.md | 4 +---
>  scripts/carray.sh     | 1 +
>  3 files changed, 4 insertions(+), 3 deletions(-)
>
> --
> 2.34.1
>

[PATCH 0/3] CArray improvements

[Ivan Orlov]

On 5/7/24 07:06, Anup Patel wrote:
> Hi Ivan,
> 
> On Tue, Apr 2, 2024 at 3:04?AM Ivan Orlov <ivan.orlov0322@gmail.com> wrote:
>>
>> Currently, there are a few inconveniences the developer may face when
>> using CArrays, including:
>>
>> - Unability to regenerate the carray-related .c files reliably without
>> removing the `build/` directory
>> - Confusion caused by source (.c) files in the `build/` directory (the
>> fact that some file was generated by carray.sh is not obvious)
>>
>> This patch series fixes them.
>>
>> Ivan Orlov (3):
>>    Makefile: clean auto-generated *.c files properly
>>    scripts/carray.sh: Add comment to generated files
>>    docs: writing tests: update cleaning instructions
> 
> I assume you (or someone else) will be sending v2 of this series ?
> 

Hi Anup,

Sorry for the late reply. I believe Ben Dooks 
<ben.dooks@codethink.co.uk> is going to send the V2 with the updated 
carray cleanup :)

Thank you!

-- 
Kind regards,
Ivan Orlov