From owner-freebsd-ports@FreeBSD.ORG Sun Apr 11 00:08:20 2010 Return-Path: Delivered-To: freebsd-ports@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id BB9D01065676 for ; Sun, 11 Apr 2010 00:08:20 +0000 (UTC) (envelope-from kamikaze@bsdforen.de) Received: from mail.bsdforen.de (bsdforen.de [212.204.60.79]) by mx1.freebsd.org (Postfix) with ESMTP id 8F3E68FC14 for ; Sun, 11 Apr 2010 00:08:19 +0000 (UTC) Received: from mobileKamikaze.norad (unknown [188.46.240.174]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (No client certificate requested) by mail.bsdforen.de (Postfix) with ESMTP id 84BC68A1A86; Sun, 11 Apr 2010 02:08:16 +0200 (CEST) Message-ID: <4BC112EB.80308@bsdforen.de> Date: Sun, 11 Apr 2010 02:08:11 +0200 From: Dominic Fandrey User-Agent: Mozilla/5.0 (X11; U; FreeBSD amd64; en-GB; rv:1.9.1.9) Gecko/20100331 Thunderbird/3.0.4 MIME-Version: 1.0 To: Garrett Cooper References: <4BC04503.4000808@bsdforen.de> <4BC0568E.6000008@bsdforen.de> <4BC0607D.9060402@bsdforen.de> In-Reply-To: X-Enigmail-Version: 1.0.1 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit Cc: freebsd-ports@freebsd.org Subject: Re: Trivial PR, fix package-noinstall X-BeenThere: freebsd-ports@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Porting software to FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 11 Apr 2010 00:08:20 -0000 On 10/04/2010 23:30, Garrett Cooper wrote: > On Sat, Apr 10, 2010 at 4:26 AM, Dominic Fandrey wrote: >> On 10/04/2010 12:49, Garrett Cooper wrote: >>> On Sat, Apr 10, 2010 at 3:44 AM, Dominic Fandrey wrote: >>>> On 10/04/2010 12:18, Garrett Cooper wrote: >>>>> On Sat, Apr 10, 2010 at 2:29 AM, Dominic Fandrey wrote: >>>>>> This morning I took a look at my outstanding PRs. There are >>>>>> is a ports PR I consider old and trivial: >>>>>> >>>>>> This one fixes a bug in the package-noinstall target. wxs told >>>>>> me that he prefers my proposed fix over his own: >>>>>> http://www.freebsd.org/cgi/query-pr.cgi?pr=ports/144164 >>>>> >>>>> This suggested fix completely breaks pkg_creates operation because it >>>>> does a chdir(2) prior to package creation (from >>>>> .../usr.sbin/pkg_install/create/perform.c:555): >>>>> >>>>> if (chdir(log_dir) == FAIL) { >>>>> warnx("can't change directory to '%s'!", log_dir); >>>>> return FALSE; >>>>> } >>>> >>>> I don't see what appears to be the problem. The fix is tested, >>>> there is no chdiring and pkg_create is not modified in any way. >>>> >>>> All it does is change the parameters pkg_create is called with. >>> >>> Have you tested in the following cases: >>> >>> 1. With the pkg_install scripts. >>> 2. Without the pkg_install scripts. >>> >>> If not, then you need to do that before asking for someone to >>> commit your code :). >> >> The do-package code is used by the package and the package-noinstall >> targets. >> >> package-noinstall is called by package-recursive on ALL-DEPENDS. >> I.e. it is only used on completely installed packages, just what >> "pkg_create -b" was made for. >> >> The regular package target is always run after install (search for >> "Main logic" in bsd.port.mk). So do-package is only called after >> install has completed, hence the code can, in every case, rely on >> logdir containing all required data. > > Ok, interesting. If you look at another spot in bsd.port.mk, there's > another area where +INSTALL and friends are installed: > > fake-pkg: > # ... > if [ -f ${PKGINSTALL} ]; then \ > ${CP} ${PKGINSTALL} ${PKG_DBDIR}/${PKGNAME}/+INSTALL; \ > fi; \ > if [ -f ${PKGDEINSTALL} ]; then \ > ${CP} ${PKGDEINSTALL} ${PKG_DBDIR}/${PKGNAME}/+DEINSTALL; \ > fi; \ > if [ -f ${PKGREQ} ]; then \ > ${CP} ${PKGREQ} ${PKG_DBDIR}/${PKGNAME}/+REQUIRE; \ > if [ -f ${PKGMESSAGE} ]; then \ > ${CP} ${PKGMESSAGE} ${PKG_DBDIR}/${PKGNAME}/+DISPLAY; \ > fi; \ > # ... > > So if I don't define NO_PKG_REGISTER and I define FORCE_PKG_REGISTER, > then this logic will be executed. Maybe it's because it's 2 AM, but once again I fail to see the connection to my patch. > This change does need to be tested for the make package-noinstall case > with pkg-install, pkg-deinstall, etc being present and not being > present [in their .in files form and non-.in files form]. Otherwise > this is going to potentially introduce a regression into bsd.port.mk. If you have a case you suspect might go wrong, please go ahead. I find myself unable to follow your logic, so I wouldn't even know the symptoms of something going wrong. As far as I can see you are talking about install, something my patch does in no way interfere with. The only connection between install and package I see is, that the order in which they can be executed is carved in stone. Regards -- A: Because it fouls the order in which people normally read text. Q: Why is top-posting such a bad thing? A: Top-posting. Q: What is the most annoying thing on usenet and in e-mail?