Re: [PATCH v2] models: Use database constraints to prevent split Series
On Sat, 2019-10-19 at 00:57 +1100, Daniel Axtens wrote:
> > Closes: #241
>
> I'm not sure this is quite correct, sadly. Using the parallel parser I
> posted, I see a difference in the number of series created by parsing in
> parallel and parsing serially.
>
> # serial (-j1)
> > > > Series.objects.count()
> 28016
>
> # parallel (-j6)
> > > > Series.objects.count()
> 28065
>
>
> I investigated just the first Untitled series from the parallel case,
> where I see that one 6-patch series has been split up a lot:
>
> > > > pprint.pprint([(p.name, p.series) for p in
> > > > Patch.objects.filter(submitter__id=42)[0:6]])
> [('[1/6] x86: implement support to synchronize RDTSC through MFENCE on AMD '
> 'CPUs',
>on AMD CPUs>),
> ('[2/6] x86: Implement support to synchronize RDTSC with LFENCE on Intel
> CPUs',
> ),
> ('[3/6] x86: move nop declarations into separate include file',
> ),
> ('[4/6] x86: introduce rdtsc_barrier()', ),
> ('[5/6] x86: remove get_cycles_sync', ),
> ('[6/6] x86: read_tsc sync', )]
>
> If I do serial parsing, this does not get split up:
>
> > > > pprint.pprint([(p.name, p.series) for p in
> > > > Patch.objects.filter(submitter__id=74)[0:6]])
> [('[1/6] x86: implement support to synchronize RDTSC through MFENCE on AMD '
> 'CPUs',
> ),
> ('[2/6] x86: Implement support to synchronize RDTSC with LFENCE on Intel
> CPUs',
> ),
> ('[3/6] x86: move nop declarations into separate include file',
> ),
> ('[4/6] x86: introduce rdtsc_barrier()',
> ),
> ('[5/6] x86: remove get_cycles_sync',
> ),
> ('[6/6] x86: read_tsc sync', )]
>
> I haven't looked at the series in any detail beyond this - it might
> still be worth including, but it just doesn't quite squash the
> bug, sadly.
So my only theory about why this might be happening is that this series
is shallow threaded (so all patches contain a reference to the cover
letter but not their preceding patches) and there is still a race in
creating the SeriesReference corresponding to the cover letter which
some patches are losing. However, what's confusing is that they should
be trying and hitting an IntegrityError in the losing case, resulting
in the rollback/retry of the entire transaction. Clearly that is not
the case though so it would be good to figure out what's missing.
Would you be able to dump the rows for these Patches (minus the diff,
headers and content fields), the Series, and the associated
SeriesReferences so I can try figure out how we ended up doing this? I
probably won't be able to work on it for the next two weeks (I'm on
PTO) but I'll try crack it then. This has to be possible.
Stephen
> Regards,
> Daniel
>
> > Cc: Daniel Axtens
> > Cc: Petr Vorel
> > ---
> > patchwork/migrations/0037_python_3.py | 298 ++
> > .../0038_unique_series_references.py | 121 +++
> > patchwork/models.py | 6 +-
> > patchwork/parser.py | 122 +++
> > patchwork/tests/test_parser.py| 9 +-
> > patchwork/tests/utils.py | 6 +-
> > 6 files changed, 496 insertions(+), 66 deletions(-)
> > create mode 100644 patchwork/migrations/0037_python_3.py
> > create mode 100644 patchwork/migrations/0038_unique_series_references.py
> >
> > diff --git patchwork/migrations/0037_python_3.py
> > patchwork/migrations/0037_python_3.py
> > new file mode 100644
> > index ..416a7045
> > --- /dev/null
> > +++ patchwork/migrations/0037_python_3.py
> > @@ -0,0 +1,298 @@
> > +import datetime
> > +
> > +from django.conf import settings
> > +from django.db import migrations, models
> > +import django.db.models.deletion
> > +
> > +import patchwork.models
> > +
> > +
> > +class Migration(migrations.Migration):
> > +
> > +dependencies = [('patchwork', '0036_project_commit_url_format')]
> > +
> > +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.', # noqa
> > +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,
> > +
[PATCH 1/2] [PW3] tests: Fix escaping in bundle tests on Django 3.0
Django 3.0 switches to using Python 3's built-in HTML escaper, which
prefers to escape entities using hex rather than decimal.
Some of our tests check rendered HTML output against pre-escaped strings,
and fail because ''' is now '''.
Fix this by using the escape function so we get consistent escaping no
matter which Django version..
Signed-off-by: Andrew Donnellan
---
patchwork/tests/test_bundles.py | 14 --
1 file changed, 8 insertions(+), 6 deletions(-)
diff --git a/patchwork/tests/test_bundles.py b/patchwork/tests/test_bundles.py
index 63f943c033d6..c5e7ee62f435 100644
--- a/patchwork/tests/test_bundles.py
+++ b/patchwork/tests/test_bundles.py
@@ -10,6 +10,7 @@ import unittest
from django.conf import settings
from django.test import TestCase
from django.urls import reverse
+from django.utils.html import escape
from django.utils.http import urlencode
from patchwork.models import Bundle
@@ -548,8 +549,8 @@ class BundleAddFromListTest(BundleTestBase):
'project_id': self.project.linkname}),
params)
-self.assertContains(response, 'Patch '%s' already in bundle'
-% patch.name, count=1, status_code=200)
+expected = escape(f"Patch '{patch.name}' already in bundle")
+self.assertContains(response, expected, count=1, status_code=200)
self.assertEqual(count, self.bundle.patches.count())
@@ -570,11 +571,12 @@ class BundleAddFromListTest(BundleTestBase):
'project_id': self.project.linkname}),
params)
-self.assertContains(response, 'Patch '%s' already in bundle'
-% patch.name, count=1, status_code=200)
-self.assertContains(response, 'Patch '%s' added to bundle'
-% self.patches[1].name, count=1,
+expected_already = escape(f"Patch '{patch.name}' already in bundle")
+expected_added = escape(
+f"Patch '{self.patches[1].name}' added to bundle")
+self.assertContains(response, expected_already, count=1,
status_code=200)
+self.assertContains(response, expected_added, count=1, status_code=200)
self.assertEqual(count + 1, self.bundle.patches.count())
--
2.20.1
___
Patchwork mailing list
Patchwork@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/patchwork
[PATCH 2/2] [PW3] tox: Add Django 3.0b1
Now that we've dropped Python 2, we can get ready for Django 3.0.
Add Django 3.0b1 as a tox environment.
Closes: #311 ("Django 3.0 support")
Signed-off-by: Andrew Donnellan
---
Not adding upper bounds on the version numbers until 3.0 actually drops.
I'm considering this as closing the issue for Django 3.0 support, as now
that Django 3.0 is in beta it's unlikely to get further breaking changes
and we'll catch anything as we test it.
---
tox.ini | 5 -
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/tox.ini b/tox.ini
index ae15ce815f26..8ee11de56d14 100644
--- a/tox.ini
+++ b/tox.ini
@@ -1,6 +1,6 @@
[tox]
minversion = 2.0
-envlist = pep8,docs,py{36,37}-django{20,21,22}
+envlist = pep8,docs,py{36,37}-django{20,21,22,30b1}
skipsdist = True
[testenv]
@@ -13,6 +13,9 @@ deps =
django22: django>=2.2,<2.3
django22: djangorestframework>=3.10,<3.11
django22: django-filter>=2.1,<3.0
+django30b1: django==3.0b1
+django30b1: djangorestframework>=3.10
+django30b1: django-filter>=2.2
setenv =
DJANGO_SETTINGS_MODULE = patchwork.settings.dev
PYTHONDONTWRITEBYTECODE = 1
--
2.20.1
___
Patchwork mailing list
Patchwork@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/patchwork
Re: RFE: use patchwork to submit a patch
Hi Steven, On Tue, Oct 15, 2019 at 12:47:49PM -0400, Steven Rostedt wrote: > On Tue, 15 Oct 2019 19:37:04 +0300 Laurent Pinchart wrote: > > > But how far could we push this reasoning ? I've worked for a company > > whose corporate policy was that all source code had to be stored in SVN, > > not exception. I didn't reach the community to move kernel development > > away from git. I've also worked with people whose corporate policy was > > that they had to do Linux kernel development on Windows, without using > > any virtual machine. There are all kind of crazy corporate policies, and > > if we don't push the pain it inflicts on all of us back to the people > > who enact those crazy policies, we'll always lose. > > I understand your sentiment. But most of your examples are one-off > "crazy corporate policies". The "sucky email server" and "you must use > this sucky email server" is rather standard. Not saying that we don't > want to pressure them to change, but I'm hearing from people (like Dave > Miller), that new developers are moving away from email for > development. I think that's two different issues though, which certainly overlap (but I'm not sure to what extent). I'm also not sure if we can say that new developers are moving away from e-mail, or simply start development with a different, non e-mail-based process. It makes a bit of a difference, because some new developers may not have any issue with e-mail, they may just not have considered it. This of course doesn't mean that nobody has issues with e-mail, that nobody is moving away from e-mail, or that nobody would have trouble moving to e-mail. > I thought part of this thread was talking about having other ways > besides email to submit a patch. Something that can help people > correctly send a patch (at least their formatting) by making sure they > fill out the proper fields and have a tool that helps them do so. I > brought up the corporate email servers just as an example of having > this help out there too. I'm very biased here, but for me a CLI tool that asks me questions and generates the patch series (or directly sends it) would be easier to use than a web UI. That's because I would stay on the CLI that I'm already using for git. This of course assumes a working git-send-email configuration, which we all no is not a given. I thus wonder if it wouldn't be better to have a CLI helper to create the patches, and provide some sort of https-to-smtp service (possibly in the form of pushing a tag with a message for instance). There are many options possible to work around broken e-mail workflows, so instead of focussing on a web UI because it was the first idea that was proposed, let's make sure we consider other ones. And maybe we'll decide that a web UI is the best option. > I plan on continuing to develop mostly in email (I still send my patch > series via quilt!). But I'm not going to enforce everyone to continue > to use email if we can come up with a better way. I also want to make > sure that whatever we do come up with will still support email. Hypothetically speaking, if there was a service that allowed sending a patch series through a git push with a tag containing a cover letter in its message, and that service would send out e-mails to mailing lists (with the option to self-host it if you want), would you consider using it ? -- Regards, Laurent Pinchart ___ Patchwork mailing list Patchwork@lists.ozlabs.org https://lists.ozlabs.org/listinfo/patchwork
RE: [PATCH] sql: Fix table lists
Hi Stephen,
> -Original Message-
> From: Stephen Finucane
> Sent: Sunday, September 8, 2019 8:51 PM
> To: Ali Alnubani ; patchwork@lists.ozlabs.org
> Cc: Thomas Monjalon
> Subject: Re: [PATCH] sql: Fix table lists
>
> On Sun, 2019-09-08 at 07:13 +, Ali Alnubani wrote:
> > Thanks Stephen.
> >
> > I see the following section in the docs
> https://patchwork.readthedocs.io/en/latest/deployment/installation/#mail-
> transfer-agent-mta:
> > """
> > This assumes your Postfix process is running as the nobody user. If
> > this is not correct (use of postfix user is also common), you should
> > change both the username in the createuser command above and
> > substitute the username in the grant-all-postgres.sql script with the
> > appropriate alternative.
> > """
> > But I believe that it doesn’t have to be the user running the postfix
> > process, It's the permissions defined by postfix's conf default_privs:
> > http://www.postfix.org/postconf.5.html#default_privs
> >
> > Can you confirm my thought?
>
> To be honest, I'll have to let somone else answer this. I wrote that quite a
> long time ago and haven't really touched it since, so I've long since
> forgotten
> the reason I included that /o\ What you've said does make sense though, so
> if you've tested it and it works as expected or you can find a blog that says
> the same thing, I'd be happy to accept a patch to correct this.
>
> Sorry I can't be of more help,
> Stephen
I just sent a patch http://patchwork.ozlabs.org/patch/1180692/.
>
> > Thanks,
> > Ali
> >
> > > -Original Message-
> > > From: Stephen Finucane
> > > Sent: Friday, September 6, 2019 4:31 PM
> > > To: Ali Alnubani ; patchwork@lists.ozlabs.org
> > > Cc: Thomas Monjalon
> > > Subject: Re: [PATCH] sql: Fix table lists
> > >
> > > On Wed, 2019-09-04 at 13:40 +, Ali Alnubani wrote:
> > > > The patch adds missing commas in the table lists where missing,
> > > > and removes where unnecessary.
> > > > This fixes errors such as the following when feeding the script to
> > > > psql:
> > > >
> > > > ```
> > > > psql:lib/sql/grant-all.postgres.sql:37: ERROR: syntax error at or
> > > > near "patchwork_emailconfirmation"
> > > > LINE 19: patchwork_emailconfirmation, ...
> > > > ```
> > > >
> > > > Fixes: ca0e79d4db34 ("sql: Sort 'grant-all' scripts
> > > > alphabetically")
> > > >
> > > > Signed-off-by: Ali Alnubani
> > >
> > > Nicely done. Applied and backported to stable/2.1.
The patch doesn't seem to be in stable/2.1 yet. I also noticed that it doesn't
apply there.
Let me know if you want me to backport it.
> > >
> > > Thanks!
> > >
> > > Stephen
Thanks,
Ali
___
Patchwork mailing list
Patchwork@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/patchwork
[PATCH] docs: Fix link to deployment guide
The old format redirects to a nonexistent page when there are multiple versions of the docs. Signed-off-by: Ali Alnubani --- docs/development/installation.rst | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/development/installation.rst b/docs/development/installation.rst index 7f17881..940f730 100644 --- a/docs/development/installation.rst +++ b/docs/development/installation.rst @@ -3,8 +3,8 @@ Installation This document describes the necessary steps to configure Patchwork in a development environment. If you are interested in deploying Patchwork in a -production environment, refer to `the deployment guide -`__ instead. +production environment, refer to :doc:`the deployment guide +` instead. To begin, you should clone Patchwork: -- 2.11.0 ___ Patchwork mailing list Patchwork@lists.ozlabs.org https://lists.ozlabs.org/listinfo/patchwork
[PATCH] docs: Fix note about the required Postfix rights
The permissions for the user running the postfix process are not the ones used for external file or command delivery by default. The ones defined by default_privs are (in case the aliases(5) file that is owned by root was being used). A privileged user or the postfix owner should not be used in this case. See http://www.postfix.org/postconf.5.html#default_privs and local(8). Signed-off-by: Ali Alnubani --- docs/deployment/installation.rst | 10 +- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/docs/deployment/installation.rst b/docs/deployment/installation.rst index c086d9a..cd5e102 100644 --- a/docs/deployment/installation.rst +++ b/docs/deployment/installation.rst @@ -617,11 +617,11 @@ they can be loaded as seen below: .. note:: - This assumes your Postfix process is running as the ``nobody`` user. If - this is not correct (use of ``postfix`` user is also common), you should - change both the username in the ``createuser`` command above and substitute - the username in the ``grant-all-postgres.sql`` script with the appropriate - alternative. + This assumes that you are using the aliases(5) file that is owned by root, + and that Postfix's ``default_privs`` configuration is set as ``nobody``. If + this is not the case, you should change both the username in the ``createuser`` + command above and substitute the username in the ``grant-all-postgres.sql`` + script with the appropriate alternative. __ http://www.postfix.org/ -- 2.11.0 ___ Patchwork mailing list Patchwork@lists.ozlabs.org https://lists.ozlabs.org/listinfo/patchwork
[PATCH 1/2] templates: Move download buttons outside h1 tag
It's not valid to put a inside an . Move the download buttons in
the submission template outside the tag.
Signed-off-by: Andrew Donnellan
---
patchwork/templates/patchwork/submission.html | 9 +++--
1 file changed, 3 insertions(+), 6 deletions(-)
diff --git a/patchwork/templates/patchwork/submission.html
b/patchwork/templates/patchwork/submission.html
index b5b55dbd9241..407f9ece85fb 100644
--- a/patchwork/templates/patchwork/submission.html
+++ b/patchwork/templates/patchwork/submission.html
@@ -27,9 +27,8 @@ function toggle_div(link_id, headers_id)
}
-{{ submission.name }}
{% include "patchwork/partials/download-buttons.html" %}
-
+{{ submission.name }}
@@ -273,10 +272,8 @@ function toggle_div(link_id, headers_id)
{% endfor %}
{% if submission.diff %}
-
- Patch
- {% include "patchwork/partials/download-buttons.html" %}
-
+{% include "patchwork/partials/download-buttons.html" %}
+Patch
--
2.20.1
___
Patchwork mailing list
Patchwork@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/patchwork
[PATCH 2/2] templates: Fix mismatched close tags
There's a rather than in the bundle list. Fix it. Signed-off-by: Andrew Donnellan --- patchwork/templates/patchwork/bundles.html | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/patchwork/templates/patchwork/bundles.html b/patchwork/templates/patchwork/bundles.html index 749aaedcf010..1bb3b0da71ed 100644 --- a/patchwork/templates/patchwork/bundles.html +++ b/patchwork/templates/patchwork/bundles.html @@ -14,7 +14,7 @@ Bundle Project Public - Patches + Patches Download Delete -- 2.20.1 ___ Patchwork mailing list Patchwork@lists.ozlabs.org https://lists.ozlabs.org/listinfo/patchwork
