[PATCHv2 0/1] deb-pkg: Add device tree blobs to the package

[PATCHv2 0/1] deb-pkg: Add device tree blobs to the package

[Arnaud Patard]

Hi,

Looks like I've never sent the v2 of this patch, so let's try to correct this.


Thanks,
Arnaud

[PATCHv2 1/1] deb-pkg: Add device tree blobs to the package

[Arnaud Patard]

[-- Attachment #1: builddeb-install-dtbs-if-needed.patch --]
[-- Type: text/plain, Size: 951 bytes --]

When building a package with make deb-pkg (say, for arm), the dtb files are
not added to the package. Given that things are still evolving on arm, it
make sense to have them along with the kernel and modules.

v2: make use of dtbs_install

Signed-off-by: Arnaud Patard <arnaud.patard@rtp-net.org>
---

Index: linux-next/scripts/package/builddeb
===================================================================
--- linux-next.orig/scripts/package/builddeb	2015-01-14 13:04:45.845922441 +0100
+++ linux-next/scripts/package/builddeb	2015-01-14 13:19:26.121883720 +0100
@@ -143,6 +143,10 @@ else
 	cp arch/$ARCH/boot/$KBUILD_IMAGE "$tmpdir/$installed_image_path"
 fi
 
+if grep -q "^CONFIG_OF=y" .config ; then
+	make INSTALL_DTBS_PATH="$tmpdir/usr/lib/$packagename" dtbs_install
+fi
+
 if grep -q '^CONFIG_MODULES=y' $KCONFIG_CONFIG ; then
 	INSTALL_MOD_PATH="$tmpdir" $MAKE KBUILD_SRC= modules_install
 	rm -f "$tmpdir/lib/modules/$version/build"

Re: [PATCHv2 1/1] deb-pkg: Add device tree blobs to the package

[Ben Hutchings]

[-- Attachment #1: Type: text/plain, Size: 1383 bytes --]

[Please submit patches inline.]

On Wed, 2015-01-14 at 13:32 +0100, Arnaud Patard wrote:
> When building a package with make deb-pkg (say, for arm), the dtb files are
> not added to the package. Given that things are still evolving on arm, it
> make sense to have them along with the kernel and modules.
> 
> v2: make use of dtbs_install
> 
> Signed-off-by: Arnaud Patard <arnaud.patard@rtp-net.org>
> ---
> 
> Index: linux-next/scripts/package/builddeb
> ===================================================================
> --- linux-next.orig/scripts/package/builddeb    2015-01-14 13:04:45.845922441 +0100
> +++ linux-next/scripts/package/builddeb 2015-01-14 13:19:26.121883720 +0100
> @@ -143,6 +143,10 @@ else
>         cp arch/$ARCH/boot/$KBUILD_IMAGE "$tmpdir/$installed_image_path"
>  fi
>  
> +if grep -q "^CONFIG_OF=y" .config ; then
> +       make INSTALL_DTBS_PATH="$tmpdir/usr/lib/$packagename" dtbs_install
> +fi

Only arm and arm64 support that target.  You should maybe run something
like 'make -n dtbs_install >/dev/null 2>&1' first to check that the
target is defined.

Ben.

>  if grep -q '^CONFIG_MODULES=y' $KCONFIG_CONFIG ; then
>         INSTALL_MOD_PATH="$tmpdir" $MAKE KBUILD_SRC= modules_install
>         rm -f "$tmpdir/lib/modules/$version/build"
> 

-- 
Ben Hutchings
The world is coming to an end.	Please log off.

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 811 bytes --]

Re: [PATCHv2 1/1] deb-pkg: Add device tree blobs to the package

[Arnaud Patard]

Ben Hutchings <ben@decadent.org.uk> writes:

> [Please submit patches inline.]

I've no way to control the way quilt is sending mails.

>
> On Wed, 2015-01-14 at 13:32 +0100, Arnaud Patard wrote:
>> When building a package with make deb-pkg (say, for arm), the dtb files are
>> not added to the package. Given that things are still evolving on arm, it
>> make sense to have them along with the kernel and modules.
>> 
>> v2: make use of dtbs_install
>> 
>> Signed-off-by: Arnaud Patard <arnaud.patard@rtp-net.org>
>> ---
>> 
>> Index: linux-next/scripts/package/builddeb
>> ===================================================================
>> --- linux-next.orig/scripts/package/builddeb    2015-01-14 13:04:45.845922441 +0100
>> +++ linux-next/scripts/package/builddeb 2015-01-14 13:19:26.121883720 +0100
>> @@ -143,6 +143,10 @@ else
>>         cp arch/$ARCH/boot/$KBUILD_IMAGE "$tmpdir/$installed_image_path"
>>  fi
>>  
>> +if grep -q "^CONFIG_OF=y" .config ; then
>> +       make INSTALL_DTBS_PATH="$tmpdir/usr/lib/$packagename" dtbs_install
>> +fi
>
> Only arm and arm64 support that target.  You should maybe run something
> like 'make -n dtbs_install >/dev/null 2>&1' first to check that the
> target is defined.

There's a 'set -e' on top of the script so using make -n will likely
result in the script failing, which wouldn't be so nice imho.

From a quick untested guess, I see 2 way of solving this:

make INSTALL_DTBS_PATH="$tmpdir/usr/lib/$packagename" dtbs_install || /bin/true

or

set +e
make -n dtbs_install >/dev/null 2>&1
ret=$?
set -e
if [ $ret -eq 0 ]; then
   make INSTALL_DTBS_PATH="$tmpdir/usr/lib/$packagename" dtbs_install
fi

Any preference ?

Arnaud

Re: [PATCHv2 1/1] deb-pkg: Add device tree blobs to the package

[Ben Hutchings]

[-- Attachment #1: Type: text/plain, Size: 1831 bytes --]

On Tue, 2015-01-20 at 00:13 +0100, Arnaud Patard wrote:
> Ben Hutchings <ben@decadent.org.uk> writes:
> 
> > [Please submit patches inline.]
> 
> I've no way to control the way quilt is sending mails.
> 
> >
> > On Wed, 2015-01-14 at 13:32 +0100, Arnaud Patard wrote:
> >> When building a package with make deb-pkg (say, for arm), the dtb files are
> >> not added to the package. Given that things are still evolving on arm, it
> >> make sense to have them along with the kernel and modules.
> >> 
> >> v2: make use of dtbs_install
> >> 
> >> Signed-off-by: Arnaud Patard <arnaud.patard@rtp-net.org>
> >> ---
> >> 
> >> Index: linux-next/scripts/package/builddeb
> >> ===================================================================
> >> --- linux-next.orig/scripts/package/builddeb    2015-01-14 13:04:45.845922441 +0100
> >> +++ linux-next/scripts/package/builddeb 2015-01-14 13:19:26.121883720 +0100
> >> @@ -143,6 +143,10 @@ else
> >>         cp arch/$ARCH/boot/$KBUILD_IMAGE "$tmpdir/$installed_image_path"
> >>  fi
> >>  
> >> +if grep -q "^CONFIG_OF=y" .config ; then
> >> +       make INSTALL_DTBS_PATH="$tmpdir/usr/lib/$packagename" dtbs_install
> >> +fi
> >
> > Only arm and arm64 support that target.  You should maybe run something
> > like 'make -n dtbs_install >/dev/null 2>&1' first to check that the
> > target is defined.
> 
> There's a 'set -e' on top of the script so using make -n will likely
> result in the script failing, which wouldn't be so nice imho.
[...]

That's why you use it with the if statement:

# Only some architectures with OF support have this target
if make -n dtbs_install >/dev/null 2>&1; then
	make INSTALL_DTBS_PATH="$tmpdir/usr/lib/$packagename" dtbs_install
fi

Ben.

-- 
Ben Hutchings
Larkinson's Law: All laws are basically false.

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 811 bytes --]

Re: [PATCHv2 1/1] deb-pkg: Add device tree blobs to the package

[Fathi Boudra]

Hi,

On 20 January 2015 at 18:04, Ben Hutchings <ben@decadent.org.uk> wrote:
> On Tue, 2015-01-20 at 00:13 +0100, Arnaud Patard wrote:
>> Ben Hutchings <ben@decadent.org.uk> writes:
>>
>> > [Please submit patches inline.]
>>
>> I've no way to control the way quilt is sending mails.
>>
>> >
>> > On Wed, 2015-01-14 at 13:32 +0100, Arnaud Patard wrote:
>> >> When building a package with make deb-pkg (say, for arm), the dtb files are
>> >> not added to the package. Given that things are still evolving on arm, it
>> >> make sense to have them along with the kernel and modules.
>> >>
>> >> v2: make use of dtbs_install
>> >>
>> >> Signed-off-by: Arnaud Patard <arnaud.patard@rtp-net.org>
>> >> ---
>> >>
>> >> Index: linux-next/scripts/package/builddeb
>> >> ===================================================================
>> >> --- linux-next.orig/scripts/package/builddeb    2015-01-14 13:04:45.845922441 +0100
>> >> +++ linux-next/scripts/package/builddeb 2015-01-14 13:19:26.121883720 +0100
>> >> @@ -143,6 +143,10 @@ else
>> >>         cp arch/$ARCH/boot/$KBUILD_IMAGE "$tmpdir/$installed_image_path"
>> >>  fi
>> >>
>> >> +if grep -q "^CONFIG_OF=y" .config ; then
>> >> +       make INSTALL_DTBS_PATH="$tmpdir/usr/lib/$packagename" dtbs_install
>> >> +fi
>> >
>> > Only arm and arm64 support that target.  You should maybe run something
>> > like 'make -n dtbs_install >/dev/null 2>&1' first to check that the
>> > target is defined.
>>
>> There's a 'set -e' on top of the script so using make -n will likely
>> result in the script failing, which wouldn't be so nice imho.
> [...]
>
> That's why you use it with the if statement:
>
> # Only some architectures with OF support have this target
> if make -n dtbs_install >/dev/null 2>&1; then
>         make INSTALL_DTBS_PATH="$tmpdir/usr/lib/$packagename" dtbs_install
> fi

Would it be acceptable to have a fallback when dtbs_install isn't available?
It will allow to cover e.g. LTS 3.10 kernel which don't have the target

for example (i.e. can't be upstreamed as-is), I'm using this hack locally:
https://git.linaro.org/kernel/linux-linaro-tracking.git/blob/HEAD:/scripts/package/builddeb#l334

> Ben.

Cheers,
Fathi

Re: [PATCHv2 1/1] deb-pkg: Add device tree blobs to the package

[Ben Hutchings]

[-- Attachment #1: Type: text/plain, Size: 2731 bytes --]

On Thu, 2015-01-22 at 12:24 +0200, Fathi Boudra wrote:
> Hi,
> 
> On 20 January 2015 at 18:04, Ben Hutchings <ben@decadent.org.uk> wrote:
> > On Tue, 2015-01-20 at 00:13 +0100, Arnaud Patard wrote:
> >> Ben Hutchings <ben@decadent.org.uk> writes:
> >>
> >> > [Please submit patches inline.]
> >>
> >> I've no way to control the way quilt is sending mails.
> >>
> >> >
> >> > On Wed, 2015-01-14 at 13:32 +0100, Arnaud Patard wrote:
> >> >> When building a package with make deb-pkg (say, for arm), the dtb files are
> >> >> not added to the package. Given that things are still evolving on arm, it
> >> >> make sense to have them along with the kernel and modules.
> >> >>
> >> >> v2: make use of dtbs_install
> >> >>
> >> >> Signed-off-by: Arnaud Patard <arnaud.patard@rtp-net.org>
> >> >> ---
> >> >>
> >> >> Index: linux-next/scripts/package/builddeb
> >> >> ===================================================================
> >> >> --- linux-next.orig/scripts/package/builddeb    2015-01-14 13:04:45.845922441 +0100
> >> >> +++ linux-next/scripts/package/builddeb 2015-01-14 13:19:26.121883720 +0100
> >> >> @@ -143,6 +143,10 @@ else
> >> >>         cp arch/$ARCH/boot/$KBUILD_IMAGE "$tmpdir/$installed_image_path"
> >> >>  fi
> >> >>
> >> >> +if grep -q "^CONFIG_OF=y" .config ; then
> >> >> +       make INSTALL_DTBS_PATH="$tmpdir/usr/lib/$packagename" dtbs_install
> >> >> +fi
> >> >
> >> > Only arm and arm64 support that target.  You should maybe run something
> >> > like 'make -n dtbs_install >/dev/null 2>&1' first to check that the
> >> > target is defined.
> >>
> >> There's a 'set -e' on top of the script so using make -n will likely
> >> result in the script failing, which wouldn't be so nice imho.
> > [...]
> >
> > That's why you use it with the if statement:
> >
> > # Only some architectures with OF support have this target
> > if make -n dtbs_install >/dev/null 2>&1; then
> >         make INSTALL_DTBS_PATH="$tmpdir/usr/lib/$packagename" dtbs_install
> > fi
> 
> Would it be acceptable to have a fallback when dtbs_install isn't available?
> It will allow to cover e.g. LTS 3.10 kernel which don't have the target
> 
> for example (i.e. can't be upstreamed as-is), I'm using this hack locally:
> https://git.linaro.org/kernel/linux-linaro-tracking.git/blob/HEAD:/scripts/package/builddeb#l334

If there are any architectures that build .dtb files but don't have a
dtbs_install target, they should be fixed rather than adding a
workaround here.

If you want this in some other branch then you should get the addition
of the dtbs_install target backported.

Ben.

-- 
Ben Hutchings
I haven't lost my mind; it's backed up on tape somewhere.

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 811 bytes --]

Re: [PATCHv2 1/1] deb-pkg: Add device tree blobs to the package

[Michal Marek]

On 2015-01-20 17:04, Ben Hutchings wrote:
> On Tue, 2015-01-20 at 00:13 +0100, Arnaud Patard wrote:
>> Ben Hutchings <ben@decadent.org.uk> writes:
>>> Only arm and arm64 support that target.  You should maybe run something
>>> like 'make -n dtbs_install >/dev/null 2>&1' first to check that the
>>> target is defined.
>>
>> There's a 'set -e' on top of the script so using make -n will likely
>> result in the script failing, which wouldn't be so nice imho.
> [...]
> 
> That's why you use it with the if statement:
> 
> # Only some architectures with OF support have this target
> if make -n dtbs_install >/dev/null 2>&1; then

The problem is that kbuild does not support make -n and some of the
commands are always executed. So the command will silently build the
blobs and just not install them. Better do

if grep -q dtbs_install "arch/$SRCARCH/Makefile"; then
  ...

and hope that there won't be any comment causing a false positive.
Another option is to grep 'make help' output.

Michal

Re: [PATCHv2 1/1] deb-pkg: Add device tree blobs to the package

[Ben Hutchings]

[-- Attachment #1: Type: text/plain, Size: 1412 bytes --]

On Thu, 2015-01-22 at 14:40 +0100, Michal Marek wrote:
> On 2015-01-20 17:04, Ben Hutchings wrote:
> > On Tue, 2015-01-20 at 00:13 +0100, Arnaud Patard wrote:
> >> Ben Hutchings <ben@decadent.org.uk> writes:
> >>> Only arm and arm64 support that target.  You should maybe run something
> >>> like 'make -n dtbs_install >/dev/null 2>&1' first to check that the
> >>> target is defined.
> >>
> >> There's a 'set -e' on top of the script so using make -n will likely
> >> result in the script failing, which wouldn't be so nice imho.
> > [...]
> > 
> > That's why you use it with the if statement:
> > 
> > # Only some architectures with OF support have this target
> > if make -n dtbs_install >/dev/null 2>&1; then
> 
> The problem is that kbuild does not support make -n and some of the
> commands are always executed. So the command will silently build the
> blobs and just not install them.

If the dtbs_install target is defined, they should already have been
built at this point.  If the target is not defined, why would it do
anything?

Of course it should be $MAKE not make.

> Better do
> 
> if grep -q dtbs_install "arch/$SRCARCH/Makefile"; then
>   ...
> 
> and hope that there won't be any comment causing a false positive.
> Another option is to grep 'make help' output.

Ugh.

Ben.

-- 
Ben Hutchings
I haven't lost my mind; it's backed up on tape somewhere.

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 811 bytes --]

Re: [PATCHv2 1/1] deb-pkg: Add device tree blobs to the package

[Michal Marek]

On 2015-01-22 15:19, Ben Hutchings wrote:
> On Thu, 2015-01-22 at 14:40 +0100, Michal Marek wrote:
>> On 2015-01-20 17:04, Ben Hutchings wrote:
>>> On Tue, 2015-01-20 at 00:13 +0100, Arnaud Patard wrote:
>>>> Ben Hutchings <ben@decadent.org.uk> writes:
>>>>> Only arm and arm64 support that target.  You should maybe run something
>>>>> like 'make -n dtbs_install >/dev/null 2>&1' first to check that the
>>>>> target is defined.
>>>>
>>>> There's a 'set -e' on top of the script so using make -n will likely
>>>> result in the script failing, which wouldn't be so nice imho.
>>> [...]
>>>
>>> That's why you use it with the if statement:
>>>
>>> # Only some architectures with OF support have this target
>>> if make -n dtbs_install >/dev/null 2>&1; then
>>
>> The problem is that kbuild does not support make -n and some of the
>> commands are always executed. So the command will silently build the
>> blobs and just not install them.
> 
> If the dtbs_install target is defined, they should already have been
> built at this point.  If the target is not defined, why would it do
> anything?

The point is that make -n does not work with kbuild. It might work for
this specific case, but there is no guarantee that it will continue to
do so.


>> if grep -q dtbs_install "arch/$SRCARCH/Makefile"; then
>>   ...
>>
>> and hope that there won't be any comment causing a false positive.
>> Another option is to grep 'make help' output.
> 
> Ugh.

I know that both options are ugly :).

Michal

Re: [PATCHv2 1/1] deb-pkg: Add device tree blobs to the package

[Arnaud Patard]

Michal Marek <mmarek@suse.cz> writes:

> On 2015-01-22 15:19, Ben Hutchings wrote:
>> On Thu, 2015-01-22 at 14:40 +0100, Michal Marek wrote:
>>> On 2015-01-20 17:04, Ben Hutchings wrote:
>>>> On Tue, 2015-01-20 at 00:13 +0100, Arnaud Patard wrote:
>>>>> Ben Hutchings <ben@decadent.org.uk> writes:
>>>>>> Only arm and arm64 support that target.  You should maybe run something
>>>>>> like 'make -n dtbs_install >/dev/null 2>&1' first to check that the
>>>>>> target is defined.
>>>>>
>>>>> There's a 'set -e' on top of the script so using make -n will likely
>>>>> result in the script failing, which wouldn't be so nice imho.
>>>> [...]
>>>>
>>>> That's why you use it with the if statement:
>>>>
>>>> # Only some architectures with OF support have this target
>>>> if make -n dtbs_install >/dev/null 2>&1; then
>>>
>>> The problem is that kbuild does not support make -n and some of the
>>> commands are always executed. So the command will silently build the
>>> blobs and just not install them.
>> 
>> If the dtbs_install target is defined, they should already have been
>> built at this point.  If the target is not defined, why would it do
>> anything?
>
> The point is that make -n does not work with kbuild. It might work for
> this specific case, but there is no guarantee that it will continue to
> do so.
>

Ok, so a patch with make -n will be refused, right ?

>
>>> if grep -q dtbs_install "arch/$SRCARCH/Makefile"; then
>>>   ...
>>>
>>> and hope that there won't be any comment causing a false positive.
>>> Another option is to grep 'make help' output.
>> 
>> Ugh.
>
> I know that both options are ugly :).

Let's go to your first options then. I feel not really motivated by
parsing 'make help' output.

Arnaud

Re: [PATCHv2 1/1] deb-pkg: Add device tree blobs to the package

[Michal Marek]

On 2015-01-22 16:58, Arnaud Patard (Rtp) wrote:
> Michal Marek <mmarek@suse.cz> writes:
> 
>> On 2015-01-22 15:19, Ben Hutchings wrote:
>>> On Thu, 2015-01-22 at 14:40 +0100, Michal Marek wrote:
>>>> On 2015-01-20 17:04, Ben Hutchings wrote:
>>>>> On Tue, 2015-01-20 at 00:13 +0100, Arnaud Patard wrote:
>>>>>> Ben Hutchings <ben@decadent.org.uk> writes:
>>>>>>> Only arm and arm64 support that target.  You should maybe run something
>>>>>>> like 'make -n dtbs_install >/dev/null 2>&1' first to check that the
>>>>>>> target is defined.
>>>>>>
>>>>>> There's a 'set -e' on top of the script so using make -n will likely
>>>>>> result in the script failing, which wouldn't be so nice imho.
>>>>> [...]
>>>>>
>>>>> That's why you use it with the if statement:
>>>>>
>>>>> # Only some architectures with OF support have this target
>>>>> if make -n dtbs_install >/dev/null 2>&1; then
>>>>
>>>> The problem is that kbuild does not support make -n and some of the
>>>> commands are always executed. So the command will silently build the
>>>> blobs and just not install them.
>>>
>>> If the dtbs_install target is defined, they should already have been
>>> built at this point.  If the target is not defined, why would it do
>>> anything?
>>
>> The point is that make -n does not work with kbuild. It might work for
>> this specific case, but there is no guarantee that it will continue to
>> do so.
>>
> 
> Ok, so a patch with make -n will be refused, right ?

I'm not trying to veto such patch, I just wanted to point out that make
-n is not the best idea to do with kbuild.


> Let's go to your first options then. I feel not really motivated by
> parsing 'make help' output.

Whatever. You can also have a static list of archs supporting
dtbs_install, like we (still) have in scripts/headers.sh. It would be
guaranteed to works as designed, but obviously not future-proof.

Michal