diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml new file mode 100644 index 000000000..d2061a919 --- /dev/null +++ b/.github/workflows/ci.yaml @@ -0,0 +1,104 @@ +--- +name: CI +on: + - push + - pull_request +jobs: + lint: + name: Run linters + runs-on: ubuntu-latest + steps: + - name: Checkout source code + uses: actions/checkout@v2 + - name: Set up Python 3.9 + uses: actions/setup-python@v2 + with: + python-version: 3.9 + - name: Install dependencies + run: python -m pip install tox + - name: Run tox + run: tox -e pep8 + test: + name: Run unit tests + runs-on: ubuntu-latest + strategy: + matrix: + python: [3.6, 3.7, 3.8, 3.9] + db: [postgres, mysql] + services: + postgres: + image: postgres:latest + env: + POSTGRES_DB: patchwork + POSTGRES_PASSWORD: patchwork + POSTGRES_USER: patchwork + ports: + - 5432:5432 + options: >- + --health-cmd pg_isready + --health-interval 10s + --health-timeout 5s + --health-retries 5 + mysql: + image: mysql:latest + env: + MYSQL_DATABASE: patchwork + MYSQL_USER: patchwork + MYSQL_PASSWORD: patchwork + MYSQL_ROOT_PASSWORD: root + ports: + - 3306:3306 + options: >- + --health-cmd="mysqladmin ping" + --health-interval 10s + --health-timeout 5s + --health-retries 5 + steps: + - name: Checkout source code + uses: actions/checkout@v2 + - name: Set up Python ${{ matrix.python }} + uses: actions/setup-python@v2 + with: + python-version: ${{ matrix.python }} + - name: Install Python dependencies + run: python -m pip install tox tox-gh-actions codecov + - name: Log database configuration (mysql) + if: ${{ matrix.db == 'mysql' }} + run: mysql -h 127.0.0.1 -e "SELECT VERSION(), CURRENT_USER();" -uroot -proot patchwork + - name: Log database configuration (postgres) + if: ${{ matrix.db == 'postgres' }} + run: psql -h 127.0.0.1 -c "SELECT VERSION(), CURRENT_USER, current_database()" -U patchwork -d patchwork + env: + PGPASSWORD: patchwork + - name: Modify database user permissions (mysql) + if: ${{ matrix.db == 'mysql' }} + run: mysql -h 127.0.0.1 -e "GRANT ALL ON \`test\\_patchwork%\`.* to 'patchwork'@'%';" -uroot -proot + - name: Run unit tests (via tox) + run: tox + env: + PW_TEST_DB_TYPE: "${{ matrix.db }}" + PW_TEST_DB_USER: "patchwork" + PW_TEST_DB_PASS: "patchwork" + PW_TEST_DB_HOST: "127.0.0.1" + docs: + name: Build docs + runs-on: ubuntu-latest + steps: + - name: Checkout source code + uses: actions/checkout@v2 + with: + fetch-depth: 0 + - name: Set up Python 3.9 + uses: actions/setup-python@v2 + with: + python-version: 3.9 + - name: Install dependencies + run: python -m pip install tox + - name: Build docs (via tox) + run: tox -e docs + - name: Archive build results + uses: actions/upload-artifact@v2 + with: + name: html-docs-build + path: docs/_build/html + retention-days: 7 diff --git a/.travis.yml b/.travis.yml deleted file mode 100644 index 50b87a6c3..000000000 --- a/.travis.yml +++ /dev/null @@ -1,59 +0,0 @@ -language: python - -os: linux -dist: xenial - -python: - - 3.6 - - 3.7 - - 3.8 - -addons: - postgresql: 9.6 - -services: - - mysql - - postgresql - -env: - jobs: - - PW_TEST_DB_TYPE=postgres PW_TEST_DB_USER=postgres - - PW_TEST_DB_TYPE=mysql PW_TEST_DB_USER=root - global: - - PW_TEST_DB_PASS="" - - PW_TEST_DB_HOST="localhost" - -jobs: - include: - - addons: - mariadb: 10.3 - env: - - PW_TEST_DB_TYPE=mysql - - PW_TEST_DB_USER=root - - addons: - postgresql: 10 - env: - - PW_TEST_DB_TYPE=postgres - - PW_TEST_DB_USER=postgres - python: 3.7 - -before_script: - - if [[ $PW_TEST_DB_TYPE == mysql ]]; then mysql -e "create database patchwork character set utf8;"; fi - - if [[ $PW_TEST_DB_TYPE == postgres ]]; then psql -c "create database patchwork with ENCODING = 'UTF8';" -U $PW_TEST_DB_USER; fi - -install: - - pip install tox-travis - - pip install codecov - -script: - - > - if [[ $PW_TEST_DB_TYPE == mysql ]]; - then - mysql -e "SELECT VERSION(), CURRENT_USER();" -u $PW_TEST_DB_USER patchwork; - else - psql -c "SELECT VERSION(), CURRENT_USER, current_database()" -U $PW_TEST_DB_USER patchwork; - fi - - tox - -after_success: - - codecov diff --git a/README.rst b/README.rst index 7af45ce72..995b7745a 100644 --- a/README.rst +++ b/README.rst @@ -6,12 +6,12 @@ Patchwork :target: https://pyup.io/repos/github/getpatchwork/patchwork/ :alt: Requirements Status -.. image:: https://codecov.io/gh/getpatchwork/patchwork/branch/master/graph/badge.svg +.. image:: https://codecov.io/gh/getpatchwork/patchwork/branch/main/graph/badge.svg :target: https://codecov.io/gh/getpatchwork/patchwork :alt: Codecov -.. image:: https://travis-ci.org/getpatchwork/patchwork.svg?branch=master - :target: https://travis-ci.org/getpatchwork/patchwork +.. image:: https://github.com/getpatchwork/patchwork/actions/workflows/ci.yaml/badge.svg + :target: https://github.com/getpatchwork/patchwork/actions/workflows/ci.yaml :alt: Build Status .. image:: https://readthedocs.org/projects/patchwork/badge/?version=latest diff --git a/docs/deployment/installation.rst b/docs/deployment/installation.rst index 6681002f8..f90000976 100644 --- a/docs/deployment/installation.rst +++ b/docs/deployment/installation.rst @@ -137,14 +137,14 @@ The first requirement is Patchwork itself. It can be downloaded like so: .. code-block:: shell - $ wget https://github.com/getpatchwork/patchwork/archive/v2.2.0.tar.gz + $ wget https://github.com/getpatchwork/patchwork/archive/v3.0.0.tar.gz We will install this under ``/opt``, though this is only a suggestion: .. code-block:: shell - $ tar -xvzf v2.2.0.tar.gz - $ sudo mv v2.2.0 /opt/patchwork + $ tar -xvzf v3.0.0.tar.gz + $ sudo mv v3.0.0 /opt/patchwork .. important:: diff --git a/docs/development/releasing.rst b/docs/development/releasing.rst index 8bb6b3145..9c18e2021 100644 --- a/docs/development/releasing.rst +++ b/docs/development/releasing.rst @@ -53,7 +53,7 @@ There is no cadence for releases: they are made available as necessary. Supported Versions ------------------ -Typically all development should occur on ``master``. While we will backport +Typically all development should occur on ``main``. While we will backport bugfixes and security updates, we will not backport any new features. This is to ensure stability for users of these versions of Patchwork. @@ -100,12 +100,12 @@ Backporting ----------- We will occasionally backport bugfixes and security updates. When backporting a -patch, said patch should first be merged into ``master``. Once merged, you can +patch, said patch should first be merged into ``main``. Once merged, you can backport by cherry-picking commits, using the ``-x`` flag for posterity: .. code-block:: shell - $ git cherry-pick -x + $ git cherry-pick -x There may be some conflicts; resolve these, uncommenting the `Conflicts` line when committing:: diff --git a/docs/requirements.txt b/docs/requirements.txt index e2641c8fb..7668b8857 100644 --- a/docs/requirements.txt +++ b/docs/requirements.txt @@ -1,5 +1,7 @@ -sphinx>=2.0 -reno>=2.2 -sphinx_rtd_theme~=0.5.0 -jinja2~=2.11.2 +sphinx>=4.2.0 +reno>=3.4.0 +sphinx-rtd-theme~=1.0 +jinja2~=3.0 sphinxcontrib-openapi~=0.7.0 +# See https://github.com/getpatchwork/patchwork/issues/442 +mistune<2.0.0 diff --git a/lib/sql/grant-all.postgres.sql b/lib/sql/grant-all.postgres.sql index a3b192b49..baaad22b6 100644 --- a/lib/sql/grant-all.postgres.sql +++ b/lib/sql/grant-all.postgres.sql @@ -55,7 +55,6 @@ GRANT SELECT, UPDATE ON patchwork_emailconfirmation_id_seq, patchwork_event_id_seq, patchwork_patch_id_seq, - patchwork_patchcomment_id_seq, patchwork_patchrelation_id_seq, patchwork_patchtag_id_seq, patchwork_person_id_seq, diff --git a/patchwork/__init__.py b/patchwork/__init__.py index 6fb547286..ccc164a87 100644 --- a/patchwork/__init__.py +++ b/patchwork/__init__.py @@ -5,7 +5,7 @@ from patchwork.version import get_latest_version -VERSION = (3, 0, 0) +VERSION = (3, 0, 7, 'alpha', 0) __version__ = get_latest_version(VERSION) diff --git a/patchwork/api/base.py b/patchwork/api/base.py index 89a43114f..6cb703b12 100644 --- a/patchwork/api/base.py +++ b/patchwork/api/base.py @@ -96,6 +96,9 @@ def to_representation(self, instance): # field was added, we drop it if not utils.has_version(request, version): for field in self.Meta.versioned_fields[version]: - data.pop(field) + # After a PATCH with an older API version, we may not see + # these fields. If they don't exist, don't panic, return + # (and then discard) None. + data.pop(field, None) return data diff --git a/patchwork/api/patch.py b/patchwork/api/patch.py index f6cb276d7..9d2227544 100644 --- a/patchwork/api/patch.py +++ b/patchwork/api/patch.py @@ -84,7 +84,8 @@ class PatchListSerializer(BaseHyperlinkedModelSerializer): checks = SerializerMethodField() tags = SerializerMethodField() related = PatchSerializer( - source='related.patches', many=True, default=[]) + source='related.patches', many=True, default=[], + style={'base_template': 'input.html'}) def get_web_url(self, instance): request = self.context.get('request') diff --git a/patchwork/migrations/0041_python3.py b/patchwork/migrations/0041_python3.py index 201c6460f..b9316bac0 100644 --- a/patchwork/migrations/0041_python3.py +++ b/patchwork/migrations/0041_python3.py @@ -1,3 +1,11 @@ +# commit 3a979ed8bfc6 ("migrations: don't go to the db for 0041_python3 migration") +# made a bunch of strings go past 79 characters, breaking flake8 checks. +# +# We're not really expecting future changes to this file so just don't run +# flake8 against it. +# +# flake8: noqa + import datetime from django.conf import settings @@ -15,319 +23,323 @@ class Migration(migrations.Migration): ] operations = [ - migrations.AlterField( - model_name='check', - name='context', - field=models.SlugField( - default='default', - help_text='A label to discern check from checks of other ' - 'testing systems.', - max_length=255, - ), - ), - migrations.AlterField( - model_name='check', - name='description', - field=models.TextField( - blank=True, - help_text='A brief description of the check.', - null=True, - ), - ), - migrations.AlterField( - model_name='check', - name='state', - field=models.SmallIntegerField( - choices=[ - (0, 'pending'), - (1, 'success'), - (2, 'warning'), - (3, 'fail'), - ], - default=0, - help_text='The state of the check.', - ), - ), - migrations.AlterField( - model_name='check', - name='target_url', - field=models.URLField( - blank=True, - help_text='The target URL to associate with this check. This ' - 'should be specific to the patch.', - null=True, - ), - ), - migrations.AlterField( - model_name='comment', - name='submission', - field=models.ForeignKey( - on_delete=django.db.models.deletion.CASCADE, - related_name='comments', - related_query_name='comment', - to='patchwork.Submission', - ), - ), - migrations.AlterField( - model_name='delegationrule', - name='path', - field=models.CharField( - help_text='An fnmatch-style pattern to match filenames ' - 'against.', - max_length=255, - ), - ), - migrations.AlterField( - model_name='delegationrule', - name='priority', - field=models.IntegerField( - default=0, - help_text='The priority of the rule. Rules with a higher ' - 'priority will override rules with lower priorities', - ), - ), - migrations.AlterField( - model_name='delegationrule', - name='user', - field=models.ForeignKey( - help_text='A user to delegate the patch to.', - on_delete=django.db.models.deletion.CASCADE, - to=settings.AUTH_USER_MODEL, - ), - ), - migrations.AlterField( - model_name='emailconfirmation', - name='type', - field=models.CharField( - choices=[ - ('userperson', 'User-Person association'), - ('registration', 'Registration'), - ('optout', 'Email opt-out'), - ], - max_length=20, - ), - ), - migrations.AlterField( - model_name='event', - name='actor', - field=models.ForeignKey( - blank=True, - help_text='The user that caused/created this event.', - null=True, - on_delete=django.db.models.deletion.SET_NULL, - related_name='+', - to=settings.AUTH_USER_MODEL, - ), - ), - migrations.AlterField( - model_name='event', - name='category', - field=models.CharField( - choices=[ - ('cover-created', 'Cover Letter Created'), - ('patch-created', 'Patch Created'), - ('patch-completed', 'Patch Completed'), - ('patch-state-changed', 'Patch State Changed'), - ('patch-delegated', 'Patch Delegate Changed'), - ('patch-relation-changed', 'Patch Relation Changed'), - ('check-created', 'Check Created'), - ('series-created', 'Series Created'), - ('series-completed', 'Series Completed'), - ], - db_index=True, - help_text='The category of the event.', - max_length=25, - ), - ), - migrations.AlterField( - model_name='event', - name='cover', - field=models.ForeignKey( - blank=True, - help_text='The cover letter that this event was created for.', - null=True, - on_delete=django.db.models.deletion.CASCADE, - related_name='+', - to='patchwork.CoverLetter', - ), - ), - migrations.AlterField( - model_name='event', - name='date', - field=models.DateTimeField( - default=datetime.datetime.utcnow, - help_text='The time this event was created.', - ), - ), - migrations.AlterField( - model_name='event', - name='patch', - field=models.ForeignKey( - blank=True, - help_text='The patch that this event was created for.', - null=True, - on_delete=django.db.models.deletion.CASCADE, - related_name='+', - to='patchwork.Patch', - ), - ), - migrations.AlterField( - model_name='event', - name='project', - field=models.ForeignKey( - help_text='The project that the events belongs to.', - on_delete=django.db.models.deletion.CASCADE, - related_name='+', - to='patchwork.Project', - ), - ), - migrations.AlterField( - model_name='event', - name='series', - field=models.ForeignKey( - blank=True, - help_text='The series that this event was created for.', - null=True, - on_delete=django.db.models.deletion.CASCADE, - related_name='+', - to='patchwork.Series', - ), - ), - migrations.AlterField( - model_name='patch', - name='number', - field=models.PositiveSmallIntegerField( - default=None, - help_text='The number assigned to this patch in the series', - null=True, - ), - ), - migrations.AlterField( - model_name='project', - name='commit_url_format', - field=models.CharField( - blank=True, - help_text='URL format for a particular commit. {} will be ' - 'replaced by the commit SHA.', - max_length=2000, - ), - ), - migrations.AlterField( - model_name='project', - name='list_archive_url_format', - field=models.CharField( - blank=True, - help_text="URL format for the list archive's Message-ID " - "redirector. {} will be replaced by the Message-ID.", - max_length=2000, - ), - ), - migrations.AlterField( - model_name='project', - name='subject_match', - field=models.CharField( - blank=True, - default='', - help_text='Regex to match the subject against if only part ' - 'of emails sent to the list belongs to this project. Will be ' - 'used with IGNORECASE and MULTILINE flags. If rules for more ' - 'projects match the first one returned from DB is chosen; ' - 'empty field serves as a default for every email which has no ' - 'other match.', - max_length=64, - validators=[patchwork.models.validate_regex_compiles], - ), - ), - migrations.AlterField( - model_name='series', - name='name', - field=models.CharField( - blank=True, - help_text='An optional name to associate with the series, ' - 'e.g. "John\'s PCI series".', - max_length=255, - null=True, - ), - ), - migrations.AlterField( - model_name='series', - name='total', - field=models.IntegerField( - help_text='Number of patches in series as indicated by the ' - 'subject prefix(es)' - ), - ), - migrations.AlterField( - model_name='series', - name='version', - field=models.IntegerField( - default=1, - help_text='Version of series as indicated by the subject ' - 'prefix(es)', - ), - ), - migrations.AlterField( - model_name='seriesreference', - name='series', - field=models.ForeignKey( - on_delete=django.db.models.deletion.CASCADE, - related_name='references', - related_query_name='reference', - to='patchwork.Series', - ), - ), - migrations.AlterField( - model_name='tag', - name='abbrev', - field=models.CharField( - help_text='Short (one-or-two letter) abbreviation for the ' - 'tag, used in table column headers', - max_length=2, - unique=True, - ), - ), - migrations.AlterField( - model_name='tag', - name='pattern', - field=models.CharField( - help_text='A simple regex to match the tag in the content of ' - 'a message. Will be used with MULTILINE and IGNORECASE flags. ' - 'eg. ^Acked-by:', - max_length=50, - validators=[patchwork.models.validate_regex_compiles], - ), - ), - migrations.AlterField( - model_name='tag', - name='show_column', - field=models.BooleanField( - default=True, - help_text="Show a column displaying this tag's count in the " - "patch list view", - ), - ), - migrations.AlterField( - model_name='userprofile', - name='items_per_page', - field=models.PositiveIntegerField( - default=100, help_text='Number of items to display per page' - ), - ), - migrations.AlterField( - model_name='userprofile', - name='send_email', - field=models.BooleanField( - default=False, - help_text='Selecting this option allows patchwork to send ' - 'email on your behalf', - ), - ), - migrations.AlterField( - model_name='userprofile', - name='show_ids', - field=models.BooleanField( - default=False, - help_text='Show click-to-copy patch IDs in the list view', - ), + migrations.SeparateDatabaseAndState( + state_operations=[ + migrations.AlterField( + model_name='check', + name='context', + field=models.SlugField( + default='default', + help_text='A label to discern check from checks of other ' + 'testing systems.', + max_length=255, + ), + ), + migrations.AlterField( + model_name='check', + name='description', + field=models.TextField( + blank=True, + help_text='A brief description of the check.', + null=True, + ), + ), + migrations.AlterField( + model_name='check', + name='state', + field=models.SmallIntegerField( + choices=[ + (0, 'pending'), + (1, 'success'), + (2, 'warning'), + (3, 'fail'), + ], + default=0, + help_text='The state of the check.', + ), + ), + migrations.AlterField( + model_name='check', + name='target_url', + field=models.URLField( + blank=True, + help_text='The target URL to associate with this check. This ' + 'should be specific to the patch.', + null=True, + ), + ), + migrations.AlterField( + model_name='comment', + name='submission', + field=models.ForeignKey( + on_delete=django.db.models.deletion.CASCADE, + related_name='comments', + related_query_name='comment', + to='patchwork.Submission', + ), + ), + migrations.AlterField( + model_name='delegationrule', + name='path', + field=models.CharField( + help_text='An fnmatch-style pattern to match filenames ' + 'against.', + max_length=255, + ), + ), + migrations.AlterField( + model_name='delegationrule', + name='priority', + field=models.IntegerField( + default=0, + help_text='The priority of the rule. Rules with a higher ' + 'priority will override rules with lower priorities', + ), + ), + migrations.AlterField( + model_name='delegationrule', + name='user', + field=models.ForeignKey( + help_text='A user to delegate the patch to.', + on_delete=django.db.models.deletion.CASCADE, + to=settings.AUTH_USER_MODEL, + ), + ), + migrations.AlterField( + model_name='emailconfirmation', + name='type', + field=models.CharField( + choices=[ + ('userperson', 'User-Person association'), + ('registration', 'Registration'), + ('optout', 'Email opt-out'), + ], + max_length=20, + ), + ), + migrations.AlterField( + model_name='event', + name='actor', + field=models.ForeignKey( + blank=True, + help_text='The user that caused/created this event.', + null=True, + on_delete=django.db.models.deletion.SET_NULL, + related_name='+', + to=settings.AUTH_USER_MODEL, + ), + ), + migrations.AlterField( + model_name='event', + name='category', + field=models.CharField( + choices=[ + ('cover-created', 'Cover Letter Created'), + ('patch-created', 'Patch Created'), + ('patch-completed', 'Patch Completed'), + ('patch-state-changed', 'Patch State Changed'), + ('patch-delegated', 'Patch Delegate Changed'), + ('patch-relation-changed', 'Patch Relation Changed'), + ('check-created', 'Check Created'), + ('series-created', 'Series Created'), + ('series-completed', 'Series Completed'), + ], + db_index=True, + help_text='The category of the event.', + max_length=25, + ), + ), + migrations.AlterField( + model_name='event', + name='cover', + field=models.ForeignKey( + blank=True, + help_text='The cover letter that this event was created for.', + null=True, + on_delete=django.db.models.deletion.CASCADE, + related_name='+', + to='patchwork.CoverLetter', + ), + ), + migrations.AlterField( + model_name='event', + name='date', + field=models.DateTimeField( + default=datetime.datetime.utcnow, + help_text='The time this event was created.', + ), + ), + migrations.AlterField( + model_name='event', + name='patch', + field=models.ForeignKey( + blank=True, + help_text='The patch that this event was created for.', + null=True, + on_delete=django.db.models.deletion.CASCADE, + related_name='+', + to='patchwork.Patch', + ), + ), + migrations.AlterField( + model_name='event', + name='project', + field=models.ForeignKey( + help_text='The project that the events belongs to.', + on_delete=django.db.models.deletion.CASCADE, + related_name='+', + to='patchwork.Project', + ), + ), + migrations.AlterField( + model_name='event', + name='series', + field=models.ForeignKey( + blank=True, + help_text='The series that this event was created for.', + null=True, + on_delete=django.db.models.deletion.CASCADE, + related_name='+', + to='patchwork.Series', + ), + ), + migrations.AlterField( + model_name='patch', + name='number', + field=models.PositiveSmallIntegerField( + default=None, + help_text='The number assigned to this patch in the series', + null=True, + ), + ), + migrations.AlterField( + model_name='project', + name='commit_url_format', + field=models.CharField( + blank=True, + help_text='URL format for a particular commit. {} will be ' + 'replaced by the commit SHA.', + max_length=2000, + ), + ), + migrations.AlterField( + model_name='project', + name='list_archive_url_format', + field=models.CharField( + blank=True, + help_text="URL format for the list archive's Message-ID " + "redirector. {} will be replaced by the Message-ID.", + max_length=2000, + ), + ), + migrations.AlterField( + model_name='project', + name='subject_match', + field=models.CharField( + blank=True, + default='', + help_text='Regex to match the subject against if only part ' + 'of emails sent to the list belongs to this project. Will be ' + 'used with IGNORECASE and MULTILINE flags. If rules for more ' + 'projects match the first one returned from DB is chosen; ' + 'empty field serves as a default for every email which has no ' + 'other match.', + max_length=64, + validators=[patchwork.models.validate_regex_compiles], + ), + ), + migrations.AlterField( + model_name='series', + name='name', + field=models.CharField( + blank=True, + help_text='An optional name to associate with the series, ' + 'e.g. "John\'s PCI series".', + max_length=255, + null=True, + ), + ), + migrations.AlterField( + model_name='series', + name='total', + field=models.IntegerField( + help_text='Number of patches in series as indicated by the ' + 'subject prefix(es)' + ), + ), + migrations.AlterField( + model_name='series', + name='version', + field=models.IntegerField( + default=1, + help_text='Version of series as indicated by the subject ' + 'prefix(es)', + ), + ), + migrations.AlterField( + model_name='seriesreference', + name='series', + field=models.ForeignKey( + on_delete=django.db.models.deletion.CASCADE, + related_name='references', + related_query_name='reference', + to='patchwork.Series', + ), + ), + migrations.AlterField( + model_name='tag', + name='abbrev', + field=models.CharField( + help_text='Short (one-or-two letter) abbreviation for the ' + 'tag, used in table column headers', + max_length=2, + unique=True, + ), + ), + migrations.AlterField( + model_name='tag', + name='pattern', + field=models.CharField( + help_text='A simple regex to match the tag in the content of ' + 'a message. Will be used with MULTILINE and IGNORECASE flags. ' + 'eg. ^Acked-by:', + max_length=50, + validators=[patchwork.models.validate_regex_compiles], + ), + ), + migrations.AlterField( + model_name='tag', + name='show_column', + field=models.BooleanField( + default=True, + help_text="Show a column displaying this tag's count in the " + "patch list view", + ), + ), + migrations.AlterField( + model_name='userprofile', + name='items_per_page', + field=models.PositiveIntegerField( + default=100, help_text='Number of items to display per page' + ), + ), + migrations.AlterField( + model_name='userprofile', + name='send_email', + field=models.BooleanField( + default=False, + help_text='Selecting this option allows patchwork to send ' + 'email on your behalf', + ), + ), + migrations.AlterField( + model_name='userprofile', + name='show_ids', + field=models.BooleanField( + default=False, + help_text='Show click-to-copy patch IDs in the list view', + ), + ), + ], ), ] diff --git a/patchwork/models.py b/patchwork/models.py index 6f90627d5..00273da9f 100644 --- a/patchwork/models.py +++ b/patchwork/models.py @@ -372,10 +372,13 @@ class SubmissionMixin(FilenameMixin, EmailMixin, models.Model): def list_archive_url(self): if not self.project.list_archive_url_format: return None + if not self.msgid: return None + return self.project.list_archive_url_format.format( - self.url_msgid) + self.url_msgid, + ) # patchwork metadata @@ -515,50 +518,13 @@ def is_editable(self, user): return True return False - @property - def combined_check_state(self): - """Return the combined state for all checks. - - Generate the combined check's state for this patch. This check - is one of the following, based on the value of each unique - check: - - * failure, if any context's latest check reports as failure - * warning, if any context's latest check reports as warning - * pending, if there are no checks, or a context's latest - Check reports as pending - * success, if latest checks for all contexts reports as - success - """ - state_names = dict(Check.STATE_CHOICES) - states = [check.state for check in self.checks] - - if not states: - return state_names[Check.STATE_PENDING] - - for state in [Check.STATE_FAIL, Check.STATE_WARNING, - Check.STATE_PENDING]: # order sensitive - if state in states: - return state_names[state] - - return state_names[Check.STATE_SUCCESS] - - @property - def checks(self): - """Return the list of unique checks. - - Generate a list of checks associated with this patch for each - type of Check. Only "unique" checks are considered, - identified by their 'context' field. This means, given n - checks with the same 'context', the newest check is the only - one counted regardless of its value. The end result will be a - association of types to number of unique checks for said - type. - """ + @staticmethod + def filter_unique_checks(checks): + """Filter the provided checks to generate the unique list.""" unique = {} duplicates = [] - for check in self.check_set.all(): + for check in checks: ctx = check.context user = check.user_id @@ -585,7 +551,50 @@ def checks(self): # prefetch_related.) So, do it 'by hand' in Python. We can # also be confident that this won't be worse, seeing as we've # just iterated over self.check_set.all() *anyway*. - return [c for c in self.check_set.all() if c.id not in duplicates] + return [c for c in checks if c.id not in duplicates] + + @property + def checks(self): + """Return the list of unique checks. + + Generate a list of checks associated with this patch for each + type of Check. Only "unique" checks are considered, + identified by their 'context' field. This means, given n + checks with the same 'context', the newest check is the only + one counted regardless of its value. The end result will be a + association of types to number of unique checks for said + type. + """ + return self.filter_unique_checks(self.check_set.all()) + + @property + def combined_check_state(self): + """Return the combined state for all checks. + + Generate the combined check's state for this patch. This check + is one of the following, based on the value of each unique + check: + + * failure, if any context's latest check reports as failure + * warning, if any context's latest check reports as warning + * pending, if there are no checks, or a context's latest check reports + as pending + * success, if latest checks for all contexts reports as success + """ + state_names = dict(Check.STATE_CHOICES) + states = [check.state for check in self.checks] + + if not states: + return state_names[Check.STATE_PENDING] + + # order sensitive + for state in ( + Check.STATE_FAIL, Check.STATE_WARNING, Check.STATE_PENDING, + ): + if state in states: + return state_names[state] + + return state_names[Check.STATE_SUCCESS] @property def check_count(self): @@ -653,9 +662,13 @@ class CoverComment(EmailMixin, models.Model): def list_archive_url(self): if not self.cover.project.list_archive_url_format: return None + if not self.msgid: return None - return self.project.list_archive_url_format.format(self.url_msgid) + + return self.cover.project.list_archive_url_format.format( + self.url_msgid, + ) def get_absolute_url(self): return reverse('comment-redirect', kwargs={'comment_id': self.id}) @@ -685,10 +698,13 @@ class PatchComment(EmailMixin, models.Model): def list_archive_url(self): if not self.patch.project.list_archive_url_format: return None + if not self.msgid: return None - return self.patch.list_archive_url_format.format( - self.url_msgid) + + return self.patch.project.list_archive_url_format.format( + self.url_msgid, + ) def get_absolute_url(self): return reverse('comment-redirect', kwargs={'comment_id': self.id}) diff --git a/patchwork/parser.py b/patchwork/parser.py index 61a812461..e8f8d8a4c 100644 --- a/patchwork/parser.py +++ b/patchwork/parser.py @@ -31,6 +31,7 @@ from patchwork.models import State +_msgid_re = re.compile(r'<[^>]+>') _hunk_re = re.compile(r'^\@\@ -\d+(?:,(\d+))? \+\d+(?:,(\d+))? \@\@') _filename_re = re.compile(r'^(---|\+\+\+) (\S+)') list_id_headers = ['List-ID', 'X-Mailing-List', 'X-list'] @@ -235,15 +236,15 @@ def _find_series_by_references(project, mail): name, prefixes = clean_subject(subject, [project.linkname]) version = parse_version(name, prefixes) - refs = find_references(mail) - h = clean_header(mail.get('Message-Id')) - if h: - refs = [h] + refs + msg_id = find_message_id(mail) + refs = [msg_id] + find_references(mail) for ref in refs: try: series = SeriesReference.objects.get( - msgid=ref[:255], project=project).series + msgid=ref[:255], + project=project, + ).series if series.version != version: # if the versions don't match, at least make sure these were @@ -472,6 +473,34 @@ def find_headers(mail): return '\n'.join(strings) +def find_message_id(mail): + """Extract the 'message-id' headers from a given mail and validate it. + + The validation here is simply checking that the Message-ID is correctly + formatted per RFC-2822. However, even if it's not we'll attempt to use what + we're given because a patch tracked in Patchwork with janky threading is + better than no patch whatsoever. + """ + header = clean_header(mail.get('Message-Id')) + if not header: + raise ValueError("Broken 'Message-Id' header") + + msgid = _msgid_re.search(header) + if msgid: + msgid = msgid.group(0) + else: + # This is only info level since the admin likely can't do anything + # about this + logger.info( + "Malformed 'Message-Id' header. The 'msg-id' component should be " + "surrounded by angle brackets. Saving raw header. This may " + "include comments and extra whitespace." + ) + msgid = header.strip() + + return msgid[:255] + + def find_references(mail): """Construct a list of possible reply message ids. @@ -481,23 +510,33 @@ def find_references(mail): """ refs = [] - if 'In-Reply-To' in mail: - for in_reply_to in mail.get_all('In-Reply-To'): - r = clean_header(in_reply_to) - if r: - refs.append(r) + for header in mail.get_all('In-Reply-To', []): + header = clean_header(header) + if not header: + continue + ref = _msgid_re.search(header) + if ref: + ref = ref.group(0) + else: + logger.info( + "Malformed 'In-Reply-To' header. The 'msg-id' component " + "should be surrounded by angle brackets. Saving raw header. " + "This may include comments and extra whitespace." + ) + ref = header.strip() + refs.append(ref) - if 'References' in mail: - for references_header in mail.get_all('References'): - h = clean_header(references_header) - if not h: - continue - references = h.split() - references.reverse() - for ref in references: - ref = ref.strip() - if ref not in refs: - refs.append(ref) + for header in mail.get_all('References', []): + header = clean_header(header) + if not header: + continue + # NOTE: We can't really implement a fallback here since without angle + # brackets there is no obvious way to delimit headers. + references = _msgid_re.findall(header) + references.reverse() + for ref in references: + if ref not in refs: + refs.append(ref) return refs @@ -813,7 +852,7 @@ def parse_patch(content): format, and splits it into the component comments and diff. Args: - patch: The patch to be split + content: The mail to be split Returns: A tuple containing the diff and comment. Either one or both of @@ -835,6 +874,7 @@ def parse_patch(content): # 4: patch hunk header line (@@ line) # 5: patch hunk content # 6: patch meta header (rename from/rename to/new file/index) + # 7: binary patch hunk # # valid transitions: # 0 -> 1 (diff, Index:) @@ -843,10 +883,12 @@ def parse_patch(content): # 2 -> 3 (+++) # 3 -> 4 (@@ line) # 4 -> 5 (patch content) - # 5 -> 1 (run out of lines from @@-specifed count) + # 5 -> 1 (ran out of lines from @@-specified count) # 1 -> 6 (extended header lines) # 6 -> 2 (---) + # 6 -> 7 (GIT binary patch) # 6 -> 1 (other text) + # 7 -> 1 (diff) # # Suspected patch header is stored into buf, and appended to # patchbuf if we find a following hunk. Otherwise, append to @@ -941,9 +983,20 @@ def fn(x): patchbuf += buf + line buf = '' state = 2 + elif line.startswith('GIT binary patch'): + patchbuf += buf + line + buf = '' + state = 7 else: buf += line state = 1 + elif state == 7: + if line.startswith('diff'): + buf += line + state = 0 + else: + patchbuf += buf + line + buf = '' else: raise Exception("Unknown state %d! (line '%s')" % (state, line)) @@ -1059,11 +1112,7 @@ def parse_mail(mail, list_id=None): # parse metadata - msgid = clean_header(mail.get('Message-Id')) - if not msgid: - raise ValueError("Broken 'Message-Id' header") - msgid = msgid[:255] - + msgid = find_message_id(mail) subject = mail.get('Subject') name, prefixes = clean_subject(subject, [project.linkname]) is_comment = subject_check(subject) diff --git a/patchwork/tests/api/test_comment.py b/patchwork/tests/api/test_comment.py index dfbf90497..5bbebf2ea 100644 --- a/patchwork/tests/api/test_comment.py +++ b/patchwork/tests/api/test_comment.py @@ -54,6 +54,18 @@ def test_list(self): self.assertEqual(status.HTTP_200_OK, resp.status_code) self.assertEqual(1, len(resp.data)) self.assertSerialized(comment, resp.data[0]) + self.assertIn('list_archive_url', resp.data[0]) + + def test_list_version_1_1(self): + """List cover letter comments using API v1.1.""" + cover = create_cover() + comment = create_cover_comment(cover=cover) + + resp = self.client.get(self.api_url(cover, version='1.1')) + self.assertEqual(status.HTTP_200_OK, resp.status_code) + self.assertEqual(1, len(resp.data)) + self.assertSerialized(comment, resp.data[0]) + self.assertNotIn('list_archive_url', resp.data[0]) def test_list_version_1_0(self): """List cover letter comments using API v1.0.""" @@ -105,6 +117,18 @@ def test_list(self): self.assertEqual(status.HTTP_200_OK, resp.status_code) self.assertEqual(1, len(resp.data)) self.assertSerialized(comment, resp.data[0]) + self.assertIn('list_archive_url', resp.data[0]) + + def test_list_version_1_1(self): + """List patch comments using API v1.1.""" + patch = create_patch() + comment = create_patch_comment(patch=patch) + + resp = self.client.get(self.api_url(patch, version='1.1')) + self.assertEqual(status.HTTP_200_OK, resp.status_code) + self.assertEqual(1, len(resp.data)) + self.assertSerialized(comment, resp.data[0]) + self.assertNotIn('list_archive_url', resp.data[0]) def test_list_version_1_0(self): """List patch comments using API v1.0.""" diff --git a/patchwork/tests/api/test_patch.py b/patchwork/tests/api/test_patch.py index da2dd6e90..b94ad2290 100644 --- a/patchwork/tests/api/test_patch.py +++ b/patchwork/tests/api/test_patch.py @@ -334,6 +334,20 @@ def test_update_maintainer(self): self.assertEqual(status.HTTP_200_OK, resp.status_code, resp) self.assertIsNone(Patch.objects.get(id=patch.id).delegate) + def test_update_maintainer_version_1_0(self): + """Update patch as maintainer on v1.1.""" + project = create_project() + patch = create_patch(project=project) + state = create_state() + user = create_maintainer(project) + + self.client.force_authenticate(user=user) + resp = self.client.patch(self.api_url(patch.id, version="1.1"), + {'state': state.slug, 'delegate': user.id}) + self.assertEqual(status.HTTP_200_OK, resp.status_code, resp) + self.assertEqual(Patch.objects.get(id=patch.id).state, state) + self.assertEqual(Patch.objects.get(id=patch.id).delegate, user) + @utils.store_samples('patch-update-error-bad-request') def test_update_invalid_state(self): """Update patch with invalid fields. diff --git a/patchwork/tests/api/test_project.py b/patchwork/tests/api/test_project.py index bf87a5630..5c2fbe12e 100644 --- a/patchwork/tests/api/test_project.py +++ b/patchwork/tests/api/test_project.py @@ -71,6 +71,26 @@ def test_list_authenticated(self): self.assertEqual(status.HTTP_200_OK, resp.status_code) self.assertEqual(1, len(resp.data)) self.assertSerialized(project, resp.data[0]) + self.assertIn('subject_match', resp.data[0]) + self.assertIn('list_archive_url', resp.data[0]) + self.assertIn('list_archive_url_format', resp.data[0]) + self.assertIn('commit_url_format', resp.data[0]) + + @utils.store_samples('project-list-1.1') + def test_list_version_1_1(self): + """List projects using API v1.1. + + Validate that newer fields are dropped for older API versions. + """ + create_project() + + resp = self.client.get(self.api_url(version='1.1')) + self.assertEqual(status.HTTP_200_OK, resp.status_code) + self.assertEqual(1, len(resp.data)) + self.assertIn('subject_match', resp.data[0]) + self.assertNotIn('list_archive_url', resp.data[0]) + self.assertNotIn('list_archive_url_format', resp.data[0]) + self.assertNotIn('commit_url_format', resp.data[0]) @utils.store_samples('project-list-1.0') def test_list_version_1_0(self): @@ -86,7 +106,7 @@ def test_list_version_1_0(self): self.assertNotIn('subject_match', resp.data[0]) @utils.store_samples('project-detail') - def test_detail_by_id(self): + def test_detail(self): """Show project using ID lookup. Validate that it's possible to filter by pk. @@ -96,6 +116,10 @@ def test_detail_by_id(self): resp = self.client.get(self.api_url(project.pk)) self.assertEqual(status.HTTP_200_OK, resp.status_code) self.assertSerialized(project, resp.data) + self.assertIn('subject_match', resp.data) + self.assertIn('list_archive_url', resp.data) + self.assertIn('list_archive_url_format', resp.data) + self.assertIn('commit_url_format', resp.data) def test_detail_by_linkname(self): """Show project using linkname lookup. @@ -119,6 +143,22 @@ def test_detail_by_numeric_linkname(self): self.assertEqual(status.HTTP_200_OK, resp.status_code) self.assertSerialized(project, resp.data) + @utils.store_samples('project-detail-1.1') + def test_detail_version_1_1(self): + """Show project using API v1.1. + + Validate that newer fields are dropped for older API versions. + """ + project = create_project() + + resp = self.client.get(self.api_url(project.pk, version='1.1')) + self.assertEqual(status.HTTP_200_OK, resp.status_code) + self.assertIn('name', resp.data) + self.assertIn('subject_match', resp.data) + self.assertNotIn('list_archive_url', resp.data) + self.assertNotIn('list_archive_url_format', resp.data) + self.assertNotIn('commit_url_format', resp.data) + @utils.store_samples('project-detail-1.0') def test_detail_version_1_0(self): """Show project using API v1.0. diff --git a/patchwork/tests/api/validator.py b/patchwork/tests/api/validator.py index 8ae891826..2b1921a35 100644 --- a/patchwork/tests/api/validator.py +++ b/patchwork/tests/api/validator.py @@ -8,14 +8,14 @@ from django.urls import resolve import openapi_core -from openapi_core.contrib.django import DjangoOpenAPIResponseFactory from openapi_core.contrib.django import DjangoOpenAPIRequestFactory -from openapi_core.schema.schemas.models import Format -from openapi_core.validation.request.validators import RequestValidator -from openapi_core.validation.response.validators import ResponseValidator -from openapi_core.schema.parameters.exceptions import OpenAPIParameterError +from openapi_core.contrib.django import DjangoOpenAPIResponseFactory from openapi_core.schema.media_types.exceptions import OpenAPIMediaTypeError +from openapi_core.schema.parameters.exceptions import OpenAPIParameterError from openapi_core.templating import util +from openapi_core.unmarshalling.schemas.formatters import Formatter +from openapi_core.validation.request.validators import RequestValidator +from openapi_core.validation.response.validators import ResponseValidator from rest_framework import status import yaml @@ -57,17 +57,25 @@ def __call__(self, value): CUSTOM_FORMATTERS = { - 'uri': Format(str, RegexValidator( - r'^(?:http|ftp)s?://' - r'(?:(?:[A-Z0-9](?:[A-Z0-9-]{0,61}[A-Z0-9])?\.)+(?:[A-Z]{2,6}\.?|[A-Z0-9-]{2,}\.?)|' # noqa - r'localhost|' - r'\d{1,3}\.\d{1,3}\.\d{1,3}\.\d{1,3})' - r'(?::\d+)?' - r'(?:/?|[/?]\S+)$')), - 'iso8601': Format(str, RegexValidator( - r'^\d{4}-\d\d-\d\dT\d\d:\d\d:\d\d\.\d{6}$')), - 'email': Format(str, RegexValidator( - r'[^@]+@[^@]+\.[^@]+')), + 'uri': Formatter.from_callables( + RegexValidator( + r'^(?:http|ftp)s?://' + r'(?:(?:[A-Z0-9](?:[A-Z0-9-]{0,61}[A-Z0-9])?\.)+(?:[A-Z]{2,6}\.?|[A-Z0-9-]{2,}\.?)|' # noqa: E501 + r'localhost|' + r'\d{1,3}\.\d{1,3}\.\d{1,3}\.\d{1,3})' + r'(?::\d+)?' + r'(?:/?|[/?]\S+)$', + ), + str, + ), + 'iso8601': Formatter.from_callables( + RegexValidator(r'^\d{4}-\d\d-\d\dT\d\d:\d\d:\d\d\.\d{6}$'), + str, + ), + 'email': Formatter.from_callables( + RegexValidator(r'[^@]+@[^@]+\.[^@]+'), + str, + ), } diff --git a/patchwork/tests/mail/0025-git-add-binary-file.mbox b/patchwork/tests/mail/0025-git-add-binary-file.mbox new file mode 100644 index 000000000..2fa9afa79 --- /dev/null +++ b/patchwork/tests/mail/0025-git-add-binary-file.mbox @@ -0,0 +1,28 @@ +From 3029b9604cf2b2eaa5f38167f59246e2fa026eb5 Mon Sep 17 00:00:00 2001 +From: Stephen Finucane +Date: Wed, 11 May 2022 10:59:59 +0100 +Subject: [PATCH] Add a single pixel bitmap image +To: foo@example.com + +Demonstrate how Git generates a patch for a binary file. + +Signed-off-by: Stephen Finucane +--- + pixel.bmp | Bin 0 -> 142 bytes + 1 file changed, 0 insertions(+), 0 deletions(-) + create mode 100644 pixel.bmp + +diff --git pixel.bmp pixel.bmp +new file mode 100644 +index 0000000000000000000000000000000000000000..9710347a13c4336e7dbaafa69af0e44a40c21172 +GIT binary patch +literal 142 +zcmZ?r?PGv|E+AC{#Eft(0hV9^lFE7z3>E+r{}~t{2+VVG4=P5;5yxUeQzj#`nE?RC +Ceh3`^ + +literal 0 +HcmV?d00001 + +-- +2.35.3 + diff --git a/patchwork/tests/mail/0026-git-add-mixed-binary-text-files.mbox b/patchwork/tests/mail/0026-git-add-mixed-binary-text-files.mbox new file mode 100644 index 000000000..9af263160 --- /dev/null +++ b/patchwork/tests/mail/0026-git-add-mixed-binary-text-files.mbox @@ -0,0 +1,40 @@ +From 499d36d946a6f1f654eebad500742eb5c3057569 Mon Sep 17 00:00:00 2001 +From: Stephen Finucane +Date: Wed, 11 May 2022 10:33:58 +0100 +Subject: [PATCH] Add a single pixel bitmap image, minimal script +To: foo@example.com + +Demonstrate how Git generates a patch for a binary file when mixed with +other file types. + +Signed-off-by: Stephen Finucane +--- + pixel.bmp | Bin 0 -> 142 bytes + quit.sh | 3 +++ + 2 files changed, 3 insertions(+) + create mode 100644 pixel.bmp + create mode 100644 quit.sh + +diff --git pixel.bmp pixel.bmp +new file mode 100644 +index 0000000000000000000000000000000000000000..9710347a13c4336e7dbaafa69af0e44a40c21172 +GIT binary patch +literal 142 +zcmZ?r?PGv|E+AC{#Eft(0hV9^lFE7z3>E+r{}~t{2+VVG4=P5;5yxUeQzj#`nE?RC +Ceh3`^ + +literal 0 +HcmV?d00001 + +diff --git quit.sh quit.sh +new file mode 100644 +index 0000000..f5f929a +--- /dev/null ++++ quit.sh +@@ -0,0 +1,3 @@ ++#/usr/bin/env bash ++echo "Wuh wuh" ++exit 1 +-- +2.35.3 + diff --git a/patchwork/tests/mail/0027-git-modify-binary-file.mbox b/patchwork/tests/mail/0027-git-modify-binary-file.mbox new file mode 100644 index 000000000..cd0f40672 --- /dev/null +++ b/patchwork/tests/mail/0027-git-modify-binary-file.mbox @@ -0,0 +1,24 @@ +From 434fe33a7ed28cc8cd0b344e55f0be2c74f1a58c Mon Sep 17 00:00:00 2001 +From: Stephen Finucane +Date: Wed, 11 May 2022 11:46:10 +0100 +Subject: [PATCH] Make changes to an existing binary file +To: foo@example.com + +Signed-off-by: Stephen Finucane +--- + pixel.bmp | Bin 142 -> 0 bytes + 1 file changed, 0 insertions(+), 0 deletions(-) + +diff --git pixel.bmp pixel.bmp +index 9710347a13c4336e7dbaafa69af0e44a40c21172..e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 100644 +GIT binary patch +literal 0 +HcmV?d00001 + +literal 142 +zcmZ?r?PGv|E+AC{#Eft(0hV9^lFE7z3>E+r{}~t{2+VVG4=P5;5yxUeQzj#`nE?RC +Ceh3`^ + +-- +2.35.3 + diff --git a/patchwork/tests/test_detail.py b/patchwork/tests/test_detail.py index 393ebc76e..5add40f92 100644 --- a/patchwork/tests/test_detail.py +++ b/patchwork/tests/test_detail.py @@ -3,14 +3,20 @@ # # SPDX-License-Identifier: GPL-2.0-or-later +from datetime import datetime as dt +from datetime import timedelta + from django.test import TestCase from django.urls import reverse +from patchwork.models import Check +from patchwork.tests.utils import create_check from patchwork.tests.utils import create_cover from patchwork.tests.utils import create_cover_comment from patchwork.tests.utils import create_patch from patchwork.tests.utils import create_patch_comment from patchwork.tests.utils import create_project +from patchwork.tests.utils import create_user class CoverViewTest(TestCase): @@ -157,6 +163,38 @@ def test_invalid_patch_id(self): response = self.client.get(requested_url) self.assertEqual(response.status_code, 404) + def test_patch_with_checks(self): + user = create_user() + patch = create_patch() + check_a = create_check( + patch=patch, user=user, context='foo', state=Check.STATE_FAIL, + date=(dt.utcnow() - timedelta(days=1))) + create_check( + patch=patch, user=user, context='foo', state=Check.STATE_SUCCESS) + check_b = create_check( + patch=patch, user=user, context='bar', state=Check.STATE_PENDING) + requested_url = reverse( + 'patch-detail', + kwargs={ + 'project_id': patch.project.linkname, + 'msgid': patch.url_msgid, + }, + ) + response = self.client.get(requested_url) + + # the response should contain checks + self.assertContains(response, '

Checks

') + + # and it should only show the unique checks + self.assertEqual( + 1, response.content.decode().count( + f'{check_a.user}/{check_a.context}' + )) + self.assertEqual( + 1, response.content.decode().count( + f'{check_b.user}/{check_b.context}' + )) + class CommentRedirectTest(TestCase): diff --git a/patchwork/tests/test_parser.py b/patchwork/tests/test_parser.py index eaf6599c5..2bd7720aa 100644 --- a/patchwork/tests/test_parser.py +++ b/patchwork/tests/test_parser.py @@ -68,7 +68,15 @@ def read_mail(filename, project=None): return mail -def _create_email(msg, msgid=None, sender=None, listid=None, in_reply_to=None): +def _create_email( + msg, + msgid=None, + subject=None, + sender=None, + listid=None, + in_reply_to=None, + references=None, +): msg['Message-Id'] = msgid or make_msgid() msg['Subject'] = 'Test subject' msg['From'] = sender or 'Test Author ' @@ -76,14 +84,32 @@ def _create_email(msg, msgid=None, sender=None, listid=None, in_reply_to=None): if in_reply_to: msg['In-Reply-To'] = in_reply_to + if references: + msg['References'] = references + return msg -def create_email(content, msgid=None, sender=None, listid=None, - in_reply_to=None): +def create_email( + content, + msgid=None, + subject=None, + sender=None, + listid=None, + in_reply_to=None, + references=None, +): msg = MIMEText(content, _charset='us-ascii') - return _create_email(msg, msgid, sender, listid, in_reply_to) + return _create_email( + msg, + msgid, + subject, + sender, + listid, + in_reply_to, + references, + ) def parse_mail(*args, **kwargs): @@ -665,6 +691,40 @@ def test_git_pull_trailing_space(self): 'linux-davinci.git tags/davinci-for-v5.6/soc', pull_url) + def test_git_add_binary_file(self): + diff, message = self._find_content('0025-git-add-binary-file.mbox') + self.assertTrue(diff is not None) + self.assertTrue(message is not None) + self.assertTrue( + diff.startswith('diff --git pixel.bmp pixel.bmp'), diff + ) + self.assertIn('GIT binary patch\n', diff) + self.assertIn('literal 142\n', diff) + self.assertIn('literal 0\n', diff) + + def test_git_add_mixed_binary_text_files(self): + diff, message = self._find_content( + '0026-git-add-mixed-binary-text-files.mbox' + ) + self.assertTrue(diff is not None) + self.assertTrue(message is not None) + self.assertTrue( + diff.startswith('diff --git pixel.bmp pixel.bmp'), diff + ) + self.assertIn('GIT binary patch\n', diff) + self.assertIn('diff --git quit.sh quit.sh\n', diff) + + def test_git_modify_binary_file(self): + diff, message = self._find_content('0027-git-modify-binary-file.mbox') + self.assertTrue(diff is not None) + self.assertTrue(message is not None) + self.assertTrue( + diff.startswith('diff --git pixel.bmp pixel.bmp'), diff + ) + self.assertIn('GIT binary patch\n', diff) + self.assertIn('literal 0\n', diff) + self.assertIn('literal 142\n', diff) + def test_git_rename(self): diff, _ = self._find_content('0008-git-rename.mbox') self.assertTrue(diff is not None) @@ -1146,7 +1206,7 @@ def log_query_errors(execute, sql, params, many, context): def test_duplicate_patch(self): diff = read_patch('0001-add-line.patch') - m = create_email(diff, listid=self.listid, msgid='1@example.com') + m = create_email(diff, listid=self.listid, msgid='<1@example.com>') self._test_duplicate_mail(m) @@ -1154,18 +1214,26 @@ def test_duplicate_patch(self): def test_duplicate_comment(self): diff = read_patch('0001-add-line.patch') - m1 = create_email(diff, listid=self.listid, msgid='1@example.com') + m1 = create_email( + diff, + listid=self.listid, + msgid='<1@example.com>', + ) _parse_mail(m1) - m2 = create_email('test', listid=self.listid, msgid='2@example.com', - in_reply_to='1@example.com') + m2 = create_email( + 'test', + listid=self.listid, + msgid='<2@example.com>', + in_reply_to='<1@example.com>', + ) self._test_duplicate_mail(m2) self.assertEqual(Patch.objects.count(), 1) self.assertEqual(PatchComment.objects.count(), 1) def test_duplicate_coverletter(self): - m = create_email('test', listid=self.listid, msgid='1@example.com') + m = create_email('test', listid=self.listid, msgid='<1@example.com>') del m['Subject'] m['Subject'] = '[PATCH 0/1] test cover letter' @@ -1174,6 +1242,96 @@ def test_duplicate_coverletter(self): self.assertEqual(Cover.objects.count(), 1) +class TestFindMessageID(TestCase): + def test_find_message_id__missing_header(self): + email = create_email('test') + del email['Message-Id'] + email['Message-Id'] = '' + + with self.assertRaises(ValueError) as cm: + parser.find_message_id(email) + self.assertIn("Broken 'Message-Id' header", str(cm.exeception)) + + def test_find_message_id__header_with_comments(self): + """Test that we strip comments from the Message-ID field.""" + message_id = ' (message ID with a comment)' + email = create_email('test', msgid=message_id) + + expected = '' + actual = parser.find_message_id(email) + + self.assertEqual(expected, actual) + + def test_find_message_id__invalid_header_fallback(self): + """Test that we accept badly formatted Message-ID fields.""" + message_id = '5899d592-8c87-47d9-92b6-d34260ce1aa4@radware.com>' + email = create_email('test', msgid=message_id) + + expected = '5899d592-8c87-47d9-92b6-d34260ce1aa4@radware.com>' + actual = parser.find_message_id(email) + + self.assertEqual(expected, actual) + + +class TestFindReferences(TestCase): + def test_find_references__header_with_comments(self): + """Test that we strip comments from References, In-Reply-To fields.""" + in_reply_to = ( + '<4574b99b-edac-d8dc-9141-79c3109d2fcc@huawei.com> (message from\n' + ' liqingqing on Thu, 1 Apr 2021 16:51:45 +0800)' + ) + email = create_email('test', in_reply_to=in_reply_to) + + expected = ['<4574b99b-edac-d8dc-9141-79c3109d2fcc@huawei.com>'] + actual = parser.find_references(email) + + self.assertEqual(expected, actual) + + def test_find_references__duplicate_references(self): + """Test that we ignore duplicate message IDs in 'References'.""" + message_id = '<20130510114450.7104c5d2@nehalam.linuxnetplumber.net>' + in_reply_to = ( + '<525534677.5312512.1368202896189.JavaMail.root@vmware.com>' + ) + references = ( + '\n' # noqa: E501 + ' \n' # noqa: E501 + ' <1676591087.5291867.1368201908283.JavaMail.root@vmware.com>\n' + ' <20130510091549.3c064df6@nehalam.linuxnetplumber.net>\n' + ' <525534677.5312512.1368202896189.JavaMail.root@vmware.com>' + ) + email = create_email( + 'test', + msgid=message_id, + in_reply_to=in_reply_to, + references=references, + ) + + expected = [ + '<525534677.5312512.1368202896189.JavaMail.root@vmware.com>', + '<20130510091549.3c064df6@nehalam.linuxnetplumber.net>', + '<1676591087.5291867.1368201908283.JavaMail.root@vmware.com>', + '', # noqa: E501 + '', + ] + actual = parser.find_references(email) + + self.assertEqual(expected, actual) + + def test_find_references__invalid_header_fallback(self): + """Test that we accept badly formatted In-Reply-To fields.""" + message_id = '' # noqa: E501 + in_reply_to = '5899d592-8c87-47d9-92b6-d34260ce1aa4@radware.com>' + email = create_email('test', msgid=message_id, in_reply_to=in_reply_to) + + expected = [ + '5899d592-8c87-47d9-92b6-d34260ce1aa4@radware.com>', + ] + actual = parser.find_references(email) + + self.assertEqual(expected, actual) + + class TestCommentCorrelation(TestCase): def test_find_patch_for_comment__no_reply(self): diff --git a/patchwork/tests/test_series.py b/patchwork/tests/test_series.py index e68ee88e9..4c0935520 100644 --- a/patchwork/tests/test_series.py +++ b/patchwork/tests/test_series.py @@ -5,6 +5,7 @@ import mailbox import os +import unittest from django.test import TestCase @@ -174,6 +175,7 @@ def test_out_of_order(self): self.assertSerialized(patches, [2]) self.assertSerialized(covers, [1]) + @unittest.skip('Flaky test') def test_duplicated(self): """Series received on multiple mailing lists. diff --git a/patchwork/tests/utils.py b/patchwork/tests/utils.py index 17dc3fcbc..cc09e84cd 100644 --- a/patchwork/tests/utils.py +++ b/patchwork/tests/utils.py @@ -61,6 +61,8 @@ def create_project(**kwargs): 'listid': 'test%d.example.com' % num, 'listemail': 'test%d@example.com' % num, 'subject_match': '', + 'list_archive_url': 'https://lists.example.com/', + 'list_archive_url_format': 'https://lists.example.com/mail/{}', } values.update(kwargs) diff --git a/patchwork/urls.py b/patchwork/urls.py index 79268e4e4..be388ac5d 100644 --- a/patchwork/urls.py +++ b/patchwork/urls.py @@ -158,7 +158,7 @@ ), re_path( r'^user/password-reset/(?P[0-9A-Za-z_\-]+)/' - r'(?P[0-9A-Za-z]{1,13}-[0-9A-Za-z]{1,20})/$', + r'(?P[0-9A-Za-z]{1,13}-[0-9A-Za-z]{1,32})/$', auth_views.PasswordResetConfirmView.as_view(), name='password_reset_confirm', ), diff --git a/patchwork/views/patch.py b/patchwork/views/patch.py index 74495714f..9f0b855b0 100644 --- a/patchwork/views/patch.py +++ b/patchwork/views/patch.py @@ -124,7 +124,9 @@ def patch_detail(request, project_id, msgid): related_different_project = [] context['comments'] = comments - context['checks'] = patch.check_set.all().select_related('user') + context['checks'] = Patch.filter_unique_checks( + patch.check_set.all().select_related('user'), + ) context['submission'] = patch context['patchform'] = form context['createbundleform'] = createbundleform diff --git a/releasenotes/config.yaml b/releasenotes/config.yaml index 975252140..ddae386fc 100644 --- a/releasenotes/config.yaml +++ b/releasenotes/config.yaml @@ -1,4 +1,5 @@ --- +default_branch: main release_tag_re: 'v\d\.\d\.\d(rc\d+)?' pre_release_tag_re: '(?P-rc(?:\d)*)$' sections: diff --git a/releasenotes/notes/issue-391-4088c856247f228e.yaml b/releasenotes/notes/issue-391-4088c856247f228e.yaml new file mode 100644 index 000000000..597902b63 --- /dev/null +++ b/releasenotes/notes/issue-391-4088c856247f228e.yaml @@ -0,0 +1,6 @@ +--- +api: + - | + The ``list_archive_url`` field will now be correctly shown for patch + comments and cover letter comments. + (`#391 `__) diff --git a/releasenotes/notes/issue-394-722c1e6384684469.yaml b/releasenotes/notes/issue-394-722c1e6384684469.yaml new file mode 100644 index 000000000..eda4f12c5 --- /dev/null +++ b/releasenotes/notes/issue-394-722c1e6384684469.yaml @@ -0,0 +1,6 @@ +--- +fixes: + - | + Fixed a compatability issue with Django 3.1 that prevented users from + resetting their password. + (`#394 `__) diff --git a/releasenotes/notes/issue-399-09d6f17aa54b14b2.yaml b/releasenotes/notes/issue-399-09d6f17aa54b14b2.yaml new file mode 100644 index 000000000..e925a6066 --- /dev/null +++ b/releasenotes/notes/issue-399-09d6f17aa54b14b2.yaml @@ -0,0 +1,11 @@ +--- +fixes: + - | + Comments and whitespace are now correctly stripped from the ``Message-ID``, + ``In-Reply-To``, and ``References`` headers. One side effect of this change + is that the parser is now stricter with regards to the format of the + ``msg-id`` component of these headers: all identifiers must now be + surrounded by angle brackets, e.g. ````. This is + mandated in the spec and a review of mailing lists archives suggest it is + broadly adhered to. Without these markers, there is no way to delimit + ``msg-id`` from any surrounding comments and whitespace. diff --git a/requirements-test.txt b/requirements-test.txt index c8ce258b6..9f56c60cb 100644 --- a/requirements-test.txt +++ b/requirements-test.txt @@ -4,3 +4,10 @@ sqlparse~=0.4.0 python-dateutil~=2.8.0 tblib~=1.7.0 openapi-core~=0.13.4 +# FIXME(stephenfin): We have to pin this to prevent a recurrence of [1]. It +# seems openapi-core needs to gain support for OpenAPI 3.1 before we can fix +# this properly +# +# [1] https://github.com/OAI/OpenAPI-Specification/issues/1368 +jsonschema<4.0 +openapi-schema-validator<0.2.0 diff --git a/tools/post-receive.hook b/tools/post-receive.hook index 9f2f0503d..67f234b02 100755 --- a/tools/post-receive.hook +++ b/tools/post-receive.hook @@ -11,7 +11,7 @@ set -eu PW_DIR=/opt/patchwork/patchwork #TODO: the state map should really live in the repo's git-config -STATE_MAP="refs/heads/master:Accepted" +STATE_MAP="refs/heads/main:Accepted" # ignore all commits already present in these refs # e.g., diff --git a/tox.ini b/tox.ini index 46d72517d..4db9b20b7 100644 --- a/tox.ini +++ b/tox.ini @@ -26,7 +26,7 @@ setenv = passenv = http_proxy HTTP_PROXY https_proxy HTTPS_PROXY no_proxy NO_PROXY PW_TEST_DB_TYPE PW_TEST_DB_USER PW_TEST_DB_PASS PW_TEST_DB_HOST - PW_TEST_DB_PORT + PW_TEST_DB_PORT PW_TEST_DB_NAME DJANGO_TEST_PROCESSES commands = python {toxinidir}/manage.py test --noinput --parallel -- {posargs:patchwork} @@ -78,6 +78,9 @@ commands = --branch {toxinidir}/manage.py test --noinput patchwork coverage report -m -[travis] +[gh-actions] python = - 3.6: py36, pep8, coverage + 3.6: py36 + 3.7: py37 + 3.8: py38 + 3.9: py39