Commit 3855f7eb authored by Frits Daalmans's avatar Frits Daalmans

The run-debootstrap option --exclude=pkg1,pkg2,pkg3 is put in

environment variable DEBOOTSTRAP_EXCLUDE_FIELD, where it is picked up
by the compiled C program pkgdetails (NOT any pkgdetails perl script,
internal to debootstrap/functions or elsewhere).

This version of pkgdetails picks up DEBOOTSTRAP_EXCLUDE_FIELD and uses it
to de-select alternatives in a Depends: line, of the form
Package: wibble
Depends: foo, altpkgA | altpkgB | altpkgC, bar (<= 42)
Normally, pkgdetails would make wibble depend on foo, altpkgA and bar
but with DEBOOTSTRAP_EXCLUDE_FIELD=altpkgA,altpkgB ,
we make wibble depend on foo, altpkgC and bar

This program currently doesn't do a check on the version number constraint.

Also, I left all the DBG comments in for now.
parent 6664926a
...@@ -10,6 +10,16 @@ ...@@ -10,6 +10,16 @@
char *checksum_field=NULL; char *checksum_field=NULL;
/* exclude_field is a comma-separated list of dependencies that you want to exclude, for example:
Package: init
Pre-Depends: systemd-sysv | sysvinit-core | upstart
This will always give systemd-sysv with GETDEPS, unless you use
DEBOOTSTRAP_EXCLUDE_FIELD=systemd-sysv,sysvinit-core pkgdetails Packages init
*/
char *exclude_field=NULL;
char **exclude_array=NULL;
static void oom_die(void) static void oom_die(void)
{ {
fputs("Out of memory!\n", stderr); fputs("Out of memory!\n", stderr);
...@@ -37,6 +47,61 @@ static char *xasprintf(const char *fmt, ...) { ...@@ -37,6 +47,61 @@ static char *xasprintf(const char *fmt, ...) {
return ret; return ret;
} }
static void parse_exclude_field(char *str) {
char *workptr, *saveptr = NULL, *tok = NULL;
workptr = strdup(str);
int nf=0,f;
tok=workptr;
do {
saveptr=strchr(tok,',');
tok++;
if (saveptr) tok=saveptr+1;
nf++;
} while(saveptr);
fprintf(stderr,"DBG parse_exclude_field(%s), %d fields\n",str, nf);
exclude_array = (char **) malloc((nf+1) * sizeof(char *));
if (exclude_array==NULL){
oom_die();
}
tok = strtok_r(workptr, ",", &saveptr);
for(f=0;f<nf;f++){
exclude_array[f] = strdup(tok);
fprintf(stderr,"DBG exclude_array[%d] = \"%s\"\n", f, exclude_array[f]);
tok = strtok_r(NULL, ",", &saveptr);
}
exclude_array[nf+1-1]=NULL;
free(workptr);
}
static void free_exclude_array(void) {
int f;
if (exclude_array==NULL) return;
f=0;
while(exclude_array[f]!=NULL){
free(exclude_array[f]);
exclude_array[f]=NULL;
f++;
}
free(exclude_array);
exclude_array=NULL;
}
static int exclude_dep(char *dep, char *versionclause) {
int f;
if (exclude_array==NULL) return(0);
f=0;
while(exclude_array[f]){
if (!strcmp(exclude_array[f], dep)){
return(1);
}
f++;
}
return(0);
}
static char *fieldcpy(char *dst, char *fld) { static char *fieldcpy(char *dst, char *fld) {
while (*fld && *fld != ':') while (*fld && *fld != ':')
fld++; fld++;
...@@ -63,6 +128,154 @@ static void outputdeps(char *deps) { ...@@ -63,6 +128,154 @@ static void outputdeps(char *deps) {
} }
} }
typedef struct deps_proto {
char *cur_pkg;
int nd;
#define DEPCHUNK 16
char ** deps;
} deps_t, *deps_p;
deps_p deps_new(void) {
deps_p n = (deps_p) malloc(sizeof(deps_t));
if (n==NULL) oom_die();
memset(n, 0x00, sizeof(deps_t));
n->cur_pkg = NULL;
n->nd = 0;
n->deps = NULL;
return(n);
}
void deps_free(deps_p d) {
int i;
if (d==NULL) return;
for(i=0;i<d->nd;i++){
if (d->deps[i]) {
free(d->deps[i]);
d->deps[i] = NULL;
}
}
d->nd = 0;
free(d);
}
void deps_setpkgname(deps_p d, char *cur_pkg) {
if (d->cur_pkg) free(d->cur_pkg);
d->cur_pkg = strdup(cur_pkg);
}
void deps_add_4(deps_p d, char *dep) {
int di;
/* Typically, a package depends on 0-10 other ones. I can't be
bothered to write a sort function for that. Slowsort it is. */
for (di=0;di<d->nd;di++){
if (!strcmp(d->deps[di], dep)) return;
}
/* extend if necessary */
if ((d->nd % DEPCHUNK) == 0) {
d->deps = (char **) realloc(d->deps, (d->nd + DEPCHUNK) * sizeof(char *));
if (d->deps == NULL) oom_die();
for(di=0;di < DEPCHUNK;di++) d->deps[d->nd + di] = NULL;
}
/* add it */
d->deps[d->nd] = strdup(dep);
fprintf(stderr,"DBG %s (pre-)depends on %s\n", d->cur_pkg, d->deps[d->nd]);
d->nd++;
}
int deps_add_3(deps_p d, char *versioneddep) {
char *workptr, *tok3, *saveptr3, *versionclause;
int used;
fprintf(stderr,"DBG deps_add_3(cur_pkg=%s , %s )\n",d->cur_pkg, versioneddep);
workptr = strdup(versioneddep);
tok3 = strtok_r(workptr, " (", &saveptr3);
versionclause = strtok_r(NULL, " (", &saveptr3);
/* versionclause is not used, but keep lint happy */
if (versionclause) {
while(*versionclause == ' ') versionclause++;
}
used = !exclude_dep(tok3, versionclause);
if (!used) {
fprintf(stderr,"W: pkgdetails: skip %s dependency on %s\n", d->cur_pkg, tok3);
} else {
/* add it to the list */
deps_add_4(d, tok3);
}
free(workptr);
return(used);
}
void deps_add_2(deps_p d, char *depsaltlist) {
char *workptr, *tok2, *saveptr2;
int used, chose_alternative;
fprintf(stderr,"DBG deps_add_2(cur_pkg=%s , %s )\n",d->cur_pkg, depsaltlist);
workptr = strdup(depsaltlist);
tok2 = strtok_r(workptr, " |", &saveptr2);
chose_alternative = 0;
used = 0;
while(tok2){
used = deps_add_3(d, tok2);
if (used) break; // we want only the first alternative from the list
tok2 = strtok_r(NULL, " |", &saveptr2);
fprintf(stderr,"I: pkgdetails: consider %s dependency on %s\n", d->cur_pkg, tok2);
chose_alternative = 1;
}
if (! used) {
fprintf(stderr,"E: pkgdetails: none of the %s dependencies chosen from alternatives %s\n", d->cur_pkg, depsaltlist);
}
if (chose_alternative) {
fprintf(stderr,"I: pkgdetails: used %s dependency on %s\n", d->cur_pkg, tok2);
}
free(workptr);
}
void deps_add(deps_p d, char *depslist) {
char *workptr, *tok, *saveptr;
fprintf(stderr,"DBG cur_pkg=%s doing (Pre-)depends on %s\n",d->cur_pkg, depslist);
/* skip initial space */
while ((depslist[0])&&(isspace(depslist[0]))) depslist++;
workptr = strdup(depslist);
tok = strtok_r(workptr, ",", &saveptr);
while(tok){
deps_add_2(d, tok);
tok = strtok_r(NULL, ",", &saveptr);
}
free(workptr);
}
void deps_output(FILE *f, deps_p d) {
int i;
if ((f==NULL)||(d==NULL)||(d->nd==0)) return;
for(i=0;i<d->nd;i++){
fputs(d->deps[i], f);
fputs("\n", f);
}
}
/* The syntax of a Debian Depends: or Pre-Depends: line is as follows:
Depends: deps
Pre-Depends: deps
deps = dep | deps "," dep
dep = versioneddep | altdep
altdep = versioneddep "|" versioneddep | versioneddep "|" altdep
versioneddep = simpledep | simpledep "(" compareclause ")"
simpledep = packagename
It needs to be parsed accurately to properly do alternatives, if we
don't like the first choice of an altdep list (hint: init)
*/
static void dogetdeps(char *pkgsfile, char **in_pkgs, int pkgc) { static void dogetdeps(char *pkgsfile, char **in_pkgs, int pkgc) {
char buf[MAX_LINE]; char buf[MAX_LINE];
char cur_pkg[MAX_LINE]; char cur_pkg[MAX_LINE];
...@@ -74,6 +287,7 @@ static void dogetdeps(char *pkgsfile, char **in_pkgs, int pkgc) { ...@@ -74,6 +287,7 @@ static void dogetdeps(char *pkgsfile, char **in_pkgs, int pkgc) {
int skip; int skip;
FILE *f; FILE *f;
int output_pkg = -1; int output_pkg = -1;
deps_p curdeps = NULL;
cur_pkg[0] = cur_deps[0] = cur_predeps[0] = prev_pkg[0] = '\0'; cur_pkg[0] = cur_deps[0] = cur_predeps[0] = prev_pkg[0] = '\0';
...@@ -85,6 +299,8 @@ static void dogetdeps(char *pkgsfile, char **in_pkgs, int pkgc) { ...@@ -85,6 +299,8 @@ static void dogetdeps(char *pkgsfile, char **in_pkgs, int pkgc) {
exit(1); exit(1);
} }
curdeps = deps_new();
skip = 1; skip = 1;
while (fgets(buf, sizeof(buf), f)) { while (fgets(buf, sizeof(buf), f)) {
if (*buf && buf[strlen(buf)-1] == '\n') buf[strlen(buf)-1] = '\0'; if (*buf && buf[strlen(buf)-1] == '\n') buf[strlen(buf)-1] = '\0';
...@@ -92,6 +308,7 @@ static void dogetdeps(char *pkgsfile, char **in_pkgs, int pkgc) { ...@@ -92,6 +308,7 @@ static void dogetdeps(char *pkgsfile, char **in_pkgs, int pkgc) {
int any = 0; int any = 0;
skip = 1; skip = 1;
fieldcpy(cur_pkg, buf); fieldcpy(cur_pkg, buf);
deps_setpkgname(curdeps, cur_pkg);
if (strcmp(cur_pkg, prev_pkg) != 0) { if (strcmp(cur_pkg, prev_pkg) != 0) {
if (output_pkg != -1) if (output_pkg != -1)
pkgs[output_pkg] = NULL; pkgs[output_pkg] = NULL;
...@@ -114,15 +331,20 @@ static void dogetdeps(char *pkgsfile, char **in_pkgs, int pkgc) { ...@@ -114,15 +331,20 @@ static void dogetdeps(char *pkgsfile, char **in_pkgs, int pkgc) {
} }
if (!any) break; if (!any) break;
} else if (!skip && strncasecmp(buf, "Depends:", 8) == 0) } else if (!skip && strncasecmp(buf, "Depends:", 8) == 0)
fieldcpy(cur_deps, buf); {
deps_add(curdeps, &buf[8]);
}
else if (!skip && strncasecmp(buf, "Pre-Depends:", 12) == 0) else if (!skip && strncasecmp(buf, "Pre-Depends:", 12) == 0)
fieldcpy(cur_predeps, buf); {
deps_add(curdeps, &buf[12]);
}
} }
if (cur_deps[0])
outputdeps(cur_deps); deps_output(stdout, curdeps);
if (cur_predeps[0])
outputdeps(cur_predeps);
fclose(f); fclose(f);
deps_free(curdeps);
} }
static void dopkgmirrorpkgs(int uniq, char *mirror, char *pkgsfile, static void dopkgmirrorpkgs(int uniq, char *mirror, char *pkgsfile,
...@@ -210,6 +432,7 @@ static void dopkgmirrorpkgs(int uniq, char *mirror, char *pkgsfile, ...@@ -210,6 +432,7 @@ static void dopkgmirrorpkgs(int uniq, char *mirror, char *pkgsfile,
} }
} }
} }
} }
static void dopkgstanzas(char *pkgsfile, char **pkgs, int pkgc) static void dopkgstanzas(char *pkgsfile, char **pkgs, int pkgc)
...@@ -304,6 +527,13 @@ int main(int argc, char *argv[]) { ...@@ -304,6 +527,13 @@ int main(int argc, char *argv[]) {
checksum_field="MD5sum"; checksum_field="MD5sum";
} }
exclude_field=getenv("DEBOOTSTRAP_EXCLUDE_FIELD");
if (exclude_field != NULL) {
parse_exclude_field(exclude_field);
} else {
exclude_array = NULL;
}
if ((argc == 6 || argc == 5) && strcmp(argv[1], "WGET%") == 0) { if ((argc == 6 || argc == 5) && strcmp(argv[1], "WGET%") == 0) {
if (dotranslatewgetpercent(atoi(argv[2]), atoi(argv[3]), if (dotranslatewgetpercent(atoi(argv[2]), atoi(argv[3]),
atoi(argv[4]), argc == 6 ? argv[5] : NULL)) atoi(argv[4]), argc == 6 ? argv[5] : NULL))
...@@ -318,6 +548,8 @@ int main(int argc, char *argv[]) { ...@@ -318,6 +548,8 @@ int main(int argc, char *argv[]) {
dogetdeps(argv[2], argv+i, MAX_PKGS); dogetdeps(argv[2], argv+i, MAX_PKGS);
} }
dogetdeps(argv[2], argv+i, argc-i); dogetdeps(argv[2], argv+i, argc-i);
printf("crash_here\n");
free_exclude_array();
exit(0); exit(0);
} else if (argc >= 5 && strcmp(argv[1], "PKGS") == 0) { } else if (argc >= 5 && strcmp(argv[1], "PKGS") == 0) {
int i; int i;
......
...@@ -442,7 +442,13 @@ main(int argc, char *argv[]) ...@@ -442,7 +442,13 @@ main(int argc, char *argv[])
args = (char **)malloc(sizeof(char *) * (argc + 1)); args = (char **)malloc(sizeof(char *) * (argc + 1));
args[0] = "/usr/sbin/debootstrap"; args[0] = "/usr/sbin/debootstrap";
for (i = 1; i < argc; i++) for (i = 1; i < argc; i++)
{
args[i] = argv[i]; args[i] = argv[i];
if (!strncmp(args[i], "--exclude=", strlen("--exclude="))){
setenv("DEBOOTSTRAP_EXCLUDE_FIELD", &args[i][strlen("--exclude=")], 1);
fprintf(stderr,"DBG: run-debootstrap with DEBOOTSTRAP_EXCLUDE_FIELD=%s\n",getenv("DEBOOTSTRAP_EXCLUDE_FIELD"));
}
}
args[argc] = NULL; args[argc] = NULL;
return exec_debootstrap(args); return exec_debootstrap(args);
} }
  • I really think this is the wrong approach. It only will serve to hide dependency issues rather then solve them. If packages depend on systemd, we need to fix those packages and not just pretend the dependency doesn't exist.

    Scratch that, after a deeper review and understanding of the code, I'm ok with this approach in principle.

    Edited
  • I'm tend to be in favor of having both: the possibility of a new DEBOOTSTRAP_EXCLUDE_FIELD into the installer AND the ongoing effort of eliminating all dependencies from packages.

    For a final decision I'd rather leave the last word to nextime, with whom we are planning already "radar" to signal new dependency changes and a parser to analyse the current base packages dependencies and signal systemd presence, which we will then fix.

    Of course this is the correct approach, yet the EXCLUDE field proposed here can still be seen as a safety belt.

    The code is rather complex and includes a C parser which may be well prone to buffer overflows. I did not read it in detail but would be very good to have some test units for this and to run Valgrind over it. The concern of maintainability is my main concern for this code here in fact...

  • I agree with Jaromil re the complexity and would like to see at a minimum this split up into function specific commits to make reviewing easier.

  • Daniel, Jaromil, thank you for looking at my patch.

    Re: the complexity: I agree that my patch is complex. And the original code of pkgdetails probably has been around for some time, and may have been tested well.

    But I found that the original code was inadequate to deal with cases like Pre-Depends: systemd-sysv | sysvinit-core | upstart , something which the "full size" debootstrap does well. If there is an alternative way, to make the low-level installer run-debootstrap deal with these cases, I'm all ears. The current pkgdetails just parses Depends: a , b | c | d , e (>= 3) | f (= 4) as: Depends: a , b , e (until the first space character it encounters)

    I'll try to test this code a bit more, then I'll turn my attention back on the live-build scripts which work quite well now.

    I'll upload my changes to live-build, with the comment that you can't use such a live DVD for installation yet.

  • I like the idea, i like less the code and the way the idea is implemented.

    I'm all to accept the feature added by this patch, but i think a serious code rewrite is needed.

  • I take the liberty to say some things asked in private conversation:

    As far as I know, there are two (three?) versions of pkgdetails: a "large" one, written in perl, included inside the debootstrap functions shellscript. This one is fixed and has no errors that I know of. There is mention of a "standalone" pkgdetails_perl but I haven't found it. The debootstrap/functions code contains the following comment:

    # NOTE

    # For the debootstrap udeb, pkgdetails is provided by the bootstrap-base

    # udeb, so the pkgdetails API needs to be kept in sync with that.

    And also in the base-installer package, which makes bootstrap-base and base-installer .udebs, a "tiny" pkgdetails written in C, that is the one we're talking about here, that I patched.

    I'm not sure, but I suspect they made a special 12 kb executable so that it runs inside the most primitive installer system, before bash and perl etc. are installed.

    I like Daniel's idea to make base-installer make a pkgdetails.deb. But I haven't done a careful review to see if the C program does exactly the same things as the perl script.

    Maybe let base-installer make a pkgdetails_getdeps.deb only :-)

    I tested the code with valgrind, no memory leaks or errors left after the next commit ahem. I only tested the "GETDEPS" functionality because I don't really know what the other functions do. 90% of my patch is boilerplate code, because that's how I'm used to work. Basically it parses commas, then | symbols (new!), then (), but it doesn't do anything with those yet.

    Neither does the perl program, if I look at line 1203 of /usr/share/debootstrap/functions : (close your eyes if you don't like perl) (I must admit the perl function is a lot shorter than my patch ;-D )

    if ($in and (/^Depends: (.)$/ or /^Pre-Depends: (.)$/)) {

                for $d (split /\s*,\s*/, $1) {
    
                        for $p (split /\s*\|\s*/, $d) {
    
                                $p =~ s/\s*[(].*[)]\s*//;
    
                                if (!$exclude{$p}) {
    
                                        push @d, $p;
    
                                        last;
    
                                }
    
                        }
    
                }
    
        }
    Edited
  • Very cool, saw your new commits. Lets continue here https://git.devuan.org/d-i/base-installer/issues/20

    I think we'll be able to include this pretty soon.

    As of normalization between the C and Perl implementation, the main solution for it is a test suite.

Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment