[PATCH 0/5] Fix do_clean for git and gitsm

[PATCH 0/5] Fix do_clean for git and gitsm

[liezhi.yang]

From: Robert Yang <liezhi.yang@windriver.com>

* Test info
$ bitbake world --runall=fetch && bitbake world --runall=cleanall
$ ls downloads/
git2  uninative
$ ls downloads/git2/
[No output]

$ bitbake-selftest
Works well

Now all known do_clean issues have been fixed.

// Robert

The following changes since commit 95e61f3dacacb3a001d9f0e2db4c4a2960d96640:

  contrib/hashserv/Dockerfile: Add libgcc to image for runtime dependency (2024-10-25 15:40:52 +0100)

are available in the Git repository at:

  https://github.com/robertlinux/bitbake rbt/clean
  https://github.com/robertlinux/bitbake/tree/rbt/clean

Robert Yang (5):
  gitsm: Add call_process_submodules() to remove duplicated code
  gitsm: Remove downloads/tmpdir when failed
  gitsm: Add clean function
  git: Clean shallow mirror tarball
  git: Clean broken symlink

 lib/bb/fetch2/git.py   |  7 +++++-
 lib/bb/fetch2/gitsm.py | 57 +++++++++++++++++++++---------------------
 2 files changed, 35 insertions(+), 29 deletions(-)

-- 
2.25.1

[PATCH 1/5] gitsm: Add call_process_submodules() to remove duplicated code

[liezhi.yang]

From: Robert Yang <liezhi.yang@windriver.com>

There are 14 lines can be removed, and can make it easy to maintain.

Signed-off-by: Robert Yang <liezhi.yang@windriver.com>
---
 lib/bb/fetch2/gitsm.py | 42 ++++++++++++++----------------------------
 1 file changed, 14 insertions(+), 28 deletions(-)

diff --git a/lib/bb/fetch2/gitsm.py b/lib/bb/fetch2/gitsm.py
index f7f3af721..17aac6357 100644
--- a/lib/bb/fetch2/gitsm.py
+++ b/lib/bb/fetch2/gitsm.py
@@ -147,6 +147,17 @@ class GitSM(Git):
 
         return submodules != []
 
+    def call_process_submodules(self, ud, d, extra_check, subfunc):
+        # If we're using a shallow mirror tarball it needs to be
+        # unpacked temporarily so that we can examine the .gitmodules file
+        if ud.shallow and os.path.exists(ud.fullshallow) and extra_check:
+            tmpdir = tempfile.mkdtemp(dir=d.getVar("DL_DIR"))
+            runfetchcmd("tar -xzf %s" % ud.fullshallow, d, workdir=tmpdir)
+            self.process_submodules(ud, tmpdir, subfunc, d)
+            shutil.rmtree(tmpdir)
+        else:
+            self.process_submodules(ud, ud.clonedir, subfunc, d)
+
     def need_update(self, ud, d):
         if Git.need_update(self, ud, d):
             return True
@@ -164,15 +175,7 @@ class GitSM(Git):
                 logger.error('gitsm: submodule update check failed: %s %s' % (type(e).__name__, str(e)))
                 need_update_result = True
 
-        # If we're using a shallow mirror tarball it needs to be unpacked
-        # temporarily so that we can examine the .gitmodules file
-        if ud.shallow and os.path.exists(ud.fullshallow) and not os.path.exists(ud.clonedir):
-            tmpdir = tempfile.mkdtemp(dir=d.getVar("DL_DIR"))
-            runfetchcmd("tar -xzf %s" % ud.fullshallow, d, workdir=tmpdir)
-            self.process_submodules(ud, tmpdir, need_update_submodule, d)
-            shutil.rmtree(tmpdir)
-        else:
-            self.process_submodules(ud, ud.clonedir, need_update_submodule, d)
+        self.call_process_submodules(ud, d, not os.path.exists(ud.clonedir), need_update_submodule)
 
         if need_update_list:
             logger.debug('gitsm: Submodules requiring update: %s' % (' '.join(need_update_list)))
@@ -195,16 +198,7 @@ class GitSM(Git):
                 raise
 
         Git.download(self, ud, d)
-
-        # If we're using a shallow mirror tarball it needs to be unpacked
-        # temporarily so that we can examine the .gitmodules file
-        if ud.shallow and os.path.exists(ud.fullshallow) and self.need_update(ud, d):
-            tmpdir = tempfile.mkdtemp(dir=d.getVar("DL_DIR"))
-            runfetchcmd("tar -xzf %s" % ud.fullshallow, d, workdir=tmpdir)
-            self.process_submodules(ud, tmpdir, download_submodule, d)
-            shutil.rmtree(tmpdir)
-        else:
-            self.process_submodules(ud, ud.clonedir, download_submodule, d)
+        self.call_process_submodules(ud, d, self.need_update(ud, d), download_submodule)
 
     def unpack(self, ud, destdir, d):
         def unpack_submodules(ud, url, module, modpath, workdir, d):
@@ -263,14 +257,6 @@ class GitSM(Git):
             newfetch = Fetch([url], d, cache=False)
             urldata.extend(newfetch.expanded_urldata())
 
-        # If we're using a shallow mirror tarball it needs to be unpacked
-        # temporarily so that we can examine the .gitmodules file
-        if ud.shallow and os.path.exists(ud.fullshallow) and ud.method.need_update(ud, d):
-            tmpdir = tempfile.mkdtemp(dir=d.getVar("DL_DIR"))
-            subprocess.check_call("tar -xzf %s" % ud.fullshallow, cwd=tmpdir, shell=True)
-            self.process_submodules(ud, tmpdir, add_submodule, d)
-            shutil.rmtree(tmpdir)
-        else:
-            self.process_submodules(ud, ud.clonedir, add_submodule, d)
+        self.call_process_submodules(ud, d, ud.method.need_update(ud, d), add_submodule)
 
         return urldata
-- 
2.25.1

[PATCH 2/5] gitsm: Remove downloads/tmpdir when failed

[liezhi.yang]

From: Robert Yang <liezhi.yang@windriver.com>

The tmpdir such as downloads/tmplp3cnemv won't be removed without this fix.

Signed-off-by: Robert Yang <liezhi.yang@windriver.com>
---
 lib/bb/fetch2/gitsm.py | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/lib/bb/fetch2/gitsm.py b/lib/bb/fetch2/gitsm.py
index 17aac6357..fab4b1164 100644
--- a/lib/bb/fetch2/gitsm.py
+++ b/lib/bb/fetch2/gitsm.py
@@ -152,9 +152,11 @@ class GitSM(Git):
         # unpacked temporarily so that we can examine the .gitmodules file
         if ud.shallow and os.path.exists(ud.fullshallow) and extra_check:
             tmpdir = tempfile.mkdtemp(dir=d.getVar("DL_DIR"))
-            runfetchcmd("tar -xzf %s" % ud.fullshallow, d, workdir=tmpdir)
-            self.process_submodules(ud, tmpdir, subfunc, d)
-            shutil.rmtree(tmpdir)
+            try:
+                runfetchcmd("tar -xzf %s" % ud.fullshallow, d, workdir=tmpdir)
+                self.process_submodules(ud, tmpdir, subfunc, d)
+            finally:
+                shutil.rmtree(tmpdir)
         else:
             self.process_submodules(ud, ud.clonedir, subfunc, d)
 
-- 
2.25.1

[PATCH 3/5] gitsm: Add clean function

[liezhi.yang]

From: Robert Yang <liezhi.yang@windriver.com>

Fixed:
$ bitbake utfcpp -cfetch && bitbake utfcpp -ccleanall

The downloads/git2/github.com.nemtrif.ftest won't be cleaned without this fix.

Signed-off-by: Robert Yang <liezhi.yang@windriver.com>
---
 lib/bb/fetch2/gitsm.py | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/lib/bb/fetch2/gitsm.py b/lib/bb/fetch2/gitsm.py
index fab4b1164..ba62517f0 100644
--- a/lib/bb/fetch2/gitsm.py
+++ b/lib/bb/fetch2/gitsm.py
@@ -249,6 +249,19 @@ class GitSM(Git):
             # should also be skipped as these files were already smudged in the fetch stage if lfs
             # was enabled.
             runfetchcmd("GIT_LFS_SKIP_SMUDGE=1 %s submodule update --recursive --no-fetch" % (ud.basecmd), d, quiet=True, workdir=ud.destdir)
+    def clean(self, ud, d):
+        def clean_submodule(ud, url, module, modpath, workdir, d):
+            url += ";bareclone=1;nobranch=1"
+            try:
+                newfetch = Fetch([url], d, cache=False)
+                newfetch.clean()
+            except Exception as e:
+                logger.warning('gitsm: submodule clean failed: %s %s' % (type(e).__name__, str(e)))
+
+        self.call_process_submodules(ud, d, True, clean_submodule)
+
+        # Clean top git dir
+        Git.clean(self, ud, d)
 
     def implicit_urldata(self, ud, d):
         import shutil, subprocess, tempfile
-- 
2.25.1

[PATCH 4/5] git: Clean shallow mirror tarball

[liezhi.yang]

From: Robert Yang <liezhi.yang@windriver.com>

Fixed:
BB_GIT_SHALLOW = "1"
BB_GENERATE_SHALLOW_TARBALLS = "1"
$ bitbake utfcpp -cfetch && bitbake utfcpp -ccleanall

The downloads/gitsmshallow_github.com.nemtrif.*.tar.gz won't be cleaned without
this fix.

Signed-off-by: Robert Yang <liezhi.yang@windriver.com>
---
 lib/bb/fetch2/git.py | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/lib/bb/fetch2/git.py b/lib/bb/fetch2/git.py
index 5b678827e..0ea55e139 100644
--- a/lib/bb/fetch2/git.py
+++ b/lib/bb/fetch2/git.py
@@ -727,6 +727,11 @@ class Git(FetchMethod):
             clonedir = os.path.realpath(ud.localpath)
             to_remove.append(clonedir)
 
+        # Remove shallow mirror tarball
+        if ud.shallow:
+            to_remove.append(ud.fullshallow)
+            to_remove.append(ud.fullshallow + ".done")
+
         for r in to_remove:
             if os.path.exists(r):
                 bb.note('Removing %s' % r)
-- 
2.25.1

[PATCH 5/5] git: Clean broken symlink

[liezhi.yang]

From: Robert Yang <liezhi.yang@windriver.com>

The file may be a symlink when mirror from local disk, and the symlink will be
broken when the linked file is removed, use os.path.islink() to check and
remove the broken symlink.

Signed-off-by: Robert Yang <liezhi.yang@windriver.com>
---
 lib/bb/fetch2/git.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/bb/fetch2/git.py b/lib/bb/fetch2/git.py
index 0ea55e139..6badda597 100644
--- a/lib/bb/fetch2/git.py
+++ b/lib/bb/fetch2/git.py
@@ -733,7 +733,7 @@ class Git(FetchMethod):
             to_remove.append(ud.fullshallow + ".done")
 
         for r in to_remove:
-            if os.path.exists(r):
+            if os.path.exists(r) or os.path.islink(r):
                 bb.note('Removing %s' % r)
                 bb.utils.remove(r, True)
 
-- 
2.25.1

Re: [bitbake-devel] [PATCH 2/5] gitsm: Remove downloads/tmpdir when failed

[Richard Purdie]

On Sat, 2024-10-26 at 02:39 -0700, Robert Yang via
lists.openembedded.org wrote:
> From: Robert Yang <liezhi.yang@windriver.com>
> 
> The tmpdir such as downloads/tmplp3cnemv won't be removed without
> this fix.
> 
> Signed-off-by: Robert Yang <liezhi.yang@windriver.com>

Thanks, I've been meaning to track down the cause of those!

Steve: We might want to consider this to help the autobuilder.

Cheers,

Richard

Re: [bitbake-devel] [PATCH 2/5] gitsm: Remove downloads/tmpdir when failed

[Steve Sakoman]

On Mon, Oct 28, 2024 at 7:06 AM Richard Purdie
<richard.purdie@linuxfoundation.org> wrote:
>
> On Sat, 2024-10-26 at 02:39 -0700, Robert Yang via
> lists.openembedded.org wrote:
> > From: Robert Yang <liezhi.yang@windriver.com>
> >
> > The tmpdir such as downloads/tmplp3cnemv won't be removed without
> > this fix.
> >
> > Signed-off-by: Robert Yang <liezhi.yang@windriver.com>
>
> Thanks, I've been meaning to track down the cause of those!
>
> Steve: We might want to consider this to help the autobuilder.

This won't apply unless I also take gitsm: Add
call_process_submodules() to remove duplicated code

I've added both to all three stable branches and will test when the
autobuilder is usable again.

Steve