In commit ab0c443691, we switched from using commit ranges to fixed
ranges. This was a good idea in so far as it ensures we're providing an
application with dependencies that are guaranteed to work. However,
Patchwork as a project isn't active enough to warrant the continued busy
work effort necessary to keep bumping these versions and it's probably
about time to abandon the experiment. However, rather than switching
back to version ranges, use the compatible releases feature introduced
in PEP 440 [1]. This gives us most of the benefits of ranges but with a
nicer syntax.
This is slightly slower to initially configure but requires less hacking
to get the same environment and should be a lot more maintainable (just
a simple modification to change the Python version).
Signed-off-by: Stephen Finucane <stephen@that.guru>
This is failing since the update to Xenial [1] with the following
warning:
$ sudo -u postgres psql -c "grant ALL on DATABASE postgres to travis WITH GRANT OPTION;"
could not change directory to "/home/travis/build/getpatchwork/patchwork": Permission denied
psql: could not connect to server: No such file or directory
Is the server running locally and accepting
connections on Unix domain socket "/var/run/postgresql/.s.PGSQL.5433"?
Bump the distro version to xenial, as this is the first version to
include Python 3.7 support. Bionic is also available but it doesn't
support Python 3.5 [1].
In addition, skip a test that was valid on Python 3.4 - 3.6 but does not
appear to be an issue for Python 3.7.
Ali Alnubani [Wed, 4 Sep 2019 13:40:31 +0000 (13:40 +0000)]
sql: Fix table lists
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,
...
Signed-off-by: Ali Alnubani <alialnu@mellanox.com> Reviewed-by: Stephen Finucane <stephen@that.guru> Fixes: ca0e79d4db34 ("sql: Sort 'grant-all' scripts alphabetically")
migrations: Correct 'unique_together' order in '0015'
This was resulting in exceptions like the following when used with MySQL
8.0:
Traceback (most recent call last):
File "../patchwork/manage.py", line 11, in <module>
execute_from_command_line(sys.argv)
...
File "../.tox/py27-django111/lib/python2.7/site-packages/django/db/backends/mysql/schema.py", line 88, in _delete_composed_index
return super(DatabaseSchemaEditor, self)._delete_composed_index(model, fields, *args)
File "../.tox/py27-django111/lib/python2.7/site-packages/django/db/backends/base/schema.py", line 394, in _delete_composed_index
", ".join(columns),
ValueError: Found wrong number (0) of constraints for patchwork_seriespatch(series_id, number)
This error was being raised by the following lines in the 0033
migration:
It appears that this is because of a mismatch between the order of
fields in a 'unique_together' constraint [1]. Correct the order in the
original migration and see the issue disappear.
Signed-off-by: Stephen Finucane <stephen@that.guru> Fixes: d67d859f40f ("models: Add 'Series' model")
As was designed, starting the interpreter would cause the State model
and its entries to be evaluated. This was an issue if, for example, the
model had been modified and you were attempting to apply the migration.
Traceback (most recent call last):
File "manage.py", line 11, in <module>
execute_from_command_line(sys.argv)
...
File "/usr/local/lib/python2.7/dist-packages/django/forms/models.py", line 1199, in _set_queryset
self.widget.choices = self.choices
File "/home/patchwork/patchwork/patchwork/forms.py", line 157, in _get_choices
super(OptionalModelChoiceField, self)._get_choices())
File "/usr/local/lib/python2.7/dist-packages/django/forms/models.py", line 1143, in __len__
return (len(self.queryset) + (1 if self.field.empty_label is not None else 0))
File "/usr/local/lib/python2.7/dist-packages/django/db/models/query.py", line 232, in __len__
self._fetch_all()
File "/usr/local/lib/python2.7/dist-packages/django/db/models/query.py", line 1118, in _fetch_all
self._result_cache = list(self._iterable_class(self))
File "/usr/local/lib/python2.7/dist-packages/django/db/models/query.py", line 53, in __iter__
results = compiler.execute_sql(chunked_fetch=self.chunked_fetch)
File "/usr/local/lib/python2.7/dist-packages/django/db/models/sql/compiler.py", line 899, in execute_sql
raise original_exception
django.db.utils.OperationalError: (1054, "Unknown column 'patchwork_state.slug' in 'field list'")
Resolve this by moving the evaluation into '__init__', meaning it will
only occur when a new form is created.
Signed-off-by: Stephen Finucane <stephen@that.guru>
Michael Ellerman [Tue, 27 Aug 2019 06:13:13 +0000 (16:13 +1000)]
models: Add commit_url_format to Project
Add a new field to Project, commit_url_format, which specifies a
format string that can be used to generate a link to a particular
commit for a project.
This is used in the display of a patch, to render the patch's commit
as a clickable link back to the commit on the SCM website.
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au> Signed-off-by: Daniel Axtens <dja@axtens.net>
Daniel Axtens [Wed, 21 Aug 2019 05:15:44 +0000 (15:15 +1000)]
mbox: do not copy Content-Type into exported mbox
Daniel reports a patch + comment combination that breaks in
git am. The patch reports a Content-Type of US-ASCII, while
the comment adds a Ack with UTF-8 characters. The exported
mbox contains both the original Content-Type, and a UTF-8
Content-Type that we set. However, because the US-ASCII one
occurs later, git am honours it instead of ours, and chokes
on the UTF-8 characters.
Strip out any subsequent Content-Type:s. We normalise things
to UTF-8 and should not allow it to be overridden.
Add a test for this, based on the original report.
Reported-by: Daniel Borkmann <daniel@iogearbox.net> Signed-off-by: Daniel Axtens <dja@axtens.net>
docker: *Actually* don't require rebuilding if unnecessary
Because we were using 'set -e', we were erroring out as soon as
something - a diff in this case - failed. Temporarily disable it for
this one check.
Signed-off-by: Stephen Finucane <stephen@that.guru> Fixes: 0b5b4e8c ("docker: Don't require rebuilding if unnecessary") Cc: Daniel Axtens <dja@axtens.net> Signed-off-by: Daniel Axtens <dja@axtens.net>
Mete Polat [Mon, 15 Jul 2019 14:50:28 +0000 (16:50 +0200)]
Add 'dumparchive' management command
Introduces a new management command which can export all patches in a
project as one mbox file. Export of multiple projects is supported.
Additionally allows to compress the output.
Signed-off-by: Mete Polat <metepolat2000@gmail.com> Signed-off-by: Stephen Finucane <stephen@that.guru>
- Remove some newlines between terms and definitions that were causing
the latter to be rendered as blockquotes instead
- Order list of settings alphabetically
- Update URLs to use latest version of Django we support
Signed-off-by: Stephen Finucane <stephen@that.guru>
templatetags: Do not mark output of msgid tag as safe
The msgid template tag exists to remove angle brackets from either side of
the Message-ID header.
It also marks its output as safe, meaning it does not get autoescaped by
Django templating.
Its output is not safe. A maliciously crafted email can include HTML tags
inside the Message-ID header, and as long as the angle brackets are not at
the start and end of the header, we will quite happily render them.
Rather than using mark_safe(), use escape() to explicitly escape the
Message-ID.
Signed-off-by: Andrew Donnellan <ajd@linux.ibm.com> Signed-off-by: Daniel Axtens <dja@axtens.net>
Stephen Finucane [Sun, 21 Oct 2018 11:05:58 +0000 (12:05 +0100)]
Remove pwclient
Let's start managing this via a separate project, which will allow the
client to evolve separately from the server. No redirect is added for
the old '/pwclient' URL as it seems wiser to return a HTTP 404 error
code.
Signed-off-by: Stephen Finucane <stephen@that.guru>
Commit 753e4572d updated the parser to consider additional header lines
when deciding where a patch message ends and the diff begins. However,
these additional lines were not captured meaning these patches didn't
have a diff associated with them and they therefore weren't patches in
the Patchwork sense of the term. Correct this and add a test.
Signed-off-by: Stephen Finucane <stephen@that.guru>
Petr Vorel [Sun, 5 May 2019 20:20:05 +0000 (22:20 +0200)]
parser: Add missing extended header lines
Patchwork didn't recognise some patches due missing some extended header
lines (e.g. "old mode" and "new mode" for renaming file mode, see [1]).
Thus adding all modes from git doc [2].
Stephen Finucane [Tue, 14 May 2019 14:56:53 +0000 (15:56 +0100)]
docs: Document backport criteria
Explain why we don't want to be in the business of backport certain
patches, in the long run. It took me a while to put this into words but
I was helped by a similar discussion ongoing in the OpenStack community
at the moment [1].
In July 2018, we received a report of OzLabs patchwork mangling
emails that have subjects containing words with internal commas,
like "Insert DT binding for foo,bar" (#197).
Stephen took a look and came up with the comment this reverts. Quoting
the commit message:
RFC2822 states that long headers can be wrapped using CRLF followed by
WSP [1]. For example:
Subject: Foo bar,
baz
Should be parsed as:
Foo bar,baz
As it turns out, this is not the case. Journey with me to
section 2.2.3 of RFC 2822:
2.2.3. Long Header Fields
Each header field is logically a single line of characters comprising
the field name, the colon, and the field body. For convenience
however, and to deal with the 998/78 character limitations per line,
the field body portion of a header field can be split into a multiple
line representation; this is called "folding". The general rule is
that wherever this standard allows for folding white space (not
simply WSP characters), a CRLF may be inserted before any WSP. For
example, the header field:
Subject: This is a test
can be represented as:
Subject: This
is a test
So the issue with the example in the reverted commit is that there is no
folding white space in "bar,baz", so it's not valid to split it.
These are valid:
Subject: Foo bar,baz
Subject: Foo
bar,baz
but splitting "bar,baz" into "bar,\n baz" is not valid.
What then is correct unfolding behaviour? Quoting the RFC again:
The process of moving from this folded multiple-line representation
of a header field to its single line representation is called
"unfolding". Unfolding is accomplished by simply removing any CRLF
that is immediately followed by WSP. Each header field should be
treated in its unfolded form for further syntactic and semantic
evaluation.
In other words, the unfolding rule requires you to strip the CRLF, but
it does not permit you to strip the WSP. Indeed, if "bar,\n baz" is
received, the correct unfolding is "bar, baz".
If you do strip the WSP, you end up mashing words together, such as in
https://patchwork.ozlabs.org/patch/1097852/
So revert the commit, restoring original behaviour, but keep a corrected
version of the test.
This presents a big question though: how did Rob's email up with a
mangled subject line?
To answer this question, you end up having to learn about OzLabs
Patchwork and how it differs from Patchwork the project.
OzLabs Patchwork (patchwork.ozlabs.org) is an installation of Patchwork.
Part of what makes it so useful for so many projects is a little
intervening layer that can massage some mail to make it end up in the
right project. Email that lands in the device tree project is an example
of email that goes through this process. I only learned about this
today and I haven't looked in any detail at precisely what is done to
the mail. The script is not part of the Patchwork project.
This intervening filter is a Python script that runs - and this is an
important detail - in Python 2.7.
Ignoring all the details, the filter basically operates in a pipe
between the mail program and patchwork's parsemail, like
(mail from system) | filter.py | parsemail
At it's very simplest, filter.py acts as follows:
import email
import sys
mail = email.parse_from_file(sys.stdin)
sys.stdout.write(mail.as_string())
Fascinatingly, if you take Rob's email from #197 and put it through this
process, you can see that it is getting mangled:
You can see that python27 has incorrectly wrapped the header, breaking
where there is not a foldable space. Python3 does not have this issue.
To summarise:
- part of the magic of OzLabs PW is a filter to make sure mail gets to
the right place. This isn't part of the Patchwork project and so is
usually invisible to patchwork developers.
- the filter is written in python27. The email module in py27 has a bug
that incorrectly breaks subjects around commas within words.
- patchwork correctly unfolds those broken subjects with a space after
the comma.
- the extra space was interpreted as a bug in patchwork, leading to a
misinterpretation of the spec to strip out the whitespace that was
believed to be in error.
- that broke other wrapped subjects.
To solve this, revert the commit and I'll work with jk to get the filter
script into py3 compatibility. (Given that py27 sunsets in ~7mo, trying
to fix it is not worth it.)
Closes: #273 Signed-off-by: Daniel Axtens <dja@axtens.net> Signed-off-by: Stephen Finucane <stephen@that.guru>
[stephenfin: Use a new release note instead of editing the original one]
Russell Currey [Wed, 1 May 2019 06:27:18 +0000 (16:27 +1000)]
docs: Mention Postgres for Docker development install
Might as well since it's there, and it gives some clue to anyone trying
to use Docker on non-x86. I figured it was best to leave this out of
the README since it's incredibly niche.
Signed-off-by: Russell Currey <ruscur@russell.cc> Reviewed-by: Andrew Donnellan <andrew.donnellan@au1.ibm.com> Signed-off-by: Daniel Axtens <dja@axtens.net>
Russell Currey [Wed, 1 May 2019 06:27:17 +0000 (16:27 +1000)]
docker: Use Ubuntu ports repositories on non-x86 architectures
This should allow Patchwork to run "out of the box" in Docker on any
architecture with a) an Ubuntu port and b) support in the Postgres
multiarch Docker image, which includes at least arm64 and ppc64le.
It's a little gross hacking the Dockerfile like this, but I'm not sure
there's a more elegant way to do it. Unfortunately it doesn't seem like
there's any way to do conditional COPY, and anything in RUN is plain
/bin/sh, so that's why it looks like it does.
Tested on ppc64le and on x86_64.
Signed-off-by: Russell Currey <ruscur@russell.cc> Reviewed-by: Andrew Donnellan <andrew.donnellan@au1.ibm.com> Signed-off-by: Daniel Axtens <dja@axtens.net>
Russell Currey [Wed, 1 May 2019 04:35:53 +0000 (14:35 +1000)]
docker: Install libpq-dev to fix psycopg2-binary build
psycopg2-binary fails if pg_config isn't installed, which is provided by
libpq-dev.
This seems strange to me since psycopg2-binary suggests that
you use psycopg2-binary instead (of itself) if you don't want to build
psycopg2 so you wouldn't need pg_config, which is very confusing.
It's possible that psycopg2-binary only needs to compile itself on
non-x86 platforms, since I hit this on ppc64le.
Anyway, it works when this is added.
Signed-off-by: Russell Currey <ruscur@russell.cc> Reviewed-by: Andrew Donnellan <andrew.donnellan@au1.ibm.com> Signed-off-by: Daniel Axtens <dja@axtens.net>
Russell Currey [Wed, 1 May 2019 03:16:43 +0000 (13:16 +1000)]
README: add .env file to installation instructions
Creating the .env file is mentioned in the installation documentation
but not in the README, so following only the steps mentioned there will
fail. Add this and add a `cd patchwork` in there for good measure so
you could straight up copy paste the steps.
Signed-off-by: Russell Currey <ruscur@russell.cc> Reviewed-by: Andrew Donnellan <andrew.donnellan@au1.ibm.com> Signed-off-by: Daniel Axtens <dja@axtens.net>
Daniel Axtens [Mon, 29 Apr 2019 15:33:16 +0000 (01:33 +1000)]
REST: Handle regular form data requests for checks
08d1459a4a40 ("Add REST API validation using OpenAPI schema") moved
all API requests to JSON blobs rather than form data.
dc48fbce99ef ("REST: Handle JSON requests") attempted to change the
check serialiser to handle this. However, because both a JSON dict
and a QueryDict satisfy isinstance(data, dict), everything was handled
as JSON and the old style requests were broken.
Found in the process of debugging issues from the OzLabs PW & Snowpatch
crew - I'm not sure if they actually hit this one, but kudos to them
anyway as we wouldn't have found it without them.
Fixes: dc48fbce99ef ("REST: Handle JSON requests") Signed-off-by: Daniel Axtens <dja@axtens.net>
Jeremy Kerr [Sat, 27 Apr 2019 11:12:16 +0000 (19:12 +0800)]
notifications: fix notification expiry when no user is associated
It's possible that an EmailConfirmation object will have no associated
user (eg, for email opt-out, which does not require a user object). In
this case, we will see a NULL value for EmailConfirmation.user_id.
However, having a NULL value appear in a SQL 'IN' clause will match
every value. This means that once one of these null-user
EmailConfirmations is present, we will never expire any non-active user
accounts.
This change adds a filter for a valid user_id when we query for active
EmailConfirmation objects. This means we'll have a valid values set to
use in the pending_confs set.
Signed-off-by: Jeremy Kerr <jk@ozlabs.org>
[dja: fix pep8 issue] Signed-off-by: Daniel Axtens <dja@axtens.net>
Daniel Axtens [Fri, 15 Mar 2019 06:27:40 +0000 (17:27 +1100)]
Fix YAML loader warning
In my tests I'm seeing:
/home/patchwork/patchwork/patchwork/tests/api/validator.py:229:
YAMLLoadWarning: calling yaml.load() without Loader=... is deprecated,
as the default Loader is unsafe. Please read https://msg.pyyaml.org/load
for full details.
Fix this by using the safe loader in the tests.
Signed-off-by: Daniel Axtens <dja@axtens.net> Reviewed-by: Stephen Finucane <stephen@that.guru>
Daniel Axtens [Thu, 28 Feb 2019 04:29:53 +0000 (15:29 +1100)]
parser: recognise git commit consisting only of empty new file
Commits with only an empty new file are liable to be missed.
The parser state machine doesn't recognise the headers "new
file mode" and "index": teach it about them.
Add a test to demonstrate.
It's a little bit academic as you don't usually send patches like
that but sometimes you do, especially if you're a snowpatch dev :)
Closes: #256 Reported-by: Andrew Donnellan <andrew.donnellan@au1.ibm.com> Signed-off-by: Daniel Axtens <dja@axtens.net> Reviewed-by: Andrew Donnellan <andrew.donnellan@au1.ibm.com> Reviewed-by: Stephen Finucane <stephen@that.guru>
Ali Alnubani [Tue, 8 Jan 2019 12:38:47 +0000 (12:38 +0000)]
Beautify check counts in the patch list view
This patch [1] adds colors to the checks in the patch list view.
The colors are set based on the check's priority, with FAILURE
having the highest priority, followed by WARNING, and then SUCCESS.
Only the check with the highest priority and non-zero count
will be colored. This is to make failures and warnings more visible.
The patch also [2] replaces zero counts with a '-' for
FAILUREs and WARNINGs.
The SUCCESS count will only be replaced by a '-'
when all other checks have zero counts too.
Suggested-by: Thomas Monjalon <thomas@monjalon.net> Signed-off-by: Ali Alnubani <alialnu@mellanox.com> Signed-off-by: Stephen Finucane <stephen@that.guru>
Ali Alnubani [Tue, 8 Jan 2019 12:38:45 +0000 (12:38 +0000)]
Fix return code when getting patch information fails
The `info` command always exits with success, even if
the patch didn't exist.
Modified to exit with a non-zero exit status and
print an error message in that case.
Signed-off-by: Ali Alnubani <alialnu@mellanox.com> Reviewed-by: Stephen Finucane <stephen@that.guru>
It turns out it is possible to make PATCH requests with JSON bodies
rather than form-encoded data - you just need to include a Content-Type
header. Document this.
Signed-off-by: Stephen Finucane <stephen@that.guru>
projects / thirdparty / patchwork.git / log
