Merge lp:~wence/fluidity/make-fixes into lp:fluidity

[Lawrence Mitchell]

Reduce spurious recompilations in clean or mostly-clean trees

Avoid touching lib/libfluidity.a unless actually necessary. This comes in a few parts:

o Use bzr revision-info to set up Fluidity's version number (and only update confdefs.h if the version changed)
o Calculate hashes of the check_options and register_diagnostics files and only update them if the hash changes
o Avoid depending on .PHONY targets that just point directly at actual targets

Various small fixes to decrease build times in mostly built trees.

The main effect of the change is to avoid touching lib/libfluidity.a as much as possible. Timing example, in a just-built tree:

before:
$ time make all
real 0m40.936s
user 0m19.053s
sys 0m21.117s

after:
$ time make all
real 0m11.736s
user 0m7.471s
sys 0m4.223s

As a bonus, the version-number fluidity reports is now meaningful again.

[Patrick Farrell (pefarrell)]

Hi Lawrence,

Looks like great work. I've always found this business of the build being slow quite annoying.

I tried it out, and it changed the time from 9s to 7s on my workstation. Quick question: there is a noticeable delay at the start of the build now, as it calls bzr version-info; it appears like it isn't doing anything for a second or two. Is there any way to speed up the bzr version-info, or is that just how long it takes, and there's nothing we can do? With your change, but without the bzr version-info, it takes just 3s to compile on my workstation.

[lp:~wence/fluidity/make-fixes]

3476. By Lawrence Mitchell on 2011-05-13

Makefile.in: Remove unnecessary --check-clean from VERSION

Should have gone with previous change, but was missed.

[Lawrence Mitchell (wence)]

It seems like the custom version-info format increases the time spent significantly:

fluidity-trunk $ time bzr version-info >/dev/null

real 0m0.293s
user 0m0.138s
sys 0m0.150s

fluidity-trunk $ time bzr version-info --custom --template="{revno} ({revision_id})" >/dev/null

real 0m0.865s
user 0m0.522s
sys 0m0.338s

If I try this:
$ cat > gen-version<<\EOF
#!/bin/bash

while read f; do
    if [[ $f == revision-id:\ * ]]; then
        revid=${f#revision-id: }
    elif [[ $f == revno:\ * ]]; then
        revno=${f#revno: }
    fi
done < <(bzr version-info) && echo "$revno (revid $revid)"
EOF

fluidity-trunk $ time ./gen-version >/dev/null

real 0m0.297s
user 0m0.123s
sys 0m0.170s

But is not portable over changes in the output format.

The clean solution is to use the format-spec, the fast solution appears to be to munge the textual output by hand.

The really fast solution is to do:

VERSION=$(shell cat .bzr/branch/last-revision)

but that is /really/ ugly, so I don't want to be the one doing it.

[lp:~wence/fluidity/make-fixes]

3477. By Lawrence Mitchell on 2011-05-13

Makefile.in: Use bzr revision-info not version-info

The latter takes a looong time, especially with a custom format, and
the former produces equally usable results.

3478. By Lawrence Mitchell on 2011-05-20

Merge from trunk

3479. By Lawrence Mitchell on 2011-05-26

Merge from trunk.

3480. By Lawrence Mitchell on 2011-05-26

scripts: Don't use with in make_check_options and make_register_diagnostics

The with statement only appeared in python 2.5 (in __future__) which
is not the minimum supported version. So just use try/finally to open
the output file instead.

3481. By Lawrence Mitchell on 2011-05-26

Merge from trunk

[Lawrence Mitchell (wence)]

I'm not quite happy with this entire series as is. Specifically, it breaks the sequence:

make
make clean-light
make

Since libwm is not cleaned by clean-light, the object files are left lying around. The default rule in libwm/Makefile with this patch series is:

default: $(LIB)

$(LIB): $(OBJS)
     $(AR) $(ARFLAGS) $(LIB) $(OBJS)
 cp *.h ../include/

But the make sequence is:

make debug
make libwm

debug touches $(LIB) and so the modtime for $(LIB) is newer than $(OBJS), so they're not linked in to the library. This causes an undefined symbol link error when we try to build bin/fluidity.

I see two ways of fixing this:

revert to the previous (.PHONY) rule:

default: $(OBJS)
        ...

So that the object files are always stuffed in $(LIB)

Or add libwm to the list of clean-light targets.

I prefer the latter, thoughts?

[lp:~wence/fluidity/make-fixes]

3482. By Lawrence Mitchell on 2011-06-27

Merge from trunk

3483. By Lawrence Mitchell on 2011-06-27

Add libwm to clean-light target

Now that the libwm target is no longer .PHONY, we need to clean it
when cleaning lib/libfluidity.a (for example via clean-light). This
ensures that make; make clean-light; make works as a build sequence.

Unmerged revisions

=== modified file 'Makefile.in'
--- Makefile.in	2011-06-02 11:01:17 +0000
+++ Makefile.in	2011-06-27 14:02:36 +0000
@@ -63,7 +63,7 @@
 
 EVAL = scripts/silenteval
 
-VERSION = $(shell svnversion .)
+VERSION = $(shell bzr revision-info)
 
 ifeq (@MBA2D@,yes)
 LIBMBA2D = lib/libmba2d.a
@@ -143,7 +143,7 @@
 	@cd @FLFEMDEM_PATH@;$(MAKE)
 endif
 
-bin/@FLUIDITY@: main.o fluidity_library
+bin/@FLUIDITY@: main.o lib/lib@FLUIDITY@.a
 	@echo "BUILD fluidity"
 	@echo "    MKDIR bin"
 	@mkdir -p bin
@@ -262,8 +262,7 @@
 fluidity_library: lib/lib@FLUIDITY@.a
 lib/lib@FLUIDITY@.a: $(OBJS) python_build sub_system 
 	@echo "DEFINE __FLUIDITY_VERSION__"
-	@echo "#undef __FLUIDITY_VERSION__" >> include/confdefs.h
-	@echo "#define __FLUIDITY_VERSION__ \"$(VERSION)\"" >> include/confdefs.h
+	@grep "$(VERSION)" include/confdefs.h >/dev/null 2>&1 || (echo "#undef __FLUIDITY_VERSION__" >> include/confdefs.h && echo "#define __FLUIDITY_VERSION__ \"$(VERSION)\"" >> include/confdefs.h)
 	@echo "BUILD libfluidity"
 	@echo "    MKDIR lib"
 	@mkdir -p lib
@@ -309,10 +308,10 @@
 	@cd main; $(MAKE)
 	@echo "    MAKE options_check"
 	@./scripts/make_check_options
-	@cd preprocessor; $(MAKE) check_options
+	@cd preprocessor; $(MAKE) check_options.o
 	@echo "    MAKE register_diagnostics"
 	@./scripts/make_register_diagnostics
-	@cd preprocessor; $(MAKE) register_diagnostics
+	@cd preprocessor; $(MAKE) register_diagnostics.o
 
 fldecomp: fluidity_library 
 	@echo "BUILD fldecomp"
@@ -334,6 +333,8 @@
 clean-light:
 	@echo "    CLEAN debug"
 	@cd debug; $(MAKE) clean
+	@echo "    CLEAN libwm"
+	@cd libwm; $(MAKE) clean
 	@echo "    CLEAN femtools"
 	@cd femtools; $(MAKE) clean
 	@echo "    CLEAN femtools/tests"

=== modified file 'libwm/Makefile.in'
--- libwm/Makefile.in	2011-05-27 13:44:17 +0000
+++ libwm/Makefile.in	2011-06-27 14:02:36 +0000
@@ -63,9 +63,10 @@
 	@echo "        CXX $<"
 	$(CXX) $(CXXFLAGS) -I. -I../include -c $<
 
-default: $(OBJS)
+$(LIB): $(OBJS)
 	$(AR) $(ARFLAGS) $(LIB) $(OBJS)
 	cp *.h ../include/
+default: $(LIB)
 
 clean:
 	 rm -f *.o *.mod

=== modified file 'preprocessor/Makefile.in'
--- preprocessor/Makefile.in	2011-05-19 14:29:56 +0000
+++ preprocessor/Makefile.in	2011-06-27 14:02:36 +0000
@@ -74,17 +74,17 @@
 	@$(AR) $(ARFLAGS) $(LIB) $(OBJS)
 default: $(LIB)
 
-.PHONY:check_options
-
 # This gets run last by the fluidity build process.
-check_options: check_options.o
+check_options.o: check_options.F90
+	@echo "        FC $<"
+	$(FC) $(FCFLAGS) -c $<
 	mkdir -p ../lib
 	$(AR) $(ARFLAGS) $(LIB) check_options.o &> check.log
 
-.PHONY:register_diagnostics
-
 # This gets run last by the fluidity build process.
-register_diagnostics: register_diagnostics.o
+register_diagnostics.o: register_diagnostics.F90
+	@echo "        FC $<"
+	$(FC) $(FCFLAGS) -c $<
 	mkdir -p ../lib
 	$(AR) $(ARFLAGS) $(LIB) register_diagnostics.o &> register_diagnostics.log
 

=== modified file 'scripts/make_check_options'
--- scripts/make_check_options	2010-11-16 23:08:06 +0000
+++ scripts/make_check_options	2011-06-27 14:02:36 +0000
@@ -1,6 +1,9 @@
 #!/usr/bin/env python
 import re
 import glob
+import hashlib
+import StringIO
+
 
 # File header
 header="""
@@ -11,9 +14,23 @@
 end subroutine check_options
 """
 
-outfile=file("preprocessor/check_options.F90", "w")
-
-outfile.write(header)
+outfile='preprocessor/check_options.F90'
+
+# get sha1 digest of existing generated file.  Can't use 'rw' here
+# because it updates the modtime of the file, which we're trying to
+# avoid doing.
+orig=hashlib.sha1()
+try:
+    f=open(outfile, 'r')
+    orig.update(f.read())
+except IOError:
+    pass
+else:
+    f.close()
+
+# Now read module files to generate potential new data
+output=StringIO.StringIO()
+output.write(header)
 
 # List of fortran source files.
 fortran_files=glob.glob("*/*.F")+glob.glob("*/*.F90")
@@ -23,7 +40,7 @@
 module_list=[]
 
 for filename in fortran_files:
-    
+
     fortran=file(filename,"r").read()
 
     modules=module_re.findall(fortran)
@@ -31,21 +48,37 @@
     for module in modules:
 
         if re.search(r"^\s*subroutine\s+"+module+"_check_options\s*$",\
-                     fortran,\
-                     re.IGNORECASE|re.MULTILINE):
+                         fortran,\
+                         re.IGNORECASE|re.MULTILINE):
             module_list.append(module)
 
 for module in module_list:
 
-        outfile.write("  use "+module+", only: "+module+"_check_options\n")
+    output.write("  use "+module+", only: "+module+"_check_options\n")
 
 # Ensure that the subroutine is legal in the trivial case.
-outfile.write("""
+output.write("""
    continue
    """)
 
 for module in module_list:
 
-    outfile.write("  call "+module+"_check_options\n")
-
-outfile.write(footer)
+    output.write("  call "+module+"_check_options\n")
+
+output.write(footer)
+
+new=hashlib.sha1()
+new.update(output.getvalue())
+
+# Only write file if sha1sums differ
+if new.digest() != orig.digest():
+    try:
+        f=open(outfile, 'w')
+        f.write(output.getvalue())
+    except IOError:
+        # Fixme, this should fail better
+        pass
+    else:
+        f.close()
+
+output.close()

=== modified file 'scripts/make_register_diagnostics'
--- scripts/make_register_diagnostics	2011-02-05 16:05:31 +0000
+++ scripts/make_register_diagnostics	2011-06-27 14:02:36 +0000
@@ -1,6 +1,9 @@
 #!/usr/bin/env python
 import re
 import glob
+import hashlib
+import StringIO
+
 
 # File header
 header="""
@@ -11,9 +14,23 @@
 end subroutine register_diagnostics
 """
 
-outfile=file("preprocessor/register_diagnostics.F90", "w")
-
-outfile.write(header)
+outfile = 'preprocessor/register_diagnostics.F90'
+
+# get sha1 digest of existing generated file.  Can't use 'rw' here
+# because it updates the modtime of the file, which we're trying to
+# avoid doing.
+orig=hashlib.sha1()
+try:
+    f=open(outfile, 'r')
+    orig.update(f.read())
+except IOError:
+    pass
+else:
+    f.close()
+
+# Now read module files to generate potential new data
+output=StringIO.StringIO()
+output.write(header)
 
 # List of fortran source files.
 fortran_files=glob.glob("*/*.F")+glob.glob("*/*.F90")
@@ -23,7 +40,7 @@
 module_list=[]
 
 for filename in fortran_files:
-    
+
     fortran=file(filename,"r").read()
 
     modules=module_re.findall(fortran)
@@ -31,21 +48,36 @@
     for module in modules:
 
         if re.search(r"^\s*subroutine\s+"+module+"_register_diagnostic\s*$",\
-                     fortran,\
-                     re.IGNORECASE|re.MULTILINE):
+                         fortran,\
+                         re.IGNORECASE|re.MULTILINE):
             module_list.append(module)
 
 for module in module_list:
 
-        outfile.write("  use "+module+", only: "+module+"_register_diagnostic\n")
+    output.write("  use "+module+", only: "+module+"_register_diagnostic\n")
 
 # Ensure that the subroutine is legal in the trivial case.
-outfile.write("""
+output.write("""
    continue
    """)
 
 for module in module_list:
 
-    outfile.write("  call "+module+"_register_diagnostic\n")
-
-outfile.write(footer)
+    output.write("  call "+module+"_register_diagnostic\n")
+
+output.write(footer)
+
+new=hashlib.sha1()
+new.update(output.getvalue())
+
+# Only write file if sha1sums differ
+if new.digest() != orig.digest():
+    try:
+        f=open(outfile, 'w')
+        f.write(output.getvalue())
+    except IOError:
+        # Fixme, this should fail better
+        pass
+    else:
+        f.close()
+output.close()