[PATCH] Wipe ${S} before unpacking sources

[PATCH] Wipe ${S} before unpacking sources

[Markus Lehtonen]

It would seem reasonable that do_unpack() always results in a pristine source
tree. This patch causes ${S} to be removed before unpacking sources.

I didn't quite understand what kind of "investigation" the following code
comment in do_unpack() was referring to:
  # TODO: Investigate if we can remove
  # the entire ${S} in this case.
What kind of special cases might we have? At least qemux86 core-image-sato
builds fine for me.

[YOCTO #9064]

Markus Lehtonen (1):
  base.bbclass wipe ${S} before unpacking source

 meta/classes/base.bbclass | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

-- 
2.6.2

[PATCH] base.bbclass wipe ${S} before unpacking source

[Markus Lehtonen]

Make sure that we have a pristine source tree after do_unpack.

[YOCTO #9064]

Signed-off-by: Markus Lehtonen <markus.lehtonen@linux.intel.com>
---
 meta/classes/base.bbclass | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/meta/classes/base.bbclass b/meta/classes/base.bbclass
index 1372f38..717a01e 100644
--- a/meta/classes/base.bbclass
+++ b/meta/classes/base.bbclass
@@ -140,12 +140,10 @@ 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.
+    # Cleanup ${S}
     s_dir = d.getVar('S', True)
-    p_dir = os.path.join(s_dir, 'patches')
-    bb.utils.remove(p_dir, True)
+    if s_dir != rootdir and not d.getVar("EXTERNALSRC", True):
+        bb.utils.remove(s_dir, True)
 
     try:
         fetcher = bb.fetch2.Fetch(src_uri, d)
-- 
2.6.2

Re: [PATCH] base.bbclass wipe ${S} before unpacking source

[Burton, Ross]

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

On 23 February 2016 at 11:36, Markus Lehtonen <
markus.lehtonen@linux.intel.com> wrote:

> Make sure that we have a pristine source tree after do_unpack.
>

Interestingly this fails on db-native rebuild for me, as db is playing
games with S (it's not the top of the tarball).  I had a look at the db
recipe, screamed a bit, and am testing a patch to make it more idiomatic.
Let's throw this at the autobuilder and see what happens...

Ross

[-- Attachment #2: Type: text/html, Size: 876 bytes --]

Re: [PATCH] base.bbclass wipe ${S} before unpacking source

[Christopher Larson]

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

On Tue, Feb 23, 2016 at 6:55 AM Burton, Ross <ross.burton@intel.com> wrote:

>
> On 23 February 2016 at 11:36, Markus Lehtonen <
> markus.lehtonen@linux.intel.com> wrote:
>
>> Make sure that we have a pristine source tree after do_unpack.
>>
>
> Interestingly this fails on db-native rebuild for me, as db is playing
> games with S (it's not the top of the tarball).  I had a look at the db
> recipe, screamed a bit, and am testing a patch to make it more idiomatic.
> Let's throw this at the autobuilder and see what happens...
>

db isn't the only recipe doing that. The fetcher unpack method knows where
it's unpacking to, I think if anyone should be clearing out the destination
first, it should.

[-- Attachment #2: Type: text/html, Size: 1368 bytes --]

Re: [PATCH] base.bbclass wipe ${S} before unpacking source

[Burton, Ross]

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

On 23 February 2016 at 18:01, Christopher Larson <clarson@kergoth.com>
wrote:

> db isn't the only recipe doing that. The fetcher unpack method knows where
> it's unpacking to, I think if anyone should be clearing out the destination
> first, it should.


If that's true for tarballs, I agree.  I was also wondering if this should
just be unpack[cleandirs] = ${S}

Ross

[-- Attachment #2: Type: text/html, Size: 766 bytes --]

Re: [PATCH] base.bbclass wipe ${S} before unpacking source

[Markus Lehtonen]

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

On Tue, 2016-02-23 at 18:23 +0000, Burton, Ross wrote:
> 
> On 23 February 2016 at 18:01, Christopher Larson <clarson@kergoth.com
> > wrote:
> > db isn't the only recipe doing that. The fetcher unpack method
> > knows where it's unpacking to, I think if anyone should be clearing
> > out the destination first, it should. 
> If that's true for tarballs, I agree.  I was also wondering if this
> should just be unpack[cleandirs] = ${S}
The problem is that the fetcher does not now the target directory, i.e.
it does not examine the content of tar/zip. Implementing that logic for
different archive formats would require a lot more work.
Using cleandirs sounds good to me. I.e. something like:
diff --git a/meta/classes/base.bbclass b/meta/classes/base.bbclass
index 1372f38..aa107d4 100644
--- a/meta/classes/base.bbclass
+++ b/meta/classes/base.bbclass
@@ -133,23 +133,15 @@ python base_do_fetch() {
 
 addtask unpack after do_fetch
 do_unpack[dirs] = "${WORKDIR}"
+do_unpack[cleandirs] = "${@d.getVar('S', True) if d.getVar('S', True)
!= d.getVar('WORKDIR', True) else ''}"
 python base_do_unpack() {
     src_uri = (d.getVar('SRC_URI', True) or "").split()
     if len(src_uri) == 0:
         return
 
-    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)
+        fetcher.unpack(d.getVar('WORKDIR', True))
     except bb.fetch2.BBFetchException as e:
         raise bb.build.FuncFailed(e)
 }

[-- Attachment #2: Type: text/html, Size: 2881 bytes --]

Re: [PATCH] base.bbclass wipe ${S} before unpacking source

[Mike Looijmans]

You might want to assure that S is a child of WORKDIR before wiping it. 
Strange things would happen if someone tried setting S=$HOME in a recipe 
(by mistake or just for experimenting).


On 23-02-16 12:36, Markus Lehtonen wrote:
> Make sure that we have a pristine source tree after do_unpack.
>
> [YOCTO #9064]
>
> Signed-off-by: Markus Lehtonen <markus.lehtonen@linux.intel.com>
> ---
>   meta/classes/base.bbclass | 8 +++-----
>   1 file changed, 3 insertions(+), 5 deletions(-)

...

-- 
Mike Looijmans

Re: [PATCH] Wipe ${S} before unpacking sources

[Petter Mabäcker]

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

 

2016-02-23 12:36 skrev Markus Lehtonen: 

> It would seem reasonable
that do_unpack() always results in a pristine source
> tree. This patch
causes ${S} to be removed before unpacking sources.
> 
> I didn't quite
understand what kind of "investigation" the following code
> comment in
do_unpack() was referring to:
> # TODO: Investigate if we can remove
> #
the entire ${S} in this case.
> What kind of special cases might we
have? At least qemux86 core-image-sato
> builds fine for me.

The text
isn't referring to any investigation already done. But me and Paul
Eggleton discussed this matter (removing entire ${S}, not just
{$S}/patches) within the scope of another bug. But since this wasn't
really the scope for that bug, the TODO was added instead. And happily
you have done the investigation and it's seems like ${S} might be able
to be removed in this case, after all :)

So please go ahead and you can
remove that TODO comment within the scope of this patch, since if this
change gets merged it's not relevant anymore..

BR Petter 

> [YOCTO
#9064] Markus Lehtonen (1): base.bbclass wipe ${S} before unpacking
source meta/classes/base.bbclass | 8 +++----- 1 file changed, 3
insertions(+), 5 deletions(-) -- 2.6.2

See inline comments. 

Petter
Mabäcker

Technux <petter@technux.se>
www.technux.se
 

[-- Attachment #2: Type: text/html, Size: 1884 bytes --]