> http://cr.grommit.com/~amaguire/onnv-quagga-updated/
Alan,
Things look great -- nice cleanup! Since I don't have access to the
source tree, I was unable to do some mechanical checks. I would recommend
scanning the tree for any use of ROOTMETHOD and any use of conditionals on
lines with "SVC" or "MANIFEST" in them, as those are likely to be suspect.
While I was reviewing the changes, I occasionally noticed other things
that are not due to your changes, but nonetheless should be cleaned up at
some point. I've marked these with `+' below; you're welcome to tackle
or defer any of them at this point, but bugs should be logged.
General:
+ I'm confused on the convention for SVCMETHOD scripts -- it seems
like sometimes we check-in the .sh file and copy the SVCMETHOD
from the .sh file during the build (such as with inetd), and
other times we just directly check-in SVCMETHOD and install it.
Is there a rationale here? Seems like we should pick a convention
and stick to it.
* In a number of Makefiles, you've added an include for
cmd/Makefile.targ. That includes a `clobber' target which often
overlaps with the one already defined in the Makefile. This
should be rationalized.
src/cmd/Makefile.cmd:
+ 238: What does this do? We set ROOTSVCBIN here, then reset it
on line 242.
src/cmd/auditd/Makefile:
+ There appears to be quite a bit of brokenness in here that would
be nice to fix up. Specifically, SUBDIRS is never set so it
should be stripped from the targets, lines 75-78 should be
removed, `local_clobber' should be torched, a proper `clean'
target should be introduced, and `clobber' should come from
cmd/Makefile.targ. (Oh, and TEXT_DOMAIN is already set from
Makefile.cmd.)
src/cmd/cmd-inet/usr.lib/Makefile.inetsvc
+ We could finish simplifying things by removing lines 30-31,
43-46, and 54, and changing lint_SRCS to lint_PROG on line 56.
src/cmd/cmd-inet/usr.lib/in.ripngd/Makefile:
+ 76: Might as well fix this to be lint_SRCS.
src/cmd/cmd-inet/usr.sbin/Makefile:
* 222: Umm, INTSUBDIRS?
src/cmd/cmd-inet/usr.sbin/in.ftpd/Makefile:
* 12-13: While this probably works, it would be more consistent
to hoist PROG and SCRIPTS as well.
src/cmd/cmd-inet/usr.sbin/in.rdisc/Makefile:
* 27: Initial include of Makefile.cmd seems unnecessary.
* 32-34, 44: None of this stuff should be needed since this is
a single-source-file program.
* 42: Are all of these CPPFLAGS really needed? Why?
* 48: Makefile.targ should come at the end to avoid overriding
the default target of `all'.
src/cmd/cmd-inet/usr.sbin/in.routed/Makefile:
* 78: Why include Makefile.targ here rather than at the end?
src/cmd/cmd-inet/usr.sbin/routeadm/Makefile:
* 30: Remove blank line.
* 34-37: None of this stuff should be needed since this is a
single-source-file program.
* 46: Are all of these CPPFLAGS really needed? Why?
src/cmd/consadm/Makefile:
* 58-60: Can't this rule be removed as well?
src/cmd/fs.d/nfs/svc/Makefile:
+ Make `all lint' the first target.
src/cmd/ipf/svc/Makefile:
+ Would be nice to change lines 17-23 `all clean lint:', since
there are no source files.
src/cmd/lp/cmd/lpsched/Makefile:
+ 29: Might as well fix this pathname.
* 38: Remove duplicate MANIFEST setting.
+ 154: Could just be lint_SRCS.
src/cmd/print/lp/Makefile:
* Why does this file show up as "new"?
src/cmd/lvm/md_monitord/Makefile:
* 92: Is this ROOTMETHOD rule still needed?
src/cmd/lvm/*/Makefile:
+ I notice all of these LVM Makefiles have a strange disease:
they all have `-$(RM) $(ROOTPROG)' in their `install' rule.
This makes no sense to me.
src/cmd/lvm/rpc.metamedd/Makefile:
+ 43: `check' doesn't descend into SUBDIRS, so this can be
removed.
src/cmd/oplhpd/Makefile:
+ 52: Take out the extra blank line too, and the two extra ones
on lines 55-56 while you're at it.
+ 70: This `clean' rule is wrong; it should be `$(RM) $(OBJS)'.
src/cmd/picl/picld/Makefile:
+ Lines 30-34, 40-41, 65-66, and 71 all appear needless since
this is a single-source-file program (and line 74 should be
lint_PROG).
src/cmd/print/gateway/Makefile:
* 114: This should be ROOTSVCMETHOD -- but the whole thing
looks pretty out-of-date. How about we just torch 103-114?
src/cmd/rpcbind/Makefile:
+ 64, 66-68: This DIRS handling is cruft.
src/cmd/rpcsvc/Makefile.rpc:
* 33-34: This sentence is incorrect and probably unnecessary.
* 40-48: Why not just "ROOTMANIFESTDIR= $(ROOTSVCNETWORK)/rpc"?
I don't grok what these additional rules are doing.
src/cmd/rpcsvc/rpc.bootparamd/Makefile:
* Why was this file changed?
src/cmd/sa/Makefile:
* 113-119: Why are these conditional assignments still here?
src/cmd/xntpd/xntpd/Makefile:
+ 12: The `all' rule should really be moved down to around
line 24 to be more traditional.
src/cmd/ypcmd/Makefile:
* 43, 258: Why hasn't ROOTMETHOD been removed and line 258
changed to ROOTSVCMETHOD?
src/cmd/zoneadm/Makefile:
* 34, 52: Likewise.
--
meem
_______________________________________________
networking-discuss mailing list
[email protected]
