Re: [OE-core] [PATCH 1/3] scripts/patchreview: rework patch detection

2023-11-02 Thread Quentin Schulz via lists.openembedded.org

Hi Ross,

On 10/27/23 17:29, Ross Burton via lists.openembedded.org wrote:

From: Ross Burton 

A previous patch[1] added the ability to allow the search pattern for
patches to be changed, so that patchreview can be used across the entire
meta-oe repository by changing the patterns.

However, this means the caller needs to write long patterns when calling
patchreview.

Instead, we can see if the specified directory contains a layer by
checking if conf/layer.conf exists.  If it does, then search for patches
inside this directory.  If it doesn't, assume that the specified
directory is a repository that contains sublayers (such as
meta-openembedded) and look through each of the directories that match
the pattern meta-*.

This means patchreview can both scan either a single layer (eg
.../poky/meta) or a repository of sublayers (eg .../meta-openembedded).

[1] oe-core 599046ea9302af0cf856d3fcd827f6a2be75b7e1

Signed-off-by: Ross Burton 
---
  scripts/contrib/patchreview.py | 36 +-
  1 file changed, 27 insertions(+), 9 deletions(-)

diff --git a/scripts/contrib/patchreview.py b/scripts/contrib/patchreview.py
index 43de105adc2..af66e32e02e 100755
--- a/scripts/contrib/patchreview.py
+++ b/scripts/contrib/patchreview.py
@@ -41,7 +41,7 @@ def blame_patch(patch):
  "--format=%s (%aN <%aE>)",
  "--", patch)).decode("utf-8").splitlines()
  
-def patchreview(path, patches):

+def patchreview(patches):
  import re, os.path
  
  # General pattern: start of line, optional whitespace, tag with optional

@@ -56,11 +56,10 @@ def patchreview(path, patches):
  
  for patch in patches:
  
-fullpath = os.path.join(path, patch)

  result = Result()
-results[fullpath] = result
+results[patch] = result
  
-content = open(fullpath, encoding='ascii', errors='ignore').read()

+content = open(patch, encoding='ascii', errors='ignore').read()
  
  # Find the Signed-off-by tag

  match = sob_re.search(content)
@@ -198,21 +197,40 @@ def histogram(results):
  for k in bars:
  print("%-20s %s (%d)" % (k.capitalize() if k else "No status", 
bars[k], counts[k]))
  
+def gather_patches(candidate):

+# candidate can either be the path to a layer directly (eg meta-intel), or 
a
+# repository that contains other layers (meta-arm). We can determine what 
by
+# looking for a conf/layer.conf file. If that file exists then it's a 
layer,
+# otherwise its a repository of layers and we can assume they're called
+# meta-*.
+


Maybe mention that this expects a pathlib.Path object?


+if (candidate / "conf" / "layer.conf").exists():
+print(f"{candidate} is a layer")
+scan = [candidate]
+else:
+print(f"{candidate} is not a layer, checking for sub-layers")
+scan = [d for d in candidate.iterdir() if d.is_dir() and (d.name == "meta" or 
d.name.startswith("meta-"))]
+print(f"Found layers {' '.join((d.name for d in scan))}")
+


What about just looking for all layer.conf files recursively instead?

e.g.:

scan = [l.parents[1].name for l in candidate.glob('**/conf/layer.conf')

This assumes a layer root directory is the parent of `conf` directory in 
which a layer.conf file is, which I think is a good assumption.


But this does not make any assumption that the layer should be prefixed 
with meta- or be meta, as opposed to the current implementation.



+patches = []
+for directory in scan:
+filenames = subprocess.check_output(("git", "-C", directory, "ls-files", 
"recipes-*/**/*.patch", "recipes-*/**/*.diff"), universal_newlines=True).split()


FWIW, recipes- prefix is NOT enforced, it is derived from BBFILES 
variable in each layer.conf. (I have used layers where a typo was there, 
with recipe- or sometimes just recipes/). I am not saying we need to 
support those, but just merely raising this as a potential shortcoming.


Cheers,
Quentin

-=-=-=-=-=-=-=-=-=-=-=-
Links: You receive all messages sent to this group.
View/Reply Online (#190071): 
https://lists.openembedded.org/g/openembedded-core/message/190071
Mute This Topic: https://lists.openembedded.org/mt/102223656/21656
Group Owner: openembedded-core+ow...@lists.openembedded.org
Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub 
[arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-



[OE-core] [PATCH 1/3] scripts/patchreview: rework patch detection

2023-10-27 Thread Ross Burton
From: Ross Burton 

A previous patch[1] added the ability to allow the search pattern for
patches to be changed, so that patchreview can be used across the entire
meta-oe repository by changing the patterns.

However, this means the caller needs to write long patterns when calling
patchreview.

Instead, we can see if the specified directory contains a layer by
checking if conf/layer.conf exists.  If it does, then search for patches
inside this directory.  If it doesn't, assume that the specified
directory is a repository that contains sublayers (such as
meta-openembedded) and look through each of the directories that match
the pattern meta-*.

This means patchreview can both scan either a single layer (eg
.../poky/meta) or a repository of sublayers (eg .../meta-openembedded).

[1] oe-core 599046ea9302af0cf856d3fcd827f6a2be75b7e1

Signed-off-by: Ross Burton 
---
 scripts/contrib/patchreview.py | 36 +-
 1 file changed, 27 insertions(+), 9 deletions(-)

diff --git a/scripts/contrib/patchreview.py b/scripts/contrib/patchreview.py
index 43de105adc2..af66e32e02e 100755
--- a/scripts/contrib/patchreview.py
+++ b/scripts/contrib/patchreview.py
@@ -41,7 +41,7 @@ def blame_patch(patch):
 "--format=%s (%aN <%aE>)",
 "--", patch)).decode("utf-8").splitlines()
 
-def patchreview(path, patches):
+def patchreview(patches):
 import re, os.path
 
 # General pattern: start of line, optional whitespace, tag with optional
@@ -56,11 +56,10 @@ def patchreview(path, patches):
 
 for patch in patches:
 
-fullpath = os.path.join(path, patch)
 result = Result()
-results[fullpath] = result
+results[patch] = result
 
-content = open(fullpath, encoding='ascii', errors='ignore').read()
+content = open(patch, encoding='ascii', errors='ignore').read()
 
 # Find the Signed-off-by tag
 match = sob_re.search(content)
@@ -198,21 +197,40 @@ def histogram(results):
 for k in bars:
 print("%-20s %s (%d)" % (k.capitalize() if k else "No status", 
bars[k], counts[k]))
 
+def gather_patches(candidate):
+# candidate can either be the path to a layer directly (eg meta-intel), or 
a
+# repository that contains other layers (meta-arm). We can determine what 
by
+# looking for a conf/layer.conf file. If that file exists then it's a 
layer,
+# otherwise its a repository of layers and we can assume they're called
+# meta-*.
+
+if (candidate / "conf" / "layer.conf").exists():
+print(f"{candidate} is a layer")
+scan = [candidate]
+else:
+print(f"{candidate} is not a layer, checking for sub-layers")
+scan = [d for d in candidate.iterdir() if d.is_dir() and (d.name == 
"meta" or d.name.startswith("meta-"))]
+print(f"Found layers {' '.join((d.name for d in scan))}")
+
+patches = []
+for directory in scan:
+filenames = subprocess.check_output(("git", "-C", directory, 
"ls-files", "recipes-*/**/*.patch", "recipes-*/**/*.diff"), 
universal_newlines=True).split()
+patches += [os.path.join(directory, f) for f in filenames]
+return patches
 
 if __name__ == "__main__":
-import argparse, subprocess, os
+import argparse, subprocess, os, pathlib
 
 args = argparse.ArgumentParser(description="Patch Review Tool")
 args.add_argument("-b", "--blame", action="store_true", help="show blame 
for malformed patches")
 args.add_argument("-v", "--verbose", action="store_true", help="show 
per-patch results")
 args.add_argument("-g", "--histogram", action="store_true", help="show 
patch histogram")
 args.add_argument("-j", "--json", help="update JSON")
-args.add_argument("-p", "--pattern", nargs=1, action="extend", 
default=["recipes-*/**/*.patch", "recipes-*/**/*.diff"], help="pattern to 
search recipes patch")
-args.add_argument("directory", help="directory to scan")
+args.add_argument("directory", type=pathlib.Path, metavar="DIRECTORY", 
help="directory to scan (layer, or repository of layers)")
 args = args.parse_args()
 
-patches = subprocess.check_output(("git", "-C", args.directory, 
"ls-files") + tuple(args.pattern)).decode("utf-8").split()
-results = patchreview(args.directory, patches)
+patches = gather_patches(args.directory)
+results = patchreview(patches)
 analyse(results, want_blame=args.blame, verbose=args.verbose)
 
 if args.json:
-- 
2.34.1


-=-=-=-=-=-=-=-=-=-=-=-
Links: You receive all messages sent to this group.
View/Reply Online (#189747): 
https://lists.openembedded.org/g/openembedded-core/message/189747
Mute This Topic: https://lists.openembedded.org/mt/102223656/21656
Group Owner: openembedded-core+ow...@lists.openembedded.org
Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub 
[arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-