[PATCH] scripts/update-linux-headers.sh: Remove temporary directory inbetween

Thomas Huth posted 1 patch 6 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20240527060243.12647-1-thuth@redhat.com
Maintainers: "Michael S. Tsirkin" <mst@redhat.com>, Cornelia Huck <cohuck@redhat.com>, Paolo Bonzini <pbonzini@redhat.com>
scripts/update-linux-headers.sh | 1 +
1 file changed, 1 insertion(+)
[PATCH] scripts/update-linux-headers.sh: Remove temporary directory inbetween
Posted by Thomas Huth 6 months ago
We are reusing the same temporary directory for installing the headers
of all targets, so there could be stale files here when switching from
one target to another. Make sure to delete the folder before installing
a new set of target headers into it.

Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 scripts/update-linux-headers.sh | 1 +
 1 file changed, 1 insertion(+)

diff --git a/scripts/update-linux-headers.sh b/scripts/update-linux-headers.sh
index 8963c39189..fbf7e119bc 100755
--- a/scripts/update-linux-headers.sh
+++ b/scripts/update-linux-headers.sh
@@ -112,6 +112,7 @@ for arch in $ARCHLIST; do
         arch_var=ARCH
     fi
 
+    rm -rf "$hdrdir"
     make -C "$linux" O="$blddir" INSTALL_HDR_PATH="$hdrdir" $arch_var=$arch headers_install
 
     rm -rf "$output/linux-headers/asm-$arch"
-- 
2.45.1
Re: [PATCH] scripts/update-linux-headers.sh: Remove temporary directory inbetween
Posted by Michael S. Tsirkin 6 months ago
On Mon, May 27, 2024 at 08:02:43AM +0200, Thomas Huth wrote:
> We are reusing the same temporary directory for installing the headers
> of all targets, so there could be stale files here when switching from
> one target to another. Make sure to delete the folder before installing
> a new set of target headers into it.
> 
> Signed-off-by: Thomas Huth <thuth@redhat.com>


Reviewed-by: Michael S. Tsirkin <mst@redhat.com>

who's merging this?

> ---
>  scripts/update-linux-headers.sh | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/scripts/update-linux-headers.sh b/scripts/update-linux-headers.sh
> index 8963c39189..fbf7e119bc 100755
> --- a/scripts/update-linux-headers.sh
> +++ b/scripts/update-linux-headers.sh
> @@ -112,6 +112,7 @@ for arch in $ARCHLIST; do
>          arch_var=ARCH
>      fi
>  
> +    rm -rf "$hdrdir"
>      make -C "$linux" O="$blddir" INSTALL_HDR_PATH="$hdrdir" $arch_var=$arch headers_install
>  
>      rm -rf "$output/linux-headers/asm-$arch"
> -- 
> 2.45.1
Re: [PATCH] scripts/update-linux-headers.sh: Remove temporary directory inbetween
Posted by Thomas Huth 6 months ago
On 28/05/2024 17.56, Michael S. Tsirkin wrote:
> On Mon, May 27, 2024 at 08:02:43AM +0200, Thomas Huth wrote:
>> We are reusing the same temporary directory for installing the headers
>> of all targets, so there could be stale files here when switching from
>> one target to another. Make sure to delete the folder before installing
>> a new set of target headers into it.
>>
>> Signed-off-by: Thomas Huth <thuth@redhat.com>
> 
> 
> Reviewed-by: Michael S. Tsirkin <mst@redhat.com>

Thanks!

> who's merging this?

I don't mind ... if nobody objects, I can put it into my next pull request.

  Thomas
Re: [PATCH] scripts/update-linux-headers.sh: Remove temporary directory inbetween
Posted by Cornelia Huck 6 months ago
On Mon, May 27 2024, Thomas Huth <thuth@redhat.com> wrote:

> We are reusing the same temporary directory for installing the headers
> of all targets, so there could be stale files here when switching from
> one target to another. Make sure to delete the folder before installing
> a new set of target headers into it.
>
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
>  scripts/update-linux-headers.sh | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/scripts/update-linux-headers.sh b/scripts/update-linux-headers.sh
> index 8963c39189..fbf7e119bc 100755
> --- a/scripts/update-linux-headers.sh
> +++ b/scripts/update-linux-headers.sh
> @@ -112,6 +112,7 @@ for arch in $ARCHLIST; do
>          arch_var=ARCH
>      fi
>  
> +    rm -rf "$hdrdir"
>      make -C "$linux" O="$blddir" INSTALL_HDR_PATH="$hdrdir" $arch_var=$arch headers_install
>  
>      rm -rf "$output/linux-headers/asm-$arch"

Hm. I presume that headers-install gives us the same set of headers
outside include/asm for every arch?
Re: [PATCH] scripts/update-linux-headers.sh: Remove temporary directory inbetween
Posted by Thomas Huth 6 months ago
On 27/05/2024 17.04, Cornelia Huck wrote:
> On Mon, May 27 2024, Thomas Huth <thuth@redhat.com> wrote:
> 
>> We are reusing the same temporary directory for installing the headers
>> of all targets, so there could be stale files here when switching from
>> one target to another. Make sure to delete the folder before installing
>> a new set of target headers into it.
>>
>> Signed-off-by: Thomas Huth <thuth@redhat.com>
>> ---
>>   scripts/update-linux-headers.sh | 1 +
>>   1 file changed, 1 insertion(+)
>>
>> diff --git a/scripts/update-linux-headers.sh b/scripts/update-linux-headers.sh
>> index 8963c39189..fbf7e119bc 100755
>> --- a/scripts/update-linux-headers.sh
>> +++ b/scripts/update-linux-headers.sh
>> @@ -112,6 +112,7 @@ for arch in $ARCHLIST; do
>>           arch_var=ARCH
>>       fi
>>   
>> +    rm -rf "$hdrdir"
>>       make -C "$linux" O="$blddir" INSTALL_HDR_PATH="$hdrdir" $arch_var=$arch headers_install
>>   
>>       rm -rf "$output/linux-headers/asm-$arch"
> 
> Hm. I presume that headers-install gives us the same set of headers
> outside include/asm for every arch?

I just double-checked and yes, apart from a file called "a.out.h", the other 
headers outside of the asm subfolder are the same. So AFAICS there is still 
a slight chance that we might pick up a wrong file from the asm folder... 
shall I change the patch to only delete that subfolder? Or is that chance 
just too small that we can ignore it?

  Thomas
Re: [PATCH] scripts/update-linux-headers.sh: Remove temporary directory inbetween
Posted by Cornelia Huck 6 months ago
On Mon, May 27 2024, Thomas Huth <thuth@redhat.com> wrote:

> On 27/05/2024 17.04, Cornelia Huck wrote:
>> On Mon, May 27 2024, Thomas Huth <thuth@redhat.com> wrote:
>> 
>>> We are reusing the same temporary directory for installing the headers
>>> of all targets, so there could be stale files here when switching from
>>> one target to another. Make sure to delete the folder before installing
>>> a new set of target headers into it.
>>>
>>> Signed-off-by: Thomas Huth <thuth@redhat.com>
>>> ---
>>>   scripts/update-linux-headers.sh | 1 +
>>>   1 file changed, 1 insertion(+)
>>>
>>> diff --git a/scripts/update-linux-headers.sh b/scripts/update-linux-headers.sh
>>> index 8963c39189..fbf7e119bc 100755
>>> --- a/scripts/update-linux-headers.sh
>>> +++ b/scripts/update-linux-headers.sh
>>> @@ -112,6 +112,7 @@ for arch in $ARCHLIST; do
>>>           arch_var=ARCH
>>>       fi
>>>   
>>> +    rm -rf "$hdrdir"
>>>       make -C "$linux" O="$blddir" INSTALL_HDR_PATH="$hdrdir" $arch_var=$arch headers_install
>>>   
>>>       rm -rf "$output/linux-headers/asm-$arch"
>> 
>> Hm. I presume that headers-install gives us the same set of headers
>> outside include/asm for every arch?
>
> I just double-checked and yes, apart from a file called "a.out.h", the other 
> headers outside of the asm subfolder are the same. So AFAICS there is still 
> a slight chance that we might pick up a wrong file from the asm folder... 
> shall I change the patch to only delete that subfolder? Or is that chance 
> just too small that we can ignore it?

The asm folder is probably fine, but there's a chance for errors; if
moving from "use union of files for all archs outside asm" to "use files
for the last arch in the list outside asm" always results in the same
set of files, your patch is probably the better choice here.

Acked-by: Cornelia Huck <cohuck@redhat.com>