Trivial PR, fix package-noinstall

[Dominic Fandrey]

On 10/04/2010 23:30, Garrett Cooper wrote:
> On Sat, Apr 10, 2010 at 4:26 AM, Dominic Fandrey <kamikaze at bsdforen.de> wrote:
>> On 10/04/2010 12:49, Garrett Cooper wrote:
>>> On Sat, Apr 10, 2010 at 3:44 AM, Dominic Fandrey <kamikaze at bsdforen.de> wrote:
>>>> On 10/04/2010 12:18, Garrett Cooper wrote:
>>>>> On Sat, Apr 10, 2010 at 2:29 AM, Dominic Fandrey <kamikaze at bsdforen.de> 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?