On Wednesday 08 July 2009 20:21:11 Garrett Cooper wrote: > On Wed, Jul 8, 2009 at 4:53 PM, Mike Frysinger<[email protected]> wrote: > > you have duplicated copyright notices here too > > The Cisco junk is just to pass by our in-house copyright checker, but > I suppose it really doesn't matter anyhow...
i also noticed the statements "all rights reserved" ... i dont think those have any business being in open source code > > the if checking in the clean target seems unnecessary > > Uh, no. If someone's root and they specify $(INSTALL_DIR) := / (or > worse, empty), it'll nuke the host machine's entire rootfs. I ran the > build as a non-root user when I ran into that issue -- and quickly > quashed it :)... sorry, i meant the "-d" part. besides, all recent distros will refuse to run something like `rm -rf /` -- the util itself rejects "/". > > for pre/post install hooks, why not declare a common name and everyone > > use that rather than forcing people to declare it. i.e. declare the > > target "post- install-hook" as a .PHONY and always call/depend/etc... on > > it. > > I'm a bit confused by this comment. Can you review to spots in the > patch (when I upload it tonight), so this can be resolved? testcases/kernel/controllers/cpuset/Makefile: why must "install" manually depend on pre-install ? that dependency should always be there leaving the subdir to only have to define the pre-install target. testcases/Makefile: i dont get all these xxx_DEPS variables and the install-testcases-bin target > > why cant testcases/kernel/Makefile leverage these .mk files ? the whole > > point was to get away from these custom written jobbies ... > > Some of these Makefiles get a bit ugly with the pre- and post- junk... > There are a small number of Makefiles that are haunted by this work, > but then again I did some more rework with the generic_trunk_target > define. so that means you'll fix the Makefile to use the .mk files > > what's this business of every Makefile having to declare MAKE_TARGETS ? > > this should be a common default. > > It defaults to a wildcard of the .c files in the directory -- hence > the `?='. If that's a bad default, please let me know. i'm talking about the Makefiles that are setting this variable. i.e. testcases/kernel/controllers/cpuset/cpuset_syscall_test/Makefile testcases/kernel/fs/proc/Makefile tools/Makefile > > doing a for loop on a `rm` command is silly. call `rm` once. > > Yeah. I should be applying xargs instead of a loop. Calling rm once > may overflow the command line though (like with testcases/bin if it > grows large enough). if you really want to go that far, but i was looking at testcases/kernel/include/Makefile. there's no way that is going to overflow. besides, i guess there shouldnt be a clean target in there anyways -- it should be using the default one. > > debugging/optimization/warning/stripping flags shouldnt be added in any > > Makefile that is being converted to these common .mk files. same for any > > DEBUG_XXX flags. > > Agreed. I was just duplicating what already existed, hoping that we > could sweep up this stuff after everything was in and the base > infrastructure was stable. i'd prefer we scrub the crap now > > i dont see the point in declaring empty rules like "install: ;". if > > they're declared .PHONY, there is no need to add explicit rules to sub > > Makefiles. > > They're there to avoid errors with targets not being found if I'm > running recursive Make rules over the entire directory tree. So, I > agree it's silly, but if you have a better idea, I'm all ears and I'll > get on it ASAP. I'm trying to avoid cramming more variables into the > Make system because it will only generate more confusion... let's look at testcases/kernel/include/Makefile. how would a missing install target here generate an error ? > > clean targets should always delete common things like "*.o". forcing > > people to have to manually declare these is a waste. > > Ok. Once upon a time (~1 year ago) I found a testcase that generated a > .o file which was just as input for a testcase (it was under > testcases/kernel/io somewhere -- maybe the floppy test). Maybe that's > a bug then that should be solved... we should fix that then as that's just craziness -mike
signature.asc
Description: This is a digitally signed message part.
------------------------------------------------------------------------------ Enter the BlackBerry Developer Challenge This is your chance to win up to $100,000 in prizes! For a limited time, vendors submitting new applications to BlackBerry App World(TM) will have the opportunity to enter the BlackBerry Developer Challenge. See full prize details at: http://p.sf.net/sfu/Challenge
_______________________________________________ Ltp-list mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/ltp-list
