[PATCH] Correctly use the bitbake variable S from now on

public inbox for isar-users@googlegroups.com
 help / color / mirror / Atom feed

* [PATCH] Correctly use the bitbake variable S from now on
@ 2018-04-19  9:26 Henning Schild
  2018-04-19  9:45 ` Jan Kiszka
  2018-04-19 16:58 ` [PATCH v2] " Henning Schild
  0 siblings, 2 replies; 12+ messages in thread
From: Henning Schild @ 2018-04-19  9:26 UTC (permalink / raw)
  To: isar-users; +Cc: Jan Kiszka, Henning Schild

S is defined as ${WORKDIR}/${P} so it already contains ${WORKDIR} by its
definition. In Isar it was often concatinated with WORKDIR again. One
example where this was a problem is if you specified a patch in SRC_URI
but did not actually overwrite S.
Align the use of the variable with OE and bitbake defaults again.

Impact: layers building on top of Isar will have to adjust their recipes
like the internal ones needed modification. The suggestion is to not set
S and make sure to unpack to ${WORKDIR}/${P}. When setting S make sure
to include §{WORKDIR}. This patch also introduces a warning and tries to
preserve the old behaviour a bit. However if your recipe uses patches in
SRC_URI and sets S you will get the warning and do_patch will fail.

Signed-off-by: Henning Schild <henning.schild@siemens.com>
---
 doc/user_manual.md                                        |  2 +-
 meta-isar/recipes-app/example-hello/example-hello.bb      |  4 +---
 meta-isar/recipes-app/libhello/libhello.bb                |  4 +---
 meta-isar/recipes-kernel/example-module/example-module.bb |  2 +-
 meta-isar/recipes-kernel/linux/linux-cip_4.4.bb           |  4 +---
 meta-isar/recipes-kernel/linux/linux-mainline_4.14.18.bb  |  4 +---
 meta/classes/dpkg-base.bbclass                            | 13 +++++++++++--
 meta/classes/dpkg.bbclass                                 |  2 +-
 meta/classes/patch.bbclass                                |  2 +-
 meta/recipes-kernel/linux-module/module.inc               |  8 ++++----
 meta/recipes-kernel/linux/linux-custom.inc                |  2 +-
 11 files changed, 24 insertions(+), 23 deletions(-)

diff --git a/doc/user_manual.md b/doc/user_manual.md
index 2bd3793..3072bd5 100644
--- a/doc/user_manual.md
+++ b/doc/user_manual.md
@@ -511,7 +511,7 @@ PV = "1.0"
 SRC_URI = "git://github.com/ilbers/hello.git"
 SRCREV = "ad7065ecc4840cc436bfcdac427386dbba4ea719"
 
-S = "git"
+S = "${WORKDIR}/git"
 
 inherit dpkg
 ```
diff --git a/meta-isar/recipes-app/example-hello/example-hello.bb b/meta-isar/recipes-app/example-hello/example-hello.bb
index 9788ec0..d23ee6c 100644
--- a/meta-isar/recipes-app/example-hello/example-hello.bb
+++ b/meta-isar/recipes-app/example-hello/example-hello.bb
@@ -15,11 +15,9 @@ PV = "0.2-86cc719"
 DEPENDS += "libhello"
 
 SRC_URI = " \
-    git://github.com/ilbers/hello.git;protocol=https \
+    git://github.com/ilbers/hello.git;protocol=https;destsuffix=${P} \
     file://0001-Add-some-help.patch \
     file://yet-another-change.txt;apply=yes;striplevel=0"
 SRCREV = "86cc719b3359adc3c4e243387feba50360a860f3"
 
-S = "git"
-
 inherit dpkg
diff --git a/meta-isar/recipes-app/libhello/libhello.bb b/meta-isar/recipes-app/libhello/libhello.bb
index 1875831..4e75f98 100644
--- a/meta-isar/recipes-app/libhello/libhello.bb
+++ b/meta-isar/recipes-app/libhello/libhello.bb
@@ -10,9 +10,7 @@ LIC_FILES_CHKSUM = "file://${LAYERDIR_isar}/licenses/COPYING.GPLv2;md5=751419260
 
 PV = "0.1-98f2e41"
 
-SRC_URI = "git://github.com/ilbers/libhello.git;protocol=https"
+SRC_URI = "git://github.com/ilbers/libhello.git;protocol=https;destsuffix=${P}"
 SRCREV = "98f2e41e7d05ab8d19b0c5d160b104b725c8fd93"
 
-S = "git"
-
 inherit dpkg
diff --git a/meta-isar/recipes-kernel/example-module/example-module.bb b/meta-isar/recipes-kernel/example-module/example-module.bb
index dbaf5ac..98d0aaa 100644
--- a/meta-isar/recipes-kernel/example-module/example-module.bb
+++ b/meta-isar/recipes-kernel/example-module/example-module.bb
@@ -9,6 +9,6 @@ include recipes-kernel/linux-module/module.inc
 
 SRC_URI += "file://src"
 
-S = "src"
+S = "${WORKDIR}/src"
 
 AUTOLOAD = "1"
diff --git a/meta-isar/recipes-kernel/linux/linux-cip_4.4.bb b/meta-isar/recipes-kernel/linux/linux-cip_4.4.bb
index 7502f70..e9aaa9f 100644
--- a/meta-isar/recipes-kernel/linux/linux-cip_4.4.bb
+++ b/meta-isar/recipes-kernel/linux/linux-cip_4.4.bb
@@ -8,12 +8,10 @@
 require recipes-kernel/linux/linux-custom.inc
 
 SRC_URI += " \
-    git://git.kernel.org/pub/scm/linux/kernel/git/bwh/linux-cip.git;branch=linux-4.4.y-cip \
+    git://git.kernel.org/pub/scm/linux/kernel/git/bwh/linux-cip.git;branch=linux-4.4.y-cip;destsuffix=${P} \
     file://x86_64_defconfig"
 
 SRCREV = "4e52cc5f668c4666e31a8485725b5f4e897b3baf"
 PV = "4.4.112-cip18"
 
-S = "git"
-
 KERNEL_DEFCONFIG_qemuamd64 = "x86_64_defconfig"
diff --git a/meta-isar/recipes-kernel/linux/linux-mainline_4.14.18.bb b/meta-isar/recipes-kernel/linux/linux-mainline_4.14.18.bb
index 2c93d40..751912f 100644
--- a/meta-isar/recipes-kernel/linux/linux-mainline_4.14.18.bb
+++ b/meta-isar/recipes-kernel/linux/linux-mainline_4.14.18.bb
@@ -8,10 +8,8 @@
 require recipes-kernel/linux/linux-custom.inc
 
 SRC_URI += " \
-    https://cdn.kernel.org/pub/linux/kernel/v4.x/linux-${PV}.tar.xz \
+    https://cdn.kernel.org/pub/linux/kernel/v4.x/linux-${PV}.tar.xz;subdir=${P} \
     file://x86_64_defconfig"
 SRC_URI[sha256sum] = "866a94c1c38d923ae18e74b683d7a8a79b674ebdfe7f40f1a3be9a27d39fe354"
 
-S = "linux-${PV}"
-
 KERNEL_DEFCONFIG_qemuamd64 = "x86_64_defconfig"
diff --git a/meta/classes/dpkg-base.bbclass b/meta/classes/dpkg-base.bbclass
index 3c3a484..3845d8c 100644
--- a/meta/classes/dpkg-base.bbclass
+++ b/meta/classes/dpkg-base.bbclass
@@ -2,8 +2,8 @@
 # Copyright (C) 2017 Siemens AG
 
 do_adjust_git() {
-    if [ -f ${WORKDIR}/${S}/.git/objects/info/alternates ]; then
-        sed -i ${WORKDIR}/${S}/.git/objects/info/alternates \
+    if [ -f ${S}/.git/objects/info/alternates ]; then
+        sed -i ${S}/.git/objects/info/alternates \
             -e 's|${DL_DIR}|/downloads|'
     fi
 }
@@ -21,9 +21,18 @@ do_build[depends] = "buildchroot:do_build"
 DEPENDS ?= ""
 do_build[deptask] = "do_deploy_deb"
 
+def get_package_srcdir(d):
+    s = d.getVar("S", True)
+    workdir = d.getVar("WORKDIR", True)
+    if s.startswith(workdir):
+        return s[len(workdir)+1:]
+    bb.warn('S does not start with WORKDIR')
+    return s
+
 # Each package should have its own unique build folder, so use
 # recipe name as identifier
 PP = "/home/builder/${PN}"
+PPS ?= "${@get_package_srcdir(d)}"
 
 BUILDROOT = "${BUILDCHROOT_DIR}/${PP}"
 do_build[stamp-extra-info] = "${DISTRO}-${DISTRO_ARCH}"
diff --git a/meta/classes/dpkg.bbclass b/meta/classes/dpkg.bbclass
index 06f0579..c8d4ac5 100644
--- a/meta/classes/dpkg.bbclass
+++ b/meta/classes/dpkg.bbclass
@@ -6,5 +6,5 @@ inherit dpkg-base
 # Build package from sources using build script
 dpkg_runbuild() {
     E="${@ bb.utils.export_proxies(d)}"
-    sudo -E chroot ${BUILDCHROOT_DIR} /build.sh ${PP}/${S}
+    sudo -E chroot ${BUILDCHROOT_DIR} /build.sh ${PP}/${PPS}
 }
diff --git a/meta/classes/patch.bbclass b/meta/classes/patch.bbclass
index 26a0c81..1559359 100644
--- a/meta/classes/patch.bbclass
+++ b/meta/classes/patch.bbclass
@@ -5,7 +5,7 @@ python do_patch() {
     import subprocess
 
     workdir = d.getVar("WORKDIR", True) + "/"
-    src_dir = workdir + (d.getVar("S", True) or "")
+    src_dir = (d.getVar("S", True) or "")
 
     for src_uri in (d.getVar("SRC_URI", True) or "").split():
         try:
diff --git a/meta/recipes-kernel/linux-module/module.inc b/meta/recipes-kernel/linux-module/module.inc
index ec1c4b0..3075f44 100644
--- a/meta/recipes-kernel/linux-module/module.inc
+++ b/meta/recipes-kernel/linux-module/module.inc
@@ -18,14 +18,14 @@ AUTOLOAD ?= "0"
 inherit dpkg
 
 dpkg_runbuild_prepend() {
-    cp -r ${WORKDIR}/debian ${WORKDIR}/${S}/
+    cp -r ${WORKDIR}/debian ${S}/
     sed -i -e 's/@PN@/${PN}/g' -e 's/@PV@/${PV}/g' \
            -e 's/@KERNEL_NAME@/${KERNEL_NAME}/g' \
            -e 's/@DESCRIPTION@/${DESCRIPTION}/g' \
-        ${WORKDIR}/${S}/debian/changelog ${WORKDIR}/${S}/debian/control
+        ${S}/debian/changelog ${S}/debian/control
 
     if [ ${AUTOLOAD} = "1" ]; then
-        echo "echo ${PN} >> /etc/modules" >> ${WORKDIR}/${S}/debian/postinst
-        chmod +x ${WORKDIR}/${S}/debian/postinst
+        echo "echo ${PN} >> /etc/modules" >> ${S}/debian/postinst
+        chmod +x ${S}/debian/postinst
     fi
 }
diff --git a/meta/recipes-kernel/linux/linux-custom.inc b/meta/recipes-kernel/linux/linux-custom.inc
index 0498dfa..2643fec 100644
--- a/meta/recipes-kernel/linux/linux-custom.inc
+++ b/meta/recipes-kernel/linux/linux-custom.inc
@@ -32,7 +32,7 @@ dpkg_runbuild() {
 	# Install package builder script
 	sudo install -m 755 ${WORKDIR}/build-kernel.sh ${BUILDCHROOT_DIR}
 
-	sudo cp ${WORKDIR}/${KERNEL_DEFCONFIG} ${WORKDIR}/${S}/.config
+	sudo cp ${WORKDIR}/${KERNEL_DEFCONFIG} ${S}/.config
 
 	E="${@ bb.utils.export_proxies(d)}"
 
-- 
2.16.1


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] Correctly use the bitbake variable S from now on
  2018-04-19  9:26 [PATCH] Correctly use the bitbake variable S from now on Henning Schild
@ 2018-04-19  9:45 ` Jan Kiszka
  2018-04-19 10:36   ` Henning Schild
  2018-04-19 16:58 ` [PATCH v2] " Henning Schild
  1 sibling, 1 reply; 12+ messages in thread
From: Jan Kiszka @ 2018-04-19  9:45 UTC (permalink / raw)
  To: Henning Schild, isar-users

On 2018-04-19 11:26, Henning Schild wrote:
> S is defined as ${WORKDIR}/${P} so it already contains ${WORKDIR} by its
> definition. In Isar it was often concatinated with WORKDIR again. One
> example where this was a problem is if you specified a patch in SRC_URI
> but did not actually overwrite S.
> Align the use of the variable with OE and bitbake defaults again.

Yeah, I missed that when migrating Isar from SRC_DIR to S.

> 
> Impact: layers building on top of Isar will have to adjust their recipes
> like the internal ones needed modification. The suggestion is to not set
> S and make sure to unpack to ${WORKDIR}/${P}. When setting S make sure
> to include §{WORKDIR}. This patch also introduces a warning and tries to
> preserve the old behaviour a bit. However if your recipe uses patches in
> SRC_URI and sets S you will get the warning and do_patch will fail.
> 
> Signed-off-by: Henning Schild <henning.schild@siemens.com>
> ---
>  doc/user_manual.md                                        |  2 +-
>  meta-isar/recipes-app/example-hello/example-hello.bb      |  4 +---
>  meta-isar/recipes-app/libhello/libhello.bb                |  4 +---
>  meta-isar/recipes-kernel/example-module/example-module.bb |  2 +-
>  meta-isar/recipes-kernel/linux/linux-cip_4.4.bb           |  4 +---
>  meta-isar/recipes-kernel/linux/linux-mainline_4.14.18.bb  |  4 +---
>  meta/classes/dpkg-base.bbclass                            | 13 +++++++++++--
>  meta/classes/dpkg.bbclass                                 |  2 +-
>  meta/classes/patch.bbclass                                |  2 +-
>  meta/recipes-kernel/linux-module/module.inc               |  8 ++++----
>  meta/recipes-kernel/linux/linux-custom.inc                |  2 +-
>  11 files changed, 24 insertions(+), 23 deletions(-)
> 
> diff --git a/doc/user_manual.md b/doc/user_manual.md
> index 2bd3793..3072bd5 100644
> --- a/doc/user_manual.md
> +++ b/doc/user_manual.md
> @@ -511,7 +511,7 @@ PV = "1.0"
>  SRC_URI = "git://github.com/ilbers/hello.git"
>  SRCREV = "ad7065ecc4840cc436bfcdac427386dbba4ea719"
>  
> -S = "git"
> +S = "${WORKDIR}/git"

Should we promote this pattern now or destsuffix?

>  
>  inherit dpkg
>  ```
> diff --git a/meta-isar/recipes-app/example-hello/example-hello.bb b/meta-isar/recipes-app/example-hello/example-hello.bb
> index 9788ec0..d23ee6c 100644
> --- a/meta-isar/recipes-app/example-hello/example-hello.bb
> +++ b/meta-isar/recipes-app/example-hello/example-hello.bb
> @@ -15,11 +15,9 @@ PV = "0.2-86cc719"
>  DEPENDS += "libhello"
>  
>  SRC_URI = " \
> -    git://github.com/ilbers/hello.git;protocol=https \
> +    git://github.com/ilbers/hello.git;protocol=https;destsuffix=${P} \
>      file://0001-Add-some-help.patch \
>      file://yet-another-change.txt;apply=yes;striplevel=0"
>  SRCREV = "86cc719b3359adc3c4e243387feba50360a860f3"
>  
> -S = "git"
> -
>  inherit dpkg
> diff --git a/meta-isar/recipes-app/libhello/libhello.bb b/meta-isar/recipes-app/libhello/libhello.bb
> index 1875831..4e75f98 100644
> --- a/meta-isar/recipes-app/libhello/libhello.bb
> +++ b/meta-isar/recipes-app/libhello/libhello.bb
> @@ -10,9 +10,7 @@ LIC_FILES_CHKSUM = "file://${LAYERDIR_isar}/licenses/COPYING.GPLv2;md5=751419260
>  
>  PV = "0.1-98f2e41"
>  
> -SRC_URI = "git://github.com/ilbers/libhello.git;protocol=https"
> +SRC_URI = "git://github.com/ilbers/libhello.git;protocol=https;destsuffix=${P}"
>  SRCREV = "98f2e41e7d05ab8d19b0c5d160b104b725c8fd93"
>  
> -S = "git"
> -
>  inherit dpkg
> diff --git a/meta-isar/recipes-kernel/example-module/example-module.bb b/meta-isar/recipes-kernel/example-module/example-module.bb
> index dbaf5ac..98d0aaa 100644
> --- a/meta-isar/recipes-kernel/example-module/example-module.bb
> +++ b/meta-isar/recipes-kernel/example-module/example-module.bb
> @@ -9,6 +9,6 @@ include recipes-kernel/linux-module/module.inc
>  
>  SRC_URI += "file://src"
>  
> -S = "src"
> +S = "${WORKDIR}/src"
>  
>  AUTOLOAD = "1"
> diff --git a/meta-isar/recipes-kernel/linux/linux-cip_4.4.bb b/meta-isar/recipes-kernel/linux/linux-cip_4.4.bb
> index 7502f70..e9aaa9f 100644
> --- a/meta-isar/recipes-kernel/linux/linux-cip_4.4.bb
> +++ b/meta-isar/recipes-kernel/linux/linux-cip_4.4.bb
> @@ -8,12 +8,10 @@
>  require recipes-kernel/linux/linux-custom.inc
>  
>  SRC_URI += " \
> -    git://git.kernel.org/pub/scm/linux/kernel/git/bwh/linux-cip.git;branch=linux-4.4.y-cip \
> +    git://git.kernel.org/pub/scm/linux/kernel/git/bwh/linux-cip.git;branch=linux-4.4.y-cip;destsuffix=${P} \
>      file://x86_64_defconfig"
>  
>  SRCREV = "4e52cc5f668c4666e31a8485725b5f4e897b3baf"
>  PV = "4.4.112-cip18"
>  
> -S = "git"
> -
>  KERNEL_DEFCONFIG_qemuamd64 = "x86_64_defconfig"
> diff --git a/meta-isar/recipes-kernel/linux/linux-mainline_4.14.18.bb b/meta-isar/recipes-kernel/linux/linux-mainline_4.14.18.bb
> index 2c93d40..751912f 100644
> --- a/meta-isar/recipes-kernel/linux/linux-mainline_4.14.18.bb
> +++ b/meta-isar/recipes-kernel/linux/linux-mainline_4.14.18.bb
> @@ -8,10 +8,8 @@
>  require recipes-kernel/linux/linux-custom.inc
>  
>  SRC_URI += " \
> -    https://cdn.kernel.org/pub/linux/kernel/v4.x/linux-${PV}.tar.xz \
> +    https://cdn.kernel.org/pub/linux/kernel/v4.x/linux-${PV}.tar.xz;subdir=${P} \
>      file://x86_64_defconfig"
>  SRC_URI[sha256sum] = "866a94c1c38d923ae18e74b683d7a8a79b674ebdfe7f40f1a3be9a27d39fe354"
>  
> -S = "linux-${PV}"
> -
>  KERNEL_DEFCONFIG_qemuamd64 = "x86_64_defconfig"
> diff --git a/meta/classes/dpkg-base.bbclass b/meta/classes/dpkg-base.bbclass
> index 3c3a484..3845d8c 100644
> --- a/meta/classes/dpkg-base.bbclass
> +++ b/meta/classes/dpkg-base.bbclass
> @@ -2,8 +2,8 @@
>  # Copyright (C) 2017 Siemens AG
>  
>  do_adjust_git() {
> -    if [ -f ${WORKDIR}/${S}/.git/objects/info/alternates ]; then
> -        sed -i ${WORKDIR}/${S}/.git/objects/info/alternates \
> +    if [ -f ${S}/.git/objects/info/alternates ]; then
> +        sed -i ${S}/.git/objects/info/alternates \
>              -e 's|${DL_DIR}|/downloads|'
>      fi
>  }
> @@ -21,9 +21,18 @@ do_build[depends] = "buildchroot:do_build"
>  DEPENDS ?= ""
>  do_build[deptask] = "do_deploy_deb"
>  
> +def get_package_srcdir(d):
> +    s = d.getVar("S", True)
> +    workdir = d.getVar("WORKDIR", True)
> +    if s.startswith(workdir):
> +        return s[len(workdir)+1:]
> +    bb.warn('S does not start with WORKDIR')
> +    return s
> +
>  # Each package should have its own unique build folder, so use
>  # recipe name as identifier
>  PP = "/home/builder/${PN}"
> +PPS ?= "${@get_package_srcdir(d)}"
>  
>  BUILDROOT = "${BUILDCHROOT_DIR}/${PP}"
>  do_build[stamp-extra-info] = "${DISTRO}-${DISTRO_ARCH}"
> diff --git a/meta/classes/dpkg.bbclass b/meta/classes/dpkg.bbclass
> index 06f0579..c8d4ac5 100644
> --- a/meta/classes/dpkg.bbclass
> +++ b/meta/classes/dpkg.bbclass
> @@ -6,5 +6,5 @@ inherit dpkg-base
>  # Build package from sources using build script
>  dpkg_runbuild() {
>      E="${@ bb.utils.export_proxies(d)}"
> -    sudo -E chroot ${BUILDCHROOT_DIR} /build.sh ${PP}/${S}
> +    sudo -E chroot ${BUILDCHROOT_DIR} /build.sh ${PP}/${PPS}
>  }
> diff --git a/meta/classes/patch.bbclass b/meta/classes/patch.bbclass
> index 26a0c81..1559359 100644
> --- a/meta/classes/patch.bbclass
> +++ b/meta/classes/patch.bbclass
> @@ -5,7 +5,7 @@ python do_patch() {
>      import subprocess
>  
>      workdir = d.getVar("WORKDIR", True) + "/"
> -    src_dir = workdir + (d.getVar("S", True) or "")
> +    src_dir = (d.getVar("S", True) or "")

That's not equivalent, but I'm undecided if it matters. S should never
be unset (bitbake.conf holds the default). So we could just pull S and
let the build fail if that assumption should ever be wrong.

>  
>      for src_uri in (d.getVar("SRC_URI", True) or "").split():
>          try:
> diff --git a/meta/recipes-kernel/linux-module/module.inc b/meta/recipes-kernel/linux-module/module.inc
> index ec1c4b0..3075f44 100644
> --- a/meta/recipes-kernel/linux-module/module.inc
> +++ b/meta/recipes-kernel/linux-module/module.inc
> @@ -18,14 +18,14 @@ AUTOLOAD ?= "0"
>  inherit dpkg
>  
>  dpkg_runbuild_prepend() {
> -    cp -r ${WORKDIR}/debian ${WORKDIR}/${S}/
> +    cp -r ${WORKDIR}/debian ${S}/
>      sed -i -e 's/@PN@/${PN}/g' -e 's/@PV@/${PV}/g' \
>             -e 's/@KERNEL_NAME@/${KERNEL_NAME}/g' \
>             -e 's/@DESCRIPTION@/${DESCRIPTION}/g' \
> -        ${WORKDIR}/${S}/debian/changelog ${WORKDIR}/${S}/debian/control
> +        ${S}/debian/changelog ${S}/debian/control
>  
>      if [ ${AUTOLOAD} = "1" ]; then
> -        echo "echo ${PN} >> /etc/modules" >> ${WORKDIR}/${S}/debian/postinst
> -        chmod +x ${WORKDIR}/${S}/debian/postinst
> +        echo "echo ${PN} >> /etc/modules" >> ${S}/debian/postinst
> +        chmod +x ${S}/debian/postinst
>      fi
>  }
> diff --git a/meta/recipes-kernel/linux/linux-custom.inc b/meta/recipes-kernel/linux/linux-custom.inc
> index 0498dfa..2643fec 100644
> --- a/meta/recipes-kernel/linux/linux-custom.inc
> +++ b/meta/recipes-kernel/linux/linux-custom.inc
> @@ -32,7 +32,7 @@ dpkg_runbuild() {
>  	# Install package builder script
>  	sudo install -m 755 ${WORKDIR}/build-kernel.sh ${BUILDCHROOT_DIR}
>  
> -	sudo cp ${WORKDIR}/${KERNEL_DEFCONFIG} ${WORKDIR}/${S}/.config
> +	sudo cp ${WORKDIR}/${KERNEL_DEFCONFIG} ${S}/.config
>  
>  	E="${@ bb.utils.export_proxies(d)}"
>  
> 

Otherwise:
Reviewed-by: Jan Kiszka <jan.kiszka@siemens.com>

Jan

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] Correctly use the bitbake variable S from now on
  2018-04-19  9:45 ` Jan Kiszka
@ 2018-04-19 10:36   ` Henning Schild
  2018-04-19 11:42     ` Jan Kiszka
  0 siblings, 1 reply; 12+ messages in thread
From: Henning Schild @ 2018-04-19 10:36 UTC (permalink / raw)
  To: Jan Kiszka; +Cc: isar-users

Am Thu, 19 Apr 2018 11:45:05 +0200
schrieb Jan Kiszka <jan.kiszka@siemens.com>:

> On 2018-04-19 11:26, Henning Schild wrote:
> > S is defined as ${WORKDIR}/${P} so it already contains ${WORKDIR}
> > by its definition. In Isar it was often concatinated with WORKDIR
> > again. One example where this was a problem is if you specified a
> > patch in SRC_URI but did not actually overwrite S.
> > Align the use of the variable with OE and bitbake defaults again.  
> 
> Yeah, I missed that when migrating Isar from SRC_DIR to S.
> 
> > 
> > Impact: layers building on top of Isar will have to adjust their
> > recipes like the internal ones needed modification. The suggestion
> > is to not set S and make sure to unpack to ${WORKDIR}/${P}. When
> > setting S make sure to include §{WORKDIR}. This patch also
> > introduces a warning and tries to preserve the old behaviour a bit.
> > However if your recipe uses patches in SRC_URI and sets S you will
> > get the warning and do_patch will fail.
> > 
> > Signed-off-by: Henning Schild <henning.schild@siemens.com>
> > ---
> >  doc/user_manual.md                                        |  2 +-
> >  meta-isar/recipes-app/example-hello/example-hello.bb      |  4 +---
> >  meta-isar/recipes-app/libhello/libhello.bb                |  4 +---
> >  meta-isar/recipes-kernel/example-module/example-module.bb |  2 +-
> >  meta-isar/recipes-kernel/linux/linux-cip_4.4.bb           |  4 +---
> >  meta-isar/recipes-kernel/linux/linux-mainline_4.14.18.bb  |  4 +---
> >  meta/classes/dpkg-base.bbclass                            | 13
> > +++++++++++--
> > meta/classes/dpkg.bbclass                                 |  2 +-
> > meta/classes/patch.bbclass                                |  2 +-
> > meta/recipes-kernel/linux-module/module.inc               |  8
> > ++++---- meta/recipes-kernel/linux/linux-custom.inc
> > |  2 +- 11 files changed, 24 insertions(+), 23 deletions(-)
> > 
> > diff --git a/doc/user_manual.md b/doc/user_manual.md
> > index 2bd3793..3072bd5 100644
> > --- a/doc/user_manual.md
> > +++ b/doc/user_manual.md
> > @@ -511,7 +511,7 @@ PV = "1.0"
> >  SRC_URI = "git://github.com/ilbers/hello.git"
> >  SRCREV = "ad7065ecc4840cc436bfcdac427386dbba4ea719"
> >  
> > -S = "git"
> > +S = "${WORKDIR}/git"  
> 
> Should we promote this pattern now or destsuffix?

Both are fine. That is why i kind of mixed them in this commit. I am
not sure which one to promote. One tells people to modify an
internal variable the other one is not consistent between fetchers.
git uses destsuffix while tar.gz uses subdir

> >  
> >  inherit dpkg
> >  ```
> > diff --git a/meta-isar/recipes-app/example-hello/example-hello.bb
> > b/meta-isar/recipes-app/example-hello/example-hello.bb index
> > 9788ec0..d23ee6c 100644 ---
> > a/meta-isar/recipes-app/example-hello/example-hello.bb +++
> > b/meta-isar/recipes-app/example-hello/example-hello.bb @@ -15,11
> > +15,9 @@ PV = "0.2-86cc719" DEPENDS += "libhello"
> >  
> >  SRC_URI = " \
> > -    git://github.com/ilbers/hello.git;protocol=https \
> > +
> > git://github.com/ilbers/hello.git;protocol=https;destsuffix=${P} \
> > file://0001-Add-some-help.patch \
> > file://yet-another-change.txt;apply=yes;striplevel=0" SRCREV =
> > "86cc719b3359adc3c4e243387feba50360a860f3" 
> > -S = "git"
> > -
> >  inherit dpkg
> > diff --git a/meta-isar/recipes-app/libhello/libhello.bb
> > b/meta-isar/recipes-app/libhello/libhello.bb index 1875831..4e75f98
> > 100644 --- a/meta-isar/recipes-app/libhello/libhello.bb
> > +++ b/meta-isar/recipes-app/libhello/libhello.bb
> > @@ -10,9 +10,7 @@ LIC_FILES_CHKSUM =
> > "file://${LAYERDIR_isar}/licenses/COPYING.GPLv2;md5=751419260 
> >  PV = "0.1-98f2e41"
> >  
> > -SRC_URI = "git://github.com/ilbers/libhello.git;protocol=https"
> > +SRC_URI =
> > "git://github.com/ilbers/libhello.git;protocol=https;destsuffix=${P}"
> > SRCREV = "98f2e41e7d05ab8d19b0c5d160b104b725c8fd93" 
> > -S = "git"
> > -
> >  inherit dpkg
> > diff --git
> > a/meta-isar/recipes-kernel/example-module/example-module.bb
> > b/meta-isar/recipes-kernel/example-module/example-module.bb index
> > dbaf5ac..98d0aaa 100644 ---
> > a/meta-isar/recipes-kernel/example-module/example-module.bb +++
> > b/meta-isar/recipes-kernel/example-module/example-module.bb @@ -9,6
> > +9,6 @@ include recipes-kernel/linux-module/module.inc SRC_URI +=
> > "file://src" 
> > -S = "src"
> > +S = "${WORKDIR}/src"
> >  
> >  AUTOLOAD = "1"
> > diff --git a/meta-isar/recipes-kernel/linux/linux-cip_4.4.bb
> > b/meta-isar/recipes-kernel/linux/linux-cip_4.4.bb index
> > 7502f70..e9aaa9f 100644 ---
> > a/meta-isar/recipes-kernel/linux/linux-cip_4.4.bb +++
> > b/meta-isar/recipes-kernel/linux/linux-cip_4.4.bb @@ -8,12 +8,10 @@
> >  require recipes-kernel/linux/linux-custom.inc
> >  
> >  SRC_URI += " \
> > -
> > git://git.kernel.org/pub/scm/linux/kernel/git/bwh/linux-cip.git;branch=linux-4.4.y-cip
> > \
> > +
> > git://git.kernel.org/pub/scm/linux/kernel/git/bwh/linux-cip.git;branch=linux-4.4.y-cip;destsuffix=${P}
> > \ file://x86_64_defconfig" 
> >  SRCREV = "4e52cc5f668c4666e31a8485725b5f4e897b3baf"
> >  PV = "4.4.112-cip18"
> >  
> > -S = "git"
> > -
> >  KERNEL_DEFCONFIG_qemuamd64 = "x86_64_defconfig"
> > diff --git
> > a/meta-isar/recipes-kernel/linux/linux-mainline_4.14.18.bb
> > b/meta-isar/recipes-kernel/linux/linux-mainline_4.14.18.bb index
> > 2c93d40..751912f 100644 ---
> > a/meta-isar/recipes-kernel/linux/linux-mainline_4.14.18.bb +++
> > b/meta-isar/recipes-kernel/linux/linux-mainline_4.14.18.bb @@ -8,10
> > +8,8 @@ require recipes-kernel/linux/linux-custom.inc 
> >  SRC_URI += " \
> > -
> > https://cdn.kernel.org/pub/linux/kernel/v4.x/linux-${PV}.tar.xz \
> > +
> > https://cdn.kernel.org/pub/linux/kernel/v4.x/linux-${PV}.tar.xz;subdir=${P}
> > \ file://x86_64_defconfig" SRC_URI[sha256sum] =
> > "866a94c1c38d923ae18e74b683d7a8a79b674ebdfe7f40f1a3be9a27d39fe354" 
> > -S = "linux-${PV}"
> > -
> >  KERNEL_DEFCONFIG_qemuamd64 = "x86_64_defconfig"
> > diff --git a/meta/classes/dpkg-base.bbclass
> > b/meta/classes/dpkg-base.bbclass index 3c3a484..3845d8c 100644
> > --- a/meta/classes/dpkg-base.bbclass
> > +++ b/meta/classes/dpkg-base.bbclass
> > @@ -2,8 +2,8 @@
> >  # Copyright (C) 2017 Siemens AG
> >  
> >  do_adjust_git() {
> > -    if [ -f ${WORKDIR}/${S}/.git/objects/info/alternates ]; then
> > -        sed -i ${WORKDIR}/${S}/.git/objects/info/alternates \
> > +    if [ -f ${S}/.git/objects/info/alternates ]; then
> > +        sed -i ${S}/.git/objects/info/alternates \
> >              -e 's|${DL_DIR}|/downloads|'
> >      fi
> >  }
> > @@ -21,9 +21,18 @@ do_build[depends] = "buildchroot:do_build"
> >  DEPENDS ?= ""
> >  do_build[deptask] = "do_deploy_deb"
> >  
> > +def get_package_srcdir(d):
> > +    s = d.getVar("S", True)
> > +    workdir = d.getVar("WORKDIR", True)
> > +    if s.startswith(workdir):
> > +        return s[len(workdir)+1:]
> > +    bb.warn('S does not start with WORKDIR')
> > +    return s
> > +
> >  # Each package should have its own unique build folder, so use
> >  # recipe name as identifier
> >  PP = "/home/builder/${PN}"
> > +PPS ?= "${@get_package_srcdir(d)}"
> >  
> >  BUILDROOT = "${BUILDCHROOT_DIR}/${PP}"
> >  do_build[stamp-extra-info] = "${DISTRO}-${DISTRO_ARCH}"
> > diff --git a/meta/classes/dpkg.bbclass b/meta/classes/dpkg.bbclass
> > index 06f0579..c8d4ac5 100644
> > --- a/meta/classes/dpkg.bbclass
> > +++ b/meta/classes/dpkg.bbclass
> > @@ -6,5 +6,5 @@ inherit dpkg-base
> >  # Build package from sources using build script
> >  dpkg_runbuild() {
> >      E="${@ bb.utils.export_proxies(d)}"
> > -    sudo -E chroot ${BUILDCHROOT_DIR} /build.sh ${PP}/${S}
> > +    sudo -E chroot ${BUILDCHROOT_DIR} /build.sh ${PP}/${PPS}
> >  }
> > diff --git a/meta/classes/patch.bbclass b/meta/classes/patch.bbclass
> > index 26a0c81..1559359 100644
> > --- a/meta/classes/patch.bbclass
> > +++ b/meta/classes/patch.bbclass
> > @@ -5,7 +5,7 @@ python do_patch() {
> >      import subprocess
> >  
> >      workdir = d.getVar("WORKDIR", True) + "/"
> > -    src_dir = workdir + (d.getVar("S", True) or "")
> > +    src_dir = (d.getVar("S", True) or "")  
> 
> That's not equivalent, but I'm undecided if it matters. S should never
> be unset (bitbake.conf holds the default). So we could just pull S and
> let the build fail if that assumption should ever be wrong.

I was wondering why the or "" was there in the first place, S was
always set even before recipes decided to overwrite it. Some python
sniplets seem to return Null on some intermediate steps when using
layers. Might that be why you introduced the 'or ""'?

Henning

> >  
> >      for src_uri in (d.getVar("SRC_URI", True) or "").split():
> >          try:
> > diff --git a/meta/recipes-kernel/linux-module/module.inc
> > b/meta/recipes-kernel/linux-module/module.inc index
> > ec1c4b0..3075f44 100644 ---
> > a/meta/recipes-kernel/linux-module/module.inc +++
> > b/meta/recipes-kernel/linux-module/module.inc @@ -18,14 +18,14 @@
> > AUTOLOAD ?= "0" inherit dpkg
> >  
> >  dpkg_runbuild_prepend() {
> > -    cp -r ${WORKDIR}/debian ${WORKDIR}/${S}/
> > +    cp -r ${WORKDIR}/debian ${S}/
> >      sed -i -e 's/@PN@/${PN}/g' -e 's/@PV@/${PV}/g' \
> >             -e 's/@KERNEL_NAME@/${KERNEL_NAME}/g' \
> >             -e 's/@DESCRIPTION@/${DESCRIPTION}/g' \
> > -        ${WORKDIR}/${S}/debian/changelog
> > ${WORKDIR}/${S}/debian/control
> > +        ${S}/debian/changelog ${S}/debian/control
> >  
> >      if [ ${AUTOLOAD} = "1" ]; then
> > -        echo "echo ${PN} >> /etc/modules" >>
> > ${WORKDIR}/${S}/debian/postinst
> > -        chmod +x ${WORKDIR}/${S}/debian/postinst
> > +        echo "echo ${PN} >> /etc/modules" >> ${S}/debian/postinst
> > +        chmod +x ${S}/debian/postinst
> >      fi
> >  }
> > diff --git a/meta/recipes-kernel/linux/linux-custom.inc
> > b/meta/recipes-kernel/linux/linux-custom.inc index 0498dfa..2643fec
> > 100644 --- a/meta/recipes-kernel/linux/linux-custom.inc
> > +++ b/meta/recipes-kernel/linux/linux-custom.inc
> > @@ -32,7 +32,7 @@ dpkg_runbuild() {
> >  	# Install package builder script
> >  	sudo install -m 755 ${WORKDIR}/build-kernel.sh
> > ${BUILDCHROOT_DIR} 
> > -	sudo cp ${WORKDIR}/${KERNEL_DEFCONFIG}
> > ${WORKDIR}/${S}/.config
> > +	sudo cp ${WORKDIR}/${KERNEL_DEFCONFIG} ${S}/.config
> >  
> >  	E="${@ bb.utils.export_proxies(d)}"
> >  
> >   
> 
> Otherwise:
> Reviewed-by: Jan Kiszka <jan.kiszka@siemens.com>
> 
> Jan


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] Correctly use the bitbake variable S from now on
  2018-04-19 10:36   ` Henning Schild
@ 2018-04-19 11:42     ` Jan Kiszka
  2018-04-19 16:56       ` Henning Schild
  0 siblings, 1 reply; 12+ messages in thread
From: Jan Kiszka @ 2018-04-19 11:42 UTC (permalink / raw)
  To: Henning Schild; +Cc: isar-users

On 2018-04-19 12:36, Henning Schild wrote:
>>> diff --git a/meta/classes/patch.bbclass b/meta/classes/patch.bbclass
>>> index 26a0c81..1559359 100644
>>> --- a/meta/classes/patch.bbclass
>>> +++ b/meta/classes/patch.bbclass
>>> @@ -5,7 +5,7 @@ python do_patch() {
>>>      import subprocess
>>>  
>>>      workdir = d.getVar("WORKDIR", True) + "/"
>>> -    src_dir = workdir + (d.getVar("S", True) or "")
>>> +    src_dir = (d.getVar("S", True) or "")  
>>
>> That's not equivalent, but I'm undecided if it matters. S should never
>> be unset (bitbake.conf holds the default). So we could just pull S and
>> let the build fail if that assumption should ever be wrong.
> 
> I was wondering why the or "" was there in the first place, S was
> always set even before recipes decided to overwrite it. Some python
> sniplets seem to return Null on some intermediate steps when using
> layers. Might that be why you introduced the 'or ""'?

Problem is that I don't remember doing this intentionally. But if that
were the case (intermediate Null state), it should not longer harm us
now (no more "x + Null").

Jan

-- 
Siemens AG, Corporate Technology, CT RDA IOT SES-DE
Corporate Competence Center Embedded Linux

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] Correctly use the bitbake variable S from now on
  2018-04-19 11:42     ` Jan Kiszka
@ 2018-04-19 16:56       ` Henning Schild
  0 siblings, 0 replies; 12+ messages in thread
From: Henning Schild @ 2018-04-19 16:56 UTC (permalink / raw)
  To: Jan Kiszka; +Cc: isar-users

Am Thu, 19 Apr 2018 13:42:22 +0200
schrieb Jan Kiszka <jan.kiszka@siemens.com>:

> On 2018-04-19 12:36, Henning Schild wrote:
> >>> diff --git a/meta/classes/patch.bbclass
> >>> b/meta/classes/patch.bbclass index 26a0c81..1559359 100644
> >>> --- a/meta/classes/patch.bbclass
> >>> +++ b/meta/classes/patch.bbclass
> >>> @@ -5,7 +5,7 @@ python do_patch() {
> >>>      import subprocess
> >>>  
> >>>      workdir = d.getVar("WORKDIR", True) + "/"
> >>> -    src_dir = workdir + (d.getVar("S", True) or "")
> >>> +    src_dir = (d.getVar("S", True) or "")    
> >>
> >> That's not equivalent, but I'm undecided if it matters. S should
> >> never be unset (bitbake.conf holds the default). So we could just
> >> pull S and let the build fail if that assumption should ever be
> >> wrong.  
> > 
> > I was wondering why the or "" was there in the first place, S was
> > always set even before recipes decided to overwrite it. Some python
> > sniplets seem to return Null on some intermediate steps when using
> > layers. Might that be why you introduced the 'or ""'?  
> 
> Problem is that I don't remember doing this intentionally. But if that
> were the case (intermediate Null state), it should not longer harm us
> now (no more "x + Null").

It was + ed later, but never seem to be Null. V2 on the way.

Henning

> Jan
> 


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH v2] Correctly use the bitbake variable S from now on
  2018-04-19  9:26 [PATCH] Correctly use the bitbake variable S from now on Henning Schild
  2018-04-19  9:45 ` Jan Kiszka
@ 2018-04-19 16:58 ` Henning Schild
  2018-04-20 11:06   ` Henning Schild
  2018-04-20 12:36   ` [PATCH v3] " Henning Schild
  1 sibling, 2 replies; 12+ messages in thread
From: Henning Schild @ 2018-04-19 16:58 UTC (permalink / raw)
  To: isar-users; +Cc: Jan Kiszka, Henning Schild

S is defined as ${WORKDIR}/${P} so it already contains ${WORKDIR} by its
definition. In Isar it was often concatinated with WORKDIR again. One
example where this was a problem is if you specified a patch in SRC_URI
but did not actually overwrite S.
Align the use of the variable with OE and bitbake defaults again.

Impact: layers building on top of Isar will have to adjust their recipes
like the internal ones needed modification. The suggestion is to not set
S and make sure to unpack to ${WORKDIR}/${P}. When setting S make sure
to include ${WORKDIR}. This patch also introduces a warning and tries to
preserve the old behaviour a bit. However if your recipe uses patches in
SRC_URI and sets S you will get the warning and do_patch will fail.

Signed-off-by: Henning Schild <henning.schild@siemens.com>
---
 doc/user_manual.md                                        |  2 +-
 meta-isar/recipes-app/example-hello/example-hello.bb      |  4 +---
 meta-isar/recipes-app/libhello/libhello.bb                |  4 +---
 meta-isar/recipes-kernel/example-module/example-module.bb |  2 +-
 meta-isar/recipes-kernel/linux/linux-cip_4.4.bb           |  4 +---
 meta-isar/recipes-kernel/linux/linux-mainline_4.14.18.bb  |  4 +---
 meta/classes/dpkg-base.bbclass                            | 13 +++++++++++--
 meta/classes/dpkg.bbclass                                 |  2 +-
 meta/classes/patch.bbclass                                |  2 +-
 meta/recipes-kernel/linux-module/module.inc               |  8 ++++----
 meta/recipes-kernel/linux/linux-custom.inc                |  2 +-
 11 files changed, 24 insertions(+), 23 deletions(-)

diff --git a/doc/user_manual.md b/doc/user_manual.md
index 2bd3793..3072bd5 100644
--- a/doc/user_manual.md
+++ b/doc/user_manual.md
@@ -511,7 +511,7 @@ PV = "1.0"
 SRC_URI = "git://github.com/ilbers/hello.git"
 SRCREV = "ad7065ecc4840cc436bfcdac427386dbba4ea719"
 
-S = "git"
+S = "${WORKDIR}/git"
 
 inherit dpkg
 ```
diff --git a/meta-isar/recipes-app/example-hello/example-hello.bb b/meta-isar/recipes-app/example-hello/example-hello.bb
index 9788ec0..d23ee6c 100644
--- a/meta-isar/recipes-app/example-hello/example-hello.bb
+++ b/meta-isar/recipes-app/example-hello/example-hello.bb
@@ -15,11 +15,9 @@ PV = "0.2-86cc719"
 DEPENDS += "libhello"
 
 SRC_URI = " \
-    git://github.com/ilbers/hello.git;protocol=https \
+    git://github.com/ilbers/hello.git;protocol=https;destsuffix=${P} \
     file://0001-Add-some-help.patch \
     file://yet-another-change.txt;apply=yes;striplevel=0"
 SRCREV = "86cc719b3359adc3c4e243387feba50360a860f3"
 
-S = "git"
-
 inherit dpkg
diff --git a/meta-isar/recipes-app/libhello/libhello.bb b/meta-isar/recipes-app/libhello/libhello.bb
index 1875831..4e75f98 100644
--- a/meta-isar/recipes-app/libhello/libhello.bb
+++ b/meta-isar/recipes-app/libhello/libhello.bb
@@ -10,9 +10,7 @@ LIC_FILES_CHKSUM = "file://${LAYERDIR_isar}/licenses/COPYING.GPLv2;md5=751419260
 
 PV = "0.1-98f2e41"
 
-SRC_URI = "git://github.com/ilbers/libhello.git;protocol=https"
+SRC_URI = "git://github.com/ilbers/libhello.git;protocol=https;destsuffix=${P}"
 SRCREV = "98f2e41e7d05ab8d19b0c5d160b104b725c8fd93"
 
-S = "git"
-
 inherit dpkg
diff --git a/meta-isar/recipes-kernel/example-module/example-module.bb b/meta-isar/recipes-kernel/example-module/example-module.bb
index dbaf5ac..98d0aaa 100644
--- a/meta-isar/recipes-kernel/example-module/example-module.bb
+++ b/meta-isar/recipes-kernel/example-module/example-module.bb
@@ -9,6 +9,6 @@ include recipes-kernel/linux-module/module.inc
 
 SRC_URI += "file://src"
 
-S = "src"
+S = "${WORKDIR}/src"
 
 AUTOLOAD = "1"
diff --git a/meta-isar/recipes-kernel/linux/linux-cip_4.4.bb b/meta-isar/recipes-kernel/linux/linux-cip_4.4.bb
index 7502f70..e9aaa9f 100644
--- a/meta-isar/recipes-kernel/linux/linux-cip_4.4.bb
+++ b/meta-isar/recipes-kernel/linux/linux-cip_4.4.bb
@@ -8,12 +8,10 @@
 require recipes-kernel/linux/linux-custom.inc
 
 SRC_URI += " \
-    git://git.kernel.org/pub/scm/linux/kernel/git/bwh/linux-cip.git;branch=linux-4.4.y-cip \
+    git://git.kernel.org/pub/scm/linux/kernel/git/bwh/linux-cip.git;branch=linux-4.4.y-cip;destsuffix=${P} \
     file://x86_64_defconfig"
 
 SRCREV = "4e52cc5f668c4666e31a8485725b5f4e897b3baf"
 PV = "4.4.112-cip18"
 
-S = "git"
-
 KERNEL_DEFCONFIG_qemuamd64 = "x86_64_defconfig"
diff --git a/meta-isar/recipes-kernel/linux/linux-mainline_4.14.18.bb b/meta-isar/recipes-kernel/linux/linux-mainline_4.14.18.bb
index 2c93d40..751912f 100644
--- a/meta-isar/recipes-kernel/linux/linux-mainline_4.14.18.bb
+++ b/meta-isar/recipes-kernel/linux/linux-mainline_4.14.18.bb
@@ -8,10 +8,8 @@
 require recipes-kernel/linux/linux-custom.inc
 
 SRC_URI += " \
-    https://cdn.kernel.org/pub/linux/kernel/v4.x/linux-${PV}.tar.xz \
+    https://cdn.kernel.org/pub/linux/kernel/v4.x/linux-${PV}.tar.xz;subdir=${P} \
     file://x86_64_defconfig"
 SRC_URI[sha256sum] = "866a94c1c38d923ae18e74b683d7a8a79b674ebdfe7f40f1a3be9a27d39fe354"
 
-S = "linux-${PV}"
-
 KERNEL_DEFCONFIG_qemuamd64 = "x86_64_defconfig"
diff --git a/meta/classes/dpkg-base.bbclass b/meta/classes/dpkg-base.bbclass
index 3c3a484..3845d8c 100644
--- a/meta/classes/dpkg-base.bbclass
+++ b/meta/classes/dpkg-base.bbclass
@@ -2,8 +2,8 @@
 # Copyright (C) 2017 Siemens AG
 
 do_adjust_git() {
-    if [ -f ${WORKDIR}/${S}/.git/objects/info/alternates ]; then
-        sed -i ${WORKDIR}/${S}/.git/objects/info/alternates \
+    if [ -f ${S}/.git/objects/info/alternates ]; then
+        sed -i ${S}/.git/objects/info/alternates \
             -e 's|${DL_DIR}|/downloads|'
     fi
 }
@@ -21,9 +21,18 @@ do_build[depends] = "buildchroot:do_build"
 DEPENDS ?= ""
 do_build[deptask] = "do_deploy_deb"
 
+def get_package_srcdir(d):
+    s = d.getVar("S", True)
+    workdir = d.getVar("WORKDIR", True)
+    if s.startswith(workdir):
+        return s[len(workdir)+1:]
+    bb.warn('S does not start with WORKDIR')
+    return s
+
 # Each package should have its own unique build folder, so use
 # recipe name as identifier
 PP = "/home/builder/${PN}"
+PPS ?= "${@get_package_srcdir(d)}"
 
 BUILDROOT = "${BUILDCHROOT_DIR}/${PP}"
 do_build[stamp-extra-info] = "${DISTRO}-${DISTRO_ARCH}"
diff --git a/meta/classes/dpkg.bbclass b/meta/classes/dpkg.bbclass
index 06f0579..c8d4ac5 100644
--- a/meta/classes/dpkg.bbclass
+++ b/meta/classes/dpkg.bbclass
@@ -6,5 +6,5 @@ inherit dpkg-base
 # Build package from sources using build script
 dpkg_runbuild() {
     E="${@ bb.utils.export_proxies(d)}"
-    sudo -E chroot ${BUILDCHROOT_DIR} /build.sh ${PP}/${S}
+    sudo -E chroot ${BUILDCHROOT_DIR} /build.sh ${PP}/${PPS}
 }
diff --git a/meta/classes/patch.bbclass b/meta/classes/patch.bbclass
index 26a0c81..0bc449f 100644
--- a/meta/classes/patch.bbclass
+++ b/meta/classes/patch.bbclass
@@ -5,7 +5,7 @@ python do_patch() {
     import subprocess
 
     workdir = d.getVar("WORKDIR", True) + "/"
-    src_dir = workdir + (d.getVar("S", True) or "")
+    src_dir = d.getVar("S", True)
 
     for src_uri in (d.getVar("SRC_URI", True) or "").split():
         try:
diff --git a/meta/recipes-kernel/linux-module/module.inc b/meta/recipes-kernel/linux-module/module.inc
index ec1c4b0..3075f44 100644
--- a/meta/recipes-kernel/linux-module/module.inc
+++ b/meta/recipes-kernel/linux-module/module.inc
@@ -18,14 +18,14 @@ AUTOLOAD ?= "0"
 inherit dpkg
 
 dpkg_runbuild_prepend() {
-    cp -r ${WORKDIR}/debian ${WORKDIR}/${S}/
+    cp -r ${WORKDIR}/debian ${S}/
     sed -i -e 's/@PN@/${PN}/g' -e 's/@PV@/${PV}/g' \
            -e 's/@KERNEL_NAME@/${KERNEL_NAME}/g' \
            -e 's/@DESCRIPTION@/${DESCRIPTION}/g' \
-        ${WORKDIR}/${S}/debian/changelog ${WORKDIR}/${S}/debian/control
+        ${S}/debian/changelog ${S}/debian/control
 
     if [ ${AUTOLOAD} = "1" ]; then
-        echo "echo ${PN} >> /etc/modules" >> ${WORKDIR}/${S}/debian/postinst
-        chmod +x ${WORKDIR}/${S}/debian/postinst
+        echo "echo ${PN} >> /etc/modules" >> ${S}/debian/postinst
+        chmod +x ${S}/debian/postinst
     fi
 }
diff --git a/meta/recipes-kernel/linux/linux-custom.inc b/meta/recipes-kernel/linux/linux-custom.inc
index 0498dfa..2643fec 100644
--- a/meta/recipes-kernel/linux/linux-custom.inc
+++ b/meta/recipes-kernel/linux/linux-custom.inc
@@ -32,7 +32,7 @@ dpkg_runbuild() {
 	# Install package builder script
 	sudo install -m 755 ${WORKDIR}/build-kernel.sh ${BUILDCHROOT_DIR}
 
-	sudo cp ${WORKDIR}/${KERNEL_DEFCONFIG} ${WORKDIR}/${S}/.config
+	sudo cp ${WORKDIR}/${KERNEL_DEFCONFIG} ${S}/.config
 
 	E="${@ bb.utils.export_proxies(d)}"
 
-- 
2.16.1


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2] Correctly use the bitbake variable S from now on
  2018-04-19 16:58 ` [PATCH v2] " Henning Schild
@ 2018-04-20 11:06   ` Henning Schild
  2018-04-20 12:36   ` [PATCH v3] " Henning Schild
  1 sibling, 0 replies; 12+ messages in thread
From: Henning Schild @ 2018-04-20 11:06 UTC (permalink / raw)
  To: isar-users; +Cc: Jan Kiszka

Hold this one, it still has issues building custom kernels. A path that
is not on CI testing ...

Henning

Am Thu, 19 Apr 2018 18:58:32 +0200
schrieb Henning Schild <henning.schild@siemens.com>:

> S is defined as ${WORKDIR}/${P} so it already contains ${WORKDIR} by
> its definition. In Isar it was often concatinated with WORKDIR again.
> One example where this was a problem is if you specified a patch in
> SRC_URI but did not actually overwrite S.
> Align the use of the variable with OE and bitbake defaults again.
> 
> Impact: layers building on top of Isar will have to adjust their
> recipes like the internal ones needed modification. The suggestion is
> to not set S and make sure to unpack to ${WORKDIR}/${P}. When setting
> S make sure to include ${WORKDIR}. This patch also introduces a
> warning and tries to preserve the old behaviour a bit. However if
> your recipe uses patches in SRC_URI and sets S you will get the
> warning and do_patch will fail.
> 
> Signed-off-by: Henning Schild <henning.schild@siemens.com>
> ---
>  doc/user_manual.md                                        |  2 +-
>  meta-isar/recipes-app/example-hello/example-hello.bb      |  4 +---
>  meta-isar/recipes-app/libhello/libhello.bb                |  4 +---
>  meta-isar/recipes-kernel/example-module/example-module.bb |  2 +-
>  meta-isar/recipes-kernel/linux/linux-cip_4.4.bb           |  4 +---
>  meta-isar/recipes-kernel/linux/linux-mainline_4.14.18.bb  |  4 +---
>  meta/classes/dpkg-base.bbclass                            | 13
> +++++++++++--
> meta/classes/dpkg.bbclass                                 |  2 +-
> meta/classes/patch.bbclass                                |  2 +-
> meta/recipes-kernel/linux-module/module.inc               |  8
> ++++---- meta/recipes-kernel/linux/linux-custom.inc                |
> 2 +- 11 files changed, 24 insertions(+), 23 deletions(-)
> 
> diff --git a/doc/user_manual.md b/doc/user_manual.md
> index 2bd3793..3072bd5 100644
> --- a/doc/user_manual.md
> +++ b/doc/user_manual.md
> @@ -511,7 +511,7 @@ PV = "1.0"
>  SRC_URI = "git://github.com/ilbers/hello.git"
>  SRCREV = "ad7065ecc4840cc436bfcdac427386dbba4ea719"
>  
> -S = "git"
> +S = "${WORKDIR}/git"
>  
>  inherit dpkg
>  ```
> diff --git a/meta-isar/recipes-app/example-hello/example-hello.bb
> b/meta-isar/recipes-app/example-hello/example-hello.bb index
> 9788ec0..d23ee6c 100644 ---
> a/meta-isar/recipes-app/example-hello/example-hello.bb +++
> b/meta-isar/recipes-app/example-hello/example-hello.bb @@ -15,11
> +15,9 @@ PV = "0.2-86cc719" DEPENDS += "libhello"
>  
>  SRC_URI = " \
> -    git://github.com/ilbers/hello.git;protocol=https \
> +    git://github.com/ilbers/hello.git;protocol=https;destsuffix=${P}
> \ file://0001-Add-some-help.patch \
>      file://yet-another-change.txt;apply=yes;striplevel=0"
>  SRCREV = "86cc719b3359adc3c4e243387feba50360a860f3"
>  
> -S = "git"
> -
>  inherit dpkg
> diff --git a/meta-isar/recipes-app/libhello/libhello.bb
> b/meta-isar/recipes-app/libhello/libhello.bb index 1875831..4e75f98
> 100644 --- a/meta-isar/recipes-app/libhello/libhello.bb
> +++ b/meta-isar/recipes-app/libhello/libhello.bb
> @@ -10,9 +10,7 @@ LIC_FILES_CHKSUM =
> "file://${LAYERDIR_isar}/licenses/COPYING.GPLv2;md5=751419260 
>  PV = "0.1-98f2e41"
>  
> -SRC_URI = "git://github.com/ilbers/libhello.git;protocol=https"
> +SRC_URI =
> "git://github.com/ilbers/libhello.git;protocol=https;destsuffix=${P}"
> SRCREV = "98f2e41e7d05ab8d19b0c5d160b104b725c8fd93" 
> -S = "git"
> -
>  inherit dpkg
> diff --git
> a/meta-isar/recipes-kernel/example-module/example-module.bb
> b/meta-isar/recipes-kernel/example-module/example-module.bb index
> dbaf5ac..98d0aaa 100644 ---
> a/meta-isar/recipes-kernel/example-module/example-module.bb +++
> b/meta-isar/recipes-kernel/example-module/example-module.bb @@ -9,6
> +9,6 @@ include recipes-kernel/linux-module/module.inc SRC_URI +=
> "file://src" 
> -S = "src"
> +S = "${WORKDIR}/src"
>  
>  AUTOLOAD = "1"
> diff --git a/meta-isar/recipes-kernel/linux/linux-cip_4.4.bb
> b/meta-isar/recipes-kernel/linux/linux-cip_4.4.bb index
> 7502f70..e9aaa9f 100644 ---
> a/meta-isar/recipes-kernel/linux/linux-cip_4.4.bb +++
> b/meta-isar/recipes-kernel/linux/linux-cip_4.4.bb @@ -8,12 +8,10 @@
>  require recipes-kernel/linux/linux-custom.inc
>  
>  SRC_URI += " \
> -
> git://git.kernel.org/pub/scm/linux/kernel/git/bwh/linux-cip.git;branch=linux-4.4.y-cip
> \
> +
> git://git.kernel.org/pub/scm/linux/kernel/git/bwh/linux-cip.git;branch=linux-4.4.y-cip;destsuffix=${P}
> \ file://x86_64_defconfig" 
>  SRCREV = "4e52cc5f668c4666e31a8485725b5f4e897b3baf"
>  PV = "4.4.112-cip18"
>  
> -S = "git"
> -
>  KERNEL_DEFCONFIG_qemuamd64 = "x86_64_defconfig"
> diff --git a/meta-isar/recipes-kernel/linux/linux-mainline_4.14.18.bb
> b/meta-isar/recipes-kernel/linux/linux-mainline_4.14.18.bb index
> 2c93d40..751912f 100644 ---
> a/meta-isar/recipes-kernel/linux/linux-mainline_4.14.18.bb +++
> b/meta-isar/recipes-kernel/linux/linux-mainline_4.14.18.bb @@ -8,10
> +8,8 @@ require recipes-kernel/linux/linux-custom.inc
>  
>  SRC_URI += " \
> -    https://cdn.kernel.org/pub/linux/kernel/v4.x/linux-${PV}.tar.xz \
> +
> https://cdn.kernel.org/pub/linux/kernel/v4.x/linux-${PV}.tar.xz;subdir=${P}
> \ file://x86_64_defconfig" SRC_URI[sha256sum] =
> "866a94c1c38d923ae18e74b683d7a8a79b674ebdfe7f40f1a3be9a27d39fe354" 
> -S = "linux-${PV}"
> -
>  KERNEL_DEFCONFIG_qemuamd64 = "x86_64_defconfig"
> diff --git a/meta/classes/dpkg-base.bbclass
> b/meta/classes/dpkg-base.bbclass index 3c3a484..3845d8c 100644
> --- a/meta/classes/dpkg-base.bbclass
> +++ b/meta/classes/dpkg-base.bbclass
> @@ -2,8 +2,8 @@
>  # Copyright (C) 2017 Siemens AG
>  
>  do_adjust_git() {
> -    if [ -f ${WORKDIR}/${S}/.git/objects/info/alternates ]; then
> -        sed -i ${WORKDIR}/${S}/.git/objects/info/alternates \
> +    if [ -f ${S}/.git/objects/info/alternates ]; then
> +        sed -i ${S}/.git/objects/info/alternates \
>              -e 's|${DL_DIR}|/downloads|'
>      fi
>  }
> @@ -21,9 +21,18 @@ do_build[depends] = "buildchroot:do_build"
>  DEPENDS ?= ""
>  do_build[deptask] = "do_deploy_deb"
>  
> +def get_package_srcdir(d):
> +    s = d.getVar("S", True)
> +    workdir = d.getVar("WORKDIR", True)
> +    if s.startswith(workdir):
> +        return s[len(workdir)+1:]
> +    bb.warn('S does not start with WORKDIR')
> +    return s
> +
>  # Each package should have its own unique build folder, so use
>  # recipe name as identifier
>  PP = "/home/builder/${PN}"
> +PPS ?= "${@get_package_srcdir(d)}"
>  
>  BUILDROOT = "${BUILDCHROOT_DIR}/${PP}"
>  do_build[stamp-extra-info] = "${DISTRO}-${DISTRO_ARCH}"
> diff --git a/meta/classes/dpkg.bbclass b/meta/classes/dpkg.bbclass
> index 06f0579..c8d4ac5 100644
> --- a/meta/classes/dpkg.bbclass
> +++ b/meta/classes/dpkg.bbclass
> @@ -6,5 +6,5 @@ inherit dpkg-base
>  # Build package from sources using build script
>  dpkg_runbuild() {
>      E="${@ bb.utils.export_proxies(d)}"
> -    sudo -E chroot ${BUILDCHROOT_DIR} /build.sh ${PP}/${S}
> +    sudo -E chroot ${BUILDCHROOT_DIR} /build.sh ${PP}/${PPS}
>  }
> diff --git a/meta/classes/patch.bbclass b/meta/classes/patch.bbclass
> index 26a0c81..0bc449f 100644
> --- a/meta/classes/patch.bbclass
> +++ b/meta/classes/patch.bbclass
> @@ -5,7 +5,7 @@ python do_patch() {
>      import subprocess
>  
>      workdir = d.getVar("WORKDIR", True) + "/"
> -    src_dir = workdir + (d.getVar("S", True) or "")
> +    src_dir = d.getVar("S", True)
>  
>      for src_uri in (d.getVar("SRC_URI", True) or "").split():
>          try:
> diff --git a/meta/recipes-kernel/linux-module/module.inc
> b/meta/recipes-kernel/linux-module/module.inc index ec1c4b0..3075f44
> 100644 --- a/meta/recipes-kernel/linux-module/module.inc
> +++ b/meta/recipes-kernel/linux-module/module.inc
> @@ -18,14 +18,14 @@ AUTOLOAD ?= "0"
>  inherit dpkg
>  
>  dpkg_runbuild_prepend() {
> -    cp -r ${WORKDIR}/debian ${WORKDIR}/${S}/
> +    cp -r ${WORKDIR}/debian ${S}/
>      sed -i -e 's/@PN@/${PN}/g' -e 's/@PV@/${PV}/g' \
>             -e 's/@KERNEL_NAME@/${KERNEL_NAME}/g' \
>             -e 's/@DESCRIPTION@/${DESCRIPTION}/g' \
> -        ${WORKDIR}/${S}/debian/changelog
> ${WORKDIR}/${S}/debian/control
> +        ${S}/debian/changelog ${S}/debian/control
>  
>      if [ ${AUTOLOAD} = "1" ]; then
> -        echo "echo ${PN} >> /etc/modules" >>
> ${WORKDIR}/${S}/debian/postinst
> -        chmod +x ${WORKDIR}/${S}/debian/postinst
> +        echo "echo ${PN} >> /etc/modules" >> ${S}/debian/postinst
> +        chmod +x ${S}/debian/postinst
>      fi
>  }
> diff --git a/meta/recipes-kernel/linux/linux-custom.inc
> b/meta/recipes-kernel/linux/linux-custom.inc index 0498dfa..2643fec
> 100644 --- a/meta/recipes-kernel/linux/linux-custom.inc
> +++ b/meta/recipes-kernel/linux/linux-custom.inc
> @@ -32,7 +32,7 @@ dpkg_runbuild() {
>  	# Install package builder script
>  	sudo install -m 755 ${WORKDIR}/build-kernel.sh
> ${BUILDCHROOT_DIR} 
> -	sudo cp ${WORKDIR}/${KERNEL_DEFCONFIG}
> ${WORKDIR}/${S}/.config
> +	sudo cp ${WORKDIR}/${KERNEL_DEFCONFIG} ${S}/.config
>  
>  	E="${@ bb.utils.export_proxies(d)}"
>  


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH v3] Correctly use the bitbake variable S from now on
  2018-04-19 16:58 ` [PATCH v2] " Henning Schild
  2018-04-20 11:06   ` Henning Schild
@ 2018-04-20 12:36   ` Henning Schild
  2018-04-20 12:37     ` Henning Schild
  2018-05-11 15:33     ` Alexander Smirnov
  1 sibling, 2 replies; 12+ messages in thread
From: Henning Schild @ 2018-04-20 12:36 UTC (permalink / raw)
  To: isar-users; +Cc: Jan Kiszka, Henning Schild

S is defined as ${WORKDIR}/${P} so it already contains ${WORKDIR} by its
definition. In Isar it was often concatinated with WORKDIR again. One
example where this was a problem is if you specified a patch in SRC_URI
but did not actually overwrite S.
Align the use of the variable with OE and bitbake defaults again.

Impact: layers building on top of Isar will have to adjust their recipes
like the internal ones needed modification. The suggestion is to not set
S and make sure to unpack to ${WORKDIR}/${P}. When setting S make sure
to include ${WORKDIR}. This patch also introduces a warning and tries to
preserve the old behaviour a bit. However if your recipe uses patches in
SRC_URI and sets S you will get the warning and do_patch will fail.

Signed-off-by: Henning Schild <henning.schild@siemens.com>
---
 doc/user_manual.md                                        |  2 +-
 meta-isar/recipes-app/example-hello/example-hello.bb      |  4 +---
 meta-isar/recipes-app/libhello/libhello.bb                |  4 +---
 meta-isar/recipes-kernel/example-module/example-module.bb |  2 +-
 meta-isar/recipes-kernel/linux/linux-cip_4.4.bb           |  4 +---
 meta-isar/recipes-kernel/linux/linux-mainline_4.14.18.bb  |  2 +-
 meta/classes/dpkg-base.bbclass                            | 13 +++++++++++--
 meta/classes/dpkg.bbclass                                 |  2 +-
 meta/classes/patch.bbclass                                |  2 +-
 meta/recipes-kernel/linux-module/module.inc               |  8 ++++----
 meta/recipes-kernel/linux/linux-custom.inc                |  4 ++--
 11 files changed, 25 insertions(+), 22 deletions(-)

diff --git a/doc/user_manual.md b/doc/user_manual.md
index 2bd3793..3072bd5 100644
--- a/doc/user_manual.md
+++ b/doc/user_manual.md
@@ -511,7 +511,7 @@ PV = "1.0"
 SRC_URI = "git://github.com/ilbers/hello.git"
 SRCREV = "ad7065ecc4840cc436bfcdac427386dbba4ea719"
 
-S = "git"
+S = "${WORKDIR}/git"
 
 inherit dpkg
 ```
diff --git a/meta-isar/recipes-app/example-hello/example-hello.bb b/meta-isar/recipes-app/example-hello/example-hello.bb
index 9788ec0..d23ee6c 100644
--- a/meta-isar/recipes-app/example-hello/example-hello.bb
+++ b/meta-isar/recipes-app/example-hello/example-hello.bb
@@ -15,11 +15,9 @@ PV = "0.2-86cc719"
 DEPENDS += "libhello"
 
 SRC_URI = " \
-    git://github.com/ilbers/hello.git;protocol=https \
+    git://github.com/ilbers/hello.git;protocol=https;destsuffix=${P} \
     file://0001-Add-some-help.patch \
     file://yet-another-change.txt;apply=yes;striplevel=0"
 SRCREV = "86cc719b3359adc3c4e243387feba50360a860f3"
 
-S = "git"
-
 inherit dpkg
diff --git a/meta-isar/recipes-app/libhello/libhello.bb b/meta-isar/recipes-app/libhello/libhello.bb
index 1875831..4e75f98 100644
--- a/meta-isar/recipes-app/libhello/libhello.bb
+++ b/meta-isar/recipes-app/libhello/libhello.bb
@@ -10,9 +10,7 @@ LIC_FILES_CHKSUM = "file://${LAYERDIR_isar}/licenses/COPYING.GPLv2;md5=751419260
 
 PV = "0.1-98f2e41"
 
-SRC_URI = "git://github.com/ilbers/libhello.git;protocol=https"
+SRC_URI = "git://github.com/ilbers/libhello.git;protocol=https;destsuffix=${P}"
 SRCREV = "98f2e41e7d05ab8d19b0c5d160b104b725c8fd93"
 
-S = "git"
-
 inherit dpkg
diff --git a/meta-isar/recipes-kernel/example-module/example-module.bb b/meta-isar/recipes-kernel/example-module/example-module.bb
index dbaf5ac..98d0aaa 100644
--- a/meta-isar/recipes-kernel/example-module/example-module.bb
+++ b/meta-isar/recipes-kernel/example-module/example-module.bb
@@ -9,6 +9,6 @@ include recipes-kernel/linux-module/module.inc
 
 SRC_URI += "file://src"
 
-S = "src"
+S = "${WORKDIR}/src"
 
 AUTOLOAD = "1"
diff --git a/meta-isar/recipes-kernel/linux/linux-cip_4.4.bb b/meta-isar/recipes-kernel/linux/linux-cip_4.4.bb
index 7502f70..e9aaa9f 100644
--- a/meta-isar/recipes-kernel/linux/linux-cip_4.4.bb
+++ b/meta-isar/recipes-kernel/linux/linux-cip_4.4.bb
@@ -8,12 +8,10 @@
 require recipes-kernel/linux/linux-custom.inc
 
 SRC_URI += " \
-    git://git.kernel.org/pub/scm/linux/kernel/git/bwh/linux-cip.git;branch=linux-4.4.y-cip \
+    git://git.kernel.org/pub/scm/linux/kernel/git/bwh/linux-cip.git;branch=linux-4.4.y-cip;destsuffix=${P} \
     file://x86_64_defconfig"
 
 SRCREV = "4e52cc5f668c4666e31a8485725b5f4e897b3baf"
 PV = "4.4.112-cip18"
 
-S = "git"
-
 KERNEL_DEFCONFIG_qemuamd64 = "x86_64_defconfig"
diff --git a/meta-isar/recipes-kernel/linux/linux-mainline_4.14.18.bb b/meta-isar/recipes-kernel/linux/linux-mainline_4.14.18.bb
index 2c93d40..fc465fd 100644
--- a/meta-isar/recipes-kernel/linux/linux-mainline_4.14.18.bb
+++ b/meta-isar/recipes-kernel/linux/linux-mainline_4.14.18.bb
@@ -12,6 +12,6 @@ SRC_URI += " \
     file://x86_64_defconfig"
 SRC_URI[sha256sum] = "866a94c1c38d923ae18e74b683d7a8a79b674ebdfe7f40f1a3be9a27d39fe354"
 
-S = "linux-${PV}"
+S = "${WORKDIR}/linux-${PV}"
 
 KERNEL_DEFCONFIG_qemuamd64 = "x86_64_defconfig"
diff --git a/meta/classes/dpkg-base.bbclass b/meta/classes/dpkg-base.bbclass
index 3c3a484..3845d8c 100644
--- a/meta/classes/dpkg-base.bbclass
+++ b/meta/classes/dpkg-base.bbclass
@@ -2,8 +2,8 @@
 # Copyright (C) 2017 Siemens AG
 
 do_adjust_git() {
-    if [ -f ${WORKDIR}/${S}/.git/objects/info/alternates ]; then
-        sed -i ${WORKDIR}/${S}/.git/objects/info/alternates \
+    if [ -f ${S}/.git/objects/info/alternates ]; then
+        sed -i ${S}/.git/objects/info/alternates \
             -e 's|${DL_DIR}|/downloads|'
     fi
 }
@@ -21,9 +21,18 @@ do_build[depends] = "buildchroot:do_build"
 DEPENDS ?= ""
 do_build[deptask] = "do_deploy_deb"
 
+def get_package_srcdir(d):
+    s = d.getVar("S", True)
+    workdir = d.getVar("WORKDIR", True)
+    if s.startswith(workdir):
+        return s[len(workdir)+1:]
+    bb.warn('S does not start with WORKDIR')
+    return s
+
 # Each package should have its own unique build folder, so use
 # recipe name as identifier
 PP = "/home/builder/${PN}"
+PPS ?= "${@get_package_srcdir(d)}"
 
 BUILDROOT = "${BUILDCHROOT_DIR}/${PP}"
 do_build[stamp-extra-info] = "${DISTRO}-${DISTRO_ARCH}"
diff --git a/meta/classes/dpkg.bbclass b/meta/classes/dpkg.bbclass
index 06f0579..c8d4ac5 100644
--- a/meta/classes/dpkg.bbclass
+++ b/meta/classes/dpkg.bbclass
@@ -6,5 +6,5 @@ inherit dpkg-base
 # Build package from sources using build script
 dpkg_runbuild() {
     E="${@ bb.utils.export_proxies(d)}"
-    sudo -E chroot ${BUILDCHROOT_DIR} /build.sh ${PP}/${S}
+    sudo -E chroot ${BUILDCHROOT_DIR} /build.sh ${PP}/${PPS}
 }
diff --git a/meta/classes/patch.bbclass b/meta/classes/patch.bbclass
index 26a0c81..0bc449f 100644
--- a/meta/classes/patch.bbclass
+++ b/meta/classes/patch.bbclass
@@ -5,7 +5,7 @@ python do_patch() {
     import subprocess
 
     workdir = d.getVar("WORKDIR", True) + "/"
-    src_dir = workdir + (d.getVar("S", True) or "")
+    src_dir = d.getVar("S", True)
 
     for src_uri in (d.getVar("SRC_URI", True) or "").split():
         try:
diff --git a/meta/recipes-kernel/linux-module/module.inc b/meta/recipes-kernel/linux-module/module.inc
index ec1c4b0..3075f44 100644
--- a/meta/recipes-kernel/linux-module/module.inc
+++ b/meta/recipes-kernel/linux-module/module.inc
@@ -18,14 +18,14 @@ AUTOLOAD ?= "0"
 inherit dpkg
 
 dpkg_runbuild_prepend() {
-    cp -r ${WORKDIR}/debian ${WORKDIR}/${S}/
+    cp -r ${WORKDIR}/debian ${S}/
     sed -i -e 's/@PN@/${PN}/g' -e 's/@PV@/${PV}/g' \
            -e 's/@KERNEL_NAME@/${KERNEL_NAME}/g' \
            -e 's/@DESCRIPTION@/${DESCRIPTION}/g' \
-        ${WORKDIR}/${S}/debian/changelog ${WORKDIR}/${S}/debian/control
+        ${S}/debian/changelog ${S}/debian/control
 
     if [ ${AUTOLOAD} = "1" ]; then
-        echo "echo ${PN} >> /etc/modules" >> ${WORKDIR}/${S}/debian/postinst
-        chmod +x ${WORKDIR}/${S}/debian/postinst
+        echo "echo ${PN} >> /etc/modules" >> ${S}/debian/postinst
+        chmod +x ${S}/debian/postinst
     fi
 }
diff --git a/meta/recipes-kernel/linux/linux-custom.inc b/meta/recipes-kernel/linux/linux-custom.inc
index 0498dfa..1176b25 100644
--- a/meta/recipes-kernel/linux/linux-custom.inc
+++ b/meta/recipes-kernel/linux/linux-custom.inc
@@ -32,7 +32,7 @@ dpkg_runbuild() {
 	# Install package builder script
 	sudo install -m 755 ${WORKDIR}/build-kernel.sh ${BUILDCHROOT_DIR}
 
-	sudo cp ${WORKDIR}/${KERNEL_DEFCONFIG} ${WORKDIR}/${S}/.config
+	sudo cp ${WORKDIR}/${KERNEL_DEFCONFIG} ${S}/.config
 
 	E="${@ bb.utils.export_proxies(d)}"
 
@@ -43,5 +43,5 @@ dpkg_runbuild() {
 	export KERNEL_DEBIAN_DEPENDS="${KERNEL_DEBIAN_DEPENDS}"
 	export KERNEL_HEADERS_DEBIAN_DEPENDS="${KERNEL_HEADERS_DEBIAN_DEPENDS}"
 
-	sudo -E chroot ${BUILDCHROOT_DIR} /build-kernel.sh ${PP}/${S}
+	sudo -E chroot ${BUILDCHROOT_DIR} /build-kernel.sh ${PP}/${PPS}
 }
-- 
2.16.1


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v3] Correctly use the bitbake variable S from now on
  2018-04-20 12:36   ` [PATCH v3] " Henning Schild
@ 2018-04-20 12:37     ` Henning Schild
  2018-04-20 12:39       ` Jan Kiszka
  2018-05-11 15:33     ` Alexander Smirnov
  1 sibling, 1 reply; 12+ messages in thread
From: Henning Schild @ 2018-04-20 12:37 UTC (permalink / raw)
  To: isar-users; +Cc: Jan Kiszka

This one is now manually tested against linux-cip and -mainline recipes
from the Isar tree.

Henning

Am Fri, 20 Apr 2018 14:36:26 +0200
schrieb Henning Schild <henning.schild@siemens.com>:

> S is defined as ${WORKDIR}/${P} so it already contains ${WORKDIR} by
> its definition. In Isar it was often concatinated with WORKDIR again.
> One example where this was a problem is if you specified a patch in
> SRC_URI but did not actually overwrite S.
> Align the use of the variable with OE and bitbake defaults again.
> 
> Impact: layers building on top of Isar will have to adjust their
> recipes like the internal ones needed modification. The suggestion is
> to not set S and make sure to unpack to ${WORKDIR}/${P}. When setting
> S make sure to include ${WORKDIR}. This patch also introduces a
> warning and tries to preserve the old behaviour a bit. However if
> your recipe uses patches in SRC_URI and sets S you will get the
> warning and do_patch will fail.
> 
> Signed-off-by: Henning Schild <henning.schild@siemens.com>
> ---
>  doc/user_manual.md                                        |  2 +-
>  meta-isar/recipes-app/example-hello/example-hello.bb      |  4 +---
>  meta-isar/recipes-app/libhello/libhello.bb                |  4 +---
>  meta-isar/recipes-kernel/example-module/example-module.bb |  2 +-
>  meta-isar/recipes-kernel/linux/linux-cip_4.4.bb           |  4 +---
>  meta-isar/recipes-kernel/linux/linux-mainline_4.14.18.bb  |  2 +-
>  meta/classes/dpkg-base.bbclass                            | 13
> +++++++++++--
> meta/classes/dpkg.bbclass                                 |  2 +-
> meta/classes/patch.bbclass                                |  2 +-
> meta/recipes-kernel/linux-module/module.inc               |  8
> ++++---- meta/recipes-kernel/linux/linux-custom.inc                |
> 4 ++-- 11 files changed, 25 insertions(+), 22 deletions(-)
> 
> diff --git a/doc/user_manual.md b/doc/user_manual.md
> index 2bd3793..3072bd5 100644
> --- a/doc/user_manual.md
> +++ b/doc/user_manual.md
> @@ -511,7 +511,7 @@ PV = "1.0"
>  SRC_URI = "git://github.com/ilbers/hello.git"
>  SRCREV = "ad7065ecc4840cc436bfcdac427386dbba4ea719"
>  
> -S = "git"
> +S = "${WORKDIR}/git"
>  
>  inherit dpkg
>  ```
> diff --git a/meta-isar/recipes-app/example-hello/example-hello.bb
> b/meta-isar/recipes-app/example-hello/example-hello.bb index
> 9788ec0..d23ee6c 100644 ---
> a/meta-isar/recipes-app/example-hello/example-hello.bb +++
> b/meta-isar/recipes-app/example-hello/example-hello.bb @@ -15,11
> +15,9 @@ PV = "0.2-86cc719" DEPENDS += "libhello"
>  
>  SRC_URI = " \
> -    git://github.com/ilbers/hello.git;protocol=https \
> +    git://github.com/ilbers/hello.git;protocol=https;destsuffix=${P}
> \ file://0001-Add-some-help.patch \
>      file://yet-another-change.txt;apply=yes;striplevel=0"
>  SRCREV = "86cc719b3359adc3c4e243387feba50360a860f3"
>  
> -S = "git"
> -
>  inherit dpkg
> diff --git a/meta-isar/recipes-app/libhello/libhello.bb
> b/meta-isar/recipes-app/libhello/libhello.bb index 1875831..4e75f98
> 100644 --- a/meta-isar/recipes-app/libhello/libhello.bb
> +++ b/meta-isar/recipes-app/libhello/libhello.bb
> @@ -10,9 +10,7 @@ LIC_FILES_CHKSUM =
> "file://${LAYERDIR_isar}/licenses/COPYING.GPLv2;md5=751419260 
>  PV = "0.1-98f2e41"
>  
> -SRC_URI = "git://github.com/ilbers/libhello.git;protocol=https"
> +SRC_URI =
> "git://github.com/ilbers/libhello.git;protocol=https;destsuffix=${P}"
> SRCREV = "98f2e41e7d05ab8d19b0c5d160b104b725c8fd93" 
> -S = "git"
> -
>  inherit dpkg
> diff --git
> a/meta-isar/recipes-kernel/example-module/example-module.bb
> b/meta-isar/recipes-kernel/example-module/example-module.bb index
> dbaf5ac..98d0aaa 100644 ---
> a/meta-isar/recipes-kernel/example-module/example-module.bb +++
> b/meta-isar/recipes-kernel/example-module/example-module.bb @@ -9,6
> +9,6 @@ include recipes-kernel/linux-module/module.inc SRC_URI +=
> "file://src" 
> -S = "src"
> +S = "${WORKDIR}/src"
>  
>  AUTOLOAD = "1"
> diff --git a/meta-isar/recipes-kernel/linux/linux-cip_4.4.bb
> b/meta-isar/recipes-kernel/linux/linux-cip_4.4.bb index
> 7502f70..e9aaa9f 100644 ---
> a/meta-isar/recipes-kernel/linux/linux-cip_4.4.bb +++
> b/meta-isar/recipes-kernel/linux/linux-cip_4.4.bb @@ -8,12 +8,10 @@
>  require recipes-kernel/linux/linux-custom.inc
>  
>  SRC_URI += " \
> -
> git://git.kernel.org/pub/scm/linux/kernel/git/bwh/linux-cip.git;branch=linux-4.4.y-cip
> \
> +
> git://git.kernel.org/pub/scm/linux/kernel/git/bwh/linux-cip.git;branch=linux-4.4.y-cip;destsuffix=${P}
> \ file://x86_64_defconfig" 
>  SRCREV = "4e52cc5f668c4666e31a8485725b5f4e897b3baf"
>  PV = "4.4.112-cip18"
>  
> -S = "git"
> -
>  KERNEL_DEFCONFIG_qemuamd64 = "x86_64_defconfig"
> diff --git a/meta-isar/recipes-kernel/linux/linux-mainline_4.14.18.bb
> b/meta-isar/recipes-kernel/linux/linux-mainline_4.14.18.bb index
> 2c93d40..fc465fd 100644 ---
> a/meta-isar/recipes-kernel/linux/linux-mainline_4.14.18.bb +++
> b/meta-isar/recipes-kernel/linux/linux-mainline_4.14.18.bb @@ -12,6
> +12,6 @@ SRC_URI += " \ file://x86_64_defconfig"
>  SRC_URI[sha256sum] =
> "866a94c1c38d923ae18e74b683d7a8a79b674ebdfe7f40f1a3be9a27d39fe354" 
> -S = "linux-${PV}"
> +S = "${WORKDIR}/linux-${PV}"
>  
>  KERNEL_DEFCONFIG_qemuamd64 = "x86_64_defconfig"
> diff --git a/meta/classes/dpkg-base.bbclass
> b/meta/classes/dpkg-base.bbclass index 3c3a484..3845d8c 100644
> --- a/meta/classes/dpkg-base.bbclass
> +++ b/meta/classes/dpkg-base.bbclass
> @@ -2,8 +2,8 @@
>  # Copyright (C) 2017 Siemens AG
>  
>  do_adjust_git() {
> -    if [ -f ${WORKDIR}/${S}/.git/objects/info/alternates ]; then
> -        sed -i ${WORKDIR}/${S}/.git/objects/info/alternates \
> +    if [ -f ${S}/.git/objects/info/alternates ]; then
> +        sed -i ${S}/.git/objects/info/alternates \
>              -e 's|${DL_DIR}|/downloads|'
>      fi
>  }
> @@ -21,9 +21,18 @@ do_build[depends] = "buildchroot:do_build"
>  DEPENDS ?= ""
>  do_build[deptask] = "do_deploy_deb"
>  
> +def get_package_srcdir(d):
> +    s = d.getVar("S", True)
> +    workdir = d.getVar("WORKDIR", True)
> +    if s.startswith(workdir):
> +        return s[len(workdir)+1:]
> +    bb.warn('S does not start with WORKDIR')
> +    return s
> +
>  # Each package should have its own unique build folder, so use
>  # recipe name as identifier
>  PP = "/home/builder/${PN}"
> +PPS ?= "${@get_package_srcdir(d)}"
>  
>  BUILDROOT = "${BUILDCHROOT_DIR}/${PP}"
>  do_build[stamp-extra-info] = "${DISTRO}-${DISTRO_ARCH}"
> diff --git a/meta/classes/dpkg.bbclass b/meta/classes/dpkg.bbclass
> index 06f0579..c8d4ac5 100644
> --- a/meta/classes/dpkg.bbclass
> +++ b/meta/classes/dpkg.bbclass
> @@ -6,5 +6,5 @@ inherit dpkg-base
>  # Build package from sources using build script
>  dpkg_runbuild() {
>      E="${@ bb.utils.export_proxies(d)}"
> -    sudo -E chroot ${BUILDCHROOT_DIR} /build.sh ${PP}/${S}
> +    sudo -E chroot ${BUILDCHROOT_DIR} /build.sh ${PP}/${PPS}
>  }
> diff --git a/meta/classes/patch.bbclass b/meta/classes/patch.bbclass
> index 26a0c81..0bc449f 100644
> --- a/meta/classes/patch.bbclass
> +++ b/meta/classes/patch.bbclass
> @@ -5,7 +5,7 @@ python do_patch() {
>      import subprocess
>  
>      workdir = d.getVar("WORKDIR", True) + "/"
> -    src_dir = workdir + (d.getVar("S", True) or "")
> +    src_dir = d.getVar("S", True)
>  
>      for src_uri in (d.getVar("SRC_URI", True) or "").split():
>          try:
> diff --git a/meta/recipes-kernel/linux-module/module.inc
> b/meta/recipes-kernel/linux-module/module.inc index ec1c4b0..3075f44
> 100644 --- a/meta/recipes-kernel/linux-module/module.inc
> +++ b/meta/recipes-kernel/linux-module/module.inc
> @@ -18,14 +18,14 @@ AUTOLOAD ?= "0"
>  inherit dpkg
>  
>  dpkg_runbuild_prepend() {
> -    cp -r ${WORKDIR}/debian ${WORKDIR}/${S}/
> +    cp -r ${WORKDIR}/debian ${S}/
>      sed -i -e 's/@PN@/${PN}/g' -e 's/@PV@/${PV}/g' \
>             -e 's/@KERNEL_NAME@/${KERNEL_NAME}/g' \
>             -e 's/@DESCRIPTION@/${DESCRIPTION}/g' \
> -        ${WORKDIR}/${S}/debian/changelog
> ${WORKDIR}/${S}/debian/control
> +        ${S}/debian/changelog ${S}/debian/control
>  
>      if [ ${AUTOLOAD} = "1" ]; then
> -        echo "echo ${PN} >> /etc/modules" >>
> ${WORKDIR}/${S}/debian/postinst
> -        chmod +x ${WORKDIR}/${S}/debian/postinst
> +        echo "echo ${PN} >> /etc/modules" >> ${S}/debian/postinst
> +        chmod +x ${S}/debian/postinst
>      fi
>  }
> diff --git a/meta/recipes-kernel/linux/linux-custom.inc
> b/meta/recipes-kernel/linux/linux-custom.inc index 0498dfa..1176b25
> 100644 --- a/meta/recipes-kernel/linux/linux-custom.inc
> +++ b/meta/recipes-kernel/linux/linux-custom.inc
> @@ -32,7 +32,7 @@ dpkg_runbuild() {
>  	# Install package builder script
>  	sudo install -m 755 ${WORKDIR}/build-kernel.sh
> ${BUILDCHROOT_DIR} 
> -	sudo cp ${WORKDIR}/${KERNEL_DEFCONFIG}
> ${WORKDIR}/${S}/.config
> +	sudo cp ${WORKDIR}/${KERNEL_DEFCONFIG} ${S}/.config
>  
>  	E="${@ bb.utils.export_proxies(d)}"
>  
> @@ -43,5 +43,5 @@ dpkg_runbuild() {
>  	export KERNEL_DEBIAN_DEPENDS="${KERNEL_DEBIAN_DEPENDS}"
>  	export
> KERNEL_HEADERS_DEBIAN_DEPENDS="${KERNEL_HEADERS_DEBIAN_DEPENDS}" 
> -	sudo -E chroot ${BUILDCHROOT_DIR} /build-kernel.sh ${PP}/${S}
> +	sudo -E chroot ${BUILDCHROOT_DIR} /build-kernel.sh
> ${PP}/${PPS} }


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v3] Correctly use the bitbake variable S from now on
  2018-04-20 12:37     ` Henning Schild
@ 2018-04-20 12:39       ` Jan Kiszka
  2018-04-20 12:43         ` Henning Schild
  0 siblings, 1 reply; 12+ messages in thread
From: Jan Kiszka @ 2018-04-20 12:39 UTC (permalink / raw)
  To: Henning Schild, isar-users

On 2018-04-20 14:37, Henning Schild wrote:
> This one is now manually tested against linux-cip and -mainline recipes
> from the Isar tree.

And the trick was...?

Jan

> 
> Henning
> 
> Am Fri, 20 Apr 2018 14:36:26 +0200
> schrieb Henning Schild <henning.schild@siemens.com>:
> 
>> S is defined as ${WORKDIR}/${P} so it already contains ${WORKDIR} by
>> its definition. In Isar it was often concatinated with WORKDIR again.
>> One example where this was a problem is if you specified a patch in
>> SRC_URI but did not actually overwrite S.
>> Align the use of the variable with OE and bitbake defaults again.
>>
>> Impact: layers building on top of Isar will have to adjust their
>> recipes like the internal ones needed modification. The suggestion is
>> to not set S and make sure to unpack to ${WORKDIR}/${P}. When setting
>> S make sure to include ${WORKDIR}. This patch also introduces a
>> warning and tries to preserve the old behaviour a bit. However if
>> your recipe uses patches in SRC_URI and sets S you will get the
>> warning and do_patch will fail.
>>
>> Signed-off-by: Henning Schild <henning.schild@siemens.com>
>> ---
>>  doc/user_manual.md                                        |  2 +-
>>  meta-isar/recipes-app/example-hello/example-hello.bb      |  4 +---
>>  meta-isar/recipes-app/libhello/libhello.bb                |  4 +---
>>  meta-isar/recipes-kernel/example-module/example-module.bb |  2 +-
>>  meta-isar/recipes-kernel/linux/linux-cip_4.4.bb           |  4 +---
>>  meta-isar/recipes-kernel/linux/linux-mainline_4.14.18.bb  |  2 +-
>>  meta/classes/dpkg-base.bbclass                            | 13
>> +++++++++++--
>> meta/classes/dpkg.bbclass                                 |  2 +-
>> meta/classes/patch.bbclass                                |  2 +-
>> meta/recipes-kernel/linux-module/module.inc               |  8
>> ++++---- meta/recipes-kernel/linux/linux-custom.inc                |
>> 4 ++-- 11 files changed, 25 insertions(+), 22 deletions(-)
>>
>> diff --git a/doc/user_manual.md b/doc/user_manual.md
>> index 2bd3793..3072bd5 100644
>> --- a/doc/user_manual.md
>> +++ b/doc/user_manual.md
>> @@ -511,7 +511,7 @@ PV = "1.0"
>>  SRC_URI = "git://github.com/ilbers/hello.git"
>>  SRCREV = "ad7065ecc4840cc436bfcdac427386dbba4ea719"
>>  
>> -S = "git"
>> +S = "${WORKDIR}/git"
>>  
>>  inherit dpkg
>>  ```
>> diff --git a/meta-isar/recipes-app/example-hello/example-hello.bb
>> b/meta-isar/recipes-app/example-hello/example-hello.bb index
>> 9788ec0..d23ee6c 100644 ---
>> a/meta-isar/recipes-app/example-hello/example-hello.bb +++
>> b/meta-isar/recipes-app/example-hello/example-hello.bb @@ -15,11
>> +15,9 @@ PV = "0.2-86cc719" DEPENDS += "libhello"
>>  
>>  SRC_URI = " \
>> -    git://github.com/ilbers/hello.git;protocol=https \
>> +    git://github.com/ilbers/hello.git;protocol=https;destsuffix=${P}
>> \ file://0001-Add-some-help.patch \
>>      file://yet-another-change.txt;apply=yes;striplevel=0"
>>  SRCREV = "86cc719b3359adc3c4e243387feba50360a860f3"
>>  
>> -S = "git"
>> -
>>  inherit dpkg
>> diff --git a/meta-isar/recipes-app/libhello/libhello.bb
>> b/meta-isar/recipes-app/libhello/libhello.bb index 1875831..4e75f98
>> 100644 --- a/meta-isar/recipes-app/libhello/libhello.bb
>> +++ b/meta-isar/recipes-app/libhello/libhello.bb
>> @@ -10,9 +10,7 @@ LIC_FILES_CHKSUM =
>> "file://${LAYERDIR_isar}/licenses/COPYING.GPLv2;md5=751419260 
>>  PV = "0.1-98f2e41"
>>  
>> -SRC_URI = "git://github.com/ilbers/libhello.git;protocol=https"
>> +SRC_URI =
>> "git://github.com/ilbers/libhello.git;protocol=https;destsuffix=${P}"
>> SRCREV = "98f2e41e7d05ab8d19b0c5d160b104b725c8fd93" 
>> -S = "git"
>> -
>>  inherit dpkg
>> diff --git
>> a/meta-isar/recipes-kernel/example-module/example-module.bb
>> b/meta-isar/recipes-kernel/example-module/example-module.bb index
>> dbaf5ac..98d0aaa 100644 ---
>> a/meta-isar/recipes-kernel/example-module/example-module.bb +++
>> b/meta-isar/recipes-kernel/example-module/example-module.bb @@ -9,6
>> +9,6 @@ include recipes-kernel/linux-module/module.inc SRC_URI +=
>> "file://src" 
>> -S = "src"
>> +S = "${WORKDIR}/src"
>>  
>>  AUTOLOAD = "1"
>> diff --git a/meta-isar/recipes-kernel/linux/linux-cip_4.4.bb
>> b/meta-isar/recipes-kernel/linux/linux-cip_4.4.bb index
>> 7502f70..e9aaa9f 100644 ---
>> a/meta-isar/recipes-kernel/linux/linux-cip_4.4.bb +++
>> b/meta-isar/recipes-kernel/linux/linux-cip_4.4.bb @@ -8,12 +8,10 @@
>>  require recipes-kernel/linux/linux-custom.inc
>>  
>>  SRC_URI += " \
>> -
>> git://git.kernel.org/pub/scm/linux/kernel/git/bwh/linux-cip.git;branch=linux-4.4.y-cip
>> \
>> +
>> git://git.kernel.org/pub/scm/linux/kernel/git/bwh/linux-cip.git;branch=linux-4.4.y-cip;destsuffix=${P}
>> \ file://x86_64_defconfig" 
>>  SRCREV = "4e52cc5f668c4666e31a8485725b5f4e897b3baf"
>>  PV = "4.4.112-cip18"
>>  
>> -S = "git"
>> -
>>  KERNEL_DEFCONFIG_qemuamd64 = "x86_64_defconfig"
>> diff --git a/meta-isar/recipes-kernel/linux/linux-mainline_4.14.18.bb
>> b/meta-isar/recipes-kernel/linux/linux-mainline_4.14.18.bb index
>> 2c93d40..fc465fd 100644 ---
>> a/meta-isar/recipes-kernel/linux/linux-mainline_4.14.18.bb +++
>> b/meta-isar/recipes-kernel/linux/linux-mainline_4.14.18.bb @@ -12,6
>> +12,6 @@ SRC_URI += " \ file://x86_64_defconfig"
>>  SRC_URI[sha256sum] =
>> "866a94c1c38d923ae18e74b683d7a8a79b674ebdfe7f40f1a3be9a27d39fe354" 
>> -S = "linux-${PV}"
>> +S = "${WORKDIR}/linux-${PV}"
>>  
>>  KERNEL_DEFCONFIG_qemuamd64 = "x86_64_defconfig"
>> diff --git a/meta/classes/dpkg-base.bbclass
>> b/meta/classes/dpkg-base.bbclass index 3c3a484..3845d8c 100644
>> --- a/meta/classes/dpkg-base.bbclass
>> +++ b/meta/classes/dpkg-base.bbclass
>> @@ -2,8 +2,8 @@
>>  # Copyright (C) 2017 Siemens AG
>>  
>>  do_adjust_git() {
>> -    if [ -f ${WORKDIR}/${S}/.git/objects/info/alternates ]; then
>> -        sed -i ${WORKDIR}/${S}/.git/objects/info/alternates \
>> +    if [ -f ${S}/.git/objects/info/alternates ]; then
>> +        sed -i ${S}/.git/objects/info/alternates \
>>              -e 's|${DL_DIR}|/downloads|'
>>      fi
>>  }
>> @@ -21,9 +21,18 @@ do_build[depends] = "buildchroot:do_build"
>>  DEPENDS ?= ""
>>  do_build[deptask] = "do_deploy_deb"
>>  
>> +def get_package_srcdir(d):
>> +    s = d.getVar("S", True)
>> +    workdir = d.getVar("WORKDIR", True)
>> +    if s.startswith(workdir):
>> +        return s[len(workdir)+1:]
>> +    bb.warn('S does not start with WORKDIR')
>> +    return s
>> +
>>  # Each package should have its own unique build folder, so use
>>  # recipe name as identifier
>>  PP = "/home/builder/${PN}"
>> +PPS ?= "${@get_package_srcdir(d)}"
>>  
>>  BUILDROOT = "${BUILDCHROOT_DIR}/${PP}"
>>  do_build[stamp-extra-info] = "${DISTRO}-${DISTRO_ARCH}"
>> diff --git a/meta/classes/dpkg.bbclass b/meta/classes/dpkg.bbclass
>> index 06f0579..c8d4ac5 100644
>> --- a/meta/classes/dpkg.bbclass
>> +++ b/meta/classes/dpkg.bbclass
>> @@ -6,5 +6,5 @@ inherit dpkg-base
>>  # Build package from sources using build script
>>  dpkg_runbuild() {
>>      E="${@ bb.utils.export_proxies(d)}"
>> -    sudo -E chroot ${BUILDCHROOT_DIR} /build.sh ${PP}/${S}
>> +    sudo -E chroot ${BUILDCHROOT_DIR} /build.sh ${PP}/${PPS}
>>  }
>> diff --git a/meta/classes/patch.bbclass b/meta/classes/patch.bbclass
>> index 26a0c81..0bc449f 100644
>> --- a/meta/classes/patch.bbclass
>> +++ b/meta/classes/patch.bbclass
>> @@ -5,7 +5,7 @@ python do_patch() {
>>      import subprocess
>>  
>>      workdir = d.getVar("WORKDIR", True) + "/"
>> -    src_dir = workdir + (d.getVar("S", True) or "")
>> +    src_dir = d.getVar("S", True)
>>  
>>      for src_uri in (d.getVar("SRC_URI", True) or "").split():
>>          try:
>> diff --git a/meta/recipes-kernel/linux-module/module.inc
>> b/meta/recipes-kernel/linux-module/module.inc index ec1c4b0..3075f44
>> 100644 --- a/meta/recipes-kernel/linux-module/module.inc
>> +++ b/meta/recipes-kernel/linux-module/module.inc
>> @@ -18,14 +18,14 @@ AUTOLOAD ?= "0"
>>  inherit dpkg
>>  
>>  dpkg_runbuild_prepend() {
>> -    cp -r ${WORKDIR}/debian ${WORKDIR}/${S}/
>> +    cp -r ${WORKDIR}/debian ${S}/
>>      sed -i -e 's/@PN@/${PN}/g' -e 's/@PV@/${PV}/g' \
>>             -e 's/@KERNEL_NAME@/${KERNEL_NAME}/g' \
>>             -e 's/@DESCRIPTION@/${DESCRIPTION}/g' \
>> -        ${WORKDIR}/${S}/debian/changelog
>> ${WORKDIR}/${S}/debian/control
>> +        ${S}/debian/changelog ${S}/debian/control
>>  
>>      if [ ${AUTOLOAD} = "1" ]; then
>> -        echo "echo ${PN} >> /etc/modules" >>
>> ${WORKDIR}/${S}/debian/postinst
>> -        chmod +x ${WORKDIR}/${S}/debian/postinst
>> +        echo "echo ${PN} >> /etc/modules" >> ${S}/debian/postinst
>> +        chmod +x ${S}/debian/postinst
>>      fi
>>  }
>> diff --git a/meta/recipes-kernel/linux/linux-custom.inc
>> b/meta/recipes-kernel/linux/linux-custom.inc index 0498dfa..1176b25
>> 100644 --- a/meta/recipes-kernel/linux/linux-custom.inc
>> +++ b/meta/recipes-kernel/linux/linux-custom.inc
>> @@ -32,7 +32,7 @@ dpkg_runbuild() {
>>  	# Install package builder script
>>  	sudo install -m 755 ${WORKDIR}/build-kernel.sh
>> ${BUILDCHROOT_DIR} 
>> -	sudo cp ${WORKDIR}/${KERNEL_DEFCONFIG}
>> ${WORKDIR}/${S}/.config
>> +	sudo cp ${WORKDIR}/${KERNEL_DEFCONFIG} ${S}/.config
>>  
>>  	E="${@ bb.utils.export_proxies(d)}"
>>  
>> @@ -43,5 +43,5 @@ dpkg_runbuild() {
>>  	export KERNEL_DEBIAN_DEPENDS="${KERNEL_DEBIAN_DEPENDS}"
>>  	export
>> KERNEL_HEADERS_DEBIAN_DEPENDS="${KERNEL_HEADERS_DEBIAN_DEPENDS}" 
>> -	sudo -E chroot ${BUILDCHROOT_DIR} /build-kernel.sh ${PP}/${S}
>> +	sudo -E chroot ${BUILDCHROOT_DIR} /build-kernel.sh
>> ${PP}/${PPS} }
> 


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v3] Correctly use the bitbake variable S from now on
  2018-04-20 12:39       ` Jan Kiszka
@ 2018-04-20 12:43         ` Henning Schild
  0 siblings, 0 replies; 12+ messages in thread
From: Henning Schild @ 2018-04-20 12:43 UTC (permalink / raw)
  To: Jan Kiszka; +Cc: isar-users

Am Fri, 20 Apr 2018 14:39:21 +0200
schrieb Jan Kiszka <jan.kiszka@siemens.com>:

> On 2018-04-20 14:37, Henning Schild wrote:
> > This one is now manually tested against linux-cip and -mainline
> > recipes from the Isar tree.  
> 
> And the trick was...?

I had to keep setting S in the tar.gz case (mainline), subdir= will
actually create another folder hierarchy instead of defining the name.

And in build-kernel.sh has to go to ${PP}/${PPS}, not PP/S like in v2.

Henning

> Jan
> 
> > 
> > Henning
> > 
> > Am Fri, 20 Apr 2018 14:36:26 +0200
> > schrieb Henning Schild <henning.schild@siemens.com>:
> >   
> >> S is defined as ${WORKDIR}/${P} so it already contains ${WORKDIR}
> >> by its definition. In Isar it was often concatinated with WORKDIR
> >> again. One example where this was a problem is if you specified a
> >> patch in SRC_URI but did not actually overwrite S.
> >> Align the use of the variable with OE and bitbake defaults again.
> >>
> >> Impact: layers building on top of Isar will have to adjust their
> >> recipes like the internal ones needed modification. The suggestion
> >> is to not set S and make sure to unpack to ${WORKDIR}/${P}. When
> >> setting S make sure to include ${WORKDIR}. This patch also
> >> introduces a warning and tries to preserve the old behaviour a
> >> bit. However if your recipe uses patches in SRC_URI and sets S you
> >> will get the warning and do_patch will fail.
> >>
> >> Signed-off-by: Henning Schild <henning.schild@siemens.com>
> >> ---
> >>  doc/user_manual.md                                        |  2 +-
> >>  meta-isar/recipes-app/example-hello/example-hello.bb      |  4
> >> +--- meta-isar/recipes-app/libhello/libhello.bb                |
> >> 4 +--- meta-isar/recipes-kernel/example-module/example-module.bb
> >> |  2 +- meta-isar/recipes-kernel/linux/linux-cip_4.4.bb
> >> |  4 +---
> >> meta-isar/recipes-kernel/linux/linux-mainline_4.14.18.bb  |  2 +-
> >> meta/classes/dpkg-base.bbclass                            | 13
> >> +++++++++++--
> >> meta/classes/dpkg.bbclass                                 |  2 +-
> >> meta/classes/patch.bbclass                                |  2 +-
> >> meta/recipes-kernel/linux-module/module.inc               |  8
> >> ++++---- meta/recipes-kernel/linux/linux-custom.inc
> >> | 4 ++-- 11 files changed, 25 insertions(+), 22 deletions(-)
> >>
> >> diff --git a/doc/user_manual.md b/doc/user_manual.md
> >> index 2bd3793..3072bd5 100644
> >> --- a/doc/user_manual.md
> >> +++ b/doc/user_manual.md
> >> @@ -511,7 +511,7 @@ PV = "1.0"
> >>  SRC_URI = "git://github.com/ilbers/hello.git"
> >>  SRCREV = "ad7065ecc4840cc436bfcdac427386dbba4ea719"
> >>  
> >> -S = "git"
> >> +S = "${WORKDIR}/git"
> >>  
> >>  inherit dpkg
> >>  ```
> >> diff --git a/meta-isar/recipes-app/example-hello/example-hello.bb
> >> b/meta-isar/recipes-app/example-hello/example-hello.bb index
> >> 9788ec0..d23ee6c 100644 ---
> >> a/meta-isar/recipes-app/example-hello/example-hello.bb +++
> >> b/meta-isar/recipes-app/example-hello/example-hello.bb @@ -15,11
> >> +15,9 @@ PV = "0.2-86cc719" DEPENDS += "libhello"
> >>  
> >>  SRC_URI = " \
> >> -    git://github.com/ilbers/hello.git;protocol=https \
> >> +
> >> git://github.com/ilbers/hello.git;protocol=https;destsuffix=${P} \
> >> file://0001-Add-some-help.patch \
> >> file://yet-another-change.txt;apply=yes;striplevel=0" SRCREV =
> >> "86cc719b3359adc3c4e243387feba50360a860f3" 
> >> -S = "git"
> >> -
> >>  inherit dpkg
> >> diff --git a/meta-isar/recipes-app/libhello/libhello.bb
> >> b/meta-isar/recipes-app/libhello/libhello.bb index 1875831..4e75f98
> >> 100644 --- a/meta-isar/recipes-app/libhello/libhello.bb
> >> +++ b/meta-isar/recipes-app/libhello/libhello.bb
> >> @@ -10,9 +10,7 @@ LIC_FILES_CHKSUM =
> >> "file://${LAYERDIR_isar}/licenses/COPYING.GPLv2;md5=751419260 
> >>  PV = "0.1-98f2e41"
> >>  
> >> -SRC_URI = "git://github.com/ilbers/libhello.git;protocol=https"
> >> +SRC_URI =
> >> "git://github.com/ilbers/libhello.git;protocol=https;destsuffix=${P}"
> >> SRCREV = "98f2e41e7d05ab8d19b0c5d160b104b725c8fd93" 
> >> -S = "git"
> >> -
> >>  inherit dpkg
> >> diff --git
> >> a/meta-isar/recipes-kernel/example-module/example-module.bb
> >> b/meta-isar/recipes-kernel/example-module/example-module.bb index
> >> dbaf5ac..98d0aaa 100644 ---
> >> a/meta-isar/recipes-kernel/example-module/example-module.bb +++
> >> b/meta-isar/recipes-kernel/example-module/example-module.bb @@ -9,6
> >> +9,6 @@ include recipes-kernel/linux-module/module.inc SRC_URI +=
> >> "file://src" 
> >> -S = "src"
> >> +S = "${WORKDIR}/src"
> >>  
> >>  AUTOLOAD = "1"
> >> diff --git a/meta-isar/recipes-kernel/linux/linux-cip_4.4.bb
> >> b/meta-isar/recipes-kernel/linux/linux-cip_4.4.bb index
> >> 7502f70..e9aaa9f 100644 ---
> >> a/meta-isar/recipes-kernel/linux/linux-cip_4.4.bb +++
> >> b/meta-isar/recipes-kernel/linux/linux-cip_4.4.bb @@ -8,12 +8,10 @@
> >>  require recipes-kernel/linux/linux-custom.inc
> >>  
> >>  SRC_URI += " \
> >> -
> >> git://git.kernel.org/pub/scm/linux/kernel/git/bwh/linux-cip.git;branch=linux-4.4.y-cip
> >> \
> >> +
> >> git://git.kernel.org/pub/scm/linux/kernel/git/bwh/linux-cip.git;branch=linux-4.4.y-cip;destsuffix=${P}
> >> \ file://x86_64_defconfig" 
> >>  SRCREV = "4e52cc5f668c4666e31a8485725b5f4e897b3baf"
> >>  PV = "4.4.112-cip18"
> >>  
> >> -S = "git"
> >> -
> >>  KERNEL_DEFCONFIG_qemuamd64 = "x86_64_defconfig"
> >> diff --git
> >> a/meta-isar/recipes-kernel/linux/linux-mainline_4.14.18.bb
> >> b/meta-isar/recipes-kernel/linux/linux-mainline_4.14.18.bb index
> >> 2c93d40..fc465fd 100644 ---
> >> a/meta-isar/recipes-kernel/linux/linux-mainline_4.14.18.bb +++
> >> b/meta-isar/recipes-kernel/linux/linux-mainline_4.14.18.bb @@
> >> -12,6 +12,6 @@ SRC_URI += " \ file://x86_64_defconfig"
> >> SRC_URI[sha256sum] =
> >> "866a94c1c38d923ae18e74b683d7a8a79b674ebdfe7f40f1a3be9a27d39fe354"
> >> -S = "linux-${PV}" +S = "${WORKDIR}/linux-${PV}"
> >>  
> >>  KERNEL_DEFCONFIG_qemuamd64 = "x86_64_defconfig"
> >> diff --git a/meta/classes/dpkg-base.bbclass
> >> b/meta/classes/dpkg-base.bbclass index 3c3a484..3845d8c 100644
> >> --- a/meta/classes/dpkg-base.bbclass
> >> +++ b/meta/classes/dpkg-base.bbclass
> >> @@ -2,8 +2,8 @@
> >>  # Copyright (C) 2017 Siemens AG
> >>  
> >>  do_adjust_git() {
> >> -    if [ -f ${WORKDIR}/${S}/.git/objects/info/alternates ]; then
> >> -        sed -i ${WORKDIR}/${S}/.git/objects/info/alternates \
> >> +    if [ -f ${S}/.git/objects/info/alternates ]; then
> >> +        sed -i ${S}/.git/objects/info/alternates \
> >>              -e 's|${DL_DIR}|/downloads|'
> >>      fi
> >>  }
> >> @@ -21,9 +21,18 @@ do_build[depends] = "buildchroot:do_build"
> >>  DEPENDS ?= ""
> >>  do_build[deptask] = "do_deploy_deb"
> >>  
> >> +def get_package_srcdir(d):
> >> +    s = d.getVar("S", True)
> >> +    workdir = d.getVar("WORKDIR", True)
> >> +    if s.startswith(workdir):
> >> +        return s[len(workdir)+1:]
> >> +    bb.warn('S does not start with WORKDIR')
> >> +    return s
> >> +
> >>  # Each package should have its own unique build folder, so use
> >>  # recipe name as identifier
> >>  PP = "/home/builder/${PN}"
> >> +PPS ?= "${@get_package_srcdir(d)}"
> >>  
> >>  BUILDROOT = "${BUILDCHROOT_DIR}/${PP}"
> >>  do_build[stamp-extra-info] = "${DISTRO}-${DISTRO_ARCH}"
> >> diff --git a/meta/classes/dpkg.bbclass b/meta/classes/dpkg.bbclass
> >> index 06f0579..c8d4ac5 100644
> >> --- a/meta/classes/dpkg.bbclass
> >> +++ b/meta/classes/dpkg.bbclass
> >> @@ -6,5 +6,5 @@ inherit dpkg-base
> >>  # Build package from sources using build script
> >>  dpkg_runbuild() {
> >>      E="${@ bb.utils.export_proxies(d)}"
> >> -    sudo -E chroot ${BUILDCHROOT_DIR} /build.sh ${PP}/${S}
> >> +    sudo -E chroot ${BUILDCHROOT_DIR} /build.sh ${PP}/${PPS}
> >>  }
> >> diff --git a/meta/classes/patch.bbclass
> >> b/meta/classes/patch.bbclass index 26a0c81..0bc449f 100644
> >> --- a/meta/classes/patch.bbclass
> >> +++ b/meta/classes/patch.bbclass
> >> @@ -5,7 +5,7 @@ python do_patch() {
> >>      import subprocess
> >>  
> >>      workdir = d.getVar("WORKDIR", True) + "/"
> >> -    src_dir = workdir + (d.getVar("S", True) or "")
> >> +    src_dir = d.getVar("S", True)
> >>  
> >>      for src_uri in (d.getVar("SRC_URI", True) or "").split():
> >>          try:
> >> diff --git a/meta/recipes-kernel/linux-module/module.inc
> >> b/meta/recipes-kernel/linux-module/module.inc index
> >> ec1c4b0..3075f44 100644 ---
> >> a/meta/recipes-kernel/linux-module/module.inc +++
> >> b/meta/recipes-kernel/linux-module/module.inc @@ -18,14 +18,14 @@
> >> AUTOLOAD ?= "0" inherit dpkg
> >>  
> >>  dpkg_runbuild_prepend() {
> >> -    cp -r ${WORKDIR}/debian ${WORKDIR}/${S}/
> >> +    cp -r ${WORKDIR}/debian ${S}/
> >>      sed -i -e 's/@PN@/${PN}/g' -e 's/@PV@/${PV}/g' \
> >>             -e 's/@KERNEL_NAME@/${KERNEL_NAME}/g' \
> >>             -e 's/@DESCRIPTION@/${DESCRIPTION}/g' \
> >> -        ${WORKDIR}/${S}/debian/changelog
> >> ${WORKDIR}/${S}/debian/control
> >> +        ${S}/debian/changelog ${S}/debian/control
> >>  
> >>      if [ ${AUTOLOAD} = "1" ]; then
> >> -        echo "echo ${PN} >> /etc/modules" >>
> >> ${WORKDIR}/${S}/debian/postinst
> >> -        chmod +x ${WORKDIR}/${S}/debian/postinst
> >> +        echo "echo ${PN} >> /etc/modules" >> ${S}/debian/postinst
> >> +        chmod +x ${S}/debian/postinst
> >>      fi
> >>  }
> >> diff --git a/meta/recipes-kernel/linux/linux-custom.inc
> >> b/meta/recipes-kernel/linux/linux-custom.inc index 0498dfa..1176b25
> >> 100644 --- a/meta/recipes-kernel/linux/linux-custom.inc
> >> +++ b/meta/recipes-kernel/linux/linux-custom.inc
> >> @@ -32,7 +32,7 @@ dpkg_runbuild() {
> >>  	# Install package builder script
> >>  	sudo install -m 755 ${WORKDIR}/build-kernel.sh
> >> ${BUILDCHROOT_DIR} 
> >> -	sudo cp ${WORKDIR}/${KERNEL_DEFCONFIG}
> >> ${WORKDIR}/${S}/.config
> >> +	sudo cp ${WORKDIR}/${KERNEL_DEFCONFIG} ${S}/.config
> >>  
> >>  	E="${@ bb.utils.export_proxies(d)}"
> >>  
> >> @@ -43,5 +43,5 @@ dpkg_runbuild() {
> >>  	export KERNEL_DEBIAN_DEPENDS="${KERNEL_DEBIAN_DEPENDS}"
> >>  	export
> >> KERNEL_HEADERS_DEBIAN_DEPENDS="${KERNEL_HEADERS_DEBIAN_DEPENDS}" 
> >> -	sudo -E chroot ${BUILDCHROOT_DIR} /build-kernel.sh
> >> ${PP}/${S}
> >> +	sudo -E chroot ${BUILDCHROOT_DIR} /build-kernel.sh
> >> ${PP}/${PPS} }  
> >   
> 


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v3] Correctly use the bitbake variable S from now on
  2018-04-20 12:36   ` [PATCH v3] " Henning Schild
  2018-04-20 12:37     ` Henning Schild
@ 2018-05-11 15:33     ` Alexander Smirnov
  1 sibling, 0 replies; 12+ messages in thread
From: Alexander Smirnov @ 2018-05-11 15:33 UTC (permalink / raw)
  To: Henning Schild, isar-users; +Cc: Jan Kiszka

On 04/20/2018 03:36 PM, Henning Schild wrote:
> S is defined as ${WORKDIR}/${P} so it already contains ${WORKDIR} by its
> definition. In Isar it was often concatinated with WORKDIR again. One
> example where this was a problem is if you specified a patch in SRC_URI
> but did not actually overwrite S.
> Align the use of the variable with OE and bitbake defaults again.
> 
> Impact: layers building on top of Isar will have to adjust their recipes
> like the internal ones needed modification. The suggestion is to not set
> S and make sure to unpack to ${WORKDIR}/${P}. When setting S make sure
> to include ${WORKDIR}. This patch also introduces a warning and tries to
> preserve the old behaviour a bit. However if your recipe uses patches in
> SRC_URI and sets S you will get the warning and do_patch will fail.
> 

Applied to next, thanks!

Alex

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2018-05-11 15:33 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-04-19  9:26 [PATCH] Correctly use the bitbake variable S from now on Henning Schild
2018-04-19  9:45 ` Jan Kiszka
2018-04-19 10:36   ` Henning Schild
2018-04-19 11:42     ` Jan Kiszka
2018-04-19 16:56       ` Henning Schild
2018-04-19 16:58 ` [PATCH v2] " Henning Schild
2018-04-20 11:06   ` Henning Schild
2018-04-20 12:36   ` [PATCH v3] " Henning Schild
2018-04-20 12:37     ` Henning Schild
2018-04-20 12:39       ` Jan Kiszka
2018-04-20 12:43         ` Henning Schild
2018-05-11 15:33     ` Alexander Smirnov

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox