[PATCH 1/7] devtool: standard: Add some missing whitespace

[PATCH 1/7] devtool: standard: Add some missing whitespace

[Peter Kjellerstedt]

Makes it a little bit easier when reading the code.

Signed-off-by: Peter Kjellerstedt <peter.kjellerstedt@axis.com>
---
 scripts/lib/devtool/standard.py | 24 ++++++++++++------------
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/scripts/lib/devtool/standard.py b/scripts/lib/devtool/standard.py
index 0243e3bc75..dd9232da1c 100644
--- a/scripts/lib/devtool/standard.py
+++ b/scripts/lib/devtool/standard.py
@@ -460,7 +460,7 @@ def sync(args, config, basepath, workspace):
     finally:
         tinfoil.shutdown()
 
-def symlink_oelocal_files_srctree(rd,srctree):
+def symlink_oelocal_files_srctree(rd, srctree):
     import oe.patch
     if os.path.abspath(rd.getVar('S')) == os.path.abspath(rd.getVar('WORKDIR')):
         # If recipe extracts to ${WORKDIR}, symlink the files into the srctree
@@ -657,9 +657,9 @@ def _extract_source(srctree, keep_temp, devbranch, sync, config, basepath, works
 
             if os.path.exists(workshareddir) and (not os.listdir(workshareddir) or kernelVersion != staging_kerVer):
                 shutil.rmtree(workshareddir)
-                oe.path.copyhardlinktree(srcsubdir,workshareddir)
+                oe.path.copyhardlinktree(srcsubdir, workshareddir)
             elif not os.path.exists(workshareddir):
-                oe.path.copyhardlinktree(srcsubdir,workshareddir)
+                oe.path.copyhardlinktree(srcsubdir, workshareddir)
 
         tempdir_localdir = os.path.join(tempdir, 'oe-local-files')
         srctree_localdir = os.path.join(srctree, 'oe-local-files')
@@ -689,7 +689,7 @@ def _extract_source(srctree, keep_temp, devbranch, sync, config, basepath, works
                 shutil.move(tempdir_localdir, srcsubdir)
 
             shutil.move(srcsubdir, srctree)
-            symlink_oelocal_files_srctree(d,srctree)
+            symlink_oelocal_files_srctree(d, srctree)
 
         if is_kernel_yocto:
             logger.info('Copying kernel config to srctree')
@@ -762,7 +762,7 @@ def get_staging_kver(srcdir):
     kerver = []
     staging_kerVer=""
     if os.path.exists(srcdir) and os.listdir(srcdir):
-        with open(os.path.join(srcdir,"Makefile")) as f:
+        with open(os.path.join(srcdir, "Makefile")) as f:
             version = [next(f) for x in range(5)][1:4]
             for word in version:
                 kerver.append(word.split('= ')[1].split('\n')[0])
@@ -843,10 +843,10 @@ def modify(args, config, basepath, workspace):
             staging_kerVer = get_staging_kver(srcdir)
             staging_kbranch = get_staging_kbranch(srcdir)
             if (os.path.exists(srcdir) and os.listdir(srcdir)) and (kernelVersion in staging_kerVer and staging_kbranch == kbranch):
-                oe.path.copyhardlinktree(srcdir,srctree)
+                oe.path.copyhardlinktree(srcdir, srctree)
                 workdir = rd.getVar('WORKDIR')
                 srcsubdir = rd.getVar('S')
-                localfilesdir = os.path.join(srctree,'oe-local-files')
+                localfilesdir = os.path.join(srctree, 'oe-local-files')
                 # Move local source files into separate subdir
                 recipe_patches = [os.path.basename(patch) for patch in oe.recipeutils.get_recipe_patches(rd)]
                 local_files = oe.recipeutils.get_recipe_local_files(rd)
@@ -870,9 +870,9 @@ def modify(args, config, basepath, workspace):
                     for fname in local_files:
                         _move_file(os.path.join(workdir, fname), os.path.join(srctree, 'oe-local-files', fname))
                     with open(os.path.join(srctree, 'oe-local-files', '.gitignore'), 'w') as f:
-                        f.write('# Ignore local files, by default. Remove this file ''if you want to commit the directory to Git\n*\n')
+                        f.write('# Ignore local files, by default. Remove this file if you want to commit the directory to Git\n*\n')
 
-                symlink_oelocal_files_srctree(rd,srctree)
+                symlink_oelocal_files_srctree(rd, srctree)
 
                 task = 'do_configure'
                 res = tinfoil.build_targets(pn, task, handle_events=True)
@@ -880,7 +880,7 @@ def modify(args, config, basepath, workspace):
                 # Copy .config to workspace
                 kconfpath = rd.getVar('B')
                 logger.info('Copying kernel config to workspace')
-                shutil.copy2(os.path.join(kconfpath, '.config'),srctree)
+                shutil.copy2(os.path.join(kconfpath, '.config'), srctree)
 
                 # Set this to true, we still need to get initial_rev
                 # by parsing the git repo
@@ -1004,7 +1004,7 @@ def modify(args, config, basepath, workspace):
                         '        mv ${S}/.config ${S}/.config.old\n'
                         '    fi\n'
                         '}\n')
-            if rd.getVarFlag('do_menuconfig','task'):
+            if rd.getVarFlag('do_menuconfig', 'task'):
                 f.write('\ndo_configure:append() {\n'
                 '    if [ ${@oe.types.boolean(d.getVar("KCONFIG_CONFIG_ENABLE_MENUCONFIG"))} = True ]; then\n'
                 '        cp ${KCONFIG_CONFIG_ROOTDIR}/.config ${S}/.config.baseline\n'
@@ -2081,7 +2081,7 @@ def _reset(recipes, no_clean, remove_work, config, basepath, workspace):
                         # We don't want to risk wiping out any work in progress
                         if srctreebase.startswith(os.path.join(config.workspace_path, 'sources')):
                             from datetime import datetime
-                            preservesrc = os.path.join(config.workspace_path, 'attic', 'sources', "{}.{}".format(pn,datetime.now().strftime("%Y%m%d%H%M%S")))
+                            preservesrc = os.path.join(config.workspace_path, 'attic', 'sources', "{}.{}".format(pn, datetime.now().strftime("%Y%m%d%H%M%S")))
                             logger.info('Preserving source tree in %s\nIf you no '
                                         'longer need it then please delete it manually.\n'
                                         'It is also possible to reuse it via devtool source tree argument.'

[PATCH 2/7] devtool: _extract_source: Correct the removal of an old backup directory

[Peter Kjellerstedt]

Also correct the comment describing what is happening.

Signed-off-by: Peter Kjellerstedt <peter.kjellerstedt@axis.com>
---
 scripts/lib/devtool/standard.py | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/scripts/lib/devtool/standard.py b/scripts/lib/devtool/standard.py
index dd9232da1c..ccb7ea851b 100644
--- a/scripts/lib/devtool/standard.py
+++ b/scripts/lib/devtool/standard.py
@@ -667,13 +667,13 @@ def _extract_source(srctree, keep_temp, devbranch, sync, config, basepath, works
         if sync:
             bb.process.run('git fetch file://' + srcsubdir + ' ' + devbranch + ':' + devbranch, cwd=srctree)
 
-            # Move oe-local-files directory to srctree
-            # As the oe-local-files is not part of the constructed git tree,
-            # remove them directly during the synchrounizating might surprise
-            # the users.  Instead, we move it to oe-local-files.bak and remind
-            # user in the log message.
+            # Move the oe-local-files directory to srctree.
+            # As oe-local-files is not part of the constructed git tree,
+            # removing it directly during the synchronization might surprise
+            # the user.  Instead, we move it to oe-local-files.bak and remind
+            # the user in the log message.
             if os.path.exists(srctree_localdir + '.bak'):
-                shutil.rmtree(srctree_localdir, srctree_localdir + '.bak')
+                shutil.rmtree(srctree_localdir + '.bak')
 
             if os.path.exists(srctree_localdir):
                 logger.info('Backing up current local file directory %s' % srctree_localdir)

[PATCH 3/7] lib/oe/patch: Make extractPatches() not extract ignored commits

[Peter Kjellerstedt]

If a commit is marked with "%% ignore" it means it is used by devtool to
keep track of changes to the source code that are not the result of
running do_patch(). These changes need to actually be ignored when
extracting the patches as they typically make no sense as actual patches
in a recipe.

This also adds a new test for oe-selftest that verifies that there are
no patches generated from ignored commits.

Signed-off-by: Peter Kjellerstedt <peter.kjellerstedt@axis.com>
---
 .../0055-Add-cstdint-for-uintXX_t-types.patch | 38 +++++++++++
 ...099-cmake-Pass-PROBE_NAME-via-CFLAGS.patch | 29 +++++++++
 .../sysdig/sysdig-selftest_0.28.0.bb          | 64 +++++++++++++++++++
 meta/lib/oe/patch.py                          | 16 ++---
 meta/lib/oeqa/selftest/cases/devtool.py       | 46 +++++++++++++
 5 files changed, 185 insertions(+), 8 deletions(-)
 create mode 100644 meta-selftest/recipes-extended/sysdig/sysdig-selftest/0055-Add-cstdint-for-uintXX_t-types.patch
 create mode 100644 meta-selftest/recipes-extended/sysdig/sysdig-selftest/0099-cmake-Pass-PROBE_NAME-via-CFLAGS.patch
 create mode 100644 meta-selftest/recipes-extended/sysdig/sysdig-selftest_0.28.0.bb

diff --git a/meta-selftest/recipes-extended/sysdig/sysdig-selftest/0055-Add-cstdint-for-uintXX_t-types.patch b/meta-selftest/recipes-extended/sysdig/sysdig-selftest/0055-Add-cstdint-for-uintXX_t-types.patch
new file mode 100644
index 0000000000..e564958dad
--- /dev/null
+++ b/meta-selftest/recipes-extended/sysdig/sysdig-selftest/0055-Add-cstdint-for-uintXX_t-types.patch
@@ -0,0 +1,38 @@
+From 3d076ea588eb3c7f334133b4c31172a14beadf5b Mon Sep 17 00:00:00 2001
+From: Khem Raj <raj.khem@gmail.com>
+Date: Thu, 2 Feb 2023 20:18:27 -0800
+Subject: [PATCH] Add <cstdint> for uintXX_t types
+
+gcc 13 moved some includes around and as a result <cstdint> is no
+longer transitively included [1]. Explicitly include it
+for uintXX_t.
+
+[1] https://gcc.gnu.org/gcc-13/porting_to.html#header-dep-changes
+
+Upstream-Status: Submitted [https://github.com/falcosecurity/libs/pull/862]
+Signed-off-by: Khem Raj <raj.khem@gmail.com>
+---
+ userspace/libsinsp/filter/parser.h | 1 +
+ userspace/libsinsp/filter_value.h  | 1 +
+ 2 files changed, 2 insertions(+)
+
+--- a/userspace/libsinsp/filter/parser.h
++++ b/userspace/libsinsp/filter/parser.h
+@@ -18,6 +18,7 @@ limitations under the License.
+ #pragma once
+ 
+ #include "ast.h"
++#include <cstdint>
+ 
+ //
+ // Context-free Grammar for Sinsp Filters
+--- a/userspace/libsinsp/filter_value.h
++++ b/userspace/libsinsp/filter_value.h
+@@ -18,6 +18,7 @@ limitations under the License.
+ #pragma once
+ 
+ #include <string.h>
++#include <cstdint>
+ #include <utility>
+ 
+ // Used for CO_IN/CO_PMATCH filterchecks using PT_CHARBUFs to allow
diff --git a/meta-selftest/recipes-extended/sysdig/sysdig-selftest/0099-cmake-Pass-PROBE_NAME-via-CFLAGS.patch b/meta-selftest/recipes-extended/sysdig/sysdig-selftest/0099-cmake-Pass-PROBE_NAME-via-CFLAGS.patch
new file mode 100644
index 0000000000..903ccdf36a
--- /dev/null
+++ b/meta-selftest/recipes-extended/sysdig/sysdig-selftest/0099-cmake-Pass-PROBE_NAME-via-CFLAGS.patch
@@ -0,0 +1,29 @@
+From ed8969a233adb6bf701de96d0fd0570e5ddcc787 Mon Sep 17 00:00:00 2001
+From: Khem Raj <raj.khem@gmail.com>
+Date: Mon, 21 Mar 2022 19:35:48 -0700
+Subject: [PATCH] cmake: Pass PROBE_NAME via CFLAGS
+
+This helps compliation of driver code where its calling modprobe on the
+given kernel module via system() API
+
+Upstream-Status: Pending
+Signed-off-by: Khem Raj <raj.khem@gmail.com>
+---
+ CMakeLists.txt | 1 +
+ 1 file changed, 1 insertion(+)
+
+diff --git a/CMakeLists.txt b/CMakeLists.txt
+index 7dceb7ae..e156c36f 100644
+--- a/CMakeLists.txt
++++ b/CMakeLists.txt
+@@ -149,6 +149,7 @@ if(CMAKE_SYSTEM_NAME MATCHES "Linux")
+ 	if(NOT DEFINED PROBE_NAME)
+ 		set(PROBE_NAME "scap")
+ 	endif()
++	add_definitions(-DPROBE_NAME="${PROBE_NAME}")
+ 
+ 	set(DRIVERS_REPO "https://download.sysdig.com/scap-drivers")
+ 	
+-- 
+2.35.1
+
diff --git a/meta-selftest/recipes-extended/sysdig/sysdig-selftest_0.28.0.bb b/meta-selftest/recipes-extended/sysdig/sysdig-selftest_0.28.0.bb
new file mode 100644
index 0000000000..e114f465a0
--- /dev/null
+++ b/meta-selftest/recipes-extended/sysdig/sysdig-selftest_0.28.0.bb
@@ -0,0 +1,64 @@
+SUMMARY = "A New System Troubleshooting Tool Built for the Way You Work"
+DESCRIPTION = "Sysdig is open source, system-level exploration: capture \
+system state and activity from a running Linux instance, then save, \
+filter and analyze."
+HOMEPAGE = "http://www.sysdig.org/"
+LICENSE = "Apache-2.0 & (MIT | GPL-2.0-only)"
+LIC_FILES_CHKSUM = "file://COPYING;md5=f8fee3d59797546cffab04f3b88b2d44"
+
+inherit cmake pkgconfig
+
+#OECMAKE_GENERATOR = "Unix Makefiles"
+JIT ?= "jit"
+JIT:mipsarchn32 = ""
+JIT:mipsarchn64 = ""
+JIT:riscv64 = ""
+JIT:riscv32 = ""
+JIT:powerpc = ""
+JIT:powerpc64le = ""
+JIT:powerpc64 = ""
+
+#DEPENDS += "libb64 lua${JIT} zlib c-ares grpc-native grpc curl ncurses jsoncpp \
+#            tbb jq openssl elfutils protobuf protobuf-native jq-native valijson"
+RDEPENDS:${PN} = "bash"
+
+SRC_URI = "git://github.com/draios/sysdig.git;branch=dev;protocol=https;name=sysdig \
+           git://github.com/falcosecurity/libs;protocol=https;branch=master;name=falco;subdir=git/falcosecurity-libs \
+           file://0055-Add-cstdint-for-uintXX_t-types.patch;patchdir=./falcosecurity-libs \
+           file://0099-cmake-Pass-PROBE_NAME-via-CFLAGS.patch \
+           "
+SRCREV_sysdig = "4fb6288275f567f63515df0ff0a6518043ecfa9b"
+SRCREV_falco= "caa0e4d0044fdaaebab086592a97f0c7f32aeaa9"
+
+SRCREV_FORMAT = "sysdig_falco"
+
+S = "${WORKDIR}/git"
+
+EXTRA_OECMAKE = "\
+                -DBUILD_DRIVER=OFF \
+                -DMINIMAL_BUILD=ON \
+                -DUSE_BUNDLED_DEPS=OFF \
+                -DCREATE_TEST_TARGETS=OFF \
+                -DDIR_ETC=${sysconfdir} \
+                -DLUA_INCLUDE_DIR=${STAGING_INCDIR}/luajit-2.1 \
+                -DFALCOSECURITY_LIBS_SOURCE_DIR=${S}/falcosecurity-libs \
+                -DVALIJSON_INCLUDE=${STAGING_INCDIR}/valijson \
+"
+
+#CMAKE_VERBOSE = "VERBOSE=1"
+
+FILES:${PN} += " \
+    ${DIR_ETC}/* \
+    ${datadir}/zsh/* \
+    ${prefix}/src/*  \
+"
+# Use getaddrinfo_a is a GNU extension in libsinsp
+# It should be fixed in sysdig, until then disable
+# on musl
+# Something like this https://code.videolan.org/ePirat/vlc/-/commit/01fd9fe4c7f6c5558f7345f38abf0152e17853ab  is needed to fix it
+COMPATIBLE_HOST:libc-musl = "null"
+COMPATIBLE_HOST:mips = "null"
+COMPATIBLE_HOST:riscv64 = "null"
+COMPATIBLE_HOST:riscv32 = "null"
+COMPATIBLE_HOST:powerpc = "null"
+COMPATIBLE_HOST:powerpc64le = "null"
diff --git a/meta/lib/oe/patch.py b/meta/lib/oe/patch.py
index d5ad4f3dc1..70cdb1d9c0 100644
--- a/meta/lib/oe/patch.py
+++ b/meta/lib/oe/patch.py
@@ -474,9 +474,9 @@ class GitApplyTree(PatchTree):
                 out = runcmd(["sh", "-c", " ".join(shellcmd)], os.path.join(tree, name))
                 if out:
                     for srcfile in out.split():
+                        outfile = os.path.basename(srcfile)
                         for encoding in ['utf-8', 'latin-1']:
                             patchlines = []
-                            outfile = None
                             try:
                                 with open(srcfile, 'r', encoding=encoding, newline='') as f:
                                     for line in f:
@@ -484,7 +484,8 @@ class GitApplyTree(PatchTree):
                                             outfile = line.split()[-1].strip()
                                             continue
                                         if line.startswith(GitApplyTree.ignore_commit_prefix):
-                                            continue
+                                            outfile = None
+                                            break
                                         patchlines.append(line)
                             except UnicodeDecodeError:
                                 continue
@@ -492,12 +493,11 @@ class GitApplyTree(PatchTree):
                         else:
                             raise PatchError('Unable to find a character encoding to decode %s' % srcfile)
 
-                        if not outfile:
-                            outfile = os.path.basename(srcfile)
-                        bb.utils.mkdirhier(os.path.join(outdir, name))
-                        with open(os.path.join(outdir, name, outfile), 'w') as of:
-                            for line in patchlines:
-                                of.write(line)
+                        if outfile:
+                            bb.utils.mkdirhier(os.path.join(outdir, name))
+                            with open(os.path.join(outdir, name, outfile), 'w') as of:
+                                for line in patchlines:
+                                    of.write(line)
         finally:
             shutil.rmtree(tempdir)
 
diff --git a/meta/lib/oeqa/selftest/cases/devtool.py b/meta/lib/oeqa/selftest/cases/devtool.py
index dd3a5e9b5c..abc85c90a7 100644
--- a/meta/lib/oeqa/selftest/cases/devtool.py
+++ b/meta/lib/oeqa/selftest/cases/devtool.py
@@ -2227,6 +2227,52 @@ class DevtoolUpgradeTests(DevtoolBase):
         if files:
             self.fail('Unexpected file(s) copied next to bbappend: %s' % ', '.join(files))
 
+    def test_devtool_finish_update_patch(self):
+        # This test uses a modified version of the sysdig recipe from meta-oe.
+        # - The patches have been renamed.
+        # - The dependencies are commented out since the recipe is not being
+        #   built.
+        #
+        # The sysdig recipe is interesting in that it fetches two different Git
+        # repositories, and there are patches for both. This leads to that
+        # devtool will create ignore commits as it uses Git submodules to keep
+        # track of the second repository.
+        #
+        # This test will verify that the ignored commits actually are ignored
+        # when a commit in between is modified. It will also verify that the
+        # updated patch keeps its original name.
+
+        # Check preconditions
+        self.assertTrue(not os.path.exists(self.workspacedir), 'This test cannot be run with a workspace directory under the build directory')
+        # Try modifying a recipe
+        self.track_for_cleanup(self.workspacedir)
+        recipe = 'sysdig-selftest'
+        recipefile = get_bb_var('FILE', recipe)
+        recipedir = os.path.dirname(recipefile)
+        result = runCmd('git status --porcelain .', cwd=recipedir)
+        if result.output.strip():
+            self.fail('Recipe directory for %s contains uncommitted changes' % recipe)
+        tempdir = tempfile.mkdtemp(prefix='devtoolqa')
+        self.track_for_cleanup(tempdir)
+        self.add_command_to_tearDown('bitbake-layers remove-layer */workspace')
+        result = runCmd('devtool modify %s %s' % (recipe, tempdir))
+        self.add_command_to_tearDown('cd %s; rm %s/*; git checkout %s %s' % (recipedir, recipe, recipe, os.path.basename(recipefile)))
+        self.assertExists(os.path.join(tempdir, 'CMakeLists.txt'), 'Extracted source could not be found')
+        # Make a change to one of the existing commits
+        result = runCmd('echo "# A comment " >> CMakeLists.txt', cwd=tempdir)
+        result = runCmd('git status --porcelain', cwd=tempdir)
+        self.assertIn('M CMakeLists.txt', result.output)
+        result = runCmd('git commit --fixup HEAD^ CMakeLists.txt', cwd=tempdir)
+        result = runCmd('git show -s --format=%s', cwd=tempdir)
+        self.assertIn('fixup! cmake: Pass PROBE_NAME via CFLAGS', result.output)
+        result = runCmd('GIT_SEQUENCE_EDITOR=true git rebase -i --autosquash devtool-base', cwd=tempdir)
+        result = runCmd('devtool finish %s meta-selftest' % recipe)
+        result = runCmd('devtool status')
+        self.assertNotIn(recipe, result.output, 'Recipe should have been reset by finish but wasn\'t')
+        self.assertNotExists(os.path.join(self.workspacedir, 'recipes', recipe), 'Recipe directory should not exist after finish')
+        expected_status = [(' M', '.*/0099-cmake-Pass-PROBE_NAME-via-CFLAGS.patch$')]
+        self._check_repo_status(recipedir, expected_status)
+
     def test_devtool_rename(self):
         # Check preconditions
         self.assertTrue(not os.path.exists(self.workspacedir), 'This test cannot be run with a workspace directory under the build directory')

[PATCH 4/7] lib/oe/patch: Add GitApplyTree.commitIgnored()

[Peter Kjellerstedt]

This function can be used to create a commit that devtool will ignore
when creating/updating the patches.

Signed-off-by: Peter Kjellerstedt <peter.kjellerstedt@axis.com>
---
 meta/lib/oe/patch.py | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/meta/lib/oe/patch.py b/meta/lib/oe/patch.py
index 70cdb1d9c0..3ded5f3601 100644
--- a/meta/lib/oe/patch.py
+++ b/meta/lib/oe/patch.py
@@ -460,6 +460,16 @@ class GitApplyTree(PatchTree):
             cmd.append('--date="%s"' % date)
         return (tmpfile, cmd)
 
+    @staticmethod
+    def commitIgnored(subject, dir=None, files=None, d=None):
+        if files:
+            runcmd(['git', 'add'] + files, dir)
+        message = "%s\n\n%s" % (subject, GitApplyTree.ignore_commit_prefix)
+        cmd = ["git"]
+        GitApplyTree.gitCommandUserOptions(cmd, d=d)
+        cmd += ["commit", "-m", message, "--no-verify"]
+        runcmd(cmd, dir)
+
     @staticmethod
     def extractPatches(tree, startcommits, outdir, paths=None):
         import tempfile

[PATCH 5/7] devtool: Make use of oe.patch.GitApplyTree.commitIgnored()

[Peter Kjellerstedt]

This makes use of the oe.patch.GitApplyTree.commitIgnored() function to
create commits that shall be ignored by `devtool finish`.

Signed-off-by: Peter Kjellerstedt <peter.kjellerstedt@axis.com>
---
 scripts/lib/devtool/__init__.py | 4 +---
 scripts/lib/devtool/standard.py | 6 +-----
 2 files changed, 2 insertions(+), 8 deletions(-)

diff --git a/scripts/lib/devtool/__init__.py b/scripts/lib/devtool/__init__.py
index c7fc3cfc41..6133c1c5b4 100644
--- a/scripts/lib/devtool/__init__.py
+++ b/scripts/lib/devtool/__init__.py
@@ -253,9 +253,7 @@ def setup_git_repo(repodir, version, devbranch, basetag='devtool-base', d=None):
                     bb.process.run('git submodule add %s %s' % (remote_url, os.path.relpath(root, os.path.join(root, ".."))), cwd=os.path.join(root, ".."))
                     found = True
                 if found:
-                    useroptions = []
-                    oe.patch.GitApplyTree.gitCommandUserOptions(useroptions, d=d)
-                    bb.process.run('git %s commit -m "Adding additionnal submodule from SRC_URI\n\n%s"' % (' '.join(useroptions), oe.patch.GitApplyTree.ignore_commit_prefix), cwd=os.path.join(root, ".."))
+                    oe.patch.GitApplyTree.commitIgnored("Add additional submodule from SRC_URI", dir=os.path.join(root, ".."), d=d)
                     found = False
     if os.path.exists(os.path.join(repodir, '.gitmodules')):
         bb.process.run('git submodule foreach --recursive  "git tag -f %s"' % basetag, cwd=repodir)
diff --git a/scripts/lib/devtool/standard.py b/scripts/lib/devtool/standard.py
index ccb7ea851b..6d7fd17fbd 100644
--- a/scripts/lib/devtool/standard.py
+++ b/scripts/lib/devtool/standard.py
@@ -484,11 +484,7 @@ def symlink_oelocal_files_srctree(rd, srctree):
                     os.symlink('oe-local-files/%s' % fn, destpth)
                 addfiles.append(os.path.join(relpth, fn))
         if addfiles:
-            bb.process.run('git add %s' % ' '.join(addfiles), cwd=srctree)
-            useroptions = []
-            oe.patch.GitApplyTree.gitCommandUserOptions(useroptions, d=rd)
-            bb.process.run('git %s commit -m "Committing local file symlinks\n\n%s"' % (' '.join(useroptions), oe.patch.GitApplyTree.ignore_commit_prefix), cwd=srctree)
-
+            oe.patch.GitApplyTree.commitIgnored("Add local file symlinks", dir=srctree, files=addfiles, d=rd)
 
 def _extract_source(srctree, keep_temp, devbranch, sync, config, basepath, workspace, fixed_setup, d, tinfoil, no_overrides=False):
     """Extract sources of a recipe"""

[PATCH 6/7] patch.bbclass: Make use of oe.patch.GitApplyTree.commitIgnored()

[Peter Kjellerstedt]

This makes use of the oe.patch.GitApplyTree.commitIgnored() function to
create commits that shall be ignored by `devtool finish`.

Signed-off-by: Peter Kjellerstedt <peter.kjellerstedt@axis.com>
---
 meta/classes-global/patch.bbclass | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/meta/classes-global/patch.bbclass b/meta/classes-global/patch.bbclass
index e3157c7b18..e5786b1c9a 100644
--- a/meta/classes-global/patch.bbclass
+++ b/meta/classes-global/patch.bbclass
@@ -79,9 +79,7 @@ python patch_task_postfunc() {
                         bb.process.run('git checkout patches', cwd=srcsubdir)
         stdout, _ = bb.process.run('git status --porcelain .', cwd=srcsubdir)
         if stdout:
-            useroptions = []
-            oe.patch.GitApplyTree.gitCommandUserOptions(useroptions, d=d)
-            bb.process.run('git add .; git %s commit -a -m "Committing changes from %s\n\n%s"' % (' '.join(useroptions), func, oe.patch.GitApplyTree.ignore_commit_prefix + ' - from %s' % func), cwd=srcsubdir)
+            oe.patch.GitApplyTree.commitIgnored("Add changes from %s" % func, dir=srcsubdir, files=['.'], d=d)
 }
 
 def src_patches(d, all=False, expand=True):

[PATCH 7/7] lib/oe/patch: Use git notes to store the filenames for the patches

[Peter Kjellerstedt]

The old way of keeping track of the filenames for the patches that
correspond to the commits was to add a special comment line to the end
of the commit message, e.g., "%% original patch: <filename>", using a
temporary git hook. This method had some drawbacks, e.g.:

* It caused problems if one wanted to push the commits upstream as the
  comment line had to be manually removed.
* The comment line would end up in patches if someone used git
  format-path rather than devtool finish to generate the patches.
* The comment line could interfere with global Git hooks used to
  validate the format of the Git commit message.
* When regenerating patches with `devtool finish --force-patch-refresh`,
  the process typically resulted in adding empty lines to the end of the
  commit messages in the updated patches.

A better way of keeping track of the patch filenames is to use Git
notes. This way the commit messages remain unaffected, but the
information is still shown when, e.g., doing `git log`. A special Git
notes space, refs/notes/devtool, is used to not intefere with the
default Git notes. It is configured to be shown in, e.g., `git log` and
to survive rewrites (i.e., `git commit --amend` and `git rebase`).

Since there is no longer any need for a temporary Git hook, the code
that manipulated the .git/hooks directory has also been removed. To
avoid potential problems due to global Git hooks, --no-verify was added
to the `git commit` command.

To not cause troubles for those who have done `devtool modify` for a
recipe with the old solution and then do `devtool finish` with the new
solution, the code will fall back to look for the old strings in the
commit message if no Git note can be found.

While not technically motivated like above, the way to keep track of
ignored commits is also changed to use Git notes to avoid having
different methods to store similar information.

Signed-off-by: Peter Kjellerstedt <peter.kjellerstedt@axis.com>
---
 meta/lib/oe/patch.py                    | 105 +++++++++++++++---------
 meta/lib/oeqa/selftest/cases/devtool.py |   9 +-
 scripts/lib/devtool/standard.py         |  15 ++--
 3 files changed, 78 insertions(+), 51 deletions(-)

diff --git a/meta/lib/oe/patch.py b/meta/lib/oe/patch.py
index 3ded5f3601..4e971f8b30 100644
--- a/meta/lib/oe/patch.py
+++ b/meta/lib/oe/patch.py
@@ -294,8 +294,9 @@ class PatchTree(PatchSet):
         self.Pop(all=True)
 
 class GitApplyTree(PatchTree):
-    patch_line_prefix = '%% original patch'
-    ignore_commit_prefix = '%% ignore'
+    notes_ref = "refs/notes/devtool"
+    original_patch = 'original patch'
+    ignore_commit = 'ignore'
 
     def __init__(self, dir, d):
         PatchTree.__init__(self, dir, d)
@@ -452,7 +453,7 @@ class GitApplyTree(PatchTree):
         # Prepare git command
         cmd = ["git"]
         GitApplyTree.gitCommandUserOptions(cmd, commituser, commitemail)
-        cmd += ["commit", "-F", tmpfile]
+        cmd += ["commit", "-F", tmpfile, "--no-verify"]
         # git doesn't like plain email addresses as authors
         if author and '<' in author:
             cmd.append('--author="%s"' % author)
@@ -460,15 +461,49 @@ class GitApplyTree(PatchTree):
             cmd.append('--date="%s"' % date)
         return (tmpfile, cmd)
 
+    @staticmethod
+    def addNote(repo, ref, key, value=None):
+        note = key + (": %s" % value if value else "")
+        notes_ref = GitApplyTree.notes_ref
+        cmd = ["git", "config", "--fixed-value", "notes.displayRef", notes_ref, notes_ref]
+        runcmd(cmd, repo)
+        cmd = ["git", "config", "--fixed-value", "notes.rewriteRef", notes_ref, notes_ref]
+        runcmd(cmd, repo)
+        cmd = ["git", "notes", "--ref", notes_ref, "append", "-m", note, ref]
+        runcmd(cmd, repo)
+
+    @staticmethod
+    def getNotes(repo, ref):
+        import re
+
+        note = None
+        cmd = ["git", "notes", "--ref", GitApplyTree.notes_ref, "show", ref]
+        try:
+            note = runcmd(cmd, repo)
+            prefix = ""
+        except CmdError:
+            cmd = ['git', 'show', '-s', '--format=%B', ref]
+            note = runcmd(cmd, repo)
+            prefix = "%% "
+
+        note_re = re.compile(r'^%s(.*?)(?::\s*(.*))?$' % prefix)
+        notes = dict()
+        for line in note.splitlines():
+            m = note_re.match(line)
+            if m:
+                notes[m.group(1)] = m.group(2)
+
+        return notes
+
     @staticmethod
     def commitIgnored(subject, dir=None, files=None, d=None):
         if files:
             runcmd(['git', 'add'] + files, dir)
-        message = "%s\n\n%s" % (subject, GitApplyTree.ignore_commit_prefix)
         cmd = ["git"]
         GitApplyTree.gitCommandUserOptions(cmd, d=d)
-        cmd += ["commit", "-m", message, "--no-verify"]
+        cmd += ["commit", "-m", subject, "--no-verify"]
         runcmd(cmd, dir)
+        GitApplyTree.addNote(dir, "HEAD", GitApplyTree.ignore_commit)
 
     @staticmethod
     def extractPatches(tree, startcommits, outdir, paths=None):
@@ -484,18 +519,20 @@ class GitApplyTree(PatchTree):
                 out = runcmd(["sh", "-c", " ".join(shellcmd)], os.path.join(tree, name))
                 if out:
                     for srcfile in out.split():
-                        outfile = os.path.basename(srcfile)
+                        # This loop, which is used to remove any line that
+                        # starts with "%% original patch", is kept for backwards
+                        # compatibility. If/when that compatibility is dropped,
+                        # it can be replaced with code to just read the first
+                        # line of the patch file to get the SHA-1, and the code
+                        # below that writes the modified patch file can be
+                        # replaced with a simple file move.
                         for encoding in ['utf-8', 'latin-1']:
                             patchlines = []
                             try:
                                 with open(srcfile, 'r', encoding=encoding, newline='') as f:
                                     for line in f:
-                                        if line.startswith(GitApplyTree.patch_line_prefix):
-                                            outfile = line.split()[-1].strip()
+                                        if line.startswith("%% " + GitApplyTree.original_patch):
                                             continue
-                                        if line.startswith(GitApplyTree.ignore_commit_prefix):
-                                            outfile = None
-                                            break
                                         patchlines.append(line)
                             except UnicodeDecodeError:
                                 continue
@@ -503,11 +540,16 @@ class GitApplyTree(PatchTree):
                         else:
                             raise PatchError('Unable to find a character encoding to decode %s' % srcfile)
 
-                        if outfile:
-                            bb.utils.mkdirhier(os.path.join(outdir, name))
-                            with open(os.path.join(outdir, name, outfile), 'w') as of:
-                                for line in patchlines:
-                                    of.write(line)
+                        sha1 = patchlines[0].split()[1]
+                        notes = GitApplyTree.getNotes(os.path.join(tree, name), sha1)
+                        if GitApplyTree.ignore_commit in notes:
+                            continue
+                        outfile = notes.get(GitApplyTree.original_patch, os.path.basename(srcfile))
+
+                        bb.utils.mkdirhier(os.path.join(outdir, name))
+                        with open(os.path.join(outdir, name, outfile), 'w') as of:
+                            for line in patchlines:
+                                of.write(line)
         finally:
             shutil.rmtree(tempdir)
 
@@ -555,28 +597,11 @@ class GitApplyTree(PatchTree):
 
             return runcmd(["sh", "-c", " ".join(shellcmd)], self.dir)
 
-        # Add hooks which add a pointer to the original patch file name in the commit message
         reporoot = (runcmd("git rev-parse --show-toplevel".split(), self.dir) or '').strip()
         if not reporoot:
             raise Exception("Cannot get repository root for directory %s" % self.dir)
-        gitdir = (runcmd("git rev-parse --absolute-git-dir".split(), self.dir) or '').strip()
-        if not gitdir:
-            raise Exception("Cannot get gitdir for directory %s" % self.dir)
-        hooks_dir = os.path.join(gitdir, 'hooks')
-        hooks_dir_backup = hooks_dir + '.devtool-orig'
-        if os.path.lexists(hooks_dir_backup):
-            raise Exception("Git hooks backup directory already exists: %s" % hooks_dir_backup)
-        if os.path.lexists(hooks_dir):
-            shutil.move(hooks_dir, hooks_dir_backup)
-        os.mkdir(hooks_dir)
-        commithook = os.path.join(hooks_dir, 'commit-msg')
-        applyhook = os.path.join(hooks_dir, 'applypatch-msg')
-        with open(commithook, 'w') as f:
-            # NOTE: the formatting here is significant; if you change it you'll also need to
-            # change other places which read it back
-            f.write('echo "\n%s: $PATCHFILE" >> $1' % GitApplyTree.patch_line_prefix)
-        os.chmod(commithook, 0o755)
-        shutil.copy2(commithook, applyhook)
+
+        patch_applied = True
         try:
             patchfilevar = 'PATCHFILE="%s"' % os.path.basename(patch['file'])
             if self._need_dirty_check():
@@ -587,7 +612,7 @@ class GitApplyTree(PatchTree):
                     pass
                 else:
                     if output:
-                        # The tree is dirty, not need to try to apply patches with git anymore
+                        # The tree is dirty, no need to try to apply patches with git anymore
                         # since they fail, fallback directly to patch
                         output = PatchTree._applypatch(self, patch, force, reverse, run)
                         output += self._commitpatch(patch, patchfilevar)
@@ -620,10 +645,12 @@ class GitApplyTree(PatchTree):
                     output = PatchTree._applypatch(self, patch, force, reverse, run)
                 output += self._commitpatch(patch, patchfilevar)
                 return output
+        except:
+            patch_applied = False
+            raise
         finally:
-            shutil.rmtree(hooks_dir)
-            if os.path.lexists(hooks_dir_backup):
-                shutil.move(hooks_dir_backup, hooks_dir)
+            if patch_applied:
+                GitApplyTree.addNote(self.dir, "HEAD", GitApplyTree.original_patch, os.path.basename(patch['file']))
 
 
 class QuiltTree(PatchSet):
diff --git a/meta/lib/oeqa/selftest/cases/devtool.py b/meta/lib/oeqa/selftest/cases/devtool.py
index abc85c90a7..a94f4a7148 100644
--- a/meta/lib/oeqa/selftest/cases/devtool.py
+++ b/meta/lib/oeqa/selftest/cases/devtool.py
@@ -988,9 +988,10 @@ class DevtoolModifyTests(DevtoolBase):
         self.assertIn(tempdir, result.output)
         # Check git repo
         self._check_src_repo(tempdir)
-        # Check that the patch is correctly applied
-        # last commit message in the tree must contain
-        # %% original patch: <patchname>
+        # Check that the patch is correctly applied.
+        # The last commit message in the tree must contain the following note:
+        # Notes (devtool):
+        #     original patch: <patchname>
         # ..
         patchname = None
         for uri in src_uri:
@@ -998,7 +999,7 @@ class DevtoolModifyTests(DevtoolBase):
                 patchname = uri.replace("file://", "").partition('.patch')[0] + '.patch'
         self.assertIsNotNone(patchname)
         result = runCmd('git -C %s log -1' % tempdir)
-        self.assertIn("%%%% original patch: %s" % patchname, result.output)
+        self.assertIn("Notes (devtool):\n    original patch: %s" % patchname, result.output)
 
         # Configure the recipe to check that the git dependencies are correctly patched in cargo config
         bitbake('-c configure %s' % testrecipe)
diff --git a/scripts/lib/devtool/standard.py b/scripts/lib/devtool/standard.py
index 6d7fd17fbd..7972b4f822 100644
--- a/scripts/lib/devtool/standard.py
+++ b/scripts/lib/devtool/standard.py
@@ -937,14 +937,13 @@ def modify(args, config, basepath, workspace):
             seen_patches = []
             for branch in branches:
                 branch_patches[branch] = []
-                (stdout, _) = bb.process.run('git log devtool-base..%s' % branch, cwd=srctree)
-                for line in stdout.splitlines():
-                    line = line.strip()
-                    if line.startswith(oe.patch.GitApplyTree.patch_line_prefix):
-                        origpatch = line[len(oe.patch.GitApplyTree.patch_line_prefix):].split(':', 1)[-1].strip()
-                        if not origpatch in seen_patches:
-                            seen_patches.append(origpatch)
-                            branch_patches[branch].append(origpatch)
+                (stdout, _) = bb.process.run('git rev-list devtool-base..%s' % branch, cwd=srctree)
+                for sha1 in stdout.splitlines():
+                    notes = oe.patch.GitApplyTree.getNotes(srctree, sha1.strip())
+                    origpatch = notes.get(oe.patch.GitApplyTree.original_patch)
+                    if origpatch and origpatch not in seen_patches:
+                        seen_patches.append(origpatch)
+                        branch_patches[branch].append(origpatch)
 
         # Need to grab this here in case the source is within a subdirectory
         srctreebase = srctree

Patchtest results for [PATCH 6/7] patch.bbclass: Make use of oe.patch.GitApplyTree.commitIgnored()

[patchtest]

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

Thank you for your submission. Patchtest identified one
or more issues with the patch. Please see the log below for
more information:

---
Testing patch /home/patchtest/share/mboxes/6-7-patch.bbclass-Make-use-of-oe.patch.GitApplyTree.commitIgnored.patch

FAIL: test max line length: Patch line too long (current length 208, maximum is 200) (test_metadata.TestMetadata.test_max_line_length)

PASS: test Signed-off-by presence (test_mbox.TestMbox.test_signed_off_by_presence)
PASS: test author valid (test_mbox.TestMbox.test_author_valid)
PASS: test commit message presence (test_mbox.TestMbox.test_commit_message_presence)
PASS: test mbox format (test_mbox.TestMbox.test_mbox_format)
PASS: test non-AUH upgrade (test_mbox.TestMbox.test_non_auh_upgrade)
PASS: test shortlog format (test_mbox.TestMbox.test_shortlog_format)
PASS: test shortlog length (test_mbox.TestMbox.test_shortlog_length)

SKIP: pretest pylint: No python related patches, skipping test (test_python_pylint.PyLint.pretest_pylint)
SKIP: pretest src uri left files: No modified recipes, skipping pretest (test_metadata.TestMetadata.pretest_src_uri_left_files)
SKIP: test CVE check ignore: No modified recipes, skipping test (test_metadata.TestMetadata.test_cve_check_ignore)
SKIP: test CVE tag format: No new CVE patches introduced (test_patch.TestPatch.test_cve_tag_format)
SKIP: test Signed-off-by presence: No new CVE patches introduced (test_patch.TestPatch.test_signed_off_by_presence)
SKIP: test Upstream-Status presence: No new CVE patches introduced (test_patch.TestPatch.test_upstream_status_presence_format)
SKIP: test bugzilla entry format: No bug ID found (test_mbox.TestMbox.test_bugzilla_entry_format)
SKIP: test lic files chksum modified not mentioned: No modified recipes, skipping test (test_metadata.TestMetadata.test_lic_files_chksum_modified_not_mentioned)
SKIP: test lic files chksum presence: No added recipes, skipping test (test_metadata.TestMetadata.test_lic_files_chksum_presence)
SKIP: test license presence: No added recipes, skipping test (test_metadata.TestMetadata.test_license_presence)
SKIP: test pylint: No python related patches, skipping test (test_python_pylint.PyLint.test_pylint)
SKIP: test series merge on head: Merge test is disabled for now (test_mbox.TestMbox.test_series_merge_on_head)
SKIP: test src uri left files: No modified recipes, skipping pretest (test_metadata.TestMetadata.test_src_uri_left_files)
SKIP: test summary presence: No added recipes, skipping test (test_metadata.TestMetadata.test_summary_presence)
SKIP: test target mailing list: Series merged, no reason to check other mailing lists (test_mbox.TestMbox.test_target_mailing_list)

---

Please address the issues identified and
submit a new revision of the patch, or alternatively, reply to this
email with an explanation of why the patch should be accepted. If you
believe these results are due to an error in patchtest, please submit a
bug at https://bugzilla.yoctoproject.org/ (use the 'Patchtest' category
under 'Yocto Project Subprojects'). For more information on specific
failures, see: https://wiki.yoctoproject.org/wiki/Patchtest. Thank
you!

Re: [OE-core] [PATCH 3/7] lib/oe/patch: Make extractPatches() not extract ignored commits

[Richard Purdie]

On Fri, 2024-02-16 at 19:59 +0100, Peter Kjellerstedt wrote:
> If a commit is marked with "%% ignore" it means it is used by devtool to
> keep track of changes to the source code that are not the result of
> running do_patch(). These changes need to actually be ignored when
> extracting the patches as they typically make no sense as actual patches
> in a recipe.
> 
> This also adds a new test for oe-selftest that verifies that there are
> no patches generated from ignored commits.
> 
> Signed-off-by: Peter Kjellerstedt <peter.kjellerstedt@axis.com>
[...]

> diff --git a/meta-selftest/recipes-extended/sysdig/sysdig-selftest_0.28.0.bb b/meta-selftest/recipes-extended/sysdig/sysdig-selftest_0.28.0.bb
> new file mode 100644
> index 0000000000..e114f465a0
> --- /dev/null
> +++ b/meta-selftest/recipes-extended/sysdig/sysdig-selftest_0.28.0.bb
> @@ -0,0 +1,64 @@
> +SUMMARY = "A New System Troubleshooting Tool Built for the Way You Work"
> +DESCRIPTION = "Sysdig is open source, system-level exploration: capture \
> +system state and activity from a running Linux instance, then save, \
> +filter and analyze."
> +HOMEPAGE = "http://www.sysdig.org/"
> +LICENSE = "Apache-2.0 & (MIT | GPL-2.0-only)"
> +LIC_FILES_CHKSUM = "file://COPYING;md5=f8fee3d59797546cffab04f3b88b2d44"
> +
> +inherit cmake pkgconfig
> +
> +#OECMAKE_GENERATOR = "Unix Makefiles"
> +JIT ?= "jit"
> +JIT:mipsarchn32 = ""
> +JIT:mipsarchn64 = ""
> +JIT:riscv64 = ""
> +JIT:riscv32 = ""
> +JIT:powerpc = ""
> +JIT:powerpc64le = ""
> +JIT:powerpc64 = ""
> +
> +#DEPENDS += "libb64 lua${JIT} zlib c-ares grpc-native grpc curl ncurses jsoncpp \
> +#            tbb jq openssl elfutils protobuf protobuf-native jq-native valijson"
> +RDEPENDS:${PN} = "bash"
> +
> +SRC_URI = "git://github.com/draios/sysdig.git;branch=dev;protocol=https;name=sysdig \
> +           git://github.com/falcosecurity/libs;protocol=https;branch=master;name=falco;subdir=git/falcosecurity-libs \
> +           file://0055-Add-cstdint-for-uintXX_t-types.patch;patchdir=./falcosecurity-libs \
> +           file://0099-cmake-Pass-PROBE_NAME-via-CFLAGS.patch \
> +           "
> +SRCREV_sysdig = "4fb6288275f567f63515df0ff0a6518043ecfa9b"
> +SRCREV_falco= "caa0e4d0044fdaaebab086592a97f0c7f32aeaa9"
> +
> +SRCREV_FORMAT = "sysdig_falco"
> +
> +S = "${WORKDIR}/git"
> +
> +EXTRA_OECMAKE = "\
> +                -DBUILD_DRIVER=OFF \
> +                -DMINIMAL_BUILD=ON \
> +                -DUSE_BUNDLED_DEPS=OFF \
> +                -DCREATE_TEST_TARGETS=OFF \
> +                -DDIR_ETC=${sysconfdir} \
> +                -DLUA_INCLUDE_DIR=${STAGING_INCDIR}/luajit-2.1 \
> +                -DFALCOSECURITY_LIBS_SOURCE_DIR=${S}/falcosecurity-libs \
> +                -DVALIJSON_INCLUDE=${STAGING_INCDIR}/valijson \
> +"
> +
> +#CMAKE_VERBOSE = "VERBOSE=1"
> +
> +FILES:${PN} += " \
> +    ${DIR_ETC}/* \
> +    ${datadir}/zsh/* \
> +    ${prefix}/src/*  \
> +"
> +# Use getaddrinfo_a is a GNU extension in libsinsp
> +# It should be fixed in sysdig, until then disable
> +# on musl
> +# Something like this https://code.videolan.org/ePirat/vlc/-/commit/01fd9fe4c7f6c5558f7345f38abf0152e17853ab  is needed to fix it
> +COMPATIBLE_HOST:libc-musl = "null"
> +COMPATIBLE_HOST:mips = "null"
> +COMPATIBLE_HOST:riscv64 = "null"
> +COMPATIBLE_HOST:riscv32 = "null"
> +COMPATIBLE_HOST:powerpc = "null"
> +COMPATIBLE_HOST:powerpc64le = "null"


This needs an EXCLUDE_FROM_WORLD = "1" else we see:

https://autobuilder.yoctoproject.org/typhoon/#/builders/117/builds/4386/steps/13/logs/stdio

It took me far too long to realise what was going wrong and where that recipe came from :/

Cheers,

Richard

Re: [OE-core] [PATCH 7/7] lib/oe/patch: Use git notes to store the filenames for the patches

[Richard Purdie]

On Fri, 2024-02-16 at 19:59 +0100, Peter Kjellerstedt wrote:
> The old way of keeping track of the filenames for the patches that
> correspond to the commits was to add a special comment line to the end
> of the commit message, e.g., "%% original patch: <filename>", using a
> temporary git hook. This method had some drawbacks, e.g.:
> 
> * It caused problems if one wanted to push the commits upstream as the
>   comment line had to be manually removed.
> * The comment line would end up in patches if someone used git
>   format-path rather than devtool finish to generate the patches.
> * The comment line could interfere with global Git hooks used to
>   validate the format of the Git commit message.
> * When regenerating patches with `devtool finish --force-patch-refresh`,
>   the process typically resulted in adding empty lines to the end of the
>   commit messages in the updated patches.
> 
> A better way of keeping track of the patch filenames is to use Git
> notes. This way the commit messages remain unaffected, but the
> information is still shown when, e.g., doing `git log`. A special Git
> notes space, refs/notes/devtool, is used to not intefere with the
> default Git notes. It is configured to be shown in, e.g., `git log` and
> to survive rewrites (i.e., `git commit --amend` and `git rebase`).
> 
> Since there is no longer any need for a temporary Git hook, the code
> that manipulated the .git/hooks directory has also been removed. To
> avoid potential problems due to global Git hooks, --no-verify was added
> to the `git commit` command.
> 
> To not cause troubles for those who have done `devtool modify` for a
> recipe with the old solution and then do `devtool finish` with the new
> solution, the code will fall back to look for the old strings in the
> commit message if no Git note can be found.
> 
> While not technically motivated like above, the way to keep track of
> ignored commits is also changed to use Git notes to avoid having
> different methods to store similar information.
> 
> Signed-off-by: Peter Kjellerstedt <peter.kjellerstedt@axis.com>
> ---
>  meta/lib/oe/patch.py                    | 105 +++++++++++++++---------
>  meta/lib/oeqa/selftest/cases/devtool.py |   9 +-
>  scripts/lib/devtool/standard.py         |  15 ++--
>  3 files changed, 78 insertions(+), 51 deletions(-)

I've tweaked the EXCLUDE_FROM_WORLD issue and I can squash that in but
with that we still saw:

https://autobuilder.yoctoproject.org/typhoon/#/builders/87/builds/6433/steps/14/logs/stdio

which I think is from this patch.

Cheers,

Richard

RE: [OE-core] [PATCH 7/7] lib/oe/patch: Use git notes to store the filenames for the patches

[Peter Kjellerstedt]

> -----Original Message-----
> From: Richard Purdie <richard.purdie@linuxfoundation.org>
> Sent: den 17 februari 2024 13:34
> To: Peter Kjellerstedt <peter.kjellerstedt@axis.com>; openembedded-
> core@lists.openembedded.org
> Subject: Re: [OE-core] [PATCH 7/7] lib/oe/patch: Use git notes to store
> the filenames for the patches
> 
> On Fri, 2024-02-16 at 19:59 +0100, Peter Kjellerstedt wrote:
> > The old way of keeping track of the filenames for the patches that
> > correspond to the commits was to add a special comment line to the end
> > of the commit message, e.g., "%% original patch: <filename>", using a
> > temporary git hook. This method had some drawbacks, e.g.:
> >
> > * It caused problems if one wanted to push the commits upstream as the
> >   comment line had to be manually removed.
> > * The comment line would end up in patches if someone used git
> >   format-path rather than devtool finish to generate the patches.
> > * The comment line could interfere with global Git hooks used to
> >   validate the format of the Git commit message.
> > * When regenerating patches with `devtool finish --force-patch-refresh`,
> >   the process typically resulted in adding empty lines to the end of the
> >   commit messages in the updated patches.
> >
> > A better way of keeping track of the patch filenames is to use Git
> > notes. This way the commit messages remain unaffected, but the
> > information is still shown when, e.g., doing `git log`. A special Git
> > notes space, refs/notes/devtool, is used to not intefere with the
> > default Git notes. It is configured to be shown in, e.g., `git log` and
> > to survive rewrites (i.e., `git commit --amend` and `git rebase`).
> >
> > Since there is no longer any need for a temporary Git hook, the code
> > that manipulated the .git/hooks directory has also been removed. To
> > avoid potential problems due to global Git hooks, --no-verify was added
> > to the `git commit` command.
> >
> > To not cause troubles for those who have done `devtool modify` for a
> > recipe with the old solution and then do `devtool finish` with the new
> > solution, the code will fall back to look for the old strings in the
> > commit message if no Git note can be found.
> >
> > While not technically motivated like above, the way to keep track of
> > ignored commits is also changed to use Git notes to avoid having
> > different methods to store similar information.
> >
> > Signed-off-by: Peter Kjellerstedt <peter.kjellerstedt@axis.com>
> > ---
> >  meta/lib/oe/patch.py                    | 105 +++++++++++++++---------
> >  meta/lib/oeqa/selftest/cases/devtool.py |   9 +-
> >  scripts/lib/devtool/standard.py         |  15 ++--
> >  3 files changed, 78 insertions(+), 51 deletions(-)
> 
> I've tweaked the EXCLUDE_FROM_WORLD issue and I can squash that in but
> with that we still saw:
> 
> https://autobuilder.yoctoproject.org/typhoon/#/builders/87/builds/6433/steps/14/logs/stdio
> 
> which I think is from this patch.
> 
> Cheers,
> 
> Richard

While I had checked when `git notes` was introduced (1.7.1 for the 
functionality I use), I had not realized that the --fixed-value option 
to `git config` was only introduced in 2.30 (way after the current OE 
minimum requirement of Git 1.8.3.1). Thankfully, the --fixed-value 
option is not really needed in this case and can just be removed.

I have an updated patch series that fixes this, the missing 
EXCLUDE_FROM_WORLD, and a problem related to how git rebase handles 
notes when a commit is fully absorbed during a rebase. The latter 
could trigger when devtool upgrade is used.

I will send the patches once oe-selftest -r devtool is done...

//Peter