Skip site navigation (1)Skip section navigation (2)
Date:      Mon, 5 Aug 2013 10:11:27 GMT
From:      mattbw@FreeBSD.org
To:        svn-soc-all@FreeBSD.org
Subject:   socsvn commit: r255525 - soc2013/mattbw/backend
Message-ID:  <201308051011.r75ABRYE080633@socsvn.freebsd.org>

next in thread | raw e-mail | index | archive | help
Author: mattbw
Date: Mon Aug  5 10:11:27 2013
New Revision: 255525
URL: http://svnweb.FreeBSD.org/socsvn/?view=rev&rev=255525

Log:
  Cleanup and refactoring for jobs.[ch].
  
  Nothing severe, just a quick bit of function introduction and tidying.
  Hopefully everything should get a bit of cleanup done on it over the course
  of the second half-term, if I have enough time.
  
  Repository setting now works properly if the repository is "installed",
  which occurs when trying to remove packages.
  

Modified:
  soc2013/mattbw/backend/jobs.c
  soc2013/mattbw/backend/jobs.h

Modified: soc2013/mattbw/backend/jobs.c
==============================================================================
--- soc2013/mattbw/backend/jobs.c	Mon Aug  5 09:53:48 2013	(r255524)
+++ soc2013/mattbw/backend/jobs.c	Mon Aug  5 10:11:27 2013	(r255525)
@@ -33,8 +33,13 @@
 #include "jobs.h"		/* jobs_... */
 #include "pkgutils.h"		/* pkgutils_... */
 
-static bool jobs_do_repo(struct pkgdb *db, const struct jobs_spec *spec, gchar **idv, unsigned int idc, const char *reponame);
-static void free_namevers(char ***namev_p, unsigned int namec);
+static bool jobs_do_check(const struct jobs_spec *spec, struct pkg_jobs *jobs, gchar **package_ids, guint count);
+static bool jobs_do_set_repo(const struct jobs_spec *spec, struct pkg_jobs *jobs, const char *reponame);
+static bool jobs_do_same_repo(struct pkgdb *db, const struct jobs_spec *spec, gchar **package_ids, guint count, const char *reponame);
+static char **jobs_do_populate(const struct jobs_spec *spec, struct pkg_jobs *jobs, gchar **package_ids, guint count);
+static char **namevers_from_package_ids(gchar **package_ids, guint count);
+static struct pkg_jobs *jobs_do_allocate(const struct jobs_spec *spec, struct pkgdb *db);
+static void free_namevers(char ***namevers_p, guint count);
 
 /* Applies a job with the given error enums and the standard event callback. */
 bool
@@ -65,10 +70,9 @@
  */
 bool
 jobs_check_package_ids(struct pkg_jobs *jobs, gchar **package_ids,
-    bool reject_non_updates)
+    guint count, bool reject_non_updates)
 {
 	bool success;
-	guint count;
 	guint i;
 	const char *name;
 	const char *version;
@@ -185,7 +189,7 @@
 	 * TODO: consider bundling PackageIDs up into separate jobs per repo?
 	 */
 	if (package_ids == NULL)
-		success = jobs_do_repo(db, spec, NULL, 0, "");
+		success = jobs_do_same_repo(db, spec, NULL, 0, "");
 	else {
 
 		for (i = 0, success = true; i < count && success; i++) {
@@ -197,7 +201,7 @@
 				repo = strdup(splits[PK_PACKAGE_ID_DATA]);
 			g_strfreev(splits);
 
-			success = jobs_do_repo(db, spec, package_ids + i,
+			success = jobs_do_same_repo(db, spec, package_ids + i,
 			    1, repo);
 			free(repo);
 		}
@@ -213,61 +217,46 @@
  * Performs a job on a certain batch of PackageIDs, each with the same repo.
  * count can be 0, in which case no PackageIDs are used.
  */
-bool
-jobs_do_repo(struct pkgdb *db, const struct jobs_spec *spec, gchar **idv,
-    unsigned int idc, const char *reponame)
+static bool
+jobs_do_same_repo(struct pkgdb *db, const struct jobs_spec *spec,
+    gchar **package_ids, guint count, const char *reponame)
 {
+	bool		check_success;
+	bool		repo_success;
 	bool		success;
 	struct pkg_jobs *jobs;
 	char	      **namevers;
 
 	assert(db != NULL);
 	assert(spec != NULL);
-	assert(idv == NULL || idc > 0);
-	assert(idv != NULL || idc == 0);
+	assert(package_ids != NULL || 0 == count);
+	assert(package_ids == NULL || 0 < count);
 
 	success = NULL;
 	namevers = NULL;
-	jobs = NULL;
 
-	if (pkg_jobs_new(&jobs, spec->type, db) != EPKG_OK) {
-		ERR(spec->backend,
-		    PK_ERROR_ENUM_INTERNAL_ERROR,
-		    "could not init pkg_jobs");
+	jobs = jobs_do_allocate(spec, db);
+	if (jobs == NULL)
+		goto cleanup;
+
+	repo_success = jobs_do_set_repo(spec, jobs, reponame);
+	if (!repo_success)
+		goto cleanup;
+
+	namevers = jobs_do_populate(spec, jobs, package_ids, count);
+	if (package_ids != NULL && namevers == NULL)
 		goto cleanup;
-	}
-	if (reponame != NULL &&
-	    (pkg_jobs_set_repository(jobs, reponame) != EPKG_OK)) {
- 		ERR(spec->backend,
-		    PK_ERROR_ENUM_REPO_NOT_FOUND,
-		    "could not set repo");
-		goto cleanup;   	
-	}
-	if (idv != NULL) {
-		STATUS(spec->backend, PK_STATUS_ENUM_DEP_RESOLVE);
 
-		namevers = jobs_add_package_ids(jobs, idv);
-		if (namevers == NULL) {
-			ERR(spec->backend,
-			    PK_ERROR_ENUM_DEP_RESOLUTION_FAILED,
-			    "could not find all requested packages");
-			goto cleanup;
-		}
-	}
 	if (pkg_jobs_solve(jobs) != EPKG_OK) {
 		ERR(spec->backend,
 		    PK_ERROR_ENUM_DEP_RESOLUTION_FAILED,
 		    "could not solve the job");
 		goto cleanup;
 	}	
-	if (idv != NULL && 
-	    (jobs_check_package_ids(jobs, idv,
-	    spec->reject_non_updates) == false)) {
-		ERR(spec->backend,
-		    PK_ERROR_ENUM_DEP_RESOLUTION_FAILED,
-		    "packages failed suitability check");
+
+	check_success = jobs_do_check(spec, jobs, package_ids, count);
+	if (!check_success)
 		goto cleanup;
-	}
 
 	STATUS(spec->backend, spec->status);
 
@@ -281,7 +270,7 @@
 
 cleanup:
 	pkg_jobs_free(jobs);
-	free_namevers(&namevers, idc);
+	free_namevers(&namevers, count);
 
 	return success;
 } 
@@ -291,44 +280,23 @@
  * on success.
  */
 char **
-jobs_add_package_ids(struct pkg_jobs *jobs, gchar **package_ids)
+jobs_add_package_ids(struct pkg_jobs *jobs, gchar **package_ids, guint count)
 {
-	bool success;
-	char **namevers;
-	guint count;
-	guint i;
+	char	      **namevers;
 
 	assert(jobs != NULL);
 	assert(package_ids != NULL);
 
 	namevers = NULL;
-	success = false;
 
 	count = g_strv_length(package_ids);
-	if (count == 0)
-		goto cleanup;
+	if (count > 0) {
+		int		err;
 
-	/* Need to convert PackageIDs into name-version pairs. */
-	namevers = calloc(count, sizeof(char *));
-	if (namevers == NULL)
-		goto cleanup;
+		namevers = namevers_from_package_ids(package_ids, count);
 
-	for (i = 0; i < count; i++) {
-		namevers[i] = pkgutils_package_id_namever(package_ids[i]);
-		if (namevers[i] == NULL)	
-			break;
-	}
-
-	/* Make sure we successfully fished out every namever */
-	if (i < count)
-		goto cleanup;
-
-	if (pkg_jobs_add(jobs, MATCH_EXACT, namevers, (int)count) == EPKG_OK)
-		success = true;
-
-cleanup:
-	if (!success) {
-		if (namevers != NULL)
+		err = pkg_jobs_add(jobs, MATCH_EXACT, namevers, (int)count);
+		if (err != EPKG_OK)
 			free_namevers(&namevers, count);
 	}
 
@@ -353,17 +321,156 @@
  	}
 }
 
+static bool
+jobs_do_check(const struct jobs_spec *spec, struct pkg_jobs *jobs,
+    gchar **package_ids, guint count)
+{
+	bool		success;
+
+	assert(spec != NULL);
+	assert(jobs != NULL);
+	assert(package_ids != NULL || 0 == count);
+	assert(package_ids == NULL || 0 < count);
+
+	success = true;
+
+	if (package_ids == NULL) {
+		success = jobs_check_package_ids(jobs, package_ids, count,
+		    spec->reject_non_updates);
+	}
+	if (!success) {
+		ERR(spec->backend,
+		    PK_ERROR_ENUM_DEP_RESOLUTION_FAILED,
+		    "packages failed suitability check");
+	}
+
+	assert(0 < count || success);
+
+	return success;
+}
+
+static bool
+jobs_do_set_repo(const struct jobs_spec *spec, struct pkg_jobs *jobs,
+    const char *reponame)
+{
+	bool		is_remote;
+	bool		success;
+
+	assert(spec != NULL);
+	assert(jobs != NULL);
+
+	success = true;
+
+	is_remote = (reponame != NULL && strcmp(reponame, "installed") != 0);
+
+	if (is_remote) {
+		int		err;
+
+		err = pkg_jobs_set_repository(jobs, reponame);
+		if (err != EPKG_OK) {
+			char	       *err_message;
+
+			(void)asprintf(&err_message,
+			    "Could not set job repository to '%s'.",
+			    reponame);
+	 		ERR(spec->backend,
+			    PK_ERROR_ENUM_REPO_NOT_FOUND,
+			    err_message);
+	 		free(err_message);
+
+	 		success = false;
+	 	}
+	}
+
+	assert(is_remote || success == true);
+
+	return success;
+}
+
+static char **
+jobs_do_populate(const struct jobs_spec *spec, struct pkg_jobs *jobs,
+    gchar **package_ids, guint count)
+{
+	char	      **namevers;
+
+	assert(spec != NULL);
+	assert(jobs != NULL);
+	assert(package_ids != NULL || count == 0);
+	assert(package_ids == NULL || count >= 1);
+
+	namevers = NULL;
+	if (package_ids != NULL) {
+		STATUS(spec->backend, PK_STATUS_ENUM_DEP_RESOLVE);
+
+		namevers = jobs_add_package_ids(jobs, package_ids, count);
+		if (namevers == NULL) {
+			ERR(spec->backend,
+			    PK_ERROR_ENUM_DEP_RESOLUTION_FAILED,
+			    "could not find all requested packages");
+		}
+	}
+
+	assert(count >= 1 || namevers == NULL);
+
+	return namevers;
+}
+
 static void
-free_namevers(char ***namev_p, unsigned int namec)
+free_namevers(char ***namevers_p, guint count)
 {
 	unsigned int		i;
 
-	assert(namev_p != NULL);
+	assert(namevers_p != NULL);
+
+	if (*namevers_p != NULL) {
+		for (i = 0; i < count; i++)
+			free(*namevers_p[i]);
+		free(*namevers_p);
+		*namevers_p = NULL;
+	}
+}
+
+static char **
+namevers_from_package_ids(gchar **package_ids, guint count)
+{
+	char	      **namevers;
+	unsigned int	i;
 
-	if (*namev_p != NULL) {
-		for (i = 0; i < namec; i++)
-			free(*namev_p[i]);
-		free(*namev_p);
-		*namev_p = NULL;
+	namevers = calloc(count, sizeof(char *));
+	if (namevers != NULL) {
+		for (i = 0; i < count; i++) {
+			namevers[i] = pkgutils_package_id_namever(
+			    package_ids[i]);
+			if (namevers[i] == NULL) {
+				free_namevers(&namevers, count);
+				break;
+			}
+		}
 	}
+
+	return namevers;
+}
+
+/* Safe and noisy wrapper around pkg_jobs_new. */
+static struct pkg_jobs *
+jobs_do_allocate(const struct jobs_spec *spec, struct pkgdb *db)
+{
+	int		err;
+	struct pkg_jobs *jobs;
+
+	assert(spec != NULL);
+	assert(db != NULL);
+
+	jobs = NULL;
+	err = pkg_jobs_new(&jobs, spec->type, db);
+	if (err != EPKG_OK) {
+		ERR(spec->backend,
+		    PK_ERROR_ENUM_INTERNAL_ERROR,
+		    "could not init pkg_jobs");
+	}
+
+	assert(err == EPKG_OK || jobs == NULL);
+	assert(err != EPKG_OK || jobs != NULL);
+
+	return jobs;
 }

Modified: soc2013/mattbw/backend/jobs.h
==============================================================================
--- soc2013/mattbw/backend/jobs.h	Mon Aug  5 09:53:48 2013	(r255524)
+++ soc2013/mattbw/backend/jobs.h	Mon Aug  5 10:11:27 2013	(r255525)
@@ -40,9 +40,9 @@
 };
 
 bool		jobs_apply(struct pkg_jobs *jobs, PkBackend *backend, PkErrorEnum no_jobs, PkErrorEnum jobs_failed);
-bool		jobs_check_package_ids(struct pkg_jobs *jobs, gchar **package_ids, bool reject_non_updates);
+bool		jobs_check_package_ids(struct pkg_jobs *jobs, gchar **package_ids, guint count, bool reject_non_updates);
 bool		jobs_do(const struct jobs_spec *spec);
-char	      **jobs_add_package_ids(struct pkg_jobs *jobs, gchar **package_ids);
+char	      **jobs_add_package_ids(struct pkg_jobs *jobs, gchar **package_ids, guint count);
 void		jobs_emit_packages(struct pkg_jobs *jobs, PkBackend *backend, pkg_info_ptr info);
 
 #endif				/* !_PKGNG_BACKEND_JOBS_H_ */



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201308051011.r75ABRYE080633>