Doug Anderson [Sat, 19 Nov 2016 19:32:09 +0000 (19:32 +0000)]
views: Don't munge the 'From' field of patches
At the moment patchwork always uses the official submitter name (as
patchwork understands it) as the "From" for patches that you receive.
This isn't quite what users expect and has some unfortunate
consequences.
The biggest problem is that patchwork saves the "official" name for an
email address the first time it sees an email from them. If that name
is wrong (or was missing) patchwork will be confused even if future
emails from this person are fixed. There are similar problems if a
user changes his/her name (get married?).
It seems better to just have each patch report the actual "From" that
was used to send that patch. We'll still return the submitter in
'X-Patchwork-Submitter' just in case someone wants it.
Reported-by: Wolfram Sang <wsa@the-dreams.de> Signed-off-by: Doug Anderson <dianders@chromium.org> Signed-off-by: Stephen Finucane <stephen@that.guru>
Robin Jarry [Thu, 15 Dec 2016 16:56:18 +0000 (17:56 +0100)]
pwclient: Fix encoding problems
All data returned by the xmlrpc object is unicode decoded with 'utf-8' (on
python 3, unicode == str). Add from __future__ import unicode_literals
to make sure that everything is unicode and avoid surprises.
On python 2, printing unicode to stdout causes it to be encoded to str
(byte string) with the 'ascii' codec:
>>> print some_unicode_string
...
UnicodeEncodeError: 'ascii' codec can't encode character u'\u0142'
in position 468: ordinal not in range(128)
Work around ths by avoiding any explicit call to unicode() and by
replacing sys.stdout and sys.stderr by unicode-aware file objects (as
returned by io.open()).
Guess the encoding of stdout and stderr by looking at (in that order):
sys.stdout.encoding, locale.getpreferredencoding(), the PYTHONIOENCODING
environment variable. If no encoding is defined, assume 'utf-8' as
output encoding.
Signed-off-by: Robin Jarry <robin.jarry@6wind.com> Tested-by: Thomas Monjalon <thomas.monjalon@6wind.com> Reviewed-by: Stephen Finucane <stephen@that.guru>
Thomas Monjalon [Tue, 13 Dec 2016 10:37:47 +0000 (11:37 +0100)]
pwclient: Fix Python 3 encoding of received strings
The conversion encode("utf-8") makes a byte stream which is
poorly printed with Python 3.
However this encoding is required for Popen.communicate() but must be
done after str.join() which applies to a real string.
Signed-off-by: Thomas Monjalon <thomas.monjalon@6wind.com> Reviewed-by: Stephen Finucane <stephen@that.guru>
Stephen Finucane [Wed, 30 Nov 2016 18:29:37 +0000 (18:29 +0000)]
tools: Update to use 'hasher'
The old 'parser' module used to extract diffs from their surrounding
mbox fluff before hashing this. Seeing as this was only used in the
context of an actual git repo, avoid all of that rigmarole by just using
'git diff', which produces a plain diff, rather than 'git show'.
Signed-off-by: Stephen Finucane <stephen@that.guru> Tested-by: Tom Rini <trini@konsulko.com>
Closes-bug: #63
Stephen Finucane [Fri, 18 Nov 2016 00:54:51 +0000 (00:54 +0000)]
tools: Trivial formatting fixes
These tools are currently broken, but before beginning surgery let's
clean things up. Use standard 4 spaces and the longer, but easier to
read, if-else-fi syntax for comparison.
Missing license headers are added for completeness sake.
Signed-off-by: Stephen Finucane <stephen@that.guru> Tested-by: Tom Rini <trini@konsulko.com>
Andrew Donnellan [Wed, 23 Nov 2016 07:01:31 +0000 (18:01 +1100)]
views: don't duplicate tags in patch message when generating mbox
When generating an mbox for a patch with tags in the original commit
message, e.g.:
Example patch
This patch is awesome!
Signed-off-by: Andrew Donnellan <andrew.donnellan@au1.ibm.com> Acked-by: Russell Currey <ruscur@russell.cc>
the tags from the original email are duplicated:
Example patch
This patch is awesome!
Signed-off-by: Andrew Donnellan <andrew.donnellan@au1.ibm.com> Acked-by: Russell Currey <ruscur@russell.cc> Signed-off-by: Andrew Donnellan <andrew.donnellan@au1.ibm.com> Acked-by: Russell Currey <ruscur@russell.cc>
It appears that during the refactoring in ef56359fb776 ("models: Merge
patch and first comment"), we added a call to patch.patch_responses() to
extract the tags from the initial patch email, which we then append to the
patch email body... which already has the tags in it.
Remove the unnecessary append of patch.patch_responses when generating an
mbox.
Fixes: ef56359fb776 ("models: Merge patch and first comment") Reported-by: Russell Currey <ruscur@russell.cc> Signed-off-by: Andrew Donnellan <andrew.donnellan@au1.ibm.com> Reviewed-by: Daniel Axtens <dja@axtens.net> Reviewed-by: Stephen Finucane <stephen@that.guru>
Stephen Finucane [Thu, 24 Nov 2016 09:24:13 +0000 (09:24 +0000)]
trivial: Resolve pep8 issues
flake8 3.2.0 updates the pycodestyle dependency to 2.2.0, which in
turn resolves a bug with E305. Upgrading results in a small number of
additional pep8 issues. Resolve these now.
Signed-off-by: Stephen Finucane <stephen@that.guru>
Stephen Finucane [Sat, 29 Oct 2016 13:18:28 +0000 (14:18 +0100)]
admin: Integrate UserProfile fields into admin
The 'User' model is extended by means of a 'UserProfile' model. These
fields are not correctly displayed in the admin UI, but they should be.
Let's fix this per the recommendations of the Django docs [1].
Daniel Axtens [Thu, 17 Nov 2016 05:11:59 +0000 (16:11 +1100)]
Fix parsing of interesing series reply structures
There are some things you probably shouldn't do on public
mailing lists, but which people do anyway.
The first, and most understandable, is this:
- [PATCH 1/2] test: Add some lorem ipsum
- [PATCH 2/2] test: Convert to Markdown
- [PATCH v2 1/2] test: Add some lorem ipsum
- [PATCH v2 2/2] test: Convert to Markdown
We should correctly parse these by:
- creating a new series if the version number changes
- when deciding whether to create a SeriesReference, search by
message-id alone, not the message-id/series pair. (Otherwise,
we try to create a series ref for v1 2/2 in the series for v2,
which breaks a uniqueness constraint.
The second, and less excusable, is this:
- [PATCH 1/2] test: Add some lorem ipsum
- [PATCH 2/2] test: Convert to Markdown
- [PATCH 1/2] test: Add some lorem ipsum
- [PATCH 2/2] test: Convert to Markdown
With this patch:
- if we get a x/n for a series that already has an x/n, create a
new series for it.
Signed-off-by: Daniel Axtens <dja@axtens.net> Reviewed-by: Stephen Finucane <stephen@that.guru>
Daniel Axtens [Wed, 16 Nov 2016 05:58:44 +0000 (16:58 +1100)]
Fix parsing of un-numbered messages in series
Say we are sent the following:
- [PATCH 0/2] A sample series
- [PATCH 1/2] test: Add some lorem ipsum
- Random message with diff
We expect that:
1) we parse normally without errors
2) we get a series with a cover letter and a patch
3) the random message is orphaned
What happens is that we get an integrity error, boiling down to:
(1048, "Column 'number' cannot be null")
This is caused because we believe that the random message belongs
to the series because of the headers, but because there are no
numbers in the Subject, we pass "None" into the number field of
SeriesPatch. That turns into a null, and rightly hits an integrity
error.
Fix this by requring that a message has a series _and_ a number
before we try to add it to the series.
Add a test to verify correctness.
Reported-by: Daniel Wagner <wagi@monom.org> Signed-off-by: Daniel Axtens <dja@axtens.net> Reviewed-by: Stephen Finucane <stephen@that.guru>
Stephen Finucane [Sat, 29 Oct 2016 13:13:40 +0000 (14:13 +0100)]
templates: Integrate series view into patches
Patches for related series are listed on the patch page - this
provides a way to quickly grok a given patches location in a
series hierarchy.
Signed-off-by: Stephen Finucane <stephen@that.guru> Reviewed-by: Andy Doan <andy.doan@linaro.org> Reviewed-by: Daniel Axtens <dja@axtens.net> Tested-by: Russell Currey <ruscur@russell.cc>
Stephen Finucane [Sat, 29 Oct 2016 13:13:39 +0000 (14:13 +0100)]
templates: Integrate series support
Integrate support for series in the web UI. This is rather
straightforward, the only significant change being the addition of a
filter for series filtering.
Signed-off-by: Stephen Finucane <stephen@that.guru> Reviewed-by: Andy Doan <andy.doan@linaro.org> Tested-by: Russell Currey <ruscur@russell.cc> Reviewed-by: Daniel Axtens <dja@axtens.net> Reviewed-by: Andrew Donnellan <andrew.donnellan@au1.ibm.com>
Stephen Finucane [Sat, 29 Oct 2016 13:13:38 +0000 (14:13 +0100)]
filters: Handle invalid ids
Two filters - SubmitterFilter and DelegateFilter - don't attempt to
handle invalid id values and will bubble an exception up as a 5xx
error. Correct this.
Signed-off-by: Stephen Finucane <stephen@that.guru> Reviewed-by: Andrew Donnellan <andrew.donnellan@au1.ibm.com>
Stephen Finucane [Sat, 29 Oct 2016 13:13:35 +0000 (14:13 +0100)]
parser: Add series parsing
It is now possible to parse and store series, so do just that.
The parsing at the moment is based on both RFC822 headers and
subject lines.
Signed-off-by: Stephen Finucane <stephen@that.guru> Reviewed-by: Andy Doan <andy.doan@linaro.org> Reviewed-by: Daniel Axtens <dja@axtens.net> Reviewed-by: Andrew Donnellan <andrew.donnellan@au1.ibm.com> Tested-by: Russell Currey <ruscur@russell.cc>
Stephen Finucane [Sat, 29 Oct 2016 13:13:34 +0000 (14:13 +0100)]
models: Add 'Series' model
Add a series model. This model is expected to act like a collection for
patches, similar to bundles but thread-orientated.
Signed-off-by: Stephen Finucane <stephen@that.guru> Reviewed-by: Andy Doan <andy.doan@linaro.org> Reviewed-by: Daniel Axtens <dja@axtens.net> Reviewed-by: Andrew Donnellan <andrew.donnellan@au1.ibm.com> Tested-by: Russell Currey <ruscur@russell.cc>
Stephen Finucane [Sat, 29 Oct 2016 13:13:33 +0000 (14:13 +0100)]
models: Convert functions to properties
A number of models contain functions that are, semantically speaking,
actually properties. Mark them as such.
Signed-off-by: Stephen Finucane <stephen@that.guru> Reviewed-by: Andy Doan <andy.doan@linaro.org> Reviewed-by: Andrew Donnellan <andrew.donnellan@au1.ibm.com>
Stephen Finucane [Sat, 22 Oct 2016 11:23:27 +0000 (12:23 +0100)]
views: Update how patch counts are retrieved
It's no longer possible to access 'Project.patch_set' as
'Project.submission_set' has replaced it. This was missed when the
cover letter feature was merged, so resolve it now.
Signed-off-by: Stephen Finucane <stephen@that.guru> Reviewed-by: Daniel Axtens <dja@axtens.net>
Closes-bug: #54
Stephen Finucane [Thu, 20 Oct 2016 07:08:39 +0000 (08:08 +0100)]
docs: Add getmail documentation
It seems a lot of people are having success using tools like fetchmail
in combination with IMAP/POP-capable email accounts like Gmail. While
fetchmail itself is rather decrepit, the Python-based getmail seems
actively supported and pretty easy to configure. Document this process.
Signed-off-by: Stephen Finucane <stephen@that.guru> Reviewed-by: Daniel Axtens <dja@axtens.net>
Stephen Finucane [Thu, 20 Oct 2016 07:28:53 +0000 (08:28 +0100)]
models: Make use of aggregates
We're well past Django 1.1 now, so resolve a TODO to use aggregate
support introduced in this version. As part of this change, replace
the use of 'count' to check for presence of matching objects with
'exists'.
Signed-off-by: Stephen Finucane <stephen@that.guru> Reviewed-by: Daniel Axtens <dja@axtens.net>
Stephen Finucane [Tue, 11 Oct 2016 17:14:36 +0000 (18:14 +0100)]
docs: Rework XML-RPC docs
Add a barebones usage guide for pwclient and simplify the existing doc
for this API. This entails move the existing doc from the deployment
section to development section, which makes more sense given its focus.
Signed-off-by: Stephen Finucane <stephen@that.guru>
The 'future.cycle' template tag is removed in Django 1.10. Since
Django 1.7 is still supported by Patchwork, it is necessary to
provide a custom 'cycle' wrapper to prevent the deprecation
warnings resolved by '9cab078' being reintroduced.
Signed-off-by: Stephen Finucane <stephen@that.guru> Reviewed-by: Daniel Axtens <dja@axtens.net>
Daniel Axtens [Thu, 22 Sep 2016 02:15:09 +0000 (12:15 +1000)]
parsemail: Convert to a management command
Management comands allow applications to register their own actions
with 'manage.py'. This provides some advantages, like automatically
configuring Django (removing the need for 'django.setup' calls) and
removing the need to set the PYTHON_PATH. The 'parsemail' script is a
natural fit for this type of application. Migrate 'parsemail' to a
management command.
This includes some extensive work on logging configuration, as logging
is moved from code into settings. In addition, it removes a lot of the
customizable logging previously introduced in the parsemail command, in
favour of modifications to the settings files.
Signed-off-by: Daniel Axtens <dja@axtens.net> Signed-off-by: Stephen Finucane <stephen@that.guru>
Partial-bug: #17
Closes-bug: #15
Daniel Axtens [Tue, 20 Sep 2016 08:29:06 +0000 (18:29 +1000)]
parser tests: test parsing of interesting mail
We have 4 new tests:
- One with a header with a character that is neither 7-bit ASCII
nor UTF-8.
- One with UTF-8 in the body.
- One with un-encoded UTF-8 in the headers.
- One with a header that claims to be UTF-8 but isn't.
This also requires us to change how we open files on Python 3. Use
the binary file and message_from_binary_file setup under Python 3.
Signed-off-by: Daniel Axtens <dja@axtens.net> Reviewed-by: Stephen Finucane <stephen@that.guru>
Daniel Axtens [Wed, 28 Sep 2016 04:06:07 +0000 (14:06 +1000)]
parser: subject_check needs to call clean_header
When testing with headers with unencoded characters on Py3,
subject_check would fail. It was getting an Header class, rather
than a str/bytes, and so the regex was failing.
Run clean_header to decode everything fully first.
Signed-off-by: Daniel Axtens <dja@axtens.net> Reviewed-by: Stephen Finucane <stephen@that.guru>
Daniel Axtens [Mon, 19 Sep 2016 14:23:58 +0000 (00:23 +1000)]
parser: parse headers containing invalid characters or codings
If there is a non-ascii character in a header, parsing fails,
even on Py27.
This has huge Py2/Py3 complexities. The Py3 email package has tools
to handle this - we just need to use them. Py2, on the other hand,
needs a lot of hand-holding, as explained in the comments.
Additionally, support headers that claim an encoding, but fail to
decode with that encoding.
This is handy for mails with malformed headers containing weird
bytes.
Signed-off-by: Daniel Axtens <dja@axtens.net> Reported-by: Thomas Monjalon <thomas.monjalon@6wind.com> Reviewed-by: Stephen Finucane <stephen@that.guru>
Daniel Axtens [Tue, 20 Sep 2016 06:43:24 +0000 (16:43 +1000)]
parsemail.sh: make testing easier
- Take PW_PYTHON, allowing the python version to be set
- Only override DJANGO_SETTINGS_MODULE if it's not in the env
- Pass parameters to parsemail.py
Signed-off-by: Daniel Axtens <dja@axtens.net> Signed-off-by: Stephen Finucane <stephen@that.guru>
Stephen Finucane [Sun, 25 Sep 2016 21:00:47 +0000 (22:00 +0100)]
Update mailmap with new email
Hotmail is unfortunately not standards compliant and has a bad habit of
rewriting message-ids. This breaks threading for both Mailman and
Patchwork. Clearly it's time to switch to another email address.
Signed-off-by: Stephen Finucane <stephen@that.guru>
The 'with' statement does not start an exception block like 'try' would,
so we get an error executing pwclient:
File "/home/ymorin/bin/pwclient", line 300
except IOError:
^
SyntaxError: invalid syntax
Instead, don't try to catch any exception: any failure will be
propagated back up the stack and eventually cause an abort, which is
anyway what the except clause was trying to do.
Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr> Cc: Stephen Finucane <stephenfinucane@hotmail.com> Reviewed-by: Stephen Finucane <stephenfinucane@hotmail.com>
Stephen Finucane [Mon, 19 Sep 2016 22:15:26 +0000 (23:15 +0100)]
trivial: noqa any 'assert' functions
There are advantages to sticking with unittest's use of camel case in
function names, but some code tools don't like this. noqa these lines
to hide the warnings.
Signed-off-by: Stephen Finucane <stephenfinucane@hotmail.com> Reviewed-by: Daniel Axtens <dja@axtens.net>
* The behavior of 'Bundle.add_patch' is changed. Previously this would
raise an exception if the provided patch already existed in the
bundle. Since this code was only used in one location, change this to
return the BundlePatch if valid else None and change the calling code
to check return value instead of catching the exception.
* Use a context manager to open the config file in pwclient. This loses
a little granularity in error messaging, but this is a worthy
compromise.
Signed-off-by: Stephen Finucane <stephenfinucane@hotmail.com> Reviewed-by: Daniel Axtens <dja@axtens.net>
Daniel Axtens [Thu, 15 Sep 2016 08:10:29 +0000 (18:10 +1000)]
Optimise querying of checks in patch list view
tl;dr: with about 300 mails from the patchwork list, according to
django-debug-toolbar, to render '/project/patchwork/list/'
Without this patch:
- ~1.35 seconds of CPU time
- 110 SQL queries, taking ~70ms
With this patch:
- < 0.3 seconds of CPU time
- 10 SQL queries, taking <20ms
How? Replace an .exclude() on a QuerySet with a list comprehension.
Yes, that's normally a pessimisation. Surprisingly, it's an
optimisation here. Why? Where we're looking at patches in anything
that uses a generic_list() in the view, we do a prefetch_related. But,
if we then do a .filter or a .exclude, that throws out the existing,
cached information, and does another query. (See the Django docs on
prefetch_related)
So, do it 'by hand' in Python instead.
Signed-off-by: Daniel Axtens <dja@axtens.net> Reviewed-by: Stephen Finucane <stephenfinucane@hotmail.com>