[Buildroot] [PATCH] linux: handle read-only dts files

[Buildroot] [PATCH] linux: handle read-only dts files

[Hollis Blanchard]

Some fine version control systems make all files read-only. The custom DTS file
may therefore be read-only, and that permission is preserved when copying into
the Linux build directory. A subsequent rebuild tries to 'cp' again, which
fails with a "Permission denied" error unless the -f option is used.

Signed-off-by: Hollis Blanchard <hollis_blanchard@mentor.com>
---
 linux/linux.mk | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/linux/linux.mk b/linux/linux.mk
index 375a9e1..a4c4733 100644
--- a/linux/linux.mk
+++ b/linux/linux.mk
@@ -270,7 +270,7 @@ endif
 # configuration has changed.
 define LINUX_BUILD_CMDS
 	$(if $(BR2_LINUX_KERNEL_USE_CUSTOM_DTS),
-		cp $(call qstrip,$(BR2_LINUX_KERNEL_CUSTOM_DTS_PATH)) $(KERNEL_ARCH_PATH)/boot/dts/)
+		cp -f $(call qstrip,$(BR2_LINUX_KERNEL_CUSTOM_DTS_PATH)) $(KERNEL_ARCH_PATH)/boot/dts/)
 	$(LINUX_MAKE_ENV) $(MAKE) $(LINUX_MAKE_FLAGS) -C $(@D) $(LINUX_TARGET_NAME)
 	@if grep -q "CONFIG_MODULES=y" $(@D)/.config; then 	\
 		$(LINUX_MAKE_ENV) $(MAKE) $(LINUX_MAKE_FLAGS) -C $(@D) modules ;	\
-- 
2.3.2

[Buildroot] [PATCH] linux: handle read-only dts files

[Nikolay Dimitrov]

Hi Holis,

On 08/12/2015 03:11 AM, Hollis Blanchard wrote:
> Some fine version control systems make all files read-only. The custom DTS file
> may therefore be read-only, and that permission is preserved when copying into
> the Linux build directory. A subsequent rebuild tries to 'cp' again, which
> fails with a "Permission denied" error unless the -f option is used.
>
> Signed-off-by: Hollis Blanchard <hollis_blanchard@mentor.com>
> ---
>   linux/linux.mk | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/linux/linux.mk b/linux/linux.mk
> index 375a9e1..a4c4733 100644
> --- a/linux/linux.mk
> +++ b/linux/linux.mk
> @@ -270,7 +270,7 @@ endif
>   # configuration has changed.
>   define LINUX_BUILD_CMDS
>   	$(if $(BR2_LINUX_KERNEL_USE_CUSTOM_DTS),
> -		cp $(call qstrip,$(BR2_LINUX_KERNEL_CUSTOM_DTS_PATH)) $(KERNEL_ARCH_PATH)/boot/dts/)
> +		cp -f $(call qstrip,$(BR2_LINUX_KERNEL_CUSTOM_DTS_PATH)) $(KERNEL_ARCH_PATH)/boot/dts/)
>   	$(LINUX_MAKE_ENV) $(MAKE) $(LINUX_MAKE_FLAGS) -C $(@D) $(LINUX_TARGET_NAME)
>   	@if grep -q "CONFIG_MODULES=y" $(@D)/.config; then 	\
>   		$(LINUX_MAKE_ENV) $(MAKE) $(LINUX_MAKE_FLAGS) -C $(@D) modules ;	\
>

Is there a reason for the custom DTS to exist in both places?

Regards,
Nikolay

[Buildroot] [PATCH] linux: handle read-only dts files

[Hollis Blanchard]

The DTS exists at $(BR2_LINUX_KERNEL_CUSTOM_DTS_PATH) because that's 
where it lives. It's copied to $(KERNEL_ARCH_PATH)/boot/dts/ so that the 
kernel build process can build a DTB out of it (which is later copied to 
build/images/).

Or did I misunderstand the question?

Hollis Blanchard
Mentor Graphics Emulation Division

On 08/12/2015 07:58 AM, Nikolay Dimitrov wrote:
> Hi Holis,
>
> On 08/12/2015 03:11 AM, Hollis Blanchard wrote:
>> Some fine version control systems make all files read-only. The 
>> custom DTS file
>> may therefore be read-only, and that permission is preserved when 
>> copying into
>> the Linux build directory. A subsequent rebuild tries to 'cp' again, 
>> which
>> fails with a "Permission denied" error unless the -f option is used.
>>
>> Signed-off-by: Hollis Blanchard <hollis_blanchard@mentor.com>
>> ---
>>   linux/linux.mk | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/linux/linux.mk b/linux/linux.mk
>> index 375a9e1..a4c4733 100644
>> --- a/linux/linux.mk
>> +++ b/linux/linux.mk
>> @@ -270,7 +270,7 @@ endif
>>   # configuration has changed.
>>   define LINUX_BUILD_CMDS
>>       $(if $(BR2_LINUX_KERNEL_USE_CUSTOM_DTS),
>> -        cp $(call qstrip,$(BR2_LINUX_KERNEL_CUSTOM_DTS_PATH)) 
>> $(KERNEL_ARCH_PATH)/boot/dts/)
>> +        cp -f $(call qstrip,$(BR2_LINUX_KERNEL_CUSTOM_DTS_PATH)) 
>> $(KERNEL_ARCH_PATH)/boot/dts/)
>>       $(LINUX_MAKE_ENV) $(MAKE) $(LINUX_MAKE_FLAGS) -C $(@D) 
>> $(LINUX_TARGET_NAME)
>>       @if grep -q "CONFIG_MODULES=y" $(@D)/.config; then     \
>>           $(LINUX_MAKE_ENV) $(MAKE) $(LINUX_MAKE_FLAGS) -C $(@D) 
>> modules ;    \
>>
>
> Is there a reason for the custom DTS to exist in both places?
>
> Regards,
> Nikolay

[Buildroot] [PATCH] linux: handle read-only dts files

[Nikolay Dimitrov]

Hi Hollis,

On 08/12/2015 07:02 PM, Hollis Blanchard wrote:
> The DTS exists at $(BR2_LINUX_KERNEL_CUSTOM_DTS_PATH) because that's
> where it lives. It's copied to $(KERNEL_ARCH_PATH)/boot/dts/ so that the
> kernel build process can build a DTB out of it (which is later copied to
> build/images/).
>
> Or did I misunderstand the question?

Sorry, I could've been probably more clear. My question was whether the
DTS needs to exist also at the destination ($(KERNEL_ARCH_PATH)/boot
/dts/) before the copying happens.

If it doesn't exist at the destination, the VCS wouldn't have checked
it out as r/o file, and the issue wouldn't happen in the first place
(e.g. the copy operation will succeed). Also, when we have 2 files with
the same name and content, this tends to create confusion with
developers (aka which file was the primary one).

>
> Hollis Blanchard
> Mentor Graphics Emulation Division
>
> On 08/12/2015 07:58 AM, Nikolay Dimitrov wrote:
>> Hi Holis,
>>
>> On 08/12/2015 03:11 AM, Hollis Blanchard wrote:
>>> Some fine version control systems make all files read-only. The
>>> custom DTS file
>>> may therefore be read-only, and that permission is preserved when
>>> copying into
>>> the Linux build directory. A subsequent rebuild tries to 'cp' again,
>>> which
>>> fails with a "Permission denied" error unless the -f option is used.
>>>
>>> Signed-off-by: Hollis Blanchard <hollis_blanchard@mentor.com>
>>> ---
>>>   linux/linux.mk | 2 +-
>>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/linux/linux.mk b/linux/linux.mk
>>> index 375a9e1..a4c4733 100644
>>> --- a/linux/linux.mk
>>> +++ b/linux/linux.mk
>>> @@ -270,7 +270,7 @@ endif
>>>   # configuration has changed.
>>>   define LINUX_BUILD_CMDS
>>>       $(if $(BR2_LINUX_KERNEL_USE_CUSTOM_DTS),
>>> -        cp $(call qstrip,$(BR2_LINUX_KERNEL_CUSTOM_DTS_PATH))
>>> $(KERNEL_ARCH_PATH)/boot/dts/)
>>> +        cp -f $(call qstrip,$(BR2_LINUX_KERNEL_CUSTOM_DTS_PATH))
>>> $(KERNEL_ARCH_PATH)/boot/dts/)
>>>       $(LINUX_MAKE_ENV) $(MAKE) $(LINUX_MAKE_FLAGS) -C $(@D)
>>> $(LINUX_TARGET_NAME)
>>>       @if grep -q "CONFIG_MODULES=y" $(@D)/.config; then     \
>>>           $(LINUX_MAKE_ENV) $(MAKE) $(LINUX_MAKE_FLAGS) -C $(@D)
>>> modules ;    \
>>>
>>
>> Is there a reason for the custom DTS to exist in both places?
>>
>> Regards,
>> Nikolay

Regards,
Nikolay

[Buildroot] [PATCH] linux: handle read-only dts files

[Hollis Blanchard]

On 08/12/2015 09:11 AM, Nikolay Dimitrov wrote:
> Hi Hollis,
>
> On 08/12/2015 07:02 PM, Hollis Blanchard wrote:
>> The DTS exists at $(BR2_LINUX_KERNEL_CUSTOM_DTS_PATH) because that's
>> where it lives. It's copied to $(KERNEL_ARCH_PATH)/boot/dts/ so that the
>> kernel build process can build a DTB out of it (which is later copied to
>> build/images/).
>>
>> Or did I misunderstand the question?

(Just to be clear: $(BR2_LINUX_KERNEL_CUSTOM_DTS_PATH) is under version 
control, while $(KERNEL_ARCH_PATH), the build directory, is not.)

> Sorry, I could've been probably more clear. My question was whether the
> DTS needs to exist also at the destination ($(KERNEL_ARCH_PATH)/boot
> /dts/) before the copying happens.

It does not need to exist there. It's only there because of a previous 
cp in a previous build.

> If it doesn't exist at the destination, the VCS wouldn't have checked
> it out as r/o file, and the issue wouldn't happen in the first place
> (e.g. the copy operation will succeed). Also, when we have 2 files with
> the same name and content, this tends to create confusion with
> developers (aka which file was the primary one).

The file needs to exist at the destination at least temporarily, for the 
DTS->DTB translation. You could say "we should remove it afterwards", 
but a) that could be technically difficult in the face of ^C and kill 
-9, and b) you could say the same about zImage, vmlinux, etc... so we'd 
really be talking about a full "make clean", which seems like overkill.

Are you suggesting "rm -f; cp" instead of "cp -f"? As far as I know the 
two are functionally equivalent, so the former seems like slightly more 
overhead to get the same effect as the latter.

cp -f seems a simple solution: try harder to copy the file. :-)

Hollis Blanchard
Mentor Graphics Emulation Division

[Buildroot] [PATCH] linux: handle read-only dts files

[Nikolay Dimitrov]

Hi Hollis,

On 08/12/2015 07:27 PM, Hollis Blanchard wrote:
> On 08/12/2015 09:11 AM, Nikolay Dimitrov wrote:
>> Hi Hollis,
>>
>> On 08/12/2015 07:02 PM, Hollis Blanchard wrote:
>>> The DTS exists at $(BR2_LINUX_KERNEL_CUSTOM_DTS_PATH) because that's
>>> where it lives. It's copied to $(KERNEL_ARCH_PATH)/boot/dts/ so that the
>>> kernel build process can build a DTB out of it (which is later copied to
>>> build/images/).
>>>
>>> Or did I misunderstand the question?
>
> (Just to be clear: $(BR2_LINUX_KERNEL_CUSTOM_DTS_PATH) is under version
> control, while $(KERNEL_ARCH_PATH), the build directory, is not.)
>
>> Sorry, I could've been probably more clear. My question was whether the
>> DTS needs to exist also at the destination ($(KERNEL_ARCH_PATH)/boot
>> /dts/) before the copying happens.
>
> It does not need to exist there. It's only there because of a previous
> cp in a previous build.

Ahh, now I see.

>
>> If it doesn't exist at the destination, the VCS wouldn't have checked
>> it out as r/o file, and the issue wouldn't happen in the first place
>> (e.g. the copy operation will succeed). Also, when we have 2 files with
>> the same name and content, this tends to create confusion with
>> developers (aka which file was the primary one).
>
> The file needs to exist at the destination at least temporarily, for the
> DTS->DTB translation. You could say "we should remove it afterwards",
> but a) that could be technically difficult in the face of ^C and kill
> -9, and b) you could say the same about zImage, vmlinux, etc... so we'd
> really be talking about a full "make clean", which seems like overkill.
>
> Are you suggesting "rm -f; cp" instead of "cp -f"? As far as I know the
> two are functionally equivalent, so the former seems like slightly more
> overhead to get the same effect as the latter.
>
> cp -f seems a simple solution: try harder to copy the file. :-)

Now your approach totally makes sense to me, thanks for taking your
time to explain it. I agree with your proposed fix.

>
> Hollis Blanchard
> Mentor Graphics Emulation Division

Kind regards,
Nikolay

[Buildroot] [PATCH] linux: handle read-only dts files

[Nikolay Dimitrov]

On 08/12/2015 03:11 AM, Hollis Blanchard wrote:
> Some fine version control systems make all files read-only. The custom DTS file
> may therefore be read-only, and that permission is preserved when copying into
> the Linux build directory. A subsequent rebuild tries to 'cp' again, which
> fails with a "Permission denied" error unless the -f option is used.
>
> Signed-off-by: Hollis Blanchard <hollis_blanchard@mentor.com>
> ---
>   linux/linux.mk | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/linux/linux.mk b/linux/linux.mk
> index 375a9e1..a4c4733 100644
> --- a/linux/linux.mk
> +++ b/linux/linux.mk
> @@ -270,7 +270,7 @@ endif
>   # configuration has changed.
>   define LINUX_BUILD_CMDS
>   	$(if $(BR2_LINUX_KERNEL_USE_CUSTOM_DTS),
> -		cp $(call qstrip,$(BR2_LINUX_KERNEL_CUSTOM_DTS_PATH)) $(KERNEL_ARCH_PATH)/boot/dts/)
> +		cp -f $(call qstrip,$(BR2_LINUX_KERNEL_CUSTOM_DTS_PATH)) $(KERNEL_ARCH_PATH)/boot/dts/)
>   	$(LINUX_MAKE_ENV) $(MAKE) $(LINUX_MAKE_FLAGS) -C $(@D) $(LINUX_TARGET_NAME)
>   	@if grep -q "CONFIG_MODULES=y" $(@D)/.config; then 	\
>   		$(LINUX_MAKE_ENV) $(MAKE) $(LINUX_MAKE_FLAGS) -C $(@D) modules ;	\
>

Acked-by: Nikolay Dimitrov <picmaster@mail.bg>

Regards,
Nikolay

[Buildroot] [PATCH] linux: handle read-only dts files

[Yann E. MORIN]

Hollis, All,

On 2015-08-11 17:11 -0700, Hollis Blanchard spake thusly:
> Some fine version control systems make all files read-only. The custom DTS file
> may therefore be read-only, and that permission is preserved when copying into
> the Linux build directory. A subsequent rebuild tries to 'cp' again, which
> fails with a "Permission denied" error unless the -f option is used.
> 
> Signed-off-by: Hollis Blanchard <hollis_blanchard@mentor.com>

Acked-by: "Yann E. MORIN" <yann.morin.1998@free.fr>

Regards,
Yann E. MORIN.

> ---
>  linux/linux.mk | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/linux/linux.mk b/linux/linux.mk
> index 375a9e1..a4c4733 100644
> --- a/linux/linux.mk
> +++ b/linux/linux.mk
> @@ -270,7 +270,7 @@ endif
>  # configuration has changed.
>  define LINUX_BUILD_CMDS
>  	$(if $(BR2_LINUX_KERNEL_USE_CUSTOM_DTS),
> -		cp $(call qstrip,$(BR2_LINUX_KERNEL_CUSTOM_DTS_PATH)) $(KERNEL_ARCH_PATH)/boot/dts/)
> +		cp -f $(call qstrip,$(BR2_LINUX_KERNEL_CUSTOM_DTS_PATH)) $(KERNEL_ARCH_PATH)/boot/dts/)
>  	$(LINUX_MAKE_ENV) $(MAKE) $(LINUX_MAKE_FLAGS) -C $(@D) $(LINUX_TARGET_NAME)
>  	@if grep -q "CONFIG_MODULES=y" $(@D)/.config; then 	\
>  		$(LINUX_MAKE_ENV) $(MAKE) $(LINUX_MAKE_FLAGS) -C $(@D) modules ;	\
> -- 
> 2.3.2
> 
> _______________________________________________
> buildroot mailing list
> buildroot at busybox.net
> http://lists.busybox.net/mailman/listinfo/buildroot

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'

[Buildroot] [PATCH] linux: handle read-only dts files

[Thomas Petazzoni]

Dear Hollis Blanchard,

On Tue, 11 Aug 2015 17:11:49 -0700, Hollis Blanchard wrote:
> Some fine version control systems make all files read-only. The custom DTS file
> may therefore be read-only, and that permission is preserved when copying into
> the Linux build directory. A subsequent rebuild tries to 'cp' again, which
> fails with a "Permission denied" error unless the -f option is used.
> 
> Signed-off-by: Hollis Blanchard <hollis_blanchard@mentor.com>
> ---
>  linux/linux.mk | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Applied, thanks.

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com