Re: [PATCH v3] patch.py: Prevent git repo reinitialization

[Christian Eggers <ceggers@arri.de>]

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>