On Fri, Oct 30, 2009 at 4:21 AM, Garrett Cooper <[email protected]> wrote: > On Fri, Oct 30, 2009 at 3:56 AM, Jiří Paleček <[email protected]> wrote: >> >> Hello, >> >> >> yaneurabeya wrote: >>> >>> 2009/10/25 Jiří Paleček <[email protected]>: >>>> On Sun, 18 Oct 2009 20:22:00 +0200, Subrata Modak >>>> <[email protected]> wrote: >>>> >>>>> Jiri, >>>>> >>>>> Any further thoughts on this ? >>>> >>>> Yes, see the attachment. >>>> >>>> I created the directories for the different tests (rt_sigtimedwait, >>>> sigwaitinfo etc.), but to avoid copy-pasting the code, they share the >>>> same >>>> source. Please, tell me if that is OK. >>> >>> I'm a bit confused by let me help analyze the Makefile and provide >>> critiques: >>> >>> The basic template for any Makefile now, is as follows: >>> >>> # LICENSING TORT, e.g. GPLv2 License, etc. >>> >>> # <TOP-SRCDIR DEFINITION> >>> >>> # <PRE-INCLUDE> >>> >>> # <ADDITIONAL MAKE LOGIC GOES HERE> >>> >>> # <POST-INCLUDE> >>> >>> The description of these components is as follows: >>> >>> =========================================================== >>> TOP-SRCDIR DEFINITION >>> =========================================================== >>> top_srcdir ?= <relative path from current Makefile to the top of the >>> source tree> >>> >>> Example 1 (say, from testcases/realtime): >>> >>> top_srcdir ?= ../.. >>> >>> Example 2 (say, from lib): >>> >>> top_srcdir ?= .. >>> >>> The ?= is important, because this value can be redefined at ANY time >>> -- I've been considering making this assignment operator := though, >>> because it should be set just once per Makefile, and isn't being >>> exported anymore (it was a design mistake I made in the beginning that >>> Mike F. corrected me because I wasn't adhering to correct autotools >>> variable conventions). >>> >>> =========================================================== >>> PRE-INCLUDE >>> =========================================================== >>> This is important, and may lead to potential confusion. >>> >>> env_pre.mk is used for most includes outside of the testcases portion >>> of the tree. This has a minimal set of assumptions and dependent >>> targets, because it's designed to work with doc, lib, tools, and >>> utils. This can also be used in `trunk' situations (discussed below) >>> where no C-files need to be compiled, and/or linked in with libltp.a. >>> This also adds $(abs_top_srcdir)/include to the CPPFLAGS so you don't >>> need to add this to 300+ Makefiles (like it was before :)...]. >>> >>> testcases.mk is used for rest of the cases under testcases. It does >>> the following for you: >>> 1. Everything in env_pre.mk. >>> 2. Adds dependencies for libltp.a. >>> 3. Adds -lltp to LDLIBS. >>> 4. Adds dependencies for testcases/kernel/include/syscall_numbers.h >>> 5. Adds an optional hook for the tst_res apps in tools/apicmds so you >>> can run your tests out of the local directory before install [assuming >>> they're coded properly :)...]. >>> >>> All of the additional steps are done for developer convenience, so >>> that running make install from testcases/kernel/syscalls/open doesn't >>> require that I manually go into lib/ and testcases/kernel/include >>> beforehand and run a make all, and have to duplicate a lot of >>> unnecessary logic in the Makefiles. >>> >>> =========================================================== >>> ADDITIONAL MAKE LOGIC GOES HERE >>> =========================================================== >>> >>> Whatever you want to put in here that isn't provided with the defaults >>> in env_post.mk, generic_leaf_target.inc, or generic_trunk_target.inc, >>> so feel free to `use your imagination' as to whatever you may put in >>> here, but the only cases that you should have to deal with are: >>> >>> 1. Multiple C files / objects -> one or more executables. >>> 2. Items without implicit rule mappings, e.g. .c -> .o. >>> >>> =========================================================== >>> POST-INCLUDE: >>> =========================================================== >>> >>> - For all directory traversing Makefiles you include the `trunk >>> target' Makefile: >>> >>> include $(top_srcdir)/include/mk/generic_trunk_target.mk >>> >>> - For all non-directory traversing Makefiles, you include the `leaf >>> target' Makefile. >>> >>> include $(top_srcdir)/include/mk/generic_leaf_target.mk >>> >>> =========================================================== >>> >>> Now, to critique your proposed Makefiles... >>> >>> testcases/kernel/syscalls/rt_sigtimedwait/Makefile: >>> >>> +CFLAGS += -I../../../../include -Wall -O2 -DTEST_RT_SIGTIMEDWAIT >>> +LDLIBS += -L../../../../lib -lltp >>> + >>> +SRCS = $(wildcard *.c) >>> +TARGETS = $(patsubst %.c,%,$(SRCS)) rt_sigtimedwait01 >>> + >>> +all: $(TARGETS) >>> + >>> +rt_sigtimedwait01.o: ../sigwaitinfo/sigwaitinfo01.c >>> + $(CC) -c -o $@ $(CPPFLAGS) $(CFLAGS) $^ >>> + >>> +install: >>> + @set -e; for i in $(TARGETS); do ln -f $$i ../../../bin/$$i ; done >>> + >>> +clean: >>> + rm -f $(TARGETS) >>> >>> Given the above explanation, I would write this like the following >>> (minus the licensing tort, and with improper indented vars -- bloody >>> GMail :(...): >>> >>> ============================================= >>> top_srcdir ?= ../../../.. >>> >>> include $(top_srcdir)/include/mk/testcases.mk >>> >>> # NOTE: Placement doesn't matter, but for readability and consistency >>> it should go here. >>> CPPFLAGS += -DTEST_RT_SIGTIMEDWAIT >>> >>> # NOTE: Placement for this Make target doesn't matter... just so long >>> as it's defined in the Makefile. >>> rt_sigtimedwait01: $(abs_srcdir)/../sigwaitinfo/sigwaitinfo01.c >>> $(COMPILE.o) $(OUTPUT_OPTION) $^ >>> >>> # NOTE: Don't modify MAKE_TARGETS here because it isn't set yet, and >>> the default is to pick up all existing C files in the >>> # current directory and make them into single-source executables, e.g. >>> # sigwaitinfo01.c -> sigwaitinfo01.o -> sigwaitinfo01 ;)... >>> >>> include $(top_srcdir)/include/mk/generic_leaf_target.mk >>> >>> MAKE_TARGETS += rt_sigtimedwait01 >>> ============================================= >>> >>> Similar logic would be applied to your other Makefile, minus the >>> CPPFLAGS assignment. >>> >>> Please note that your binary is no longer linked into >>> $(abs_top_builddir)/testcases/bin -- it's instead install(1)'ed >>> directly into $(DESTDIR)/$(prefix)/testcases/bin . This was done to >>> facilitate proper installation by using the correct mode, and also to >>> avoid broken/hacky symlinking operations. >>> >>> Hope that helps describe things a bit better... let me know if I >>> should add this to a README or something... >>> >>> >> >> Thank you, it is definitely clearer now. I think it should be mentioned in >> the README, including some list of the ltp makefile variables (like >> $(abs_srcdir)). I'll wait with a new patch for the new build system to >> appear in a release, though. >> >> What I still don't get is where does the ZIPFILE variable get its value >> (this was the first place where I sought inspiration): >> >> ji...@debian:/usr/src/ltp-cvs$ fgrep -RI * -e ZIPFILE >> testcases/commands/unzip/Makefile:$(INSTALL_TARGETS): $(ZIPFILE) >> testcases/commands/unzip/Makefile:$(ZIPFILE): $(DIR) >> ji...@debian:/usr/src/ltp-cvs$ > > Ok, I'll add this information as an additional reference to README.mk-devel. > > ZIPFILE is most likely an ad-hoc variable defined in that Makefile > that needs to be added to the new Makefile (it's missing from install > and it's something that I forgot to add to TODO even though I noticed > it once or twice -- I need to note all of the issues clearly this > weekend after I go over the logs manually to ensure that I didn't miss > anything by accident). Assuming that ZIPFILE's generated at build time > (which it should be) you're fine getting away with ZIPFILE being a > `bareword' value relative to the current directory. The only possible > issue is that you'll have to ensure that the destination directory > exists in order for out-of-build-tree support to continue to function > properly (I don't expect people to get this correct all of the time, > but I'd like to keep breakage down to a minimum because it impacts > folks who cross-compile for multiple targets using the same src > directory like my group). You can do this similar to the following: > > foo: > mkdir -p $@ > foo/bar: foo > # fake tab; do something to create foo/bar > > If foo/bar isn't a variable, then it can be expressed as: > > foo: > mkdir -p $@
Sorry. I meant: .SECONDEXPANSION: foo/bar: $$(@D) # fake tab; do something to create foo/bar Most of the time this isn't the case though because specifying INSTALL_TARGETS, uses generate_install_rule, which uses a variable for the target value, and thus mucks up the `.SECONDEXPANSION:' call. > Specifying the directory as a dependency of the target has the two > following benefits: > 1. Ensures that the dependency remains constant and the requirement is > ensured from make (this is key). > 2. Speeds up the build because adding comparable logic to each build > and install rule (test -d foo || mkdir -p foo) I've discovered is > considerably slower than being dependent on the directory and having > make fill in the blanks as necessary. The difference was on the order > of at least 1~2 mins from what I saw on the Cisco build servers I use > and my personal machine. > > The dependency targets (e.g. foo) should be automatically available > via some default logic called up in .../include/mk/env_post.mk using > generate_install_rule_dir_dep .../include/mk/functions.mk, but I've > run into some corner cases where this isn't correct (some of the > INSTALL_TARGETS specified in Makefile.inc, IIRC), so I'll need to fix > something minor after I stare at the problem from a different angle > (it's most likely something really simple that I overlooked by > accident). > > I've made some bad assumptions too that I've had to fix after testing > in out-of-build-tree, so testing this new functionality is more > complicated than traditional in-build-tree support, but if I can get a > build-bot up and going for LTP which uses out-of-build-tree, this > would be easy to automate longterm. I don't forsee completing this > before the end of November though. > > So -- in summary: relative paths from current directory are ok -- just > be sure that the directory is a dependency of the target and is > created before the target is created or functional issues will occur > (best case consistent build errors -- worst case race conditions in > the source tree :[...). > > Cheers, > -Garrett > ------------------------------------------------------------------------------ Come build with us! The BlackBerry(R) Developer Conference in SF, CA is the only developer event you need to attend this year. Jumpstart your developing skills, take BlackBerry mobile applications to market and stay ahead of the curve. Join us from November 9 - 12, 2009. Register now! http://p.sf.net/sfu/devconference _______________________________________________ Ltp-list mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/ltp-list
