PERFORCE change 129669 for review

Garrett Cooper gcooper at FreeBSD.org
Tue Nov 27 19:46:21 PST 2007


http://perforce.freebsd.org/chv.cgi?CH=129669

Change 129669 by gcooper at shiina-ibook on 2007/11/28 03:45:37

	1. Correct accidental poor style on my part.
	2. Add a bit more stylishy items.
	3. Solve a bug in freebsd_run_script [system(3) does in fact return -1 sometimes, ya know?].
	4. Finish off most of the deinstall script support.

Affected files ...

.. //depot/projects/soc2007/revised_fbsd_pkgtools/pkg_revised/v2/contrib/libpkg/pkg_db_freebsd.c#3 edit
.. //depot/projects/soc2007/revised_fbsd_pkgtools/pkg_revised/v2/contrib/libpkg/pkg_freebsd.c#2 edit

Differences ...

==== //depot/projects/soc2007/revised_fbsd_pkgtools/pkg_revised/v2/contrib/libpkg/pkg_db_freebsd.c#3 (text+ko) ====

@@ -193,9 +193,8 @@
 	assert(pkg != NULL);
 	assert(pkg_action != NULL);
 
-	if (getwd(cwd) == NULL) {
+	if (getwd(cwd) == NULL)
 		return -1;
-	}
 
 	/* Set the package environment */
 	if (prefix == NULL) {
@@ -427,32 +426,42 @@
 		return -1;
 	}
 
-	/** @todo Check if package is dependended on */
+	/*
+	 * Calculate reverse dependencies -- is this package required by
+	 * anything else?
+	 */
 	deps = pkg_get_reverse_dependencies(real_pkg);
+
+	/* No dependencies */
 	if (deps == NULL) {
 		return -1;
-	} else if (deps[0] != NULL) {
+	}
+	/* We have dependencies to go through.. */
+	else if (deps[0] != NULL) {
 		unsigned int pos, buf_size, buf_used;
 		char *buf;
-		/* XXX */
+
 		buf_used = 0;
-		buf_size = 1024;
-		buf = malloc(buf_size);
+		buf_size = PKGDB_DEPS_READAHEAD_SIZE;
+		buf = (char*) malloc(buf_size);
+
 		if (buf == NULL) {
 			pkg_action(PKG_DB_INFO,
 			    "package '%s' is required by other packages and "
-			    "may not be deinstalled however an error occured "
-			    "while retrieving the list of packages",
+			    "may not be deinstalled.\n"
+			    "An error occurred while retrieving the list of "
+			    "packages (buffer memory could not be allocated)",
 			    pkg_get_name(real_pkg));
 			return -1;
 		}
-		/* Load the names of the packages into a buffer */
+
+		/* Load the package names into a buffer */
 		for (pos = 0; deps[pos] != NULL; pos++) {
 			size_t len;
 
 			len = strlen(pkg_get_name(deps[pos]));
 			if (buf_used + len >= buf_size) {
-				buf_size += 1024;
+				buf_size += PKGDB_DEPS_READAHEAD_SIZE;
 				buf = realloc(buf, buf_size);
 			}
 			strlcat(buf, pkg_get_name(deps[pos]), buf_size);
@@ -461,45 +470,79 @@
 		}
 
 		/*
-		 * There is a sligntly different
-		 * message when the force flag is set
+		 * The message is slightly different when force is set 
 		 */
-		if (force != 0) {
-			pkg_action(PKG_DB_INFO,
-			    "package '%s' is required by these other packages "
-			    "and may not be deinstalled (but I'll delete it "
-			    "anyway):\n%s", pkg_get_name(real_pkg), buf);
-		} else {
-			pkg_action(PKG_DB_INFO,
-			    "package '%s' is required by these other packages "
-			    "and may not be deinstalled:\n%s",
-			    pkg_get_name(real_pkg), buf);
-		}
+		pkg_action(PKG_DB_INFO,
+		    "package '%s' is required by these packages and may not be"
+		    " deinstalled%s:\n%s",
+		    pkg_get_name(real_pkg),
+		    ( force == ? "" : "(but I'll delete it anyway)"), buf);
 		free(buf);
 
 		/* Only return when the not being forced to */
 		if (force != 0)
 			return -1;
+
 	}
 
+	/*
+	 * This is equivalent to the:
+	 *
+	 *	if (fexists(POST_DEINSTALL_FNAME)) .. else { .. }
+	 *
+	 * block in "pkg_install/delete/perform.c".
+	 */
 	if (fake == 0 && exec_pkg_scripts != NULL) {
+
+		/* Try +REQUIRE */ 
 		if (pkg_run_script(real_pkg, NULL,
-		    pkg_script_require_deinstall) != 0 && !force) {
-			/* XXX */
-			return -1;
+			pkg_script_require_deinstall) != 0 &&
+		    errno != ENOFILE && force == 0) {
+
+		    warnx("deinstall +REQUIRE script execution failed");
+		    return -1;
+
 		}
+ 
+		/* +REQUIRE didn't exist; try +PRE-DEINSTALL */
+		else if (errno == ENOFILE) {
 
-		if (pkg_run_script(real_pkg, NULL, pkg_script_pre_deinstall)
-		    != 0  && !force) {
-			/* XXX */
-			return -1;
-		}
+		    /* Try +PRE-DEINSTALL */
+		    if (pkg_run_script(real_pkg, NULL,
+			    pkg_script_pre_deinstall) != 0 &&
+		        errno != ENOFILE && force == 0) {
+
+		        warnx("pre-deinstall script execution failed");
+		        return -1;
+
+		    }
+  
+		    /* +PRE-DEINSTALL didn't exist; try +DEINSTALL */
+		    else if (errno == ENOFILE) {
+
+			/* Try +DEINSTALL */
+			if (pkg_run_script(real_pkg, NULL,
+				pkg_script_deinstall) != 0 &&
+		    	    errno != ENOFILE && force == 0) {
+		    	    warnx("deinstall +REQUIRE script execution failed");
+		    	    return -1;
+			}
+
+			/*
+			 * XXX: Else, oddly enough no deinstall scripts exist.
+			 *
+			 * Is this correct behavior?!
+			 *
+			 * According to pkg_delete(1)'s existing code, yes
+			 * it's supposed to be this way, but logically does
+			 * it make sense?
+			 *
+			 */
+
+		    }
+
+		} 
 
-		if (pkg_run_script(real_pkg, NULL, pkg_script_deinstall) != 0
-		    && !force) {
-			/* XXX */
-			return -1;
-		}
 	}
 
 	/* Remove the reverse dependencies */
@@ -513,7 +556,7 @@
 			pkg_action(PKG_DB_INFO, "Trying to remove "
 			    "dependency on package '%s' with '%s' origin.",
 			    pkg_get_name(deps[pos]), pkg_get_origin(deps[pos]));
-			if (!fake) {
+			if (fake == 0) {
 				file = pkg_get_control_file(deps[pos],
 				    "+REQUIRED_BY");
 				pkgfile_remove_line(file,
@@ -531,16 +574,16 @@
 	deinstall_data.directory[0] = '\0';
 	if (pkg_deinstall(real_pkg, pkg_action, &deinstall_data,
 	    freebsd_do_chdir, freebsd_deinstall_file,
-	    freebsd_do_exec, freebsd_deregister) != 0 && !force) {
+	    freebsd_do_exec, freebsd_deregister) != 0 && force != 0) {
 		return -1;
 	}
 
 	if (fake == 0 && exec_pkg_scripts == 1) {
 		/** @todo Run +POST-DEINSTALL <pkg-name>/+DEINSTALL <pkg-name> POST-DEINSTALL */
-		if (pkg_run_script(real_pkg, NULL, pkg_script_post_deinstall)
-		    != 0 && !force) {
-			/* XXX */
-			return -1;
+		if (pkg_run_script(real_pkg, NULL, pkg_script_post_deinstall) != 0 &&
+		    force != 0) {
+		    warnx("post-deinstall script execution failed");
+		    return -1;
 		}
 	}
 
@@ -603,7 +646,7 @@
 		    install_data->directory);
 	}
 
-	if (!install_data->fake) {
+	if (install_data->fake == 0) {
 		pkg_dir_build(install_data->directory, 0);
 		return chdir(install_data->directory);
 	}

==== //depot/projects/soc2007/revised_fbsd_pkgtools/pkg_revised/v2/contrib/libpkg/pkg_freebsd.c#2 (text+ko) ====

@@ -994,7 +994,9 @@
 
 /**
  * @brief Callback for pkg_run_script()
- * @return 0
+ * @return 0 on success
+ * @return -1 on fail, noop or script not found.
+ * If script not found, set errno to ENOFILE.
  */
 static int
 freebsd_run_script(struct pkg *pkg, const char *prefix, pkg_script script)
@@ -1006,6 +1008,9 @@
 	char *cwd;
 	int ret = -1;
 
+	/* Make sure errno is 0.. */
+	errno = 0;
+
 	assert(pkg != NULL);
 
 	fpkg = pkg->data;
@@ -1056,8 +1061,10 @@
 	}
 
 	/* The script was not found so ignore it */
-	if (script_file == NULL)
-		return 0;
+	if (script_file == NULL) {
+		errno = ENOFILE;
+		return -1;
+	}
 
 	if (fpkg->pkg_type == fpkg_from_file) {
 		/**


More information about the p4-projects mailing list