From owner-p4-projects@FreeBSD.ORG Mon May 31 00:28:19 2010 Return-Path: Delivered-To: p4-projects@freebsd.org Received: by hub.freebsd.org (Postfix, from userid 32767) id 4E630106567A; Mon, 31 May 2010 00:28:19 +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 0FD8A1065676; Mon, 31 May 2010 00:28:19 +0000 (UTC) (envelope-from yanegomi@gmail.com) Received: from mail-gw0-f54.google.com (mail-gw0-f54.google.com [74.125.83.54]) by mx1.freebsd.org (Postfix) with ESMTP id A6B058FC08; Mon, 31 May 2010 00:28:18 +0000 (UTC) Received: by gwj23 with SMTP id 23so2489995gwj.13 for ; Sun, 30 May 2010 17:28:17 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=domainkey-signature:mime-version:received:sender:received :in-reply-to:references:date:x-google-sender-auth:message-id:subject :from:to:cc:content-type:content-transfer-encoding; bh=Z3+9j8EdxaPWbQ5rwalFew7xIn0fncxRwGRxVOsfyAQ=; b=OGUplM/4epnAU07pnDHfCZwuzXf/MBSPEhx3vqVkwNTx3YoURHi3qLX2AgOhHc1lVS FCkG82gd4P9GKwWNX/+2QZR1WCU3MNZm5om7Dk7raK3fgRpg1DdX3scM2LvI6NYFNehP Nyuk19fuXyOy0zEuz59I2l+XSB22UJeO5Bpqw= DomainKey-Signature: a=rsa-sha1; c=nofws; d=gmail.com; s=gamma; h=mime-version:sender:in-reply-to:references:date :x-google-sender-auth:message-id:subject:from:to:cc:content-type :content-transfer-encoding; b=IpmP1nF8Bh1/aQ1tvpn5cSdxUHMbLHXqLIZ49+wg2G1G8FX1LXSMvOLrLI65daDQtQ TC4CO8Z3+yt5GwZ74Btfhncud+ZBA+3nSLsmZMXuSdFhkbJAObhW5PDa8LaGgcuc1mlb cJ4EDhrMTv6Y+wjVe8Z/h4i7D3VBBkvIrk0eU= MIME-Version: 1.0 Received: by 10.231.120.37 with SMTP id b37mr4745296ibr.81.1275265697611; Sun, 30 May 2010 17:28:17 -0700 (PDT) Sender: yanegomi@gmail.com Received: by 10.231.70.16 with HTTP; Sun, 30 May 2010 17:28:17 -0700 (PDT) In-Reply-To: <201005302335.o4UNZLsl028791@repoman.freebsd.org> References: <201005302335.o4UNZLsl028791@repoman.freebsd.org> Date: Sun, 30 May 2010 17:28:17 -0700 X-Google-Sender-Auth: XwEDuqi6oYO3gjj7Pt7NCwFkeeE Message-ID: From: Garrett Cooper To: Ivan Voras Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable Cc: Perforce Change Reviews Subject: Re: PERFORCE change 178984 for review X-BeenThere: p4-projects@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: p4 projects tree changes List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 31 May 2010 00:28:19 -0000 On Sun, May 30, 2010 at 4:35 PM, Ivan Voras 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