Trivial PR, fix package-noinstall

[Dominic Fandrey]

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

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?

[Garrett Cooper]

On Sat, Apr 10, 2010 at 2:29 AM, Dominic Fandrey <kami...@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;
}

The goal is to expand use of the install and deinstall scripts, not
break them in ports; this would be counterproductive to what we want
to do.

FWIW, I've thought this over and and user modifiable scripts should
not be in packages; they should instead be example files which don't
conflict with real configuration files. This is already the case for
several ports, but not all ports. If we did this, it would solve the
problem we've had with ports removing or overwriting user config files
simply and easily. I wonder if other folks agree with me or not.

Thanks,
-Garrett

[Gary Jennejohn]

On Sat, 10 Apr 2010 03:18:42 -0700
Garrett Cooper <yane...@gmail.com> wrote:

> FWIW, I've thought this over and and user modifiable scripts should
> not be in packages; they should instead be example files which don't
> conflict with real configuration files. This is already the case for
> several ports, but not all ports. If we did this, it would solve the
> problem we've had with ports removing or overwriting user config files
> simply and easily. I wonder if other folks agree with me or not.
>

I agree as long as the port emits a message pointing the user at the
example configuration files.

In some cases more than this may be needed since man pages might refer
to configuration files which no longer exist.

--
Gary Jennejohn

[Dominic Fandrey]

On 10/04/2010 13:11, Gary Jennejohn wrote:
> On Sat, 10 Apr 2010 03:18:42 -0700
> Garrett Cooper <yane...@gmail.com> wrote:
>> FWIW, I've thought this over and and user modifiable scripts should
>> not be in packages; they should instead be example files which don't
>> conflict with real configuration files. This is already the case for
>> several ports, but not all ports. If we did this, it would solve the
>> problem we've had with ports removing or overwriting user config files
>> simply and easily. I wonder if other folks agree with me or not.
>>
>
> I agree as long as the port emits a message pointing the user at the
> example configuration files.

I think noone ever agreed that installing user changeable configuration
files was a good idea and .sample files or include folders are both
prominent and widely accepted solutions. However this only ever entered
the discussion because of a misunderstanding.

I'd prefer to keep the discussion on topic and avoid the million
"I agree" posts on something no one ever disagreed to.

[Dominic Fandrey]

On 10/04/2010 12:49, Garrett Cooper wrote:
> On Sat, Apr 10, 2010 at 3:44 AM, Dominic Fandrey <kami...@bsdforen.de> wrote:

>> On 10/04/2010 12:18, Garrett Cooper wrote:
>>> On Sat, Apr 10, 2010 at 2:29 AM, Dominic Fandrey <kami...@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.

[Garrett Cooper]

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.

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.

Thanks,
-Garrett

[Dominic Fandrey]

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.