base.bbclass: detect when S has been set incorrectly

Currently base.bbclass is creating S if it's not created by unpacking an archive or fetching a repository. If we avoid creating S we can detect when S hasn't been set correctly, since it will not exist. Then we can tell the user that they should set S to a proper value, instead of just failing with odd errors in later tasks.

Besides removing the auto-creation of S this change will introduce a warning if S is set incorrectly. The reason for not display an error and return is due to all external layers that might have recipes that will fail otherwise and that might be a bit to hard to start with. So use a warning until people have had a chance to cleanup affected recipes.

[YOCTO #5627]

Signed-off-by Petter Mabäcker <[email protected]>

---
 meta/classes/base.bbclass | 17 +++++++++++++----
 1 file changed, 13 insertions(+), 4 deletions(-)

meta/classes/base.bbclass +12 -3

index de50be1..789af3b 100644
@@ -113,7 +113,6 @@ python base_do_fetch() {
 
 addtask unpack after do_fetch
 do_unpack[dirs] = "${WORKDIR}"
-do_unpack[cleandirs] = "${S}/patches"
 python base_do_unpack() {
     src_uri = (d.getVar('SRC_URI', True) or "").split()
     if len(src_uri) == 0:

@@ -121,11 +120,21 @@ python base_do_unpack() {
 
     rootdir = d.getVar('WORKDIR', True)
 
+    # Ensure that we cleanup ${S}/patches
+    # TODO: Investigate if we can remove
+    # the entire ${S} in this case.
+    s_dir = d.getVar('S', True)
+    p_dir = os.path.join(s_dir, 'patches')
+    bb.utils.remove(p_dir, True)
+
     try:
         fetcher = bb.fetch2.Fetch(src_uri, d)
         fetcher.unpack(rootdir)
     except bb.fetch2.BBFetchException as e:
         raise bb.build.FuncFailed(e)
+
+    if not os.path.exists(s_dir):
+        bb.warn("%s ('S') don't exist, you must set 'S' to a proper value"
 % s_dir) }
 
 def pkgarch_mapping(d):

@@ -220,7 +229,7 @@ CONFIGURESTAMPFILE = "${WORKDIR}/configure.sstate"
 CLEANBROKEN = "0"
 
 addtask configure after do_patch
-do_configure[dirs] = "${S} ${B}"
+do_configure[dirs] = "${B}"
 do_configure[deptask] = "do_populate_sysroot"
 base_do_configure() {
 	if [ -n "${CONFIGURESTAMPFILE}" -a -e "${CONFIGURESTAMPFILE}" ]; then

@@ -238,7 +247,7 @@ base_do_configure() {
 }
 
 addtask compile after do_configure
-do_compile[dirs] = "${S} ${B}"
+do_compile[dirs] = "${B}"
 base_do_compile() {
 	if [ -e Makefile -o -e makefile -o -e GNUmakefile ]; then
 		oe_runmake || die "make failed"

Hi Petter,

Quoted 72 lines · On Saturday 10 January 2015 15:40:10 Petter Mabäcker wrote:

Currently base.bbclass is creating S if it's not created by unpacking an archive or fetching a repository. If we avoid creating S we can detect when S hasn't been set correctly, since it will not exist. Then we can tell the user that they should set S to a proper value, instead of just failing with odd errors in later tasks.

Besides removing the auto-creation of S this change will introduce a warning if S is set incorrectly. The reason for not display an error and return is due to all external layers that might have recipes that will fail otherwise and that might be a bit to hard to start with. So use a warning until people have had a chance to cleanup affected recipes.

[YOCTO #5627]

Signed-off-by Petter Mabäcker <[email protected]>

---
  meta/classes/base.bbclass | 17 +++++++++++++----
  1 file changed, 13 insertions(+), 4 deletions(-)

meta/classes/base.bbclass +12 -3

index de50be1..789af3b 100644
@@ -113,7 +113,6 @@ python base_do_fetch() {
 
  addtask unpack after do_fetch
  do_unpack[dirs] = "${WORKDIR}"
-do_unpack[cleandirs] = "${S}/patches"
  python base_do_unpack() {
      src_uri = (d.getVar('SRC_URI', True) or "").split()
      if len(src_uri) == 0:

@@ -121,11 +120,21 @@ python base_do_unpack() {
 
      rootdir = d.getVar('WORKDIR', True)
 
+    # Ensure that we cleanup ${S}/patches
+    # TODO: Investigate if we can remove
+    # the entire ${S} in this case.
+    s_dir = d.getVar('S', True)
+    p_dir = os.path.join(s_dir, 'patches')
+    bb.utils.remove(p_dir, True)
+
      try:
          fetcher = bb.fetch2.Fetch(src_uri, d)
          fetcher.unpack(rootdir)
      except bb.fetch2.BBFetchException as e:
          raise bb.build.FuncFailed(e)
+
+    if not os.path.exists(s_dir):
+        bb.warn("%s ('S') don't exist, you must set 'S' to a proper value"
 % s_dir) }
 
  def pkgarch_mapping(d):

@@ -220,7 +229,7 @@ CONFIGURESTAMPFILE = "${WORKDIR}/configure.sstate"
  CLEANBROKEN = "0"
 
  addtask configure after do_patch
-do_configure[dirs] = "${S} ${B}"
+do_configure[dirs] = "${B}"
  do_configure[deptask] = "do_populate_sysroot"
  base_do_configure() {
  	if [ -n "${CONFIGURESTAMPFILE}" -a -e "${CONFIGURESTAMPFILE}" ]; then

@@ -238,7 +247,7 @@ base_do_configure() {
  }
 
  addtask compile after do_configure
-do_compile[dirs] = "${S} ${B}"
+do_compile[dirs] = "${B}"
  base_do_compile() {
  	if [ -e Makefile -o -e makefile -o -e GNUmakefile ]; then
  		oe_runmake || die "make failed"

meta/classes/base.bbclass base_do_compile() { · line 262

Quoted 7 lines

meta/classes/base.bbclass +1 -1

@@ -248,7 +257,7 @@ base_do_compile() {
  }
 
  addtask install after do_compile
-do_install[dirs] = "${D} ${S} ${B}"
+do_install[dirs] = "${D} ${B}"
  # Remove and re-create ${D} so that is it guaranteed to be empty
  do_install[cleandirs] = "${D}"

This looks reasonable to me; however I'm sure I recall Chris talking about some kind of complication with this issue a year or so ago - Chris do you remember what that was about?

Cheers,
Paul

Quoted 5 lines · Paul Eggleton

Hi Petter,

Quoted 2 lines · On Saturday 10 January 2015 15:40:10 Petter Mabäcker wrote:

Currently base.bbclass is creating S if it's not created by unpacking an archive or fetching a repository. If we avoid creating S we can

detect

Quoted 1 line

Quoted 1 line

when S hasn't been set correctly, since it will not exist. Then we can

tell

Quoted 1 line

Quoted 1 line

the user that they should set S to a proper value, instead of just

failing

Quoted 3 lines

Quoted 3 lines

with odd errors in later tasks.

Besides removing the auto-creation of S this change will introduce a

warning

Quoted 1 line

Quoted 1 line

if S is set incorrectly. The reason for not display an error and return

is

Quoted 1 line

Quoted 1 line

due to all external layers that might have recipes that will fail

otherwise

Quoted 26 lines

Quoted 26 lines

and that might be a bit to hard to start with. So use a warning until people have had a chance to cleanup affected recipes.

[YOCTO #5627]

Signed-off-by Petter Mabäcker <[email protected]>

---
  meta/classes/base.bbclass | 17 +++++++++++++----
  1 file changed, 13 insertions(+), 4 deletions(-)

meta/classes/base.bbclass +0 -1

index de50be1..789af3b 100644
@@ -113,7 +113,6 @@ python base_do_fetch() {
 
  addtask unpack after do_fetch
  do_unpack[dirs] = "${WORKDIR}"
-do_unpack[cleandirs] = "${S}/patches"
  python base_do_unpack() {
      src_uri = (d.getVar('SRC_URI', True) or "").split()
      if len(src_uri) == 0:

@@ -121,11 +120,21 @@ python base_do_unpack() {
 
      rootdir = d.getVar('WORKDIR', True)
 

Quoted 15 lines

Quoted 5 lines

meta/classes/base.bbclass +5 -0

+    # Ensure that we cleanup ${S}/patches
+    # TODO: Investigate if we can remove
+    # the entire ${S} in this case.
+    s_dir = d.getVar('S', True)
+    p_dir = os.path.join(s_dir, 'patches')

meta/classes/base.bbclass python base_do_unpack() {

Quoted 10 lines

meta/classes/base.bbclass +5 -0

+    bb.utils.remove(p_dir, True)
+
      try:
          fetcher = bb.fetch2.Fetch(src_uri, d)
          fetcher.unpack(rootdir)
      except bb.fetch2.BBFetchException as e:
          raise bb.build.FuncFailed(e)
+
+    if not os.path.exists(s_dir):
+        bb.warn("%s ('S') don't exist, you must set 'S' to a proper

value"

Quoted 6 lines

Quoted 6 lines

meta/classes/base.bbclass +0 -0

 % s_dir) }
 
  def pkgarch_mapping(d):

@@ -220,7 +229,7 @@ CONFIGURESTAMPFILE = "${WORKDIR}/configure.sstate"
  CLEANBROKEN = "0"
 

Quoted 6 lines

meta/classes/base.bbclass CONFIGURESTAMPFILE = "${WORKDIR}/configure.sstate"

Quoted 6 lines

meta/classes/base.bbclass +1 -1

  addtask configure after do_patch
-do_configure[dirs] = "${S} ${B}"
+do_configure[dirs] = "${B}"
  do_configure[deptask] = "do_populate_sysroot"
  base_do_configure() {
     if [ -n "${CONFIGURESTAMPFILE}" -a -e "${CONFIGURESTAMPFILE}" ];

then

Quoted 12 lines

Quoted 12 lines

meta/classes/base.bbclass +1 -1

@@ -238,7 +247,7 @@ base_do_configure() {
  }
 
  addtask compile after do_configure
-do_compile[dirs] = "${S} ${B}"
+do_compile[dirs] = "${B}"
  base_do_compile() {
     if [ -e Makefile -o -e makefile -o -e GNUmakefile ]; then
             oe_runmake || die "make failed"

@@ -248,7 +257,7 @@ base_do_compile() {
  }
 

Quoted 6 lines

meta/classes/base.bbclass base_do_compile() { · line 262

Quoted 5 lines

meta/classes/base.bbclass +1 -1

  addtask install after do_compile
-do_install[dirs] = "${D} ${S} ${B}"
+do_install[dirs] = "${D} ${B}"
  # Remove and re-create ${D} so that is it guaranteed to be empty
  do_install[cleandirs] = "${D}"

This looks reasonable to me; however I'm sure I recall Chris talking

about

Quoted 6 lines

some kind of complication with this issue a year or so ago - Chris do you remember what that was about?

Cheers,
Paul

Ok, I will wait and see if Chris remembers anything sneaky about this. I have run quite some tests without seeing any problems so far. Perhaps it was about removing ${S} instead of just ${S}/patches in do_unpack ? Because I think there have been some discussions about that in the past. I also did some test with that in the scope of 'bug 5627', but it is far more complex to do such a chance, a lot of recipes didn't liked that at all =/ It seems more correct to really wipe entire ${S} in that case, but I guess it have to wait..

BR,
Petter