| Message ID | 20150114123658.185884720@rtp-net.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
[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 <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 -- To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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.
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 -- To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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.
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 -- To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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.
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 -- To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 -- To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 -- To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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"
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> --- -- To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html