[v3] patch.py: Prevent git repo reinitialization

Message ID 20220221191728.445-1-pavel@zhukoff.net
State Accepted, archived
Commit 80500ecda4c1bc8812e6e078b6b0db5ec46624de
Headers show
Series [v3] patch.py: Prevent git repo reinitialization | expand

Commit Message

Pavel Zhukov Feb. 21, 2022, 7:17 p.m. UTC
There were few bugs in the _isInitialized() function which might trigger
git repo to be reinitialized and patches failing to apply.

Signed-off-by: Pavel Zhukov <pavel.zhukov@huawei.com>
---
 .../recipes-test/gitrepotest/gitrepotest.bb   | 16 ++++++++++++++++
 .../gitrepotest/0001-testpatch.patch          |  9 +++++++++
 meta/lib/oe/patch.py                          | 11 ++++++++---
 meta/lib/oeqa/selftest/cases/bbtests.py       | 19 +++++++++++++++++--
 4 files changed, 50 insertions(+), 5 deletions(-)
 create mode 100644 meta-selftest/recipes-test/gitrepotest/gitrepotest.bb
 create mode 100644 meta-selftest/recipes-test/gitrepotest/gitrepotest/0001-testpatch.patch

Comments

Christian Eggers Feb. 22, 2022, 6:16 p.m. UTC | #1
On Monday, 21 February 2022, 20:17:29 CET, Pavel Zhukov wrote:
> There were few bugs in the _isInitialized() function which might trigger
> git repo to be reinitialized and patches failing to apply.
> 
> Signed-off-by: Pavel Zhukov <pavel.zhukov@huawei.com>
> ---
>  .../recipes-test/gitrepotest/gitrepotest.bb   | 16 ++++++++++++++++
>  .../gitrepotest/0001-testpatch.patch          |  9 +++++++++
>  meta/lib/oe/patch.py                          | 11 ++++++++---
>  meta/lib/oeqa/selftest/cases/bbtests.py       | 19 +++++++++++++++++--
>  4 files changed, 50 insertions(+), 5 deletions(-)
>  create mode 100644 meta-selftest/recipes-test/gitrepotest/gitrepotest.bb
>  create mode 100644 meta-selftest/recipes-test/gitrepotest/gitrepotest/0001-testpatch.patch
> 
> diff --git a/meta-selftest/recipes-test/gitrepotest/gitrepotest.bb b/meta-selftest/recipes-test/gitrepotest/gitrepotest.bb
> new file mode 100644
> index 0000000000..f1b6c55833
> --- /dev/null
> +++ b/meta-selftest/recipes-test/gitrepotest/gitrepotest.bb
> @@ -0,0 +1,16 @@
> +SUMMARY = "Test recipe for git repo initialization"
> +HOMEPAGE = "https://git.yoctoproject.org/git/matchbox-panel-2"
> +LICENSE = "GPL-2.0-or-later"
> +LIC_FILES_CHKSUM = "file://COPYING;md5=94d55d512a9ba36caa9b7df079bae19f"
> +
> +INHIBIT_DEFAULT_DEPS = "1"
> +
> +PATCHTOOL="git"
> +
> +SRC_URI = "git://git.yoctoproject.org/git/matchbox-panel-2;branch=master;protocol=https \
> +           file://0001-testpatch.patch \
> +          "
> +
> +SRCREV = "f82ca3f42510fb3ef10f598b393eb373a2c34ca7"
> +
> +S = "${WORKDIR}/git"
> diff --git a/meta-selftest/recipes-test/gitrepotest/gitrepotest/0001-testpatch.patch b/meta-selftest/recipes-test/gitrepotest/gitrepotest/0001-testpatch.patch
> new file mode 100644
> index 0000000000..bccda17ee9
> --- /dev/null
> +++ b/meta-selftest/recipes-test/gitrepotest/gitrepotest/0001-testpatch.patch
> @@ -0,0 +1,9 @@
> +diff --git a/Makefile.am b/Makefile.am
> +index 432a9b4..bbf7c74 100644
> +--- a/Makefile.am
> ++++ b/Makefile.am
> +@@ -1,3 +1,4 @@
> ++## This is useless comment to test if patch works
> + ACLOCAL_AMFLAGS = -I m4
> + 
> + SUBDIRS = matchbox-panel applets data po
> diff --git a/meta/lib/oe/patch.py b/meta/lib/oe/patch.py
> index 950fe723dc..9034fcae03 100644
> --- a/meta/lib/oe/patch.py
> +++ b/meta/lib/oe/patch.py
> @@ -304,14 +304,19 @@ class GitApplyTree(PatchTree):
>  
>      def _isInitialized(self):
>          cmd = "git rev-parse --show-toplevel"
> -        (status, output) = subprocess.getstatusoutput(cmd.split())
> +        try:
> +            output = runcmd(cmd.split(), self.dir).strip()
> +        except CmdError as err:
> +            ## runcmd returned non-zero which most likely means 128
> +            ## Not a git directory
> +            return False
>          ## Make sure repo is in builddir to not break top-level git repos
> -        return status == 0 and os.path.samedir(output, self.dir)
> +        return os.path.samefile(output, self.dir)

output = /home/eggers/<...>/build/devel-kirkstone/tmp/work-shared/orbiter/kernel-source
self.dir = /home/eggers/<...>/build/devel-kirkstone/tmp/work/orbiter-poky-linux-musleabi/linux-stable-rt/5.10.100-rt62-orbiter+gitAUTOINC+ca44e6dff2-r0/git
os.path.samefile(output, self.dir) = 1

Looks fine now. The build on the affected CI machine is also fixed.

>  
>      def _initRepo(self):
>          runcmd("git init".split(), self.dir)
>          runcmd("git add .".split(), self.dir)
> -        runcmd("git commit -a --allow-empty -m Patching_started".split(), self.dir)
> +        runcmd("git commit -a --allow-empty -m bitbake_patching_started".split(), self.dir)
>  
>      @staticmethod
>      def extractPatchHeader(patchfile):
> diff --git a/meta/lib/oeqa/selftest/cases/bbtests.py b/meta/lib/oeqa/selftest/cases/bbtests.py
> index ce72c4bcc6..35ad9f3cd6 100644
> --- a/meta/lib/oeqa/selftest/cases/bbtests.py
> +++ b/meta/lib/oeqa/selftest/cases/bbtests.py
> @@ -307,11 +307,26 @@ INHERIT:remove = \"report-error\"
>          src = get_bb_var("SRC_URI",test_recipe)
>          gitscm = re.search("git://", src)
>          self.assertFalse(gitscm, "test_git_patchtool pre-condition failed: {} test recipe contains git repo!".format(test_recipe))
> -        result = bitbake('man-db -c patch', ignore_status=False)
> +        result = bitbake('{} -c patch'.format(test_recipe), ignore_status=False)
>          fatal = re.search("fatal: not a git repository (or any of the parent directories)", result.output)
>          self.assertFalse(fatal, "Failed to patch using PATCHTOOL=\"git\"")
>          self.delete_recipeinc(test_recipe)
> -        bitbake('-cclean man-db')
> +        bitbake('-cclean {}'.format(test_recipe))
> +
> +    def test_git_patchtool2(self):
> +        """ Test if PATCHTOOL=git works with git repo and doesn't reinitialize it
> +        """
> +        test_recipe = "gitrepotest"
> +        src = get_bb_var("SRC_URI",test_recipe)
> +        gitscm = re.search("git://", src)
> +        self.assertTrue(gitscm, "test_git_patchtool pre-condition failed: {} test recipe doesn't contains git repo!".format(test_recipe))
> +        result = bitbake('{} -c patch'.format(test_recipe), ignore_status=False)
> +        srcdir = get_bb_var('S', test_recipe)
> +        result = runCmd("git log", cwd = srcdir)
> +        self.assertFalse("bitbake_patching_started" in result.output, msg = "Repository has been reinitialized. {}".format(srcdir))
> +        self.delete_recipeinc(test_recipe)
> +        bitbake('-cclean {}'.format(test_recipe))
> +
>  
>      def test_git_unpack_nonetwork(self):
>          """
> 
Tested-by: Christian Eggers <ceggers@arri.de>

Patch

diff --git a/meta-selftest/recipes-test/gitrepotest/gitrepotest.bb b/meta-selftest/recipes-test/gitrepotest/gitrepotest.bb
new file mode 100644
index 0000000000..f1b6c55833
--- /dev/null
+++ b/meta-selftest/recipes-test/gitrepotest/gitrepotest.bb
@@ -0,0 +1,16 @@ 
+SUMMARY = "Test recipe for git repo initialization"
+HOMEPAGE = "https://git.yoctoproject.org/git/matchbox-panel-2"
+LICENSE = "GPL-2.0-or-later"
+LIC_FILES_CHKSUM = "file://COPYING;md5=94d55d512a9ba36caa9b7df079bae19f"
+
+INHIBIT_DEFAULT_DEPS = "1"
+
+PATCHTOOL="git"
+
+SRC_URI = "git://git.yoctoproject.org/git/matchbox-panel-2;branch=master;protocol=https \
+           file://0001-testpatch.patch \
+          "
+
+SRCREV = "f82ca3f42510fb3ef10f598b393eb373a2c34ca7"
+
+S = "${WORKDIR}/git"
diff --git a/meta-selftest/recipes-test/gitrepotest/gitrepotest/0001-testpatch.patch b/meta-selftest/recipes-test/gitrepotest/gitrepotest/0001-testpatch.patch
new file mode 100644
index 0000000000..bccda17ee9
--- /dev/null
+++ b/meta-selftest/recipes-test/gitrepotest/gitrepotest/0001-testpatch.patch
@@ -0,0 +1,9 @@ 
+diff --git a/Makefile.am b/Makefile.am
+index 432a9b4..bbf7c74 100644
+--- a/Makefile.am
++++ b/Makefile.am
+@@ -1,3 +1,4 @@
++## This is useless comment to test if patch works
+ ACLOCAL_AMFLAGS = -I m4
+ 
+ SUBDIRS = matchbox-panel applets data po
diff --git a/meta/lib/oe/patch.py b/meta/lib/oe/patch.py
index 950fe723dc..9034fcae03 100644
--- a/meta/lib/oe/patch.py
+++ b/meta/lib/oe/patch.py
@@ -304,14 +304,19 @@  class GitApplyTree(PatchTree):
 
     def _isInitialized(self):
         cmd = "git rev-parse --show-toplevel"
-        (status, output) = subprocess.getstatusoutput(cmd.split())
+        try:
+            output = runcmd(cmd.split(), self.dir).strip()
+        except CmdError as err:
+            ## runcmd returned non-zero which most likely means 128
+            ## Not a git directory
+            return False
         ## Make sure repo is in builddir to not break top-level git repos
-        return status == 0 and os.path.samedir(output, self.dir)
+        return os.path.samefile(output, self.dir)
 
     def _initRepo(self):
         runcmd("git init".split(), self.dir)
         runcmd("git add .".split(), self.dir)
-        runcmd("git commit -a --allow-empty -m Patching_started".split(), self.dir)
+        runcmd("git commit -a --allow-empty -m bitbake_patching_started".split(), self.dir)
 
     @staticmethod
     def extractPatchHeader(patchfile):
diff --git a/meta/lib/oeqa/selftest/cases/bbtests.py b/meta/lib/oeqa/selftest/cases/bbtests.py
index ce72c4bcc6..35ad9f3cd6 100644
--- a/meta/lib/oeqa/selftest/cases/bbtests.py
+++ b/meta/lib/oeqa/selftest/cases/bbtests.py
@@ -307,11 +307,26 @@  INHERIT:remove = \"report-error\"
         src = get_bb_var("SRC_URI",test_recipe)
         gitscm = re.search("git://", src)
         self.assertFalse(gitscm, "test_git_patchtool pre-condition failed: {} test recipe contains git repo!".format(test_recipe))
-        result = bitbake('man-db -c patch', ignore_status=False)
+        result = bitbake('{} -c patch'.format(test_recipe), ignore_status=False)
         fatal = re.search("fatal: not a git repository (or any of the parent directories)", result.output)
         self.assertFalse(fatal, "Failed to patch using PATCHTOOL=\"git\"")
         self.delete_recipeinc(test_recipe)
-        bitbake('-cclean man-db')
+        bitbake('-cclean {}'.format(test_recipe))
+
+    def test_git_patchtool2(self):
+        """ Test if PATCHTOOL=git works with git repo and doesn't reinitialize it
+        """
+        test_recipe = "gitrepotest"
+        src = get_bb_var("SRC_URI",test_recipe)
+        gitscm = re.search("git://", src)
+        self.assertTrue(gitscm, "test_git_patchtool pre-condition failed: {} test recipe doesn't contains git repo!".format(test_recipe))
+        result = bitbake('{} -c patch'.format(test_recipe), ignore_status=False)
+        srcdir = get_bb_var('S', test_recipe)
+        result = runCmd("git log", cwd = srcdir)
+        self.assertFalse("bitbake_patching_started" in result.output, msg = "Repository has been reinitialized. {}".format(srcdir))
+        self.delete_recipeinc(test_recipe)
+        bitbake('-cclean {}'.format(test_recipe))
+
 
     def test_git_unpack_nonetwork(self):
         """