[PATCH v3 1/1] management: introduce replacerelations command

[Daniel Axtens]

Hi Rohit,

Apologies for the delay.

My one comment is that the code silently ignores patch IDs that are not
in the database. I don't really mind that much (although I probably
would have thrown a warning?), and I'm pleased that the tests cover it,
but it's completely undocumented. Please could you add a note about that
to the documentation.

> +This is a script used to ingest relations into Patchwork.
> +A patch groups file contains on each line patchwork ids of patches that form a relation.

Within the context of patchwork we call them 'patch IDs', not 'patchwork ids'. 

> +Eg contents of a patch groups file:

'Eg' should either have dots ('E.g.') or preferably be spelled out ("For
example, ...")

> +
> +    1 3 5
> +    2
> +    7 10 11 12
> +
> +In this case the script will identify 2 relations, (1, 3, 5) and (7, 10, 11, 12).

It would also be good to highlight that a line with only 1 patch will be
ignored as it's not possible to create a relation with only one patch.

> +Running this script will remove all existing relations and replace them with the relations found in the file.

Please could you also wrap your lines as in the rest of the file.

> +
> +.. option:: infile
> +
> +    input patch groups file.
> +


Lastly, when applying the patch, I see:

dja at dja-thinkpad ~/d/p/patchwork (master)> git pw patch apply 1321548 -s
.git/rebase-apply/patch:80: trailing whitespace.
    
.git/rebase-apply/patch:81: trailing whitespace.
    def handle(self, *args, **options):   
.git/rebase-apply/patch:98: trailing whitespace.
        
.git/rebase-apply/patch:102: trailing whitespace.
        
.git/rebase-apply/patch:122: trailing whitespace.
                
warning: squelched 4 whitespace errors
warning: 9 lines add whitespace errors.
Applying: management: introduce replacerelations command

Could you please fix those whitespace errors?

Kind regards,
Daniel

>  rehash
>  ~~~~~~
>  
> diff --git a/patchwork/management/commands/replacerelations.py b/patchwork/management/commands/replacerelations.py
> new file mode 100644
> index 0000000..0d9581d
> --- /dev/null
> +++ b/patchwork/management/commands/replacerelations.py
> @@ -0,0 +1,72 @@
> +# Patchwork - automated patch tracking system
> +# Copyright (C) 2020 Rohit Sarkar <rohitsarkar5398 at gmail.com>
> +#
> +# SPDX-License-Identifier: GPL-2.0-or-later
> +
> +import logging
> +import os
> +import sys
> +
> +from django.db import transaction
> +from django.core.management.base import BaseCommand
> +
> +from patchwork.models import Patch
> +from patchwork.models import PatchRelation
> +
> +logger = logging.getLogger(__name__)
> +
> +class Command(BaseCommand):
> +    help = 'Parse a relations file generated by PaStA and replace existing relations with the ones parsed'
> +
> +    def add_arguments(self, parser):
> +        parser.add_argument(
> +            'infile',
> +            help='input relations filename')
> +    
> +    def handle(self, *args, **options):   
> +        verbosity = int(options['verbosity'])
> +        if not verbosity:
> +            level = logging.CRITICAL
> +        elif verbosity == 1:
> +            level = logging.ERROR
> +        elif verbosity == 2:
> +            level = logging.INFO
> +        else:
> +            level = logging.DEBUG
> +
> +        logger.setLevel(level)
> +
> +        path = args and args[0] or options['infile']
> +        if not os.path.exists(path):
> +            logger.error('Invalid path: %s', path)
> +            sys.exit(1)
> +        
> +
> +        with open(path, 'r') as f:
> +            lines = f.readlines()
> +        
> +        # filter out trailing empty lines
> +        while len(lines) and not lines[-1]:
> +            lines.pop()
> +
> +        relations = [line.split(' ') for line in lines]
> +
> +        with transaction.atomic():
> +            PatchRelation.objects.all().delete()
> +            count = len(relations)
> +            ingested = 0
> +            logger.info('Parsing %d relations' % count)
> +            for i, patch_ids in enumerate(relations):
> +                related_patches = Patch.objects.filter(id__in=patch_ids)
> +
> +                if len(related_patches) > 1:
> +                    relation = PatchRelation()
> +                    relation.save()
> +                    related_patches.update(related=relation)
> +                    ingested += 1
> +                
> +                if i % 10 == 0:
> +                    self.stdout.write('%06d/%06d\r' % (i, count), ending='')
> +                    self.stdout.flush()
> +            
> +            self.stdout.write('Ingested %d relations' % ingested)
> diff --git a/patchwork/tests/__init__.py b/patchwork/tests/__init__.py
> index 8f78ea7..83358a6 100644
> --- a/patchwork/tests/__init__.py
> +++ b/patchwork/tests/__init__.py
> @@ -8,3 +8,4 @@ import os
>  TEST_MAIL_DIR = os.path.join(os.path.dirname(__file__), 'mail')
>  TEST_PATCH_DIR = os.path.join(os.path.dirname(__file__), 'patches')
>  TEST_FUZZ_DIR = os.path.join(os.path.dirname(__file__), 'fuzztests')
> +TEST_RELATIONS_DIR = os.path.join(os.path.dirname(__file__), 'relations')
> diff --git a/patchwork/tests/relations/patch-groups b/patchwork/tests/relations/patch-groups
> new file mode 100644
> index 0000000..593cf0f
> --- /dev/null
> +++ b/patchwork/tests/relations/patch-groups
> @@ -0,0 +1,3 @@
> +1 2
> +3 4 5
> +6
> diff --git a/patchwork/tests/relations/patch-groups-missing-patch-ids b/patchwork/tests/relations/patch-groups-missing-patch-ids
> new file mode 100644
> index 0000000..091b195
> --- /dev/null
> +++ b/patchwork/tests/relations/patch-groups-missing-patch-ids
> @@ -0,0 +1,4 @@
> +1 2
> +3 4 5 7
> +6 8
> +9 10
> diff --git a/patchwork/tests/test_management.py b/patchwork/tests/test_management.py
> index 66c6bad..c648cc7 100644
> --- a/patchwork/tests/test_management.py
> +++ b/patchwork/tests/test_management.py
> @@ -11,7 +11,7 @@ from django.core.management import call_command
>  from django.test import TestCase
>  
>  from patchwork import models
> -from patchwork.tests import TEST_MAIL_DIR
> +from patchwork.tests import TEST_MAIL_DIR, TEST_RELATIONS_DIR
>  from patchwork.tests import utils
>  
>  
> @@ -124,3 +124,28 @@ class ParsearchiveTest(TestCase):
>  
>          self.assertIn('Processed 1 messages -->', out.getvalue())
>          self.assertIn('  1 dropped', out.getvalue())
> +
> +class ReplacerelationsTest(TestCase):
> +    def test_invalid_path(self):
> +        out = StringIO()
> +        with self.assertRaises(SystemExit) as exc:
> +            call_command('replacerelations', 'xyz123random', stdout=out)
> +        self.assertEqual(exc.exception.code, 1)
> +
> +    def test_valid_relations(self):
> +        utils.create_patches(6)
> +        out = StringIO()
> +        call_command('replacerelations',
> +                     os.path.join(TEST_RELATIONS_DIR,
> +                                  'patch-groups'),
> +                                  stdout=out)
> +        
> +        self.assertEqual(models.PatchRelation.objects.count(), 2)
> +
> +        call_command('replacerelations',
> +                     os.path.join(TEST_RELATIONS_DIR,
> +                                  'patch-groups-missing-patch-ids'),
> +                                  stdout=out)
> +        
> +        self.assertEqual(models.PatchRelation.objects.count(), 2)
> +
> -- 
> 2.23.0.385.gbc12974a89