Expanding on NO_ROOT: Categorizing installed files

[Brooks Davis]

On Wed, Jul 16, 2014 at 12:13:48PM -0700, Sean Fagan wrote:
> 
> On Jul 16, 2014, at 10:07 AM, Brooks Davis <brooks at freebsd.org> wrote:
> 
> > The vast majorify of the diff is make debugging garbage that looks like
> > it was committed by accident.  I won't provided any detailed review
> > of the current patch except to say that there are a lot of apparently
> > redundent instances of setting META_CATEGORY in Makefiles and still
> > quite a lot of instances of .EXPORTVAR: META_CATEGORY.
> 
> Okay.  Again, my apologies about that; I've excised it, and removed all
> of the EXPORTVAR directives, even the ones that had been commented out.
> (Note that the patch includes a change to xinstall, that is commented out; that's
> because I think I still like the concept of it, to at least some degree, and didn't
> want to forget where I put it.  But it *is* commented out, and so the functionality
> is simply not there.)
> 
> Without my idiotic error, the diff is much smaller -- just under 50,000 bytes.  However,
> rather than include it again, I've replaced the file at http://earth.kithrup.com/~sef/auto-diffs.txt
> for everyone's perusal.

Some comments with limited context:

In Makefile.inc1 you write:

LIB32WMAKEENV+=	META_CATEGORY=compat32

Is that so downstream makefiles can modify it or should it be in
LIB32WMAKEFLAGS?

I don't think bin/Makefile should need a META_CATEGORY line.  Similarly
lib/Makefile, libexec/Makefile, sbin/Makefile, secure/Makefile, ...

cddl/Makefile.inc uses META_CATEGORY?= but many others use META_CATEGORY=
and I don't understand why which suggests some comments.

gnu/usr.bin/cc/Makefile sets META_CATEGORY=dev when ../Makefile.inc already
does.

I think I'd put META_CATEGORY=dev in lib/clang/clang.lib.mk rather than
in the environment from lib/clang/Makefile.

Setting META_CATAGORY in lib/csu/amd64/Makefile is probably not quite
right.  Since it needs to be base rather than lib I think setting it in
lib/csu/Makefile.inc is probably correct.  You'll also need to add -P (or
what ever we decide on) to INSTALL there or patch all of
lib/csu/*/Makefile.

It doesn't look like the changes to lib/libc/Makefile are needed.
Similarly lib/libelf/Makefile.

There is debug stuff in rescue/Makefile.

Writing:

+.if exists(../Makefile.inc)
+.include "../Makefile.inc"
+.endif

doesn't make sense.  You can use .sinclude or just not bother to test since
the directory shouldn't move around.

Whitespace around = or ?= which assigning to META_CATEGORY is quite inconsistent.

-- Brooks
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 181 bytes
Desc: not available
URL: <http://lists.freebsd.org/pipermail/freebsd-hackers/attachments/20140716/0a88a4fb/attachment.sig>