@@ -67,8 +67,8 @@ mod utils;
static USAGE: &'static str = "
Usage:
- snowpatch <config-file> [--count=<count>] [--project <name>]
- snowpatch <config-file> --mbox <mbox> --project <name>
+ snowpatch <config-file> --project <name> [--count=<count>]
+ snowpatch <config-file> --project <name> --mbox <mbox>
snowpatch <config-file> --patch <id>
snowpatch <config-file> --series <id>
snowpatch -v | --version
@@ -77,11 +77,11 @@ Usage:
By default, snowpatch runs as a long-running daemon.
Options:
+ --project <name> Test patches for the given project.
--count <count> Run tests on <count> recent series.
--patch <id> Run tests on the given Patchwork patch.
--series <id> Run tests on the given Patchwork series.
--mbox <mbox> Run tests on the given mbox file. Requires --project
- --project <name> Test patches for the given project.
-v, --version Output version information.
-h, --help Output this help text.
";
@@ -349,19 +349,6 @@ fn main() {
panic!("Can't specify both --series and --patch");
}
- if args.flag_mbox != "" && args.flag_project != "" {
- info!("snowpatch is testing a local patch.");
- let patch = Path::new(&args.flag_mbox);
- match settings.projects.get(&args.flag_project) {
- None => panic!("Couldn't find project {}", args.flag_project),
- Some(project) => {
- test_patch(&settings, &client, project, patch, true);
- }
- }
-
- return;
- }
-
if args.flag_patch > 0 {
info!("snowpatch is testing a patch from Patchwork.");
let patch = patchwork.get_patch(&(args.flag_patch as u64)).unwrap();
@@ -400,6 +387,17 @@ fn main() {
return;
}
+ // At this point, specifying a project is required
+ let project = settings.projects.get(&args.flag_project).unwrap();
+
+ if args.flag_mbox != "" {
+ info!("snowpatch is testing a local patch.");
+ let patch = Path::new(&args.flag_mbox);
+ test_patch(&settings, &client, project, patch, true);
+
+ return;
+ }
+
// The number of patches tested so far. If --count isn't provided, this is unused.
let mut patch_count = 0;
@@ -411,7 +409,7 @@ fn main() {
*/
'daemon: loop {
let patch_list = patchwork
- .get_patch_query()
+ .get_patch_query(&args.flag_project)
.unwrap_or_else(|err| panic!("Failed to obtain patch list: {}", err));
info!("snowpatch is ready to test new revisions from Patchwork.");
for patch in patch_list {
@@ -426,72 +424,60 @@ fn main() {
continue;
}
- //let project = patchwork.get_project(&patch.project).unwrap();
- // Skip if we're using -p and it's the wrong project
- if args.flag_project != "" && patch.project.link_name != args.flag_project {
- debug!(
+ // Skip if it's the wrong project
+ if patch.project.link_name != args.flag_project {
+ warn!(
"Skipping patch {} ({}) (wrong project: {})",
patch.name, patch.id, patch.project.link_name
);
continue;
}
- match settings.projects.get(&patch.project.link_name) {
- None => {
- debug!(
- "Project {} not configured for patch {}",
- &patch.project.link_name, patch.name
- );
- continue;
- }
- Some(project) => {
- // TODO(ajd): Refactor this.
- let hefty_tests;
- let mbox = if patch.has_series() {
- debug!(
- "Patch {} has a series at {}!",
- &patch.name, &patch.series[0].url
- );
- let series = patchwork.get_series_by_url(&patch.series[0].url);
- match series {
- Ok(series) => {
- if !series.received_all {
- debug!("Series is incomplete, skipping patch for now");
- continue;
- }
- let dependencies = patchwork.get_patch_dependencies(&patch);
- hefty_tests = dependencies.len() == series.patches.len();
- patchwork.get_patches_mbox(dependencies)
- }
- Err(e) => {
- debug!("Series is not OK: {}", e);
- hefty_tests = true;
- patchwork.get_patch_mbox(&patch)
- }
+ // TODO(ajd): Refactor this.
+ let hefty_tests;
+ let mbox = if patch.has_series() {
+ debug!(
+ "Patch {} has a series at {}!",
+ &patch.name, &patch.series[0].url
+ );
+ let series = patchwork.get_series_by_url(&patch.series[0].url);
+ match series {
+ Ok(series) => {
+ if !series.received_all {
+ debug!("Series is incomplete, skipping patch for now");
+ continue;
}
- } else {
+ let dependencies = patchwork.get_patch_dependencies(&patch);
+ hefty_tests = dependencies.len() == series.patches.len();
+ patchwork.get_patches_mbox(dependencies)
+ }
+ Err(e) => {
+ debug!("Series is not OK: {}", e);
hefty_tests = true;
patchwork.get_patch_mbox(&patch)
- };
-
- let results = test_patch(&settings, &client, project, &mbox, hefty_tests);
-
- // Delete the temporary directory with the patch in it
- fs::remove_dir_all(mbox.parent().unwrap())
- .unwrap_or_else(|err| error!("Couldn't delete temp directory: {}", err));
- if project.push_results {
- for result in results {
- patchwork.post_test_result(result, &patch.checks).unwrap();
- }
- }
- if args.flag_count > 0 {
- patch_count += 1;
- debug!("Tested {} patches out of {}", patch_count, args.flag_count);
- if patch_count >= args.flag_count {
- break 'daemon;
- }
}
}
+ } else {
+ hefty_tests = true;
+ patchwork.get_patch_mbox(&patch)
+ };
+
+ let results = test_patch(&settings, &client, project, &mbox, hefty_tests);
+
+ // Delete the temporary directory with the patch in it
+ fs::remove_dir_all(mbox.parent().unwrap())
+ .unwrap_or_else(|err| error!("Couldn't delete temp directory: {}", err));
+ if project.push_results {
+ for result in results {
+ patchwork.post_test_result(result, &patch.checks).unwrap();
+ }
+ }
+ if args.flag_count > 0 {
+ patch_count += 1;
+ debug!("Tested {} patches out of {}", patch_count, args.flag_count);
+ if patch_count >= args.flag_count {
+ break 'daemon;
+ }
}
}
info!("Finished testing new revisions, sleeping.");
@@ -300,8 +300,12 @@ impl PatchworkServer {
serde_json::from_str(&self.get_url_string(url).unwrap())
}
- pub fn get_patch_query(&self) -> Result<Vec<Patch>, serde_json::Error> {
- let url = format!("{}{}/patches/{}", &self.url, PATCHWORK_API, PATCHWORK_QUERY);
+ pub fn get_patch_query(&self, project: &str) -> Result<Vec<Patch>, serde_json::Error> {
+ let url = format!(
+ "{}{}/patches/{}&project={}",
+ &self.url, PATCHWORK_API, PATCHWORK_QUERY, project
+ );
+
serde_json::from_str(&self.get_url_string(&url)
.unwrap_or_else(|err| panic!("Failed to connect to Patchwork: {}", err)))
}
So this might seem out of place, but bear with me here. Right now, the way that we check for patches is to get the latest patches "page" from the API. For each of them, we see if the project the patch is from matches one we have settings for, and if so we test it. We exhaust the patch list page we got, and then we get a new one. There are lots of problems with this. The first one is that if we're testing lots of patches and the Patchwork instance is high traffic, we risk missing patches that fall to page 2 or beyond in that time. We currently don't have a solution for this. The next issue is that when I originally implemented this, filtering by project didn't work properly in the patches API, making it difficult to track all patches, especially for niche projects with less activity (since running into one patch of a series will lead to them all being tested, even if snowpatch doesn't find them all individually in the patch list). This is no longer the case however, and we can filter by project on the patch list and actually get a full page of patches in that project. As a result, until both the following conditions are true: - patchwork has a suitable events API and/or better pagination - snowpatch has a proper threading model for distributed work Limit snowpatch to one project at a time to minimise the chance of missing patches, and users should just spin up more than one instance if they want to test multiple projects at once. Signed-off-by: Russell Currey <ruscur@russell.cc> --- src/main.rs | 130 +++++++++++++++++++++-------------------------- src/patchwork.rs | 8 ++- 2 files changed, 64 insertions(+), 74 deletions(-)