[PATCH] pw_maintainers_cli: enhance tree selection

[PATCH] pw_maintainers_cli: enhance tree selection

[pbhagavatula]

From: Pavan Nikhilesh <pbhagavatula@marvell.com>

When longest prefix match doesnt find a suitable tree, pick the
tree which has the highest modified file count instead of defauting
to main tree.

Signed-off-by: Pavan Nikhilesh <pbhagavatula@marvell.com>
---
 tools/pw_maintainers_cli.py | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/tools/pw_maintainers_cli.py b/tools/pw_maintainers_cli.py
index c7b5ba0..4aa46ae 100755
--- a/tools/pw_maintainers_cli.py
+++ b/tools/pw_maintainers_cli.py
@@ -46,6 +46,7 @@ import re
 import argparse
 import fnmatch
 
+from collections import Counter
 from requests.exceptions import HTTPError
 
 from git_pw import config
@@ -276,6 +277,7 @@ class Maintainers(object):
           dpdk-next-crypto + dpdk-next-virtio = dpdk
           dpdk-next-net-intel + dpdk-next-net-mlx = dpdk-next-net
         """
+        highest_tree = Counter(tree_list).most_common(1)[0][0]
         # Make sure the list is unique.
         tree_list = list(set(tree_list))
 
@@ -287,7 +289,9 @@ class Maintainers(object):
             os.path.commonprefix(_tree_list).rstrip('-').replace(
                     'dpdk-next-net-virtio', 'dpdk-next-virtio')
         # There is no 'dpdk-next' named tree.
-        if common_prefix.endswith('dpdk-next') or common_prefix.endswith('/'):
+        if common_prefix.endswith('dpdk-next'):
+            common_prefix = highest_tree
+        elif common_prefix.endswith('/'):
             common_prefix = 'git://dpdk.org/dpdk'
         return common_prefix
 
-- 
2.25.1

RE: [PATCH] pw_maintainers_cli: enhance tree selection

[Pavan Nikhilesh Bhagavatula]

> -----Original Message-----
> From: pbhagavatula@marvell.com <pbhagavatula@marvell.com>
> Sent: Friday, September 29, 2023 2:05 PM
> To: Jerin Jacob Kollanukkaran <jerinj@marvell.com>; alialnu@nvidia.com;
> aconole@redhat.com
> Cc: ci@dpdk.org; Pavan Nikhilesh Bhagavatula <pbhagavatula@marvell.com>
> Subject: [PATCH] pw_maintainers_cli: enhance tree selection
> 
> From: Pavan Nikhilesh <pbhagavatula@marvell.com>
> 
> When longest prefix match doesnt find a suitable tree, pick the
> tree which has the highest modified file count instead of defauting
> to main tree.
> 

This change is need to find the correct branch when a patch has a specification
Change followed by a implementation of driver layer example:

https://patches.dpdk.org/project/dpdk/list/?series=29675

> Signed-off-by: Pavan Nikhilesh <pbhagavatula@marvell.com>
> ---
>  tools/pw_maintainers_cli.py | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/pw_maintainers_cli.py b/tools/pw_maintainers_cli.py
> index c7b5ba0..4aa46ae 100755
> --- a/tools/pw_maintainers_cli.py
> +++ b/tools/pw_maintainers_cli.py
> @@ -46,6 +46,7 @@ import re
>  import argparse
>  import fnmatch
> 
> +from collections import Counter
>  from requests.exceptions import HTTPError
> 
>  from git_pw import config
> @@ -276,6 +277,7 @@ class Maintainers(object):
>            dpdk-next-crypto + dpdk-next-virtio = dpdk
>            dpdk-next-net-intel + dpdk-next-net-mlx = dpdk-next-net
>          """
> +        highest_tree = Counter(tree_list).most_common(1)[0][0]
>          # Make sure the list is unique.
>          tree_list = list(set(tree_list))
> 
> @@ -287,7 +289,9 @@ class Maintainers(object):
>              os.path.commonprefix(_tree_list).rstrip('-').replace(
>                      'dpdk-next-net-virtio', 'dpdk-next-virtio')
>          # There is no 'dpdk-next' named tree.
> -        if common_prefix.endswith('dpdk-next') or
> common_prefix.endswith('/'):
> +        if common_prefix.endswith('dpdk-next'):
> +            common_prefix = highest_tree
> +        elif common_prefix.endswith('/'):
>              common_prefix = 'git://dpdk.org/dpdk'
>          return common_prefix
> 
> --
> 2.25.1

Re: [PATCH] pw_maintainers_cli: enhance tree selection

[David Marchand]

On Fri, Sep 29, 2023 at 11:21 AM Pavan Nikhilesh Bhagavatula
<pbhagavatula@marvell.com> wrote:
> > -----Original Message-----
> > From: pbhagavatula@marvell.com <pbhagavatula@marvell.com>
> > Sent: Friday, September 29, 2023 2:05 PM
> > To: Jerin Jacob Kollanukkaran <jerinj@marvell.com>; alialnu@nvidia.com;
> > aconole@redhat.com
> > Cc: ci@dpdk.org; Pavan Nikhilesh Bhagavatula <pbhagavatula@marvell.com>
> > Subject: [PATCH] pw_maintainers_cli: enhance tree selection
> >
> > From: Pavan Nikhilesh <pbhagavatula@marvell.com>
> >
> > When longest prefix match doesnt find a suitable tree, pick the
> > tree which has the highest modified file count instead of defauting
> > to main tree.
> >

This script helps with the delegation but nothing prevents submitters
or maintainers from updating this in patchwork themselves.

I did not test it but this heuristic seems complex in how it is
affected by existing git history.
It makes this script harder to understand / predict the outcome.
I fear side effects.


>
> This change is need to find the correct branch when a patch has a specification
> Change followed by a implementation of driver layer example:

This should be in the commitlog from the start as it exposes the need,
rather than the solution implemented by the patch.

>
> https://patches.dpdk.org/project/dpdk/list/?series=29675



-- 
David Marchand

RE: [EXT] Re: [PATCH] pw_maintainers_cli: enhance tree selection

[Pavan Nikhilesh Bhagavatula]

> On Fri, Sep 29, 2023 at 11:21 AM Pavan Nikhilesh Bhagavatula
> <pbhagavatula@marvell.com> wrote:
> > > -----Original Message-----
> > > From: pbhagavatula@marvell.com <pbhagavatula@marvell.com>
> > > Sent: Friday, September 29, 2023 2:05 PM
> > > To: Jerin Jacob Kollanukkaran <jerinj@marvell.com>; alialnu@nvidia.com;
> > > aconole@redhat.com
> > > Cc: ci@dpdk.org; Pavan Nikhilesh Bhagavatula
> <pbhagavatula@marvell.com>
> > > Subject: [PATCH] pw_maintainers_cli: enhance tree selection
> > >
> > > From: Pavan Nikhilesh <pbhagavatula@marvell.com>
> > >
> > > When longest prefix match doesnt find a suitable tree, pick the
> > > tree which has the highest modified file count instead of defauting
> > > to main tree.
> > >
> 
> This script helps with the delegation but nothing prevents submitters
> or maintainers from updating this in patchwork themselves.
>

The script is used by CI to decide which branch to apply the patches on
and run the tests.

For example in the CI run http://mails.dpdk.org/archives/test-report/2023-September/468555.html
the script incorrectly picks dpdk main tree instead of dpdk-next-event and fails.

 
> I did not test it but this heuristic seems complex in how it is
> affected by existing git history.
> It makes this script harder to understand / predict the outcome.
> I fear side effects.
> 

It is not based on git history, It picks the tree based on the files that were changed in the patch.
Also, it only effects the case where its not able to find the common tree.

> 
> >
> > This change is need to find the correct branch when a patch has a
> specification
> > Change followed by a implementation of driver layer example:
> 
> This should be in the commitlog from the start as it exposes the need,
> rather than the solution implemented by the patch.
> 

I can change this in the next version.

> >
> > https://urldefense.proofpoint.com/v2/url?u=https-
> 3A__patches.dpdk.org_project_dpdk_list_-3Fseries-
> 3D29675&d=DwIFaQ&c=nKjWec2b6R0mOyPaz7xtfQ&r=E3SgYMjtKCMVsB-
> fmvgGV3o-
> g_fjLhk5Pupi9ijohpc&m=QbsEVS0vy8B7ZVY0WPAMjlHbfOxASgG_XP4P7_pri
> aunQeURVo3SJaIvt3GsjfMe&s=kdARtlZA5zKbOuliw3uJFntF-
> IjXGY8OXli9SkD2rDg&e=
> 
> 
> 
> --
> David Marchand

Re: [PATCH] pw_maintainers_cli: enhance tree selection

[Thomas Monjalon]

29/09/2023 11:21, Pavan Nikhilesh Bhagavatula:
> 
> > -----Original Message-----
> > From: pbhagavatula@marvell.com <pbhagavatula@marvell.com>
> > Sent: Friday, September 29, 2023 2:05 PM
> > To: Jerin Jacob Kollanukkaran <jerinj@marvell.com>; alialnu@nvidia.com;
> > aconole@redhat.com
> > Cc: ci@dpdk.org; Pavan Nikhilesh Bhagavatula <pbhagavatula@marvell.com>
> > Subject: [PATCH] pw_maintainers_cli: enhance tree selection
> > 
> > From: Pavan Nikhilesh <pbhagavatula@marvell.com>
> > 
> > When longest prefix match doesnt find a suitable tree, pick the
> > tree which has the highest modified file count instead of defauting
> > to main tree.
> > 
> 
> This change is need to find the correct branch when a patch has a specification
> Change followed by a implementation of driver layer example:
> 
> https://patches.dpdk.org/project/dpdk/list/?series=29675

That's expected: when a series touches more than a tree scope,
it goes to main.
But that's not the issue here.
Both eventdev lib, test and drivers belong to the eventdev tree.
So why it is not already delegated to eventdev?
Please dig more.

RE: [EXT] Re: [PATCH] pw_maintainers_cli: enhance tree selection

[Pavan Nikhilesh Bhagavatula]

> >
> > > -----Original Message-----
> > > From: pbhagavatula@marvell.com <pbhagavatula@marvell.com>
> > > Sent: Friday, September 29, 2023 2:05 PM
> > > To: Jerin Jacob Kollanukkaran <jerinj@marvell.com>; alialnu@nvidia.com;
> > > aconole@redhat.com
> > > Cc: ci@dpdk.org; Pavan Nikhilesh Bhagavatula
> <pbhagavatula@marvell.com>
> > > Subject: [PATCH] pw_maintainers_cli: enhance tree selection
> > >
> > > From: Pavan Nikhilesh <pbhagavatula@marvell.com>
> > >
> > > When longest prefix match doesnt find a suitable tree, pick the
> > > tree which has the highest modified file count instead of defauting
> > > to main tree.
> > >
> >
> > This change is need to find the correct branch when a patch has a
> specification
> > Change followed by a implementation of driver layer example:
> >
> > https://urldefense.proofpoint.com/v2/url?u=https-
> 3A__patches.dpdk.org_project_dpdk_list_-3Fseries-
> 3D29675&d=DwICAg&c=nKjWec2b6R0mOyPaz7xtfQ&r=E3SgYMjtKCMVsB-
> fmvgGV3o-g_fjLhk5Pupi9ijohpc&m=WKN1erKSjH9MuEyyRs-
> R3jYC5geZeUeNipx2GQlQpQSuQQCet70Torr1oSdZNvZp&s=6XFs2Ggg5G0VFd
> YEaRexumEXM5JKc2vs5dmrRNcIUZw&e=
> 
> That's expected: when a series touches more than a tree scope,
> it goes to main.
> But that's not the issue here.
> Both eventdev lib, test and drivers belong to the eventdev tree.
> So why it is not already delegated to eventdev?
> Please dig more.
> 
> 

The main issue is the driver implementation touches common/cnxk which ties it to 
next-net-mrvl tree which causes the conflict.

We have few options here, 
1. Based on max number of files touched per tree (Current patch)
2. Ignore  driver/common from tree selection when there is a conflict with other tree.
3. When there is a conflict between trees choose the common tree instead of company specific tree 
    Example, dpdk-next-eventdev + dpdk-next-net-mrvl = dpdk-next-eventdev

Re: [EXT] Re: [PATCH] pw_maintainers_cli: enhance tree selection

[Thomas Monjalon]

29/09/2023 12:54, Pavan Nikhilesh Bhagavatula:
> > > From: pbhagavatula@marvell.com <pbhagavatula@marvell.com>
> > > > From: Pavan Nikhilesh <pbhagavatula@marvell.com>
> > > >
> > > > When longest prefix match doesnt find a suitable tree, pick the
> > > > tree which has the highest modified file count instead of defauting
> > > > to main tree.
> > > >
> > >
> > > This change is need to find the correct branch when a patch has a
> > specification
> > > Change followed by a implementation of driver layer example:
> > >
> > > https://urldefense.proofpoint.com/v2/url?u=https-
> > 3A__patches.dpdk.org_project_dpdk_list_-3Fseries-
> > 3D29675&d=DwICAg&c=nKjWec2b6R0mOyPaz7xtfQ&r=E3SgYMjtKCMVsB-
> > fmvgGV3o-g_fjLhk5Pupi9ijohpc&m=WKN1erKSjH9MuEyyRs-
> > R3jYC5geZeUeNipx2GQlQpQSuQQCet70Torr1oSdZNvZp&s=6XFs2Ggg5G0VFd
> > YEaRexumEXM5JKc2vs5dmrRNcIUZw&e=
> > 
> > That's expected: when a series touches more than a tree scope,
> > it goes to main.
> > But that's not the issue here.
> > Both eventdev lib, test and drivers belong to the eventdev tree.
> > So why it is not already delegated to eventdev?
> > Please dig more.
> > 
> > 
> 
> The main issue is the driver implementation touches common/cnxk which ties it to 
> next-net-mrvl tree which causes the conflict.
> 
> We have few options here, 
> 1. Based on max number of files touched per tree (Current patch)
> 2. Ignore  driver/common from tree selection when there is a conflict with other tree.
> 3. When there is a conflict between trees choose the common tree instead of company specific tree 
>     Example, dpdk-next-eventdev + dpdk-next-net-mrvl = dpdk-next-eventdev

I have a preference for option 2.

RE: [EXT] Re: [PATCH] pw_maintainers_cli: enhance tree selection

[Jerin Jacob Kollanukkaran]

> -----Original Message-----
> From: Thomas Monjalon <thomas@monjalon.net>
> Sent: Friday, September 29, 2023 4:40 PM
> To: Jerin Jacob Kollanukkaran <jerinj@marvell.com>; alialnu@nvidia.com;
> aconole@redhat.com; ci@dpdk.org; Pavan Nikhilesh Bhagavatula
> <pbhagavatula@marvell.com>
> Subject: Re: [EXT] Re: [PATCH] pw_maintainers_cli: enhance tree selection
> 
> 29/09/2023 12:54, Pavan Nikhilesh Bhagavatula:
> > > > From: pbhagavatula@marvell.com <pbhagavatula@marvell.com>
> > > > > From: Pavan Nikhilesh <pbhagavatula@marvell.com>
> > > > >
> > > > > When longest prefix match doesnt find a suitable tree, pick the
> > > > > tree which has the highest modified file count instead of
> > > > > defauting to main tree.
> > > > >
> > > >
> > > > This change is need to find the correct branch when a patch has a
> > > specification
> > > > Change followed by a implementation of driver layer example:
> > > >
> > > > https://urldefense.proofpoint.com/v2/url?u=https-
> > > 3A__patches.dpdk.org_project_dpdk_list_-3Fseries-
> > > 3D29675&d=DwICAg&c=nKjWec2b6R0mOyPaz7xtfQ&r=E3SgYMjtKCMVsB-
> > > fmvgGV3o-g_fjLhk5Pupi9ijohpc&m=WKN1erKSjH9MuEyyRs-
> > >
> R3jYC5geZeUeNipx2GQlQpQSuQQCet70Torr1oSdZNvZp&s=6XFs2Ggg5G0VFd
> > > YEaRexumEXM5JKc2vs5dmrRNcIUZw&e=
> > >
> > > That's expected: when a series touches more than a tree scope, it
> > > goes to main.
> > > But that's not the issue here.
> > > Both eventdev lib, test and drivers belong to the eventdev tree.
> > > So why it is not already delegated to eventdev?
> > > Please dig more.
> > >
> > >
> >
> > The main issue is the driver implementation touches common/cnxk which
> > ties it to next-net-mrvl tree which causes the conflict.
> >
> > We have few options here,
> > 1. Based on max number of files touched per tree (Current patch) 2.
> > Ignore  driver/common from tree selection when there is a conflict with other
> tree.
> > 3. When there is a conflict between trees choose the common tree instead of
> company specific tree
> >     Example, dpdk-next-eventdev + dpdk-next-net-mrvl =
> > dpdk-next-eventdev
> 
> I have a preference for option 2.

+1 for option 2.

> 
> 
> 

[PATCH v2] pw_maintainers_cli: enhance ci tree selection

[pbhagavatula]

From: Pavan Nikhilesh <pbhagavatula@marvell.com>

When longest prefix match doesnt find a suitable tree, remove the
trees of files belonging to 'drivers/common' and check if there
is any unique tree for the patchset.

Signed-off-by: Pavan Nikhilesh <pbhagavatula@marvell.com>
---
 v2 Chnages:
 - Find tree by removing 'drivers/common' instead of count based
 approach.

 tools/pw_maintainers_cli.py | 31 ++++++++++++++++++++++++++-----
 1 file changed, 26 insertions(+), 5 deletions(-)

diff --git a/tools/pw_maintainers_cli.py b/tools/pw_maintainers_cli.py
index c7b5ba0..ef60df8 100755
--- a/tools/pw_maintainers_cli.py
+++ b/tools/pw_maintainers_cli.py
@@ -203,13 +203,15 @@ class Maintainers(object):
         """
         Return a git tree that matches a list of files."""
         tree_list = []
+        file_tree_map = {}
         for _file in files:
             _tree = self._get_tree(_file)
             # Having no tree means that we accept those changes going through a
             # subtree (e.g. release notes).
             if _tree:
                 tree_list.append(_tree)
-        tree = self.get_common_denominator(tree_list)
+                file_tree_map[_file] = _tree
+        tree = self.get_common_denominator(tree_list, file_tree_map)
         if not tree:
             tree = 'git://dpdk.org/dpdk'
         return tree
@@ -268,7 +270,7 @@ class Maintainers(object):
         self.matched[matching_pattern] = tree
         return tree

-    def get_common_denominator(self, tree_list):
+    def get_common_denominator(self, tree_list, file_tree_map):
         """Finds a common tree by finding the longest common prefix.
         Examples for expected output:
           dpdk-next-virtio + dpdk = dpdk
@@ -278,7 +280,6 @@ class Maintainers(object):
         """
         # Make sure the list is unique.
         tree_list = list(set(tree_list))
-
         # Rename dpdk-next-virtio internally to match dpdk-next-net
         _tree_list = [
                 tree.replace('dpdk-next-virtio', 'dpdk-next-net-virtio')
@@ -286,11 +287,31 @@ class Maintainers(object):
         common_prefix = \
             os.path.commonprefix(_tree_list).rstrip('-').replace(
                     'dpdk-next-net-virtio', 'dpdk-next-virtio')
-        # There is no 'dpdk-next' named tree.
-        if common_prefix.endswith('dpdk-next') or common_prefix.endswith('/'):
+        # There is no 'dpdk-next' named tree, remove files that belong
+        # to 'drivers/common' and see if we find a tree.
+        if common_prefix.endswith('dpdk-next'):
+            common_prefix = self.get_filtered_tree(file_tree_map)
+        elif common_prefix.endswith('/'):
             common_prefix = 'git://dpdk.org/dpdk'
         return common_prefix

+    def get_common_files(self, files):
+        match_list = []
+        for f in files:
+            if re.match(r"drivers\/common", f) is not None:
+                match_list.append(f)
+        return match_list
+
+    def get_filtered_tree(self, file_tree_map):
+        # Get list of files that are in 'drivers/common'
+        common_list = self.get_common_files(file_tree_map.keys())
+        for c in common_list:
+            file_tree_map.pop(c, None)
+        tree_list = list(set(file_tree_map.values()))
+        if len(tree_list) == 1:
+            return tree_list[0]
+        return None
+

 if __name__ == '__main__':
     """Main procedure."""
--
2.25.1

Re: [PATCH v2] pw_maintainers_cli: enhance ci tree selection

[Aaron Conole]

<pbhagavatula@marvell.com> writes:

> From: Pavan Nikhilesh <pbhagavatula@marvell.com>
>
> When longest prefix match doesnt find a suitable tree, remove the
> trees of files belonging to 'drivers/common' and check if there
> is any unique tree for the patchset.
>
> Signed-off-by: Pavan Nikhilesh <pbhagavatula@marvell.com>
> ---
>  v2 Chnages:
>  - Find tree by removing 'drivers/common' instead of count based
>  approach.
>
>  tools/pw_maintainers_cli.py | 31 ++++++++++++++++++++++++++-----
>  1 file changed, 26 insertions(+), 5 deletions(-)
>
> diff --git a/tools/pw_maintainers_cli.py b/tools/pw_maintainers_cli.py
> index c7b5ba0..ef60df8 100755
> --- a/tools/pw_maintainers_cli.py
> +++ b/tools/pw_maintainers_cli.py
> @@ -203,13 +203,15 @@ class Maintainers(object):
>          """
>          Return a git tree that matches a list of files."""
>          tree_list = []
> +        file_tree_map = {}
>          for _file in files:
>              _tree = self._get_tree(_file)
>              # Having no tree means that we accept those changes going through a
>              # subtree (e.g. release notes).
>              if _tree:
>                  tree_list.append(_tree)
> -        tree = self.get_common_denominator(tree_list)
> +                file_tree_map[_file] = _tree
> +        tree = self.get_common_denominator(tree_list, file_tree_map)
>          if not tree:
>              tree = 'git://dpdk.org/dpdk'
>          return tree
> @@ -268,7 +270,7 @@ class Maintainers(object):
>          self.matched[matching_pattern] = tree
>          return tree
>
> -    def get_common_denominator(self, tree_list):
> +    def get_common_denominator(self, tree_list, file_tree_map):
>          """Finds a common tree by finding the longest common prefix.
>          Examples for expected output:
>            dpdk-next-virtio + dpdk = dpdk
> @@ -278,7 +280,6 @@ class Maintainers(object):
>          """
>          # Make sure the list is unique.
>          tree_list = list(set(tree_list))
> -
>          # Rename dpdk-next-virtio internally to match dpdk-next-net
>          _tree_list = [
>                  tree.replace('dpdk-next-virtio', 'dpdk-next-net-virtio')

Any reason why this whitespace is dropped here?  Otherwise, the patch
looks okay - but I don't think this line should be dropped.

If you agree, I can correct when I merge it.

> @@ -286,11 +287,31 @@ class Maintainers(object):
>          common_prefix = \
>              os.path.commonprefix(_tree_list).rstrip('-').replace(
>                      'dpdk-next-net-virtio', 'dpdk-next-virtio')
> -        # There is no 'dpdk-next' named tree.
> -        if common_prefix.endswith('dpdk-next') or common_prefix.endswith('/'):
> +        # There is no 'dpdk-next' named tree, remove files that belong
> +        # to 'drivers/common' and see if we find a tree.
> +        if common_prefix.endswith('dpdk-next'):
> +            common_prefix = self.get_filtered_tree(file_tree_map)
> +        elif common_prefix.endswith('/'):
>              common_prefix = 'git://dpdk.org/dpdk'
>          return common_prefix
>
> +    def get_common_files(self, files):
> +        match_list = []
> +        for f in files:
> +            if re.match(r"drivers\/common", f) is not None:
> +                match_list.append(f)
> +        return match_list
> +
> +    def get_filtered_tree(self, file_tree_map):
> +        # Get list of files that are in 'drivers/common'
> +        common_list = self.get_common_files(file_tree_map.keys())
> +        for c in common_list:
> +            file_tree_map.pop(c, None)
> +        tree_list = list(set(file_tree_map.values()))
> +        if len(tree_list) == 1:
> +            return tree_list[0]
> +        return None
> +
>
>  if __name__ == '__main__':
>      """Main procedure."""
> --
> 2.25.1

RE: [EXT] Re: [PATCH v2] pw_maintainers_cli: enhance ci tree selection

[Pavan Nikhilesh Bhagavatula]

> > From: Pavan Nikhilesh <pbhagavatula@marvell.com>
> >
> > When longest prefix match doesnt find a suitable tree, remove the
> > trees of files belonging to 'drivers/common' and check if there
> > is any unique tree for the patchset.
> >
> > Signed-off-by: Pavan Nikhilesh <pbhagavatula@marvell.com>
> > ---
> >  v2 Chnages:
> >  - Find tree by removing 'drivers/common' instead of count based
> >  approach.
> >
> >  tools/pw_maintainers_cli.py | 31 ++++++++++++++++++++++++++-----
> >  1 file changed, 26 insertions(+), 5 deletions(-)
> >
> > diff --git a/tools/pw_maintainers_cli.py b/tools/pw_maintainers_cli.py
> > index c7b5ba0..ef60df8 100755
> > --- a/tools/pw_maintainers_cli.py
> > +++ b/tools/pw_maintainers_cli.py
> > @@ -203,13 +203,15 @@ class Maintainers(object):
> >          """
> >          Return a git tree that matches a list of files."""
> >          tree_list = []
> > +        file_tree_map = {}
> >          for _file in files:
> >              _tree = self._get_tree(_file)
> >              # Having no tree means that we accept those changes going through
> a
> >              # subtree (e.g. release notes).
> >              if _tree:
> >                  tree_list.append(_tree)
> > -        tree = self.get_common_denominator(tree_list)
> > +                file_tree_map[_file] = _tree
> > +        tree = self.get_common_denominator(tree_list, file_tree_map)
> >          if not tree:
> >              tree = 'git://dpdk.org/dpdk'
> >          return tree
> > @@ -268,7 +270,7 @@ class Maintainers(object):
> >          self.matched[matching_pattern] = tree
> >          return tree
> >
> > -    def get_common_denominator(self, tree_list):
> > +    def get_common_denominator(self, tree_list, file_tree_map):
> >          """Finds a common tree by finding the longest common prefix.
> >          Examples for expected output:
> >            dpdk-next-virtio + dpdk = dpdk
> > @@ -278,7 +280,6 @@ class Maintainers(object):
> >          """
> >          # Make sure the list is unique.
> >          tree_list = list(set(tree_list))
> > -
> >          # Rename dpdk-next-virtio internally to match dpdk-next-net
> >          _tree_list = [
> >                  tree.replace('dpdk-next-virtio', 'dpdk-next-net-virtio')
> 
> Any reason why this whitespace is dropped here?  Otherwise, the patch
> looks okay - but I don't think this line should be dropped.

The new line removal can be ignored.

> 
> If you agree, I can correct when I merge it.
> 

Yes, please.

Thanks, 
Pavan.

> > @@ -286,11 +287,31 @@ class Maintainers(object):
> >          common_prefix = \
> >              os.path.commonprefix(_tree_list).rstrip('-').replace(
> >                      'dpdk-next-net-virtio', 'dpdk-next-virtio')
> > -        # There is no 'dpdk-next' named tree.
> > -        if common_prefix.endswith('dpdk-next') or
> common_prefix.endswith('/'):
> > +        # There is no 'dpdk-next' named tree, remove files that belong
> > +        # to 'drivers/common' and see if we find a tree.
> > +        if common_prefix.endswith('dpdk-next'):
> > +            common_prefix = self.get_filtered_tree(file_tree_map)
> > +        elif common_prefix.endswith('/'):
> >              common_prefix = 'git://dpdk.org/dpdk'
> >          return common_prefix
> >
> > +    def get_common_files(self, files):
> > +        match_list = []
> > +        for f in files:
> > +            if re.match(r"drivers\/common", f) is not None:
> > +                match_list.append(f)
> > +        return match_list
> > +
> > +    def get_filtered_tree(self, file_tree_map):
> > +        # Get list of files that are in 'drivers/common'
> > +        common_list = self.get_common_files(file_tree_map.keys())
> > +        for c in common_list:
> > +            file_tree_map.pop(c, None)
> > +        tree_list = list(set(file_tree_map.values()))
> > +        if len(tree_list) == 1:
> > +            return tree_list[0]
> > +        return None
> > +
> >
> >  if __name__ == '__main__':
> >      """Main procedure."""
> > --
> > 2.25.1

RE: [EXT] Re: [PATCH v2] pw_maintainers_cli: enhance ci tree selection

[Pavan Nikhilesh Bhagavatula]

> -----Original Message-----
> From: Pavan Nikhilesh Bhagavatula <pbhagavatula@marvell.com>
> Sent: Friday, October 13, 2023 11:25 AM
> To: Aaron Conole <aconole@redhat.com>
> Cc: Jerin Jacob Kollanukkaran <jerinj@marvell.com>; alialnu@nvidia.com;
> thomas@monjalon.net; david.marchand@redhat.com; ci@dpdk.org
> Subject: RE: [EXT] Re: [PATCH v2] pw_maintainers_cli: enhance ci tree
> selection
> 
> > > From: Pavan Nikhilesh <pbhagavatula@marvell.com>
> > >
> > > When longest prefix match doesnt find a suitable tree, remove the
> > > trees of files belonging to 'drivers/common' and check if there
> > > is any unique tree for the patchset.
> > >
> > > Signed-off-by: Pavan Nikhilesh <pbhagavatula@marvell.com>
> > > ---
> > >  v2 Chnages:
> > >  - Find tree by removing 'drivers/common' instead of count based
> > >  approach.
> > >
> > >  tools/pw_maintainers_cli.py | 31 ++++++++++++++++++++++++++-----
> > >  1 file changed, 26 insertions(+), 5 deletions(-)
> > >
> > > diff --git a/tools/pw_maintainers_cli.py b/tools/pw_maintainers_cli.py
> > > index c7b5ba0..ef60df8 100755
> > > --- a/tools/pw_maintainers_cli.py
> > > +++ b/tools/pw_maintainers_cli.py
> > > @@ -203,13 +203,15 @@ class Maintainers(object):
> > >          """
> > >          Return a git tree that matches a list of files."""
> > >          tree_list = []
> > > +        file_tree_map = {}
> > >          for _file in files:
> > >              _tree = self._get_tree(_file)
> > >              # Having no tree means that we accept those changes going
> through
> > a
> > >              # subtree (e.g. release notes).
> > >              if _tree:
> > >                  tree_list.append(_tree)
> > > -        tree = self.get_common_denominator(tree_list)
> > > +                file_tree_map[_file] = _tree
> > > +        tree = self.get_common_denominator(tree_list, file_tree_map)
> > >          if not tree:
> > >              tree = 'git://dpdk.org/dpdk'
> > >          return tree
> > > @@ -268,7 +270,7 @@ class Maintainers(object):
> > >          self.matched[matching_pattern] = tree
> > >          return tree
> > >
> > > -    def get_common_denominator(self, tree_list):
> > > +    def get_common_denominator(self, tree_list, file_tree_map):
> > >          """Finds a common tree by finding the longest common prefix.
> > >          Examples for expected output:
> > >            dpdk-next-virtio + dpdk = dpdk
> > > @@ -278,7 +280,6 @@ class Maintainers(object):
> > >          """
> > >          # Make sure the list is unique.
> > >          tree_list = list(set(tree_list))
> > > -
> > >          # Rename dpdk-next-virtio internally to match dpdk-next-net
> > >          _tree_list = [
> > >                  tree.replace('dpdk-next-virtio', 'dpdk-next-net-virtio')
> >
> > Any reason why this whitespace is dropped here?  Otherwise, the patch
> > looks okay - but I don't think this line should be dropped.
> 
> The new line removal can be ignored.
> 
> >
> > If you agree, I can correct when I merge it.
> >
> 
> Yes, please.
> 

Ping

> Thanks,
> Pavan.
> 
> > > @@ -286,11 +287,31 @@ class Maintainers(object):
> > >          common_prefix = \
> > >              os.path.commonprefix(_tree_list).rstrip('-').replace(
> > >                      'dpdk-next-net-virtio', 'dpdk-next-virtio')
> > > -        # There is no 'dpdk-next' named tree.
> > > -        if common_prefix.endswith('dpdk-next') or
> > common_prefix.endswith('/'):
> > > +        # There is no 'dpdk-next' named tree, remove files that belong
> > > +        # to 'drivers/common' and see if we find a tree.
> > > +        if common_prefix.endswith('dpdk-next'):
> > > +            common_prefix = self.get_filtered_tree(file_tree_map)
> > > +        elif common_prefix.endswith('/'):
> > >              common_prefix = 'git://dpdk.org/dpdk'
> > >          return common_prefix
> > >
> > > +    def get_common_files(self, files):
> > > +        match_list = []
> > > +        for f in files:
> > > +            if re.match(r"drivers\/common", f) is not None:
> > > +                match_list.append(f)
> > > +        return match_list
> > > +
> > > +    def get_filtered_tree(self, file_tree_map):
> > > +        # Get list of files that are in 'drivers/common'
> > > +        common_list = self.get_common_files(file_tree_map.keys())
> > > +        for c in common_list:
> > > +            file_tree_map.pop(c, None)
> > > +        tree_list = list(set(file_tree_map.values()))
> > > +        if len(tree_list) == 1:
> > > +            return tree_list[0]
> > > +        return None
> > > +
> > >
> > >  if __name__ == '__main__':
> > >      """Main procedure."""
> > > --
> > > 2.25.1

Re: [EXT] Re: [PATCH v2] pw_maintainers_cli: enhance ci tree selection

[Aaron Conole]

Pavan Nikhilesh Bhagavatula <pbhagavatula@marvell.com> writes:

>> -----Original Message-----
>> From: Pavan Nikhilesh Bhagavatula <pbhagavatula@marvell.com>
>> Sent: Friday, October 13, 2023 11:25 AM
>> To: Aaron Conole <aconole@redhat.com>
>> Cc: Jerin Jacob Kollanukkaran <jerinj@marvell.com>; alialnu@nvidia.com;
>> thomas@monjalon.net; david.marchand@redhat.com; ci@dpdk.org
>> Subject: RE: [EXT] Re: [PATCH v2] pw_maintainers_cli: enhance ci tree
>> selection
>> 
>> > > From: Pavan Nikhilesh <pbhagavatula@marvell.com>
>> > >
>> > > When longest prefix match doesnt find a suitable tree, remove the
>> > > trees of files belonging to 'drivers/common' and check if there
>> > > is any unique tree for the patchset.
>> > >
>> > > Signed-off-by: Pavan Nikhilesh <pbhagavatula@marvell.com>
>> > > ---
>> > >  v2 Chnages:
>> > >  - Find tree by removing 'drivers/common' instead of count based
>> > >  approach.
>> > >
>> > >  tools/pw_maintainers_cli.py | 31 ++++++++++++++++++++++++++-----
>> > >  1 file changed, 26 insertions(+), 5 deletions(-)
>> > >
>> > > diff --git a/tools/pw_maintainers_cli.py b/tools/pw_maintainers_cli.py
>> > > index c7b5ba0..ef60df8 100755
>> > > --- a/tools/pw_maintainers_cli.py
>> > > +++ b/tools/pw_maintainers_cli.py
>> > > @@ -203,13 +203,15 @@ class Maintainers(object):
>> > >          """
>> > >          Return a git tree that matches a list of files."""
>> > >          tree_list = []
>> > > +        file_tree_map = {}
>> > >          for _file in files:
>> > >              _tree = self._get_tree(_file)
>> > >              # Having no tree means that we accept those changes going
>> through
>> > a
>> > >              # subtree (e.g. release notes).
>> > >              if _tree:
>> > >                  tree_list.append(_tree)
>> > > -        tree = self.get_common_denominator(tree_list)
>> > > +                file_tree_map[_file] = _tree
>> > > +        tree = self.get_common_denominator(tree_list, file_tree_map)
>> > >          if not tree:
>> > >              tree = 'git://dpdk.org/dpdk'
>> > >          return tree
>> > > @@ -268,7 +270,7 @@ class Maintainers(object):
>> > >          self.matched[matching_pattern] = tree
>> > >          return tree
>> > >
>> > > -    def get_common_denominator(self, tree_list):
>> > > +    def get_common_denominator(self, tree_list, file_tree_map):
>> > >          """Finds a common tree by finding the longest common prefix.
>> > >          Examples for expected output:
>> > >            dpdk-next-virtio + dpdk = dpdk
>> > > @@ -278,7 +280,6 @@ class Maintainers(object):
>> > >          """
>> > >          # Make sure the list is unique.
>> > >          tree_list = list(set(tree_list))
>> > > -
>> > >          # Rename dpdk-next-virtio internally to match dpdk-next-net
>> > >          _tree_list = [
>> > >                  tree.replace('dpdk-next-virtio', 'dpdk-next-net-virtio')
>> >
>> > Any reason why this whitespace is dropped here?  Otherwise, the patch
>> > looks okay - but I don't think this line should be dropped.
>> 
>> The new line removal can be ignored.
>> 
>> >
>> > If you agree, I can correct when I merge it.
>> >
>> 
>> Yes, please.
>> 
>
> Ping

Apologies - applied, Thanks!

>> Thanks,
>> Pavan.
>> 
>> > > @@ -286,11 +287,31 @@ class Maintainers(object):
>> > >          common_prefix = \
>> > >              os.path.commonprefix(_tree_list).rstrip('-').replace(
>> > >                      'dpdk-next-net-virtio', 'dpdk-next-virtio')
>> > > -        # There is no 'dpdk-next' named tree.
>> > > -        if common_prefix.endswith('dpdk-next') or
>> > common_prefix.endswith('/'):
>> > > +        # There is no 'dpdk-next' named tree, remove files that belong
>> > > +        # to 'drivers/common' and see if we find a tree.
>> > > +        if common_prefix.endswith('dpdk-next'):
>> > > +            common_prefix = self.get_filtered_tree(file_tree_map)
>> > > +        elif common_prefix.endswith('/'):
>> > >              common_prefix = 'git://dpdk.org/dpdk'
>> > >          return common_prefix
>> > >
>> > > +    def get_common_files(self, files):
>> > > +        match_list = []
>> > > +        for f in files:
>> > > +            if re.match(r"drivers\/common", f) is not None:
>> > > +                match_list.append(f)
>> > > +        return match_list
>> > > +
>> > > +    def get_filtered_tree(self, file_tree_map):
>> > > +        # Get list of files that are in 'drivers/common'
>> > > +        common_list = self.get_common_files(file_tree_map.keys())
>> > > +        for c in common_list:
>> > > +            file_tree_map.pop(c, None)
>> > > +        tree_list = list(set(file_tree_map.values()))
>> > > +        if len(tree_list) == 1:
>> > > +            return tree_list[0]
>> > > +        return None
>> > > +
>> > >
>> > >  if __name__ == '__main__':
>> > >      """Main procedure."""
>> > > --
>> > > 2.25.1