On Wed, Jun 18, 2008 at 9:36 AM, Nate Straz <[EMAIL PROTECTED]> wrote: > On May 27 02:46, Garrett Cooper wrote: >> Keeping with the FLOSS analog, I want to build up commentary so I >> don't go off too far in the wrong direction.. > > A good idea because this is already a giant patch.
Indeed... >> 2. MAKE_TARGETS is required for all inclusions right now. I tried >> pushing it into master_rules.mk, but it didn't work. Then again I'm >> not a GNU Make pro, so... > > I noticed that you had a .DEFAULT_GOAL as a rule instead of a variable > in master_rules.mk. The line should look like this: > > .DEFAULT_GOAL := all Oops... my bad. I'll fix that right away. >> 3. I'm moving all relevant libs that can be separated out into lib, >> and moving all scripts into scripts, just to better sort out everything. > > That sounds like a good plan. Although doing this as a separate step > from the makefile rewrite would make it easier to review. Ok. I'll be sure to submit that as a different patch then beforehand. >> b. compiler_detect.mk contains compiler detection items. This helps >> when trying to determine what target host one's building for, and >> in >> the future what arguments should be specified for $(CC), etc. > > Someone with cross-compiler experience will have to review > compiler_detect.mk and target_detect.mk. Ok. My concern (as I stated in "compiler_detect.mk") was that I knowingly broke icc builds, and we (my team that is) really needs to build everything LTP with icc in addition to gcc on x86. The change should be trivial, but I hope I'm moving along in the correct direction with that. >> c. master_rules.mk contains a number of master Makefile rules. >> i. __generic_bin is used for generic binaries (the leaf >> building target). You need to specify MAKE_TARGETS for this >> one (hopefully >> this will be fixed in the next "patch release"). >> ii. __generic_dir is used for generic directories. You can >> specify what dirs you don't want to include with >> FILTER_OUT_DIRS and you >> can specify what directories you want to include with SUBDIRS. I was >> thinking of doing an analog like FILTER_DIRS, but if one goes to great >> lengths to do that, they could always just remake the __get_make_dirs >> macro. >> iii. __testcases_generic_bin is an extension of >> __generic_bin to set $(prefix) to $(srcdir)/testcases/bin. >> Just trying to >> reduce duplicated text. >> iv. __testcases_kernel_syscalls should probably be tossed >> since it's the same thing as __testcases_generic_bin. > > In 8a you talk about not touching anything that starts with __, but here > to define stuff to be used which starts with __. I think you should > name these defines in a way that shows they are makefile templates. Hmmm. Most of the config.mk stuff that shouldn't be touched are constants, which I denote in all caps, whereas the define'd sections are all lowercase. You're right though... if someone doesn't stick to this convention, knowing which is what can be confusing. I think I'll suffix everything with _rule just to be safe. >> 9. Standardizing variables: >> a. -L arguments are specified in LDFLAGS. >> b. -l (L) arguments are specified in LOADLIBES. >> c. -I (i) arguments are specified with CPPFLAGS. This might need >> revising (should this be with CFLAGS?)... > > CPPFLAGS are for C++ compliles. I didn't think we had anything in LTP > that was written in C++. Well, actually ballista's written (for the most part) in C++ IIRC. Also, CPPFLAGS are a set of flags to be passed into the preprocessor, not C++ (that's what I've gleamed from past experience/advice and http://sources.redhat.com/ml/automake/2004-11/msg00121.html confirms it). CXXFLAGS are for C++ (unsure what C++ preprocessor flags would be titled =P). There's so much mucky-muck with make(1) and variables names around that one source doesn't match the other source in terms of meaning. >> 11. The make operation is recursive, so make(1) includes Makefile.inc >> files in current work directory all the way up the tree, until the >> top. The logic should be along the lines: >> >> rootpath ?= . >> rootpath := $(rootpath)/.. >> >> include $(rootpath)/Makefile.inc > > So it's recursive on the way down and crawls back up for each Makefile > too. Yikes! So instead of marking the depth in each makefile so it can > find something in the root, you're scattering files in every directory > so you can find the root still. Yup. > A novel approach, but I think you'd be better off going completely > non-recursive. I'm not sure whether or not this was the best method to solve the problem, but it was the easiest to work with at the time... are there any better ideas? I'd love to hear them because better solutions are what this is striving for, even if it's not a method that I came up with *shrugs*. >> # Do all required logic here, like set variables, etc >> >> The recursive make(1) idea is a double edged sword: >> Pro: >> a. It's cool because it allows you to avoid duplicating >> LOADLIBES inclusions, etc, and you can build up the make >> process. > > That's funny. I thought that was an advantage of a non-recursive make. ;) As it was there were a ton of "-L ../../../lib -lltp"-like lines in the Makefiles that didn't need to be there IMO. >> Cons: >> a. A lot more files in the SCM. > > I would prefer if there were a way to collapse Makefile and Makefile.inc > in each directory into one file. I would _love_ to do that. However, all is defined n-times for n-makefiles, so needless to say if I did this GNU make would complain a great deal and I don't like whiny GNU tools =). >> b. Understanding where something at fault is coming from, >> may be confusing... > > I think that's just a trait of make in general. The most helpful thing > it to analyze the output of `make -p`. I combined that with make -d and it produces a mound of output, but at least I can sift through and find the issue, in most cases. >> 12. .../Makefile.inc had a bug in it. "sh -c 'MI=Makefile.inc; cp $MI >> $MK.bk; sed -e 's/rootdir/rootpath/' < $MI.bk > $MI; rm $MI.bk'" would >> fix that. > > sed now has an inplace flag, -i. Yeah, but for whatever reason it doesn't always work as expected on all OS'es so I just stick with the cp-sed-rm song-n-dance.. > I decided to look back at your original post. [ ... ] > What parts of your patch address this point? Did you take into account > Mike's suggestion of out-of-tree builds? I didn't catch that reply. I'll reread the threads and try and spot that. >> 2. Assuming that the user has gmake (which they should if they plan on >> working with LTP and Linux), there are a number of simplifications that >> can >> be made to the Makefiles to allow for simpler variable and make target >> definition. > > Could you elaborate on the things you did you address this goal? 1. Improve target clarity. 2. Reduce needed Makefile code. 3. Make compile process "relative-path independent". The builds were failing in our make infrastructure because of (what I thought was) relative-path dependence and (what I know now) was implicit-rules being disabled; then again it could have been my brilliant work and misunderstanding in the beginning which is leading me down this road unnecessarily now =\. 4. Leverage some functional niceties of GNU make. >> I am more than happy to complete all of the work and submit it to LTP, >> but I need to know whether or not there is any interest from the LTP >> community before going down that road too far. >> >> PS I have no intention of using automake / autoconf. I find them to be >> somewhat atrocious build utilities :)... > > Thanks for digging into this. I know how long and hard a makefile > rewrite can be. Luckily mine was only about 130 makefiles. Yes. At least this will soon be over though... looking forward to the home stretch. -Garrett ------------------------------------------------------------------------- Check out the new SourceForge.net Marketplace. It's the best place to buy or sell services for just about anything Open Source. http://sourceforge.net/services/buy/index.php _______________________________________________ Ltp-list mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/ltp-list
