Switch from a rake-based build to a make-based build.

This commit beefs up the Makefile to do the build, instead of the
Rakefile.

It also removes from the Rakefile the dependency on rake_utils, which
should mean it's ok to build in a schroot.

The files are reorganised to make the Makefile rules more tractable,
although the reorganisation reveals a problem with our current code
organisation.

The problem is that the proxy-specific code transitively depends on the
server code via flexnbd.h, which has a circular dependency on the server
and client structs. This should be broken in a future commit by
separating the flexnbd struct into a shared config struct and
server-specific parts, so that the server code can be moved into
src/server to more accurately show the functional dependencies.

Makefile

@@ -1,10 +1,95 @@
 #!/usr/bin/make -f
 
-all:
+VPATH=src:tests/unit
-	rake build
+	
+ifdef DEBUG
+	CFLAGS_EXTRA=-g -DDEBUG
+	LDFLAGS_EXTRA=-g
+else
+	CFLAGS_EXTRA=-O2
+endif
+
+CCFLAGS=-D_GNU_SOURCE=1 -Wall -Wextra -Werror-implicit-function-declaration -Wstrict-prototypes -Wno-missing-field-initializers $(CFLAGS_EXTRA) $(CFLAGS)
+LLDFLAGS=-lrt -lev $(LDFLAGS_EXTRA) $(LDFLAGS)
+
+CC?=gcc
+
+LIBS=-lpthread
+INC=-I/usr/include/libev -Isrc/common -Isrc/server -Isrc/proxy
+COMPILE=$(CC) $(INC) -c $(CCFLAGS)
+SAVEDEP=$(CC) $(INC) -MM $(CCFLAGS)
+LINK=$(CC) $(LLDFLAGS) -Isrc $(LIBS)
+
+
+EXISTING_OBJS := $(wildcard build/*.o)
+-include $(EXISTING_OBJS:.o=.d)
+
+COMMON_SRC  := $(wildcard src/common/*.c)
+SERVER_SRC := $(wildcard src/server/*.c)
+PROXY_SRC   := $(wildcard src/proxy/*.c)
+
+COMMON_OBJ  := $(COMMON_SRC:src/%.c=build/%.o)
+SERVER_OBJ := $(SERVER_SRC:src/%.c=build/%.o)
+PROXY_OBJ   := $(PROXY_SRC:src/%.c=build/%.o)
+
+SRCS := $(COMMON_SRC) $(SERVER_SRC) $(PROXY_SRC)
+OBJS := $(COMMON_OBJ) $(SERVER_OBJ) $(PROXY_OBJ)
+
+
+build/%.o: %.c
+	mkdir -p $(dir $@)
+	$(COMPILE) $< -o $@
+	$(SAVEDEP) $< > build/$*.d
+
+objs: $(OBJS)
+
+build/flexnbd: $(COMMON_OBJ) $(SERVER_OBJ) build/main.o
+	$(LINK) $^ -o $@
+
+build/flexnbd-proxy: $(COMMON_OBJ) $(PROXY_OBJ) build/proxy-main.o
+	$(LINK) $^ -o $@
+
+server: build/flexnbd
+proxy: build/flexnbd-proxy
+all: build/flexnbd build/flexnbd-proxy
+
+
+CHECK_SRC := $(wildcard tests/unit/*.c)
+CHECK_OBJ := $(CHECK_SRC:tests/unit/%.c=build/tests/%.o)
+# Why can't we reuse the build/%.o rule above? Not sure.
+build/tests/%.o: tests/unit/%.c
+	mkdir -p $(dir $@)
+	$(COMPILE) $< -o $@
+	$(SAVEDEP) $< > build/tests/$*.d
+
+CHECK_BINS := $(CHECK_OBJ:build/tests/%.o=build/tests/%)
+build/tests/%: build/tests/%.o $(OBJS)
+	$(LINK) $^ -o $@ -lcheck
+
+check_objs: $(CHECK_OBJ)
+
+check_bins: $(CHECK_BINS)
+check: $(CHECK_BINS)
+	for bin in $^; do $$bin; done
+
+build/flexnbd.1: README.txt
+	a2x --destination-dir build --format manpage $<
+build/flexnbd-proxy.1: README.proxy.txt
+	a2x --destination-dir build --format manpage $<
+# If we don't pipe to file, gzip clobbers the original, causing make
+# to rebuild each time
+%.1.gz: %.1
+	gzip -c -f $< > $@
+
+
+server-man: build/flexnbd.1.gz
+proxy-man: build/flexnbd-proxy.1.gz
+
+doc: server-man proxy-man
 
-all-debug:
-	DEBUG=1 rake build
 
 clean:
-	rake clean
+	rm -rf build/*
+
+
+.PHONY: clean objs check_objs all server proxy check_bins check server-man proxy-man doc

Rakefile

@@ -1,89 +1,34 @@
-$: << '../rake_utils/lib'
+# encoding: utf-8
-require 'rake_utils/debian'
-include RakeUtils::DSL
 
-CC=ENV['CC'] || "gcc"
+def make(*targets)
-
+  sh "make #{targets.map{|t| t.to_s}.join(" ")}"
-DEBUG  = ENV.has_key?('DEBUG') &&
-  %w|yes y ok 1 true t|.include?(ENV['DEBUG'])
-
-ALL_SOURCES = FileList['src/*']
-
-PROXY_ONLY_SOURCES = FileList['src/{proxy-main,proxy}.c']
-PROXY_ONLY_OBJECTS = PROXY_ONLY_SOURCES.pathmap( "%{^src,build}X.o" )
-
-SOURCES = ALL_SOURCES.select { |c| c =~ /\.c$/ } - PROXY_ONLY_SOURCES
-OBJECTS = SOURCES.pathmap( "%{^src,build}X.o" ) - PROXY_ONLY_OBJECTS
-
-PROXY_SOURCES = FileList['src/{ioutil,nbdtypes,readwrite,sockutil,util,parse}.c'] + PROXY_ONLY_SOURCES
-PROXY_OBJECTS = PROXY_SOURCES.pathmap( "%{^src,build}X.o" )
-
-TEST_SOURCES = FileList['tests/unit/*.c']
-TEST_OBJECTS = TEST_SOURCES.pathmap( "%{^tests/unit,build/tests}X.o" )
-
-LIBS    = %w( pthread )
-LDFLAGS = ["-lrt -lev"]
-CCFLAGS = %w(
-             -D_GNU_SOURCE=1
-             -Wall
-             -Wextra
-             -Werror-implicit-function-declaration
-             -Wstrict-prototypes
-             -Wno-missing-field-initializers
-            ) + # Added -Wno-missing-field-initializers to shut GCC up over {0} struct initialisers
-            [ENV['CFLAGS']]
-
-LIBCHECK = if File.exists?("/usr/lib/libcheck.a")
-  "/usr/lib/libcheck.a"
-elsif File.exists?("/usr/local/lib/libcheck.a")
-  "/usr/local/lib/libcheck.a"
-else
-  "-lcheck"
 end
 
-TEST_MODULES = Dir["tests/unit/check_*.c"].map { |n|
+def maketask( opts )
-  File.basename( n )[%r{check_(.+)\.c},1] }
+  case opts
-
+  when Symbol
-if DEBUG
+    maketask opts => opts
-  LDFLAGS << ["-g"]
+  else
-  CCFLAGS << ["-g -DDEBUG"]
+    opts.each do |name, targets|
-else
+      task( name ){make *[*targets]}
-  CCFLAGS << "-O2"
+    end
+  end
 end
 
+
 desc "Build the binary and man page"
-task :build => [:flexnbd, :flexnbd_proxy, :man]
+maketask :build => [:all, :doc]
-task :default => :build
 
 desc "Build just the flexnbd binary"
-task :flexnbd => "build/flexnbd"
+maketask :flexnbd => [:server]
+file "build/flexnbd" => :flexnbd
 
 desc "Build just the flexnbd-proxy binary"
-task :flexnbd_proxy => "build/flexnbd-proxy"
+maketask :flexnbd_proxy => [:proxy]
-
+file "build/flexnbd-proxy" => :flexnbd_proxy
-def check(m)
-  "build/tests/check_#{m}"
-end
-
-file "README.txt"
-file "README.proxy.txt"
-
-def manpage(name, src)
-  FileUtils.mkdir_p( "build" )
-  sh "a2x --destination-dir build --format manpage #{src}"
-  sh "gzip -f build/#{name}"
-end
-
-file "build/flexnbd.1.gz" => "README.txt" do
-  manpage("flexnbd.1", "README.txt")
-end
-
-file "build/flexnbd-proxy.1.gz" => "README.proxy.txt" do
-  manpage("flexnbd-proxy.1", "README.proxy.txt")
-end
 
 desc "Build just the man page"
-task :man => ["build/flexnbd.1.gz", "build/flexnbd-proxy.1.gz"]
+maketask :man => :doc
 
 
 namespace "test" do
@@ -91,226 +36,17 @@ namespace "test" do
   task 'run' => ["unit", "scenarios"]
 
   desc "Build C tests"
-  task 'build' => TEST_MODULES.map { |n| check n}
+  maketask :build => :check_bins
-
-  TEST_MODULES.each do |m|
-    desc "Run tests for #{m}"
-    task "check_#{m}" => check(m) do
-      sh check m
-    end
-  end
 
   desc "Run C tests"
-  task 'unit' => 'build' do
+  maketask :unit => :check
-    TEST_MODULES.each do |n|
-      ENV['EF_DISABLE_BANNER'] = '1'
-      sh check n
-    end
-  end
 
   desc "Run NBD test scenarios"
-  task 'scenarios' => ['build/flexnbd', 'build/flexnbd-proxy'] do
+  task 'scenarios' => ["build/flexnbd", "build/flexnbd-proxy"] do
-    sh "cd tests/acceptance; ruby nbd_scenarios -v"
+    sh "cd tests/acceptance && RUBYOPT='-I.' ruby nbd_scenarios -v"
   end
 end
 
 
-
-def gcc_compile( target, source )
-  FileUtils.mkdir_p File.dirname( target )
-  sh "#{CC} -I/usr/include/libev -Isrc -c #{CCFLAGS.join(' ')} -o #{target} #{source} "
-end
-
-def gcc_link(target, objects)
-  FileUtils.mkdir_p File.dirname( target )
-
-  sh "#{CC} #{LDFLAGS.join(' ')} "+
-    " -Isrc " +
-    " -o #{target} "+
-    objects.join(" ") +
-    " "+LIBS.map { |l| "-l#{l}" }.join(" ")
-end
-
-def headers(c)
-  `#{CC} -Isrc -MM #{c}`.gsub("\\\n", " ").split(" ")[2..-1]
-end
-
-rule 'build/flexnbd-proxy' => PROXY_OBJECTS do |t|
-  gcc_link(t.name, t.sources)
-end
-
-rule 'build/flexnbd' => OBJECTS do |t|
-  gcc_link(t.name, t.sources)
-end
-
-
-file check("client") =>
-%w{build/tests/check_client.o
-  build/self_pipe.o
-  build/nbdtypes.o
-  build/flexnbd.o
-  build/flexthread.o
-  build/control.o
-  build/readwrite.o
-  build/parse.o
-  build/client.o
-  build/serve.o
-  build/acl.o
-  build/ioutil.o
-  build/mbox.o
-  build/mirror.o
-  build/status.o
-  build/sockutil.o
-  build/util.o} do |t|
-  gcc_link t.name, t.prerequisites + [LIBCHECK]
-end
-
-file check("acl") =>
-%w{build/tests/check_acl.o
-  build/parse.o
-  build/acl.o
-  build/util.o} do |t|
-  gcc_link t.name, t.prerequisites + [LIBCHECK]
-end
-
-file check( "util" ) =>
-%w{build/tests/check_util.o
-  build/util.o
-  build/self_pipe.o} do |t|
-  gcc_link t.name, t.prerequisites + [LIBCHECK]
-end
-
-file check("serve") =>
-%w{build/tests/check_serve.o
-  build/self_pipe.o
-  build/nbdtypes.o
-  build/control.o
-  build/readwrite.o
-  build/parse.o
-  build/client.o
-  build/flexthread.o
-  build/serve.o
-  build/flexnbd.o
-  build/mirror.o
-  build/status.o
-  build/acl.o
-  build/mbox.o
-  build/ioutil.o
-  build/sockutil.o
-  build/util.o} do |t|
-  gcc_link t.name, t.prerequisites + [LIBCHECK]
-end
-
-file check("status") =>
-%w{
-build/tests/check_status.o
-  build/self_pipe.o
-  build/nbdtypes.o
-  build/control.o
-  build/readwrite.o
-  build/parse.o
-  build/client.o
-  build/flexthread.o
-  build/serve.o
-  build/flexnbd.o
-  build/mirror.o
-  build/status.o
-  build/acl.o
-  build/mbox.o
-  build/ioutil.o
-  build/sockutil.o
-  build/util.o
-} do |t|
-  gcc_link t.name, t.prerequisites + [LIBCHECK]
-end
-
-
-file check("readwrite") =>
-%w{build/tests/check_readwrite.o
-  build/readwrite.o
-  build/client.o
-  build/self_pipe.o
-  build/serve.o
-  build/parse.o
-  build/acl.o
-  build/flexthread.o
-  build/control.o
-  build/flexnbd.o
-  build/mirror.o
-  build/status.o
-  build/nbdtypes.o
-  build/mbox.o
-  build/ioutil.o
-  build/sockutil.o
-  build/util.o} do |t|
-  gcc_link t.name, t.prerequisites + [LIBCHECK]
-end
-
-
-file check("flexnbd") =>
-%w{build/tests/check_flexnbd.o
-  build/flexnbd.o
-  build/ioutil.o
-  build/sockutil.o
-  build/util.o
-  build/control.o
-  build/mbox.o
-  build/flexthread.o
-  build/status.o
-  build/self_pipe.o
-  build/client.o
-  build/acl.o
-  build/parse.o
-  build/nbdtypes.o
-  build/readwrite.o
-  build/mirror.o
-  build/serve.o} do |t|
-  gcc_link t.name, t.prerequisites + [LIBCHECK]
-end
-
-
-file check("control") =>
-  %w{build/tests/check_control.o} + OBJECTS - ["build/main.o", 'build/proxy-main.o', 'build/proxy.o'] do |t|
-  gcc_link t.name, t.prerequisites + [LIBCHECK]
-end
-
-(TEST_MODULES- %w{status control flexnbd acl client serve readwrite util}).each do |m|
-  tgt = "build/tests/check_#{m}.o"
-  maybe_obj_name = "build/#{m}.o"
-  # Take it out in case we're testing one of the utils
-  deps = ["build/ioutil.o", "build/util.o", "build/sockutil.o"] - [maybe_obj_name]
-
-  # Add it back in if it's something we need to compile
-  deps << maybe_obj_name if OBJECTS.include?( maybe_obj_name )
-
-  file check( m ) => deps + [tgt] do |t|
-    gcc_link(t.name, deps + [tgt, LIBCHECK])
-  end
-end
-
-
-OBJECTS.zip( SOURCES ).each do |o,c|
-  file o => [c]+headers(c) do |t| gcc_compile( o, c ) end
-end
-
-PROXY_ONLY_OBJECTS.zip( PROXY_ONLY_SOURCES).each do |o, c|
-  file o => [c]+headers(c) do |t| gcc_compile( o, c ) end
-end
-
-TEST_OBJECTS.zip( TEST_SOURCES ).each do |o,c|
-  file o => [c] + headers(c) do |t| gcc_compile( o, c ) end
-end
-
 desc "Remove all build targets, binaries and temporary files"
-task :clean do
+maketask :clean
-  sh "rm -rf *~ build"
-end
-
-namespace :pkg do
-  deb do |t|
-    t.code_files = ALL_SOURCES + ["Rakefile", "README.txt", "README.proxy.txt"]
-    t.pkg_name = "flexnbd"
-    t.generate_changelog!
-  end
-end
-

tests/acceptance/flexnbd.rb

@@ -458,12 +458,18 @@ module FlexNBD
 
     def maybe_timeout(cmd, timeout=nil )
       stdout, stderr = "",""
+      stat = nil
       run = Proc.new do
-        Open3.popen3( cmd ) do |io_in, io_out, io_err|
+        # Ruby 1.9 changed the popen3 api. instead of 3 args, the block
+        # gets 4. Not only that, but it no longer sets $?, so we have to
+        # go elsewhere for the process' exit status.
+        Open3.popen3( cmd ) do |io_in, io_out, io_err, maybe_thr|
           io_in.close
           stdout.replace io_out.read
           stderr.replace io_err.read
+          stat = maybe_thr.value if maybe_thr
         end
+        stat ||= $?
       end
 
       if timeout
@@ -472,13 +478,13 @@ module FlexNBD
         run.call
       end
 
-      [stdout, stderr]
+      [stdout, stderr, stat]
     end
 
 
     def mirror(dest_ip, dest_port, bandwidth=nil, action=nil)
-      stdout, stderr = mirror_unchecked( dest_ip, dest_port, bandwidth, action )
+      stdout, stderr, status = mirror_unchecked( dest_ip, dest_port, bandwidth, action )
-      raise IOError.new( "Migrate command failed\n" + stderr) unless $?.success?
+      raise IOError.new( "Migrate command failed\n" + stderr) unless status.success?
 
       stdout
     end

tests/acceptance/flexnbd/constants.rb

@@ -2,6 +2,14 @@
 
 module FlexNBD
 
+  def self.binary( str )
+    if str.respond_to? :force_encoding
+      str.force_encoding "ASCII-8BIT"
+    else
+      str
+    end
+  end
+
   # eeevil is his one and only name...
   def self.read_constants
     parents = []
@@ -17,7 +25,7 @@ module FlexNBD
 
     fail "No source root!" unless source_root
 
-    headers = Dir[File.join( source_root, "src", "*.h" ) ]
+    headers = Dir[File.join( source_root, "src", "{common,proxy,server}","*.h" ) ]
 
     headers.each do |header_filename|
       txt_lines = File.readlines( header_filename )
@@ -33,8 +41,8 @@ module FlexNBD
 
   read_constants()
 
-  REQUEST_MAGIC = "\x25\x60\x95\x13" unless defined?(REQUEST_MAGIC)
+  REQUEST_MAGIC = binary("\x25\x60\x95\x13") unless defined?(REQUEST_MAGIC)
-  REPLY_MAGIC = "\x67\x44\x66\x98" unless defined?(REPLY_MAGIC)
+  REPLY_MAGIC = binary("\x67\x44\x66\x98") unless defined?(REPLY_MAGIC)
 
 end # module FlexNBD
 

tests/acceptance/test_happy_path.rb

@@ -2,12 +2,17 @@
 
 require 'test/unit'
 require 'environment'
+require 'flexnbd/constants'
 
 class TestHappyPath < Test::Unit::TestCase
   def setup
     @env = Environment.new
   end
 
+  def bin(str)
+    FlexNBD.binary str
+  end
+
   def teardown
     @env.nbd1.can_die(0)
     @env.nbd2.can_die(0)
@@ -22,13 +27,13 @@ class TestHappyPath < Test::Unit::TestCase
     [0, 12, 63].each do |num|
 
       assert_equal(
-        @env.nbd1.read(num*@env.blocksize, @env.blocksize),
+        bin( @env.nbd1.read(num*@env.blocksize, @env.blocksize) ),
-        @env.file1.read(num*@env.blocksize, @env.blocksize)
+        bin( @env.file1.read(num*@env.blocksize, @env.blocksize) )
       )
     end
 
     [124, 1200, 10028, 25488].each do |num|
-      assert_equal(@env.nbd1.read(num, 4), @env.file1.read(num, 4))
+      assert_equal(bin(@env.nbd1.read(num, 4)), bin(@env.file1.read(num, 4)))
     end
   end