Date: Thu, 15 Mar 2001 02:10:51 +0200 (EET) From: Maxim Sobolev <sobomax@freebsd.org> To: jkh@freebsd.org Cc: ports@freebsd.org, asami@freebsd.org, kris@freebsd.org Subject: Serious misbehaviour of ``pkg_add -r'' [patch] Message-ID: <200103150010.f2F0AqK53921@vic.sabbo.net>
next in thread | raw e-mail | index | archive | help
--%--multipart-mixed-boundary-1.53716.984615051--%
Content-Type: text/plain; charset=us-ascii
Content-Transfer-Encoding: 7bit
Hi folks,
As Kris pointed me out, a serious bug exists in our "remote fetch" pkg_add(1)
mode, i.e. the mode when using supplied name pkg_add automatically fetches
and installs requested package and all its dependencies.
This bug affects most of the cases when user tries to install package that
has multi-level dependencies, i.e. ``pkg_add -r (A)'', while (A) depends on
package (B) which in turn depends on package (C) and so on.
Unfortunately pkg_add(1) in this case doesn't apply any efforts to ensure
proper installation order, so in many cases it will try to install (B)
before (C), obviously failing miserably due to missed dependency and user
will end up with only (C) installed.
Good example of such misbehaviour is tkcvs-6.4 package. You can verify
this bug by deinstalling tcl/tk any to doing ``pkg_add -r tkcvs-6.4''.
After several error messages you will end with only tcl installed.
It appears that pkg_add(1) tries to install dependencies exactly in the
order in which they were specifies in the +CONTENTS file, which is a
reversed alphabetical order (bsd.ports.mk sorts them alphabetically and
passes to pkg_create, which reverses it then). For the tkcvs package
relevant part of +CONTENTS looks like:
@cwd /usr/local
@pkgdep tk-8.3.2
@pkgdep tcl-8.3.2
@pkgdep XFree86-libraries-4.0.1_2
@comment ORIGIN:devel/tkcvs
Thus, after fetching and unpacking tkcvs pkg_add(1) fetches and tries to
install tk, but fails because of tcl and XFree packages are missed. After
that it fetches/installs tcl and XFree, but fails to install tkcvs itself
due to missed tk package.
The first approach to that problem that came to my mind was to enhance
pkg_add(1) to analyse dependencies between packages after fetching each
particular package, but it is not very clear and easy solution because
of the following reasons:
1. In many cases this require that several packages to be fetched before
pkg_add would be able to install something (as you can see from the
tkcvs example, pkg_add have to fetch tkcvs, tk and tcl and only then
it will be able to install tcl);
2. as a result of 1 pkg_add have to store unpacked but have not installed
yet packages on the user's disk;
3. it is not very clear how to handle interrupted transmission (i.e. what
to do with already downloaded but not yet installed packages).
After thinking it over I came to a conclusion that there is at least one
much more easier approach. Instead of adding complexity into pkg_add(1)
we can enhance pkg_create to sort dependency list in such a way that it
will list packages in the order in which they could be subsequently added.
When creating package pkg_create already have all dependencies installed,
so it is easy for it to do proper sorting. Another advantage is that the
users will not have to upgrade their pkg_* tools to get correct functionality.
With this message I'm attaching patch, which adds reordering into pkg_create.
Please note that it is not completely finished (I'm planning to add some more
errors checking), but it worksfor me. I would like to get your comments on
it, because we have to move quickly as the problem described above should
be resolved *before* 4.3 ports freeze.
Thanks!
-Maxim
--%--multipart-mixed-boundary-1.53716.984615051--%
Content-Type: text/plain; charset=us-ascii
Content-Transfer-Encoding: 7bit
Content-Description: ASCII C program text
Content-Disposition: attachment; filename="pi.diff"
Index: create/perform.c
===================================================================
RCS file: /home/ncvs/src/usr.sbin/pkg_install/create/perform.c,v
retrieving revision 1.53
diff -d -u -r1.53 perform.c
--- create/perform.c 2001/01/22 12:01:54 1.53
+++ create/perform.c 2001/03/14 22:35:51
@@ -97,15 +97,37 @@
/* Stick the dependencies, if any, at the top */
if (Pkgdeps) {
+ char **deps;
+ char *tmp;
+ int i, ndeps;
+
if (Verbose && !PlistOnly)
printf("Registering depends:");
- while (Pkgdeps) {
+
+ /* Count number of dependencies */
+ tmp = strdup(Pkgdeps);
+ ndeps = 0;
+ while (tmp) {
+ cp = strsep(&tmp, " \t\n");
+ if (*cp)
+ ndeps++;
+ }
+ free(tmp);
+
+ /* Create easy to use NULL-terminated list */
+ deps = alloca(sizeof(*deps) * ndeps + 1);
+ for (i = 0; Pkgdeps; i++) {
cp = strsep(&Pkgdeps, " \t\n");
- if (*cp) {
- add_plist_top(&plist, PLIST_PKGDEP, cp);
- if (Verbose && !PlistOnly)
- printf(" %s", cp);
- }
+ if (*cp)
+ deps[i] = cp;
+ }
+ deps[ndeps] = NULL;
+
+ sortdeps(deps);
+ for (i = 0; i < ndeps; i++) {
+ add_plist_top(&plist, PLIST_PKGDEP, deps[i]);
+ if (Verbose && !PlistOnly)
+ printf(" %s", deps[i]);
}
if (Verbose && !PlistOnly)
printf(".\n");
Index: delete/perform.c
===================================================================
RCS file: /home/ncvs/src/usr.sbin/pkg_install/delete/perform.c,v
retrieving revision 1.28
diff -d -u -r1.28 perform.c
--- delete/perform.c 2001/03/04 17:38:38 1.28
+++ delete/perform.c 2001/03/14 22:35:51
@@ -30,7 +30,6 @@
static int pkg_do(char *);
static void sanity_check(char *);
static void undepend(PackingList, char *);
-static int chkifdepends(char *pkgname1, char *pkgname2);
static char LogDir[FILENAME_MAX];
@@ -38,10 +37,9 @@
pkg_perform(char **pkgs)
{
char **matched;
- char *tmp;
- int i, j;
+ int i;
int err_cnt = 0;
- int loop_cnt, errcode;
+ int errcode;
if (MatchType != MATCH_EXACT) {
matched = matchinstalled(MatchType, pkgs, &errcode);
@@ -65,36 +63,8 @@
}
}
+ err_cnt = sortdeps(pkgs);
for (i = 0; pkgs[i]; i++) {
- /*
- * Check to see if any other package in pkgs[i+1:] depends
- * on pkgs[i] and deffer removal of pkgs[i] if so.
- */
- loop_cnt = 0;
- for (j = i + 1; pkgs[j]; j++) {
- if (chkifdepends(pkgs[j], pkgs[i]) == 1) {
- /*
- * Try to avoid deadlock if package A depends on B which in
- * turn depends on C and C due to an error depends on A.
- * Use ugly but simple method, becase it Should Never
- * Happen[tm] in the real life anyway.
- */
- if (loop_cnt > 4096) {
- warnx("dependency loop detected for package %s", pkgs[j]);
- err_cnt++;
- break;
- }
- loop_cnt++;
- tmp = pkgs[i];
- pkgs[i] = pkgs[j];
- pkgs[j] = tmp;
- /*
- * Another iteration requred to check if new pkgs[i]
- * itself has any packages that depend on it
- */
- j = i + 1;
- }
- }
err_cnt += pkg_do(pkgs[i]);
}
@@ -364,38 +334,3 @@
return;
}
-/*
- * Check to see if pkgname1 depends on pkgname2.
- * Returns 1 if depends, 0 if not, and -1 if error occured.
- */
-static int
-chkifdepends(char *pkgname1, char *pkgname2)
-{
- FILE *fp;
- char fname[FILENAME_MAX];
- char fbuf[FILENAME_MAX];
- char *tmp;
- int retval;
-
- sprintf(fname, "%s/%s/%s",
- (tmp = getenv(PKG_DBDIR)) ? tmp : DEF_LOG_DIR,
- pkgname2, REQUIRED_BY_FNAME);
- fp = fopen(fname, "r");
- if (fp == NULL) {
- /* Probably pkgname2 doesn't have any packages that depend on it */
- return 0;
- }
-
- retval = 0;
- while (fgets(fbuf, sizeof(fbuf), fp) != NULL) {
- if (fbuf[strlen(fbuf)-1] == '\n')
- fbuf[strlen(fbuf)-1] = '\0';
- if (strcmp(fbuf, pkgname1) == 0) { /* match */
- retval = 1;
- break;
- }
- }
-
- fclose(fp);
- return retval;
-}
Index: lib/Makefile
===================================================================
RCS file: /home/ncvs/src/usr.sbin/pkg_install/lib/Makefile,v
retrieving revision 1.7
diff -d -u -r1.7 Makefile
--- lib/Makefile 2001/02/27 09:00:18 1.7
+++ lib/Makefile 2001/03/14 22:35:51
@@ -1,7 +1,8 @@
# $FreeBSD$
LIB= install
-SRCS= file.c msg.c plist.c str.c exec.c global.c pen.c match.c
+SRCS= file.c msg.c plist.c str.c exec.c global.c pen.c match.c \
+ deps.c
CFLAGS+= ${DEBUG}
NOPROFILE= yes
NOPIC= yes
Index: lib/deps.c
===================================================================
RCS file: deps.c
diff -N deps.c
--- /dev/null Wed Mar 14 14:35:25 2001
+++ deps.c Wed Mar 14 14:35:52 2001
@@ -0,0 +1,107 @@
+#ifndef lint
+static const char rcsid[] =
+ "$FreeBSD$";
+#endif
+
+/*
+ * FreeBSD install - a package for the installation and maintainance
+ * of non-core utilities.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ * 1. Redistributions of source code must retain the above copyright
+ * notice, this list of conditions and the following disclaimer.
+ * 2. Redistributions in binary form must reproduce the above copyright
+ * notice, this list of conditions and the following disclaimer in the
+ * documentation and/or other materials provided with the distribution.
+ *
+ * Maxim Sobolev
+ * 14 March 2001
+ *
+ * Routines used to do various operations with dependencies
+ * among installed packages.
+ *
+ */
+
+#include "lib.h"
+
+#include <err.h>
+#include <stdio.h>
+
+int
+sortdeps(char **pkgs)
+{
+ char *tmp;
+ int i, j, loop_cnt;
+ int err_cnt = 0;
+
+ for (i = 0; pkgs[i]; i++) {
+ /*
+ * Check to see if any other package in pkgs[i+1:] depends
+ * on pkgs[i] and swap those two packages if so.
+ */
+ loop_cnt = 0;
+ for (j = i + 1; pkgs[j]; j++) {
+ if (chkifdepends(pkgs[j], pkgs[i]) == 1) {
+ /*
+ * Try to avoid deadlock if package A depends on B which in
+ * turn depends on C and C due to an error depends on A.
+ * Use ugly but simple method, becase it Should Never
+ * Happen[tm] in the real life anyway.
+ */
+ if (loop_cnt > 4096) {
+ warnx("dependency loop detected for package %s", pkgs[j]);
+ err_cnt++;
+ break;
+ }
+ loop_cnt++;
+ tmp = pkgs[i];
+ pkgs[i] = pkgs[j];
+ pkgs[j] = tmp;
+ /*
+ * Another iteration requred to check if new pkgs[i]
+ * itself has any packages that depend on it
+ */
+ j = i + 1;
+ }
+ }
+ }
+ return err_cnt;
+}
+
+/*
+ * Check to see if pkgname1 depends on pkgname2.
+ * Returns 1 if depends, 0 if not, and -1 if error occured.
+ */
+int
+chkifdepends(char *pkgname1, char *pkgname2)
+{
+ FILE *fp;
+ char fname[FILENAME_MAX];
+ char fbuf[FILENAME_MAX];
+ char *tmp;
+ int retval;
+
+ sprintf(fname, "%s/%s/%s",
+ (tmp = getenv(PKG_DBDIR)) ? tmp : DEF_LOG_DIR,
+ pkgname2, REQUIRED_BY_FNAME);
+ fp = fopen(fname, "r");
+ if (fp == NULL) {
+ /* Probably pkgname2 doesn't have any packages that depend on it */
+ return 0;
+ }
+
+ retval = 0;
+ while (fgets(fbuf, sizeof(fbuf), fp) != NULL) {
+ if (fbuf[strlen(fbuf)-1] == '\n')
+ fbuf[strlen(fbuf)-1] = '\0';
+ if (strcmp(fbuf, pkgname1) == 0) { /* match */
+ retval = 1;
+ break;
+ }
+ }
+
+ fclose(fp);
+ return retval;
+}
Index: lib/lib.h
===================================================================
RCS file: /home/ncvs/src/usr.sbin/pkg_install/lib/lib.h,v
retrieving revision 1.32
diff -d -u -r1.32 lib.h
--- lib/lib.h 2001/02/27 09:00:18 1.32
+++ lib/lib.h 2001/03/14 22:35:52
@@ -177,6 +177,10 @@
/* Query installed packages */
char **matchinstalled(match_t, char **, int *);
+/* Dependencies */
+int sortdeps(char **);
+int chkifdepends(char *, char *);
+
/* Externs */
extern Boolean Verbose;
extern Boolean Fake;
--%--multipart-mixed-boundary-1.53716.984615051--%--
To Unsubscribe: send mail to majordomo@FreeBSD.org
with "unsubscribe freebsd-ports" in the body of the message
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200103150010.f2F0AqK53921>