Jacob Keller [Thu, 9 Jan 2014 23:28:32 +0000 (15:28 -0800)]
email-lda: use cover letter subject if available
Often, a patch series will be sent with a cover letter which describes the
series and is given the '0/n' patch number. Rather than dropping these
patches, keep track of them in series. We do this by checking whether the 1/n
patch has an "In-Reply-To" header. This usually means that their will be an
associated cover letter. Once this is found, rather than sending the patch
directly on to the test program, we use the cover letter's subject by using
formail to add a special "Series-Subject". This makes it so that the
aiaiai-email will end up as a reply to the cover letter which is a bit more
aesthetically pleasing to users.
To help this process, a new "series_is_complete" function is added in place of
the original series complete check. This function reports whether the series
is completely queued, for both cases of cover letter and no cover letter.
Artem:
I've modified this patch.
1. Removed a couple of '[[' bash constructs
2. Also preserved the Message ID of the cover letter to make the reply not only
have the subject of the cover letter, but also refer the cover letter
properly.
3. Use common prefix for the special cover letter e-mail headers that we add:
X-Aiaiai-Cover-Letter-Subject
X-Aiaiai-Cover-Letter-Message-Id
4. Dropped sponge dependency. Generally, this is a nice tool, and I did not
hear about it before. I'll definitely start using it when constructing pipes
in the console. But this tool is not that well-known, so it is not typically
present in systems. E.g., mine do not have it. And I decided to avoid adding
another dependency, we already require a lot of tools. Instead, I used
another trick, which is actual just as elegant as using sponge. Well, needs
more lines of code, but it is really nice trick anyway, I like it.
I just open the mbox file, then unlink it, which means it won't be visible
in the file-system, but won't be deleted since we have the file descriptor
opened. Then I pipe the data from the file descriptor to 'formail', and
redirect the results to the mbox file. So a new mbox file will be created
and all the new contents will be there. I think this is elegant, and does
not add a new dependency.
Signed-off-by: Jacob Keller <jacob.e.keller@intel.com> Signed-off-by: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>
Artem Bityutskiy [Wed, 5 Feb 2014 14:20:13 +0000 (16:20 +0200)]
email-lda: improve consistency and readability
Similarly to what we did in 'aiaiai-email-test-patchset', use 'reply_*'
variable to specify the reply recipient, the Cc list, the subject, and the
message ID. This is a bit easier to read and consistent.
This patch is a bug-fix. It fixes the 'strip_address()' and 'merge_addresses()'
which were for some reason removing all the blanks from the list of addresses.
The result was that "Artem Bityutskiy" became "ArtemBityutskiy".
Instead of removing all blanks, it is supposed to make multiple blanks to
become a single white-space.
Artem Bityutskiy [Wed, 5 Feb 2014 13:22:33 +0000 (15:22 +0200)]
email-test-patchset: alwasy include the "always_cc" list
I do not know when we broke this, but some day that worked, and the "always_cc"
list from the configuration file was always Cced to all replies. But now it is
ignored, which is exactly what this patch fixes.
Artem Bityutskiy [Wed, 5 Feb 2014 13:08:42 +0000 (15:08 +0200)]
email-test-patchset: further improve readability
This script was confusing and difficult to follow because it used 'to', 'cc',
and 'from' variables when composing the reply. E.g., "to" means the "To:"
header of the patch under test, but when used in context of the reply, we used
it for creating the "Cc:" header. Very confusing and easy to mess things up.
This patch cleans that up. Let's leave 'to', 'cc', 'from', 'subj', and 'id'
variables to represent the _original_ "To:", "Cc:", "From:", "Subj:", and
"Id:".
For replies, introduce 'reply_to', 'reply_cc', 'reply_subj', and 'reply_id'
variables.
The code becames a lot easier to follow with this change.
Artem Bityutskiy [Wed, 5 Feb 2014 12:41:30 +0000 (14:41 +0200)]
email-test-patchset: remove a useless variable
This is another cosmetic clean-up. Just trying to make this script a bit
smaller and more readable. Remove the 'msgname' variable since we only use it
once.
Also, substitute 'echo' with 'printf', since we are trying to not using echo as
a general policy, see this URL for more infor why echo may be dangeros and
printf is safe:
This is another cosmetic change which is supposed to improve the readability.
Let's distinguish between global and project parameters by using slightly
differnet variabls prefix: "cfg_" for global parameters and "pcfg_" for
per-project. Then in the contexts where we do not yet know the project, we'd
not use any of the "pcfg_" variables, and possibly avoid introducing bugs.
Artem Bityutskiy [Wed, 5 Feb 2014 12:19:11 +0000 (14:19 +0200)]
email-sh-functions: rename cfg_descr
All the configuration variables we use internally have the same name as the
corresponding configuration option. The only exception is 'cfg_descr'. Let's
re-name it to 'cfg_description' to match this pattern and be consistent.
Artem Bityutskiy [Wed, 5 Feb 2014 12:14:00 +0000 (14:14 +0200)]
Rename 'make_options' to 'kmake_opts'
The command-line option corresponding to the 'make_options' configuration knob
is called '--kmake-opts'. Let's be consistent and use the same names for both,
just to make things be more logical.
Alternatively, if 'kmake_opts' is not the best name, we could rename both to
'make_options' instead, or to something else.
Artem Bityutskiy [Wed, 5 Feb 2014 11:56:23 +0000 (13:56 +0200)]
Remove email/aiaiai-email.cfg
I did not notice that we have 'email/aiaiai-email.cfg' file, and created the
'doc/email/example-aiaiai.cfg' file instead. This file has more comments and
looks better, so let's delete the old file and leave the new one.
Artem Bityutskiy [Wed, 5 Feb 2014 10:14:03 +0000 (12:14 +0200)]
Switch to dash when possible
Aiaiai is written in a portable manner, meaning that it does not use Bash
features and should work with different shells, like dash and ksh. Actually,
dash is a nice, small, and fast shell implementing only POSIX feature (or may
be mostly), and it is good to use it when it is available. This will ensure
that we do not accidentally break protability.
This patch adds a small code snippet to each script which will check if dash is
installed in the system, and if it is, "re-start" with dash.
People hacking on Aiaiai are encouradge to install the 'dash' package on their
systems.
Jacob Keller [Thu, 9 Jan 2014 23:28:26 +0000 (15:28 -0800)]
email-test-patchset: add support for Make options.
aiaiai-test-patchset has support for kernel make options which allows the
user to have more fine grained control over the compilation process. However,
the email form did not support passing these options to the test program. This
patch adds a configuration (per project) for specifying make options, so that
the user can have this fine grained control over the kernel build process.
Artem: amend commentaries a bit.
Signed-off-by: Jacob Keller <jacob.e.keller@intel.com> Signed-off-by: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>
Jacob Keller [Thu, 9 Jan 2014 23:28:21 +0000 (15:28 -0800)]
aiaiai: remove checkpatch.pl from repository
This patch removes the file tracked in this repository, since we now use the
in-kernel checkpatch version, or a user specified location. This enables
better control of what checkpatch.pl is used for those who care, without
having to manually update the file in git every time there is a change
upstream.
Signed-off-by: Jacob Keller <jacob.e.keller@intel.com> Signed-off-by: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>
Artem Bityutskiy [Tue, 4 Feb 2014 15:54:40 +0000 (17:54 +0200)]
email-test-patchset: improve readability
I've noticed that this script is rather difficult to read because of many
"DocHere" usage, which break indentation. These are used when we send replies
back to the patch submitter.
This patch is an attempt to improve this. It introduces a number of helper
functions which incorporate the e-mail sending. The main script then contains
just a function calls, which improves readability.
The idea to revert was stupid. This option is used internally to optimize
Aiaiai and only check those files which were modified by the patch under test.
Thus, reverting the change.
Artem Bityutskiy [Tue, 4 Feb 2014 15:17:49 +0000 (17:17 +0200)]
email-test-patchset: fix projects list generation
This patch fixes the 'list_projects()' which did not work because it was using
the 'cfg_descr' variable which was undefined. Indeed, this function is called
when the user either forgot to specify the project, or specified a non-existing
project.
This patch uses 'ini_config_get_or_die' to get project description. The whole
loop conctruct is cleaned up a bit too to make it more readable.
In this project we try to mark all internal functions with leading "__". Of
course, we do this only for "shared" files like "aiaiai-email-sh-functions".
Any symbol which is not supposed to be used ouside of the shared function
starts with "__".
Thie patch removes the prefix from the "__ini_config_get_or_die" helper because
we will need it in the next patch. Signed-off-by: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>
Artem Bityutskiy [Tue, 4 Feb 2014 14:25:09 +0000 (16:25 +0200)]
email-test-patchset: move project configuration parsing down a bit
This patch is a little clean-up which makes the code a bit more consistent. It
makes sure we first check if the user specified the project, and only then try
to parse project configuration, not vice-versa.
Artem Bityutskiy [Mon, 3 Feb 2014 15:17:03 +0000 (17:17 +0200)]
doc: refresh the Aiaiai overview text
Rename the 'README.announcement' file to descriptive 'Overview.txt'. Refresh
its contents too. This file is supposed to give general information about what
Aiaiai is.
Jacob Keller [Thu, 9 Jan 2014 23:28:10 +0000 (15:28 -0800)]
email-sh-functions: add local variables to parse_prj_config
Rather than assuming that the $cfgfile and $prj will be correctly defined, use
the passed parameters. Use local to create local variables to use, just like
in the regular config parsing.
Signed-off-by: Jacob Keller <jacob.e.keller@intel.com> Signed-off-by: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>
Jacob Keller [Thu, 9 Jan 2014 23:28:05 +0000 (15:28 -0800)]
email-test-patchset: fix a bug when project is unknown
This patch corrects an issue due to attempting to use cfg_reply_to_all (a
per-project configuration option) before we have checked whether the project
is valid. This causes the whole aiaiai-email service to die. To fix this, move
the checks for no project and unknown project above the cfg_reply_to_all use.
Also unassign the to variable when sending these emails so that we don't spam
everyone, (regardless of the reply_to_all setting). This restores the behavior
to the previous default.
Signed-off-by: Jacob Keller <jacob.e.keller@intel.com> Signed-off-by: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>
Artem Bityutskiy [Mon, 3 Feb 2014 08:04:45 +0000 (10:04 +0200)]
doc: add the TODO list
Add the TODO file. I've added some of my current wishes there. This does not
mean that I am going to implement this. But may be some time, when I have time
or if users request. Or may be someone else implements one of those.
Artem Bityutskiy [Wed, 4 Dec 2013 15:20:25 +0000 (17:20 +0200)]
test-bisectability: do not print base commit failure details
Now, when we allow for the base commit to be broken if the first patch fixes
it, it makes little sense providing the base commit failure details in that
case.
Jacob Keller [Mon, 25 Nov 2013 18:57:03 +0000 (10:57 -0800)]
bisectability: don't fail if base commit can't compile
This makes the bisectability test only fail when it can't compile
inbetween a patch series. It is perfectly reasonable that the first
patch in the series could fix the compile issue, so we should still
check the patch for bisection issues.
Signed-off-by: Jacob Keller <jacob.e.keller@intel.com> Signed-off-by: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>
Artem Bityutskiy [Wed, 4 Dec 2013 07:33:10 +0000 (09:33 +0200)]
test-patchset: improve build failure output
The previous patch made it acceptable for the base commit to fail. However, the
e-mail message we generate in this case contained something like:
"Failed to build the following commit for configuration ..."
which is not too readable, because it does not make it clear that this is the
base commit which fails to compile.
This patch improves the output and makes it look like this:
"Failed to build base commit ..."
This is just a bit nicer.
Additionally, when the base commit fails, and the first patch fixes it, the
resulting build diff is rather messy. This patch makes aiaiai-test-patchset
print a short explanation for the messy build diff.
Jacob Keller [Mon, 25 Nov 2013 18:56:37 +0000 (10:56 -0800)]
test-patchset: don't skip email when pre-patch build fails
Currently, whenever the pre-patched kernel fails to build, we
don't show the results for the post-patched kernel. This is faulty,
because we could easily be providing a patch which fixes the issue.
Instead, we should show the results for the failed build, but still
continue. If the post-patched kernel fails to build there is little need
to show a complex diff, but we can at least show the results for both
the pre-patched and post-patched kernel.
This patch also additionally adds --keep-going to the pre-patched kernel
in order to obtain more data for comparison.
Artem: add a commentary
Signed-off-by: Jacob Keller <jacob.e.keller@intel.com> Signed-off-by: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>
Artem Bityutskiy [Mon, 2 Dec 2013 12:14:04 +0000 (14:14 +0200)]
Use a helper for the 'fatal()' function
Unfortunately libshell's "fatal()" does not prefix messages with an "Error"
prefix. Introduce a new helper called "die()" which prepends messages with
"Fatal error".
Artem Bityutskiy [Mon, 2 Dec 2013 08:56:31 +0000 (10:56 +0200)]
email-lda: honor 'always_cc' and 'reply_to_all'
When rejecting a patch we should try to honor the 'always_cc' and
'reply_to_all' per-project configuration variables. This patch does exactly
that - parses the 'To' and 'Cc' list, finds out which project the e-mail
belongs to, parses the config file and CCes the 'always_cc' list, as well as
preserves the 'To' and 'Cc' addresses if 'reply_to_all' is true.
Artem Bityutskiy [Mon, 2 Dec 2013 08:28:01 +0000 (10:28 +0200)]
email-lda: send a reply when chaining was incorrect
This patch is based on Jacob Keller's work. It teaches the aiaiai-email-lda
script sending rejection notifications to the patch submitter. Right now it
only sends a reply when the submitter missed the 'In-Reply-To:' e-mail header.
Later more conditions may be added.
At the moment we do not CC the 'always_cc' list, neither honor the
'reply_to_all' flag. This may be changed later, if needed.
I've added several 'program_required's too, I was too lazy to separate them,
again, sorry.
Artem Bityutskiy [Mon, 2 Dec 2013 08:00:31 +0000 (10:00 +0200)]
email-lda: introduce --test-mode option
We are going to teach this script sending replies, and this option will disable
the functionality, just like in the aiaia-email-test-patchset script. This
patch is just a preparation.
Artem Bityutskiy [Fri, 29 Nov 2013 16:49:19 +0000 (18:49 +0200)]
email-lda: start parsing the configuration file
This is a preparation for the further changes where we teach the email-lda
script sending notifications when it rejects a patch. We'll need to know some
basic information like own e-mail address, and this is stored in the config
file. So parse it.
Artem Bityutskiy [Fri, 29 Nov 2013 16:43:28 +0000 (18:43 +0200)]
email-sh-functions: rework e-mail sending some more
Leave only the email composition stuff in the common code, and move the e-mail
sending stuff to email-test-patchset. This way the common code does not depend
on the "verbose" and "test_mode" variables anymore, which is a bit cleaner.
Artem Bityutskiy [Fri, 29 Nov 2013 14:46:17 +0000 (16:46 +0200)]
email-sh-functions: misc improvements
This is a huge patch and it should really be split. But I am being lazy and
committing all at once. Yes, this is bad, sorry, but I am trying to save time.
Anyway, this patch does 2 big changes:
1. Improves the strip_address function so that it does not anymore depend on
any global 'cfg_*' variables and gets the e-mail address to strip as a
parameter.
2. Simplifies the 'compose_email' function interface and teach it to get a
single list of addresses to CC. This required some an additional function
for merging 2 lists of e-mails. But the end result is that
email-test-patchset becomes simpler.
Artem Bityutskiy [Thu, 28 Nov 2013 12:44:53 +0000 (14:44 +0200)]
email-sh-function: new file for shared e-mail code
Let's have a separate file for the shared code which is only relevant to
e-mail handling. For now we only have the subject parsing there, but we'll add
more stuff to this file later.
Commit '07837a6 email-test-patchset: add cppcheck support' introduced a
breakage by adding an unexisting variable named "commit", which causes the
following error:
aiaiai-email-test-patchset: line 334: commit: unbound variable
The variable should be named 'branch'. This patch fixes the problem.
Artem Bityutskiy [Wed, 27 Nov 2013 13:00:24 +0000 (15:00 +0200)]
sh-functions: rename __prfx to __blah
I am going to add support for [Prefix PATCH Suffix n/n] subject, so
the prefix word would be needed, and thus in this patch I use word
"blah" for the [blah] prefixes.
Artem Bityutskiy [Wed, 27 Nov 2013 12:46:22 +0000 (14:46 +0200)]
sh-function: rename __ws to __blanks
The internal __ws variable is a short-cut for "zero or reasonable amount of
white-space or tab characters". This patch renames it to "__blanks", which
is a bit more readable. Additionally I make the reasonable amount to be 8
blanks instead of 4.
Artem Bityutskiy [Wed, 27 Nov 2013 11:42:21 +0000 (13:42 +0200)]
sh-functions: refine the 'prefix_format' variable
First of all, the whole subject parsing thing belongs to the e-mail stuff, so
it really should be moved to the email sub-directory.
The 'prefix_format' is a variable which is supposed to explain in a short form
what is the format of the subject we expect. Currently we have a confusing
<project> part there, and this patch removes it. I do not think the result is
ideal, since it does not tell that the format is rather flexible, but still
it is a bit better. Later we can change this to something better.
Jacob Keller [Tue, 26 Nov 2013 22:35:06 +0000 (14:35 -0800)]
sh-functions: fix print_separator
print_separator needs to be run as a subshell, because we need its output, not
the string itself. This fixes a print display bug where we show
"print_separator" instead of the actual separator.
Signed-off-by: Jacob Keller <jacob.e.keller@intel.com> Signed-off-by: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>
Jacob Keller [Tue, 26 Nov 2013 20:02:17 +0000 (12:02 -0800)]
aiaiai: don't assume kernel tree will be checked out
This change addresses a possible issue if someone uses a bare repsitory
as their remote. We handle this by using a new sh-function git_dir()
which will return $path/.git if that is a directly, then path if that is
a directory, or will complain if it's not a valid directory. This
enables us to automatically use the correct directory without issue even
if the repository is bare.
Signed-off-by: Jacob Keller <jacob.e.keller@intel.com> Signed-off-by: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>
Artem Bityutskiy [Tue, 26 Nov 2013 17:03:33 +0000 (19:03 +0200)]
email: fix ini_config_get_or_die
Similarly to what we did in the previous patch, change ini_config_get_or_die
interface so that the caller would not have to call it in a subshell, which
makes dying impossible.
Artem Bityutskiy [Tue, 26 Nov 2013 16:52:44 +0000 (18:52 +0200)]
Fix fetch_header_or_die
The fetch_header_or_die function is suppoed to make the caller exit if the
header is not found. However, this did not happen, because the interface of
this function assumed that it is called in a subshell in order to intercept the
results. And the exiting happened in the subshell, which did not affect the
caller at all.
Fix this issue by changing the interface of fetch_header_or_die() so that the
result is written to the first parameter.
Jacob Keller [Mon, 25 Nov 2013 18:56:25 +0000 (10:56 -0800)]
sh-functions: show merge results when patch fails to apply
This patch enables the email notification an attempt at showing you the
result of a merge based on the patch utility. This might enable more
easily debugging why a patch failed to apply.
Signed-off-by: Jacob Keller <jacob.e.keller@intel.com> Signed-off-by: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>
Jacob Keller [Mon, 25 Nov 2013 18:56:15 +0000 (10:56 -0800)]
aiaiai: make formail allow non-strict mbox format
Sometimes patches are not properly emailed in strict mbox format. This
can result in some bogus issues, which are simply automatically resolved
by just working in non-strict mbox format. This patch makes sure all
executions of formail accept non-strict formats.
Signed-off-by: Jacob Keller <jacob.e.keller@intel.com> Signed-off-by: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>
Jacob Keller [Mon, 25 Nov 2013 18:56:02 +0000 (10:56 -0800)]
email-test-patchset: add cppcheck support
The aiaiai-email-test-patchset script did not have a cppcheck option,
even though the lower level aiaiai-test-patchset supports this. Add the
option along with the other check options.
Signed-off-by: Jacob Keller <jacob.e.keller@intel.com> Signed-off-by: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>
Jacob Keller [Mon, 25 Nov 2013 18:55:57 +0000 (10:55 -0800)]
email-test-patchset: allow a configuration directory
The aiaiai-test-patchset has a configuration directory option, but the
aiaiai-email-test-patchset does not have this option. Update the
email-test-patchset script to support passing this option onto the lower
level aiaiai-test-patchset script.
Signed-off-by: Jacob Keller <jacob.e.keller@intel.com> Signed-off-by: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>
Jacob Keller [Mon, 25 Nov 2013 18:55:52 +0000 (10:55 -0800)]
make-kernel: add parameter to enable endian checking
Endian checking is not turned on by default, and requires a separate
parameter in order to enable it. As it can be quite useful to have, this
patch adds the proper syntax to enable it when the kernel checker is
enabled.
Signed-off-by: Jacob Keller <jacob.e.keller@intel.com> Signed-off-by: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>
Jacob Keller [Mon, 25 Nov 2013 18:55:40 +0000 (10:55 -0800)]
sh-functions: rename fetch_all_headers
The fetch_all_headers name is not descriptive, as it actually splits the
mailbox up and processes the first header of each message. It does not
obtain every copy of a given header from the mbox. This patch renames
the function, and updates the comment so that it is clear what the
function does.
Signed-off-by: Jacob Keller <jacob.e.keller@intel.com> Signed-off-by: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>
Jacob Keller [Mon, 25 Nov 2013 18:55:35 +0000 (10:55 -0800)]
aiaiai: use --shared since we know we will be safe
We want to use a shared clone here, because it will prevent duplicate copies
of all the object data, and we know it will be ok, since we won't be mucking
with the tree, and the commits will be tagged.
Signed-off-by: Jacob Keller <jacob.e.keller@intel.com> Signed-off-by: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>
Jacob Keller [Mon, 25 Nov 2013 18:55:30 +0000 (10:55 -0800)]
make-kernel: fix --keep-going option
The make-kernel utility has an option "--keep-going" which can be useful
if you want to continue testing past kernel build failures, in order to try
and find as many issues as you can. However, it previously did not work,
as getopt-long format required a parameter, and the case statement did
not check for the correct long syntax.
Signed-off-by: Jacob Keller <jacob.e.keller@intel.com> Signed-off-by: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>