Date: Sun, 30 May 2010 17:28:17 -0700 From: Garrett Cooper <gcooper@FreeBSD.org> To: Ivan Voras <ivoras@freebsd.org> Cc: Perforce Change Reviews <perforce@freebsd.org> Subject: Re: PERFORCE change 178984 for review Message-ID: <AANLkTikLZZ4VjkR97k53p-wYeXhny5yyKrSeBzYBAU4E@mail.gmail.com> In-Reply-To: <201005302335.o4UNZLsl028791@repoman.freebsd.org> References: <201005302335.o4UNZLsl028791@repoman.freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On Sun, May 30, 2010 at 4:35 PM, Ivan Voras <ivoras@freebsd.org> wrote: > http://p4web.freebsd.org/@@178984?ac=3D10 > > Change 178984 by ivoras@betelgeuse on 2010/05/30 23:34:24 > > =A0 =A0 =A0 =A0Step 4: (not finished): Start creating the actual patch bi= nary > > Affected files ... > > .. //depot/projects/soc2010/pkg_patch/src/patch/Makefile#8 edit > .. //depot/projects/soc2010/pkg_patch/src/patch/hashjob.c#7 edit > .. //depot/projects/soc2010/pkg_patch/src/patch/hashjob.h#7 edit > .. //depot/projects/soc2010/pkg_patch/src/patch/main.c#8 edit > .. //depot/projects/soc2010/pkg_patch/src/patch/mkpatch.c#6 edit > .. //depot/projects/soc2010/pkg_patch/src/patch/mkpatch.h#6 edit > .. //depot/projects/soc2010/pkg_patch/src/patch/pkg_patch.h#6 edit > .. //depot/projects/soc2010/pkg_patch/src/patch/support.c#5 edit > [...] > + > + =A0 =A0 =A0 /* > + =A0 =A0 =A0 =A0* XXX: Possibly reimplement with libarchive. If I finall= y get how it > + =A0 =A0 =A0 =A0* stores directories. > + =A0 =A0 =A0 =A0*/ > + =A0 =A0 =A0 sprintf(tmp, "%s/%s", dpatch, PKGPATCH_FNAME); snprintf ? This will still be an issue because you have to use archive_entry_copy_pathname anyhow for just files anyways. > + =A0 =A0 =A0 fp =3D fopen(tmp, "w"); > + =A0 =A0 =A0 if (fp =3D=3D NULL) > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 err(1, "Cannot open file for writing: %s", = tmp); > + =A0 =A0 =A0 time(&tm); from time(3): DESCRIPTION The time() function returns the value of time in seconds since 0 hours= , 0 minutes, 0 seconds, January 1, 1970, Coordinated Universal Time. If a= n error occurs, time() returns the value (time_t)-1. The return value is also stored in *tloc, provided that tloc is non-nu= ll. ERRORS The time() function may fail for any of the reasons described in gettimeofday(2). > + =A0 =A0 =A0 fprintf(fp, "# FreeBSD package patch archive created on %s\= n", > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 ctime(&tm)); > + =A0 =A0 =A0 fprintf(fp, "@version %s\n", PKGPATCH_VERSION); > + =A0 =A0 =A0 parse_package_name(fold, tmp, tmp2, NULL); > + =A0 =A0 =A0 fprintf(fp, "@source %s-%s\n", tmp, tmp2); > + =A0 =A0 =A0 parse_package_name(fnew, tmp, tmp2, NULL); > + =A0 =A0 =A0 fprintf(fp, "@target %s-%s\n", tmp, tmp2); > + =A0 =A0 =A0 SLIST_FOREACH(fl, &fldiff_new_old, linkage) > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 fprintf(fp, "@add %s\n", fl->filename); > + =A0 =A0 =A0 SLIST_FOREACH(fl, &fldiff_old_new, linkage) > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 fprintf(fp, "@remove %s\n", fl->filename); > + =A0 =A0 =A0 SLIST_FOREACH(fl, &flchanged, linkage) > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 fprintf(fp, "@patch [method=3Dcp] %s\n", fl= ->filename); > + =A0 =A0 =A0 if (fclose(fp) !=3D 0) > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 err(1, "Cannot close %s", PKGPATCH_FNAME); > + > + =A0 =A0 =A0 /* Include all metadata files from the new package. */ > + =A0 =A0 =A0 SLIST_FOREACH(fl, &flnew, linkage) { > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 if (fl->filename[0] =3D=3D '+') { I talked to kientzle about this in the past and while he and I both agree that + prefixed files are a package metadata, the problem is that it's not a widely held standard and non-conforming packages could definitely cause minor performance and functional problems in this area if someone upstream creates a package with filenames prefixed in the `+'. With the current mess trying to integrate archive(5) support into pkg_create, I have deeply considered creating a directory within each package called .fpkg-metadata/ where the metadata files will live. That way it would make the purpose of said files easy, and would make it easy to extract the files and inspect them. It would require a trivial amount of rework within pkg_install. My only concern would be with existing tools outside of pkg_install that might depend on +CONTENTS existing within the package at a particular location. > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 sprintf(tmp, "%s/%s", dnew,= fl->filename); > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 sprintf(tmp2, "%s/%s", dpat= ch, fl->filename); snprintf (x2) ? > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 if (copy_file_absolute(tmp,= tmp2) !=3D 0) > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 err(1, "Can= not copy file: %s to file: %s", > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0tmp,= tmp2); > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 } > + =A0 =A0 =A0 } > + > + =A0 =A0 =A0 /* Simply copy the directory hierarchy of the new package. = */ > + =A0 =A0 =A0 replicate_dirtree(dnew, dpatch); > + > + =A0 =A0 =A0 SLIST_FOREACH(fl, &flchanged, linkage) { > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 sprintf(tmp, "%s/%s", dnew, fl->filename); > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 sprintf(tmp2, "%s/%s", dpatch, fl->filename= ); > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 if (copy_file_absolute(tmp, tmp2) !=3D 0) > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 err(1, "Cannot copy file: %= s to file: %s", tmp, tmp2); > + =A0 =A0 =A0 } > + > + =A0 =A0 =A0 sprintf(tmp, "%s -c -j -C %s -f %s *", _PATH_TAR, dpatch, f= patch); > + =A0 =A0 =A0 fp =3D popen(tmp, "r+"); > + =A0 =A0 =A0 if (fp =3D=3D NULL) > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 err(1, "Final tar execution failed for: %s"= , fpatch); > + =A0 =A0 =A0 rm_rf(dold); > + =A0 =A0 =A0 rm_rf(dnew); There's existing code in pkg_delete that matches this purpose. [...] > +void > +parse_package_name(char *pkgfile, char *basename, char *version, char *s= uff) > +{ > + =A0 =A0 =A0 char *tmp, *p; > + > + =A0 =A0 =A0 /* Strip directory path, if any */ > + =A0 =A0 =A0 p =3D strrchr(pkgfile, '/'); > + =A0 =A0 =A0 if (p !=3D NULL) > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 tmp =3D strdup(p + 1); > + =A0 =A0 =A0 else > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 tmp =3D strdup(pkgfile); This could be done using basename(3) (and to that effect you could drop tmp, maybe...). I would look at the functions implemented through pkg_version as there might be some code in there that can be leveraged for what you're doing here. > + =A0 =A0 =A0 p =3D strrchr(tmp, '.'); > + =A0 =A0 =A0 if (suff !=3D NULL) > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 strcpy(suff, p); > + =A0 =A0 =A0 *p =3D '\0'; > + =A0 =A0 =A0 p =3D strrchr(tmp, '-'); > + =A0 =A0 =A0 if (version !=3D NULL) > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 strcpy(version, p + 1); > + =A0 =A0 =A0 *p =3D '\0'; > + =A0 =A0 =A0 if (basename !=3D NULL) > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 strcpy(basename, tmp); > + =A0 =A0 =A0 free(tmp); > +} > + > +/* > + * File copy, preserving attributes: ownership, mtime, mode. Knows how t= o handle > + * (re-create) symlinks. > + */ This doesn't handle POSIX extended attributes nor ACLs. > +int > +copy_file_absolute(char *from, char *to) > +{ > + =A0 =A0 =A0 char *buf; > + =A0 =A0 =A0 const ssize_t bufsize =3D 256*1024; > + =A0 =A0 =A0 ssize_t bs; > + =A0 =A0 =A0 int fdfrom, fdto; > + =A0 =A0 =A0 struct stat st; > + =A0 =A0 =A0 struct timeval tv; > + > + =A0 =A0 =A0 if (lstat(from, &st) !=3D 0) > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 return (errno); > + > + =A0 =A0 =A0 if (S_ISLNK(st.st_mode)) { > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 char tmp[PATH_MAX]; > + > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 if (readlink(from, tmp, PATH_MAX) < 0) > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 return (errno); > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 if (symlink(tmp, to) < 0) > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 return (errno); > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 return (0); > + =A0 =A0 =A0 } > + > + =A0 =A0 =A0 fdfrom =3D open(from, O_RDONLY); > + =A0 =A0 =A0 if (fdfrom < 0) > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 return (errno); > + =A0 =A0 =A0 fdto =3D open(to, O_CREAT | O_WRONLY | O_TRUNC); > + =A0 =A0 =A0 if (fdto < 0) > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 return (errno); > + =A0 =A0 =A0 buf =3D malloc(bufsize); > + =A0 =A0 =A0 if (buf =3D=3D NULL) > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 return (ENOMEM); > + =A0 =A0 =A0 while (1) { > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 bs =3D read(fdfrom, buf, bufsize); > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 if (bs < 0) > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 err(1, "read() failure"); > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 else if (bs > 0) > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 if (write(fdto, buf, bs) != =3D bs) > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 err(1, "wri= te() failure"); > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 if (bs =3D=3D 0) > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 break; > + =A0 =A0 =A0 } > + =A0 =A0 =A0 free(buf); > + =A0 =A0 =A0 close(fdto); > + =A0 =A0 =A0 close(fdfrom); > + > + =A0 =A0 =A0 if (chown(to, st.st_uid, st.st_gid) < 0) > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 return (errno); > + =A0 =A0 =A0 tv.tv_usec =3D 0; > + =A0 =A0 =A0 tv.tv_sec =3D st.st_mtime; > + =A0 =A0 =A0 if (lutimes(to, &tv) < 0) > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 return (errno); > + =A0 =A0 =A0 if (lchmod(to, st.st_mode) < 0) > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 return (errno); > + =A0 =A0 =A0 return (0); > +} Thanks! -Garrett
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?AANLkTikLZZ4VjkR97k53p-wYeXhny5yyKrSeBzYBAU4E>