From owner-p4-projects@FreeBSD.ORG Wed Jul 28 08:21:54 2010 Return-Path: Delivered-To: p4-projects@freebsd.org Received: by hub.freebsd.org (Postfix, from userid 32767) id 0298E106682D; Wed, 28 Jul 2010 08:21:42 +0000 (UTC) Delivered-To: perforce@FreeBSD.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 2AF1F1065F18 for ; Wed, 28 Jul 2010 08:21:40 +0000 (UTC) (envelope-from jlaffaye@FreeBSD.org) Received: from repoman.freebsd.org (repoman.freebsd.org [IPv6:2001:4f8:fff6::29]) by mx1.freebsd.org (Postfix) with ESMTP id 21C338FC12 for ; Wed, 28 Jul 2010 08:21:39 +0000 (UTC) Received: from repoman.freebsd.org (localhost [127.0.0.1]) by repoman.freebsd.org (8.14.4/8.14.4) with ESMTP id o6S8LdKn009244 for ; Wed, 28 Jul 2010 08:21:39 GMT (envelope-from jlaffaye@FreeBSD.org) Received: (from perforce@localhost) by repoman.freebsd.org (8.14.4/8.14.4/Submit) id o6S8LcR2009242 for perforce@freebsd.org; Wed, 28 Jul 2010 08:21:38 GMT (envelope-from jlaffaye@FreeBSD.org) Date: Wed, 28 Jul 2010 08:21:38 GMT Message-Id: <201007280821.o6S8LcR2009242@repoman.freebsd.org> X-Authentication-Warning: repoman.freebsd.org: perforce set sender to jlaffaye@FreeBSD.org using -f From: Julien Laffaye To: Perforce Change Reviews Precedence: bulk Cc: Subject: PERFORCE change 181486 for review X-BeenThere: p4-projects@freebsd.org X-Mailman-Version: 2.1.5 List-Id: p4 projects tree changes List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 28 Jul 2010 08:21:54 -0000 http://p4web.freebsd.org/@@181486?ac=10 Change 181486 by jlaffaye@jlaffaye-chulak on 2010/07/27 22:04:27 - Re-implemented the automatic installation of packages dependencies. - While I'm here, do not call pkg_add recursively but use pkg_do() - Moved the 'URL guess' logic from fetch_archive() to find_package_url(). - While I'm here, do not use a hard-coded value of '.tbz' for the archive extension but find it dynamically. Affected files ... .. //depot/projects/soc2010/pkg_complete/lib/libpkg/pkg.h#8 edit .. //depot/projects/soc2010/pkg_complete/lib/libpkg/url.c#4 edit .. //depot/projects/soc2010/pkg_complete/usr.sbin/pkg_install/add/add.h#4 edit .. //depot/projects/soc2010/pkg_complete/usr.sbin/pkg_install/add/extract.c#8 edit .. //depot/projects/soc2010/pkg_complete/usr.sbin/pkg_install/add/perform.c#8 edit Differences ... ==== //depot/projects/soc2010/pkg_complete/lib/libpkg/pkg.h#8 (text+ko) ==== @@ -180,8 +180,8 @@ Boolean issymlink(const char *); Boolean isURL(const char *); const char *fileGetURL(const char *, const char *, int); -int fetch_archive(struct archive *, const char *, const char *, - Boolean); +int find_package_url(char * restrict, const char *, const char *); +int fetch_archive(struct archive *, const char *, Boolean); char *fileFindByPath(const char *, const char *); char *fileGetContents(const char *); ssize_t write_file(const char *, const char *); ==== //depot/projects/soc2010/pkg_complete/lib/libpkg/url.c#4 (text+ko) ==== @@ -24,6 +24,7 @@ #include #include #include +#include #include #include #include @@ -192,70 +193,76 @@ } /* - * Setup the archive `a' callbacks to read data from an URL `spec' via fetch(3). - * If `spec' is not an URL, the function try to find the location of the file - * via `base' or via the environment variable `PKG_ADD_BASE'. + * Given a know-good URL `base', construct the URL to fetch `pkgname'. + * If the environment variable `PKG_ADD_BASE' (setted by sysinstall) exist, + * it is used to construct the URL ($PKG_ADD_BASE concatened with pkgname), + * and `base' is only used to find the file extension. + * The resulting URL string is stored at the memory of size FILENAME_MAX + * pointed by `p'. + * Returns 0 on success, 1 otherwise. + */ +int +find_package_url(char * restrict p, const char *base, const char *pkgname) +{ + char *cp; + char *ext; + + assert(p != NULL); + assert(base != NULL); + assert(pkgname != NULL); + + if ((ext = strrchr(base, '.')) == NULL) + ext = ".tbz"; + + /* Special tip that sysinstall left for us */ + if ((cp = getenv("PKG_ADD_BASE")) != NULL) { + strlcpy(p, cp, FILENAME_MAX); + strlcat(p, pkgname, FILENAME_MAX); + strlcat(p, ext, FILENAME_MAX); + } else { + strlcpy(p, base, FILENAME_MAX); + /* + * Advance back two slashes to get to the root of the + * package hierarchy, then append pkgname. + */ + cp = strrchr(p, '/'); + if (cp != NULL) { + *cp = '\0'; /* chop name */ + cp = strrchr(p, '/'); + } + if (cp != NULL) { + *(cp + 1) = '\0'; + strlcat(p, "All/", FILENAME_MAX); + strlcat(p, pkgname, FILENAME_MAX); + strlcat(p, ext, FILENAME_MAX); + } else + return (1); + } + + return (0); +} + +/* + * Setup the archive `a' callbacks to read data from an URL `url' via fetch(3). * Returns 0 on success, 1 otherwise. */ int -fetch_archive(struct archive *a, const char *base, const char *spec, - Boolean keep_package) +fetch_archive(struct archive *a, const char *url, Boolean keep_package) { struct fetch_data *data = NULL; - char *cp, *hint, *tmp; + char *tmp; char fname[FILENAME_MAX]; char pkg[FILENAME_MAX]; int retcode = 0; + if (!isURL(url)) { + warnx("fetch_archive(): '%s' is not an URL!", url); + return (1); + } + if ((data = malloc(sizeof(struct fetch_data))) == NULL) err(EXIT_FAILURE, "malloc()"); - if (!isURL(spec)) { - /* - * We've been given an existing URL (that's known-good) and now - * we need to construct a composite one out of that and the - * basename we were handed as a dependency. - */ - if (base != NULL) { - strcpy(fname, base); - /* - * Advance back two slashes to get to the root of the - * package hierarchy - */ - cp = strrchr(fname, '/'); - if (cp) { - *cp = '\0'; /* chop name */ - cp = strrchr(fname, '/'); - } - if (cp != NULL) { - *(cp + 1) = '\0'; - strcat(cp, "All/"); - strcat(cp, spec); - strcat(cp, ".tbz"); - } else { - retcode = 1; - goto cleanup; - } - } - /* Special tip that sysinstall left for us */ - else if ((hint = getenv("PKG_ADD_BASE")) != NULL) { - /* - * Otherwise, we've been given an environment variable - * hinting at the right location from sysinstall - */ - strcpy(fname, hint); - strcat(fname, spec); - strcat(fname, ".tbz"); - } - /* We dont have an url and are unable to guess one */ - else { - retcode = 1; - goto cleanup; - } - } - else - strcpy(fname, spec); - if (keep_package) { tmp = getenv("PKGDIR"); strlcpy(pkg, tmp ? tmp : ".", sizeof(pkg)); ==== //depot/projects/soc2010/pkg_complete/usr.sbin/pkg_install/add/add.h#4 (text+ko) ==== @@ -42,7 +42,8 @@ int make_hierarchy(char *); void apply_perms(const char *, const char *); -int extract_package(struct archive *, Package *); +int extract_package(struct archive *, Package *, const char *); int extract_plist(struct archive *, struct archive_entry *, Package *); +int pkg_do(const char *); #endif /* _INST_ADD_H_INCLUDE */ ==== //depot/projects/soc2010/pkg_complete/usr.sbin/pkg_install/add/extract.c#8 (text+ko) ==== @@ -66,7 +66,7 @@ } int -extract_package(struct archive *a, Package *pkg) +extract_package(struct archive *a, Package *pkg, const char *fname) { PackingList p; struct archive_entry *entry; @@ -188,113 +188,108 @@ } #endif - /* Now check the packing list for dependencies */ - for (p = pkg->head; p ; p = p->next) { - char *deporigin; + /* Now check the packing list for dependencies */ + for (p = pkg->head; p ; p = p->next) { + char *deporigin; - if (p->type != PLIST_PKGDEP) - continue; - deporigin = (p->next->type == PLIST_DEPORIGIN) ? p->next->name : NULL; - if (Verbose) { - printf("Package '%s' depends on '%s'", pkg->name, p->name); - if (deporigin != NULL) - printf(" with '%s' origin", deporigin); - printf(".\n"); - } -#if 0 - if (isinstalledpkg(p->name) <= 0 && - !(deporigin != NULL && matchbyorigin(deporigin, NULL) != NULL)) { - char path[FILENAME_MAX]; - const char *cp = NULL; + if (p->type != PLIST_PKGDEP) + continue; - if (!Fake) { - char prefixArg[2 + MAXPATHLEN]; /* "-P" + Prefix */ - if (PrefixRecursive) { - strlcpy(prefixArg, "-P", sizeof(prefixArg)); - strlcat(prefixArg, Prefix, sizeof(prefixArg)); + if (p->next->type == PLIST_DEPORIGIN) + deporigin = p->next->name; + else + deporigin = NULL; + if (Verbose) { + printf("Package '%s' depends on '%s'", pkg->name, p->name); + if (deporigin != NULL) + printf(" with '%s' origin", deporigin); + printf(".\n"); } - if (!isURL(pkg) && !getenv("PKG_ADD_BASE")) { - const char *ext; + + if (isinstalledpkg(p->name) <= 0 && + !(deporigin != NULL && matchbyorigin(deporigin, NULL) != + NULL)) { + char path[FILENAME_MAX]; + const char *cp = NULL; + + if (!Fake) { + /* + * Since we are calling pkg_do() recursively, + * set the global variable `Prefix' to NULL. + * It is safe to do so because we are already done + * with it at this level. + */ + if (PrefixRecursive == FALSE) + Prefix = NULL; + + if (!isURL(fname) && !getenv("PKG_ADD_BASE")) { + const char *ext; + + ext = strrchr(fname, '.'); + if (ext == NULL) + ext = ".tbz"; + snprintf(path, FILENAME_MAX, "%s/%s%s", + getenv("_TOP"), p->name, ext); + if (fexists(path)) + cp = path; + else + cp = fileFindByPath(fname, p->name); + + if (cp) { + if (Verbose) + printf("Loading it from %s.\n", cp); + if (pkg_do(cp) != 0) { + warnx("autoload of dependency '%s' " + "failed%s", cp, + Force ? " (proceeding anyway)" : "!"); + if (!Force) + ++code; + } + } + else { + warnx("could not find package %s %s", p->name, + Force ? "(proceeding anyway)" : "!"); + if (!Force) + ++code; + } + } + else { + char dep_url[FILENAME_MAX]; + if (find_package_url(dep_url, fname, p->name) != 1) + errx(1, "Can not make an URL to get %s", + p->name); - ext = strrchr(pkg, '.'); - if (ext == NULL) - ext = ".tbz"; - snprintf(path, FILENAME_MAX, "%s/%s%s", getenv("_TOP"), - p->name, ext); - if (fexists(path)) - cp = path; - else - cp = fileFindByPath(pkg, p->name); - if (cp) { - if (Verbose) - printf("Loading it from %s.\n", cp); - if (vsystem("%s %s %s '%s'", PkgAddCmd, Verbose ? "-v " -: "", PrefixRecursive ? prefixArg : "", cp)) { - warnx("autoload of dependency '%s' failed%s", - cp, Force ? " (proceeding anyway)" : "!"); - if (!Force) - ++code; + if (pkg_do(dep_url) != 0) { + warnx("pkg_add of dependency %s failed%s", + p->name, + Force ? " (proceeding anyway)" : "!"); + if (!Force) + ++code; + } else + if (Verbose) + printf("%s added successfully\n", p->name); } + /* pkg_do() modified this global, so reset it */ + extract_state = 0; } + /* XXX: WTF is this logic ? */ else { - warnx("could not find package %s %s", - p->name, Force ? " (proceeding anyway)" : "!"); + if (Verbose) + printf("and was not found%s.\n", + Force ? " (proceeding anyway)" : ""); + else + printf("Package dependency %s for %s not found%s\n", + p->name, pkg->name, + Force ? " (proceeding anyway)" : "!"); if (!Force) ++code; } } - else { - - if ((cp = fileGetURL(pkg, p->name, KeepPackage)) == NULL) { - cleanup(); - warnx("Getting pkg '%s' by URL failed", pkg); - goto bomb; - } else { - - if (Verbose) - printf("Finished loading %s via a URL\n", p->name); - if (!fexists(CONTENTS_FNAME)) { - warnx("autoloaded package %s has no %s file?", - p->name, CONTENTS_FNAME); - if (!Force) - ++code; - } - else if (vsystem("(pwd; /bin/cat " CONTENTS_FNAME - ") | %s %s %s %s -S", PkgAddCmd, - Verbose ? "-v" : "", - PrefixRecursive ? prefixArg : "", - KeepPackage ? "-K" : "")) { - warnx("pkg_add of dependency '%s' failed%s", - p->name, Force ? " (proceeding anyway)" : "!"); - if (!Force) - ++code; - } - else if (Verbose) - printf("\t'%s' loaded successfully.\n", p->name); - /* Nuke the temporary playpen */ - leave_playpen(); - - } - - } + else if (Verbose) + printf(" - already installed.\n"); } - else { - if (Verbose) - printf("and was not found%s.\n", - Force ? " (proceeding anyway)" : ""); - else - printf("Package dependency %s for %s not found%s\n", - p->name, pkg, Force ? " (proceeding anyway)" : "!"); - if (!Force) - ++code; - } - } - else if (Verbose) - printf(" - already installed.\n"); -#endif - } - } /* if (!IgnoreDeps) */ + } /* if (!IgnoreDeps) */ if (code != 0) return (1); ==== //depot/projects/soc2010/pkg_complete/usr.sbin/pkg_install/add/perform.c#8 (text+ko) ==== @@ -33,7 +33,6 @@ #include "cleanup.h" void cleanup(void); -static int pkg_do(char *); extern int extract_state; extern char db_dir_tmp[FILENAME_MAX]; @@ -61,8 +60,8 @@ * as first parameter and install it. * Returns 0 on success, 1 otherwise. */ -static int -pkg_do(char *fname) +int +pkg_do(const char *fname) { Package pkg; struct archive *a = NULL; @@ -81,7 +80,7 @@ * add support for complete packages */ if (isURL(fname)) { - if (fetch_archive(a, NULL, fname, KeepPackage) != 0) { + if (fetch_archive(a, fname, KeepPackage) != 0) { warnx("Can not fetch '%s' - aborting", fname); retcode = 1; goto cleanup; @@ -113,7 +112,7 @@ retcode = 1; goto cleanup; } - retcode = extract_package(a, &pkg); + retcode = extract_package(a, &pkg, fname); free_plist(&pkg); } else if (strcmp(pathname, "+PKG_COMPLETE") == 0) { if (Verbose)