Merge pull request #293 from liskin/port-deps

Check C source dependencies in needs_compile

inttest/port/c_src/test1.c

@@ -0,0 +1 @@
+#include "test1.h"

inttest/port/c_src/test2.c

@@ -0,0 +1 @@
+#include "test2.h"

inttest/port/c_src/test2.h

@@ -0,0 +1 @@
+#include "test1.h"

inttest/port/port_rt.erl

@@ -0,0 +1,76 @@
+%% -*- erlang-indent-level: 4;indent-tabs-mode: nil -*-
+%% ex: ts=4 sw=4 et
+
+-module(port_rt).
+-export([files/0,
+         run/1]).
+
+-include_lib("eunit/include/eunit.hrl").
+
+files() ->
+    [
+     {copy, "../../rebar", "rebar"},
+     {copy, "rebar.config", "rebar.config"},
+     {copy, "c_src", "c_src"},
+     {create, "ebin/foo.app", app(foo, [])}
+    ].
+
+run(_Dir) ->
+    % wait a bit for new files to have different timestamps
+    wait(),
+    % test.so is created during first compile
+    ?assertEqual(0, filelib:last_modified("priv/test.so")),
+    ?assertMatch({ok, _}, retest_sh:run("./rebar compile", [])),
+    TestSo1 = filelib:last_modified("priv/test.so"),
+    ?assert(TestSo1 > 0),
+    wait(),
+    % nothing happens during second compile
+    ?assertMatch({ok, _}, retest_sh:run("./rebar compile", [])),
+    TestSo2 = filelib:last_modified("priv/test.so"),
+    Test1o2 = filelib:last_modified("c_src/test1.o"),
+    Test2o2 = filelib:last_modified("c_src/test2.o"),
+    ?assertEqual(TestSo1, TestSo2),
+    ?assert(TestSo1 >= Test1o2),
+    ?assert(TestSo1 >= Test2o2),
+    wait(),
+    % when test2.c changes, at least test2.o and test.so are rebuilt
+    ?assertMatch({ok, _}, retest_sh:run("touch c_src/test2.c", [])),
+    ?assertMatch({ok, _}, retest_sh:run("./rebar compile", [])),
+    TestSo3 = filelib:last_modified("priv/test.so"),
+    Test2o3 = filelib:last_modified("c_src/test2.o"),
+    ?assert(TestSo3 > TestSo2),
+    ?assert(Test2o3 > TestSo2),
+    wait(),
+    % when test2.h changes, at least test2.o and test.so are rebuilt
+    ?assertMatch({ok, _}, retest_sh:run("touch c_src/test2.h", [])),
+    ?assertMatch({ok, _}, retest_sh:run("./rebar compile", [])),
+    TestSo4 = filelib:last_modified("priv/test.so"),
+    Test2o4 = filelib:last_modified("c_src/test2.o"),
+    ?assert(TestSo4 > TestSo3),
+    ?assert(Test2o4 > TestSo3),
+    wait(),
+    % when test1.h changes, everything is rebuilt
+    ?assertMatch({ok, _}, retest_sh:run("touch c_src/test1.h", [])),
+    ?assertMatch({ok, _}, retest_sh:run("./rebar compile", [])),
+    TestSo5 = filelib:last_modified("priv/test.so"),
+    Test1o5 = filelib:last_modified("c_src/test1.o"),
+    Test2o5 = filelib:last_modified("c_src/test2.o"),
+    ?assert(TestSo5 > TestSo4),
+    ?assert(Test1o5 > TestSo4),
+    ?assert(Test2o5 > TestSo4),
+    ok.
+
+wait() ->
+    timer:sleep(1000).
+
+%%
+%% Generate the contents of a simple .app file
+%%
+app(Name, Modules) ->
+    App = {application, Name,
+           [{description, atom_to_list(Name)},
+            {vsn, "1"},
+            {modules, Modules},
+            {registered, []},
+            {applications, [kernel, stdlib]}]},
+    io_lib:format("~p.\n", [App]).

inttest/port/rebar.config

@@ -0,0 +1 @@
+{port_specs, [{"priv/test.so", ["c_src/*.c"]}]}.

src/rebar_port_compiler.erl

@@ -141,7 +141,8 @@ clean(Config, AppFile) ->
         Specs ->
             lists:foreach(fun(#spec{target=Target, objects=Objects}) ->
                                   rebar_file_utils:delete_each([Target]),
-                                  rebar_file_utils:delete_each(Objects)
+                                  rebar_file_utils:delete_each(Objects),
+                                  rebar_file_utils:delete_each(port_deps(Objects))
                           end, Specs)
     end,
     ok.
@@ -251,9 +252,28 @@ exec_compiler(Config, Source, Cmd, ShOpts) ->
     end.
 
 needs_compile(Source, Bin) ->
-    %% TODO: Generate depends using gcc -MM so we can also
-    %% check for include changes
-    filelib:last_modified(Bin) < filelib:last_modified(Source).
+    needs_link(Bin, [Source|bin_deps(Bin)]).
+
+%% NOTE: This relies on -MMD being passed to the compiler and returns an
+%% empty list if the .d file is not available.  This means header deps are
+%% ignored on win32.
+bin_deps(Bin) ->
+    [DepFile] = port_deps([Bin]),
+    case file:read_file(DepFile) of
+        {ok, Deps} ->
+            Ds = parse_bin_deps(list_to_binary(Bin), Deps),
+            ?DEBUG("Deps of ~p: ~p\n", [Bin, Ds]),
+            Ds;
+        {error, Err} ->
+            ?DEBUG("Skipping deps parse of ~s: ~p\n", [DepFile, Err]),
+            []
+    end.
+
+parse_bin_deps(Bin, Deps) ->
+    Sz = size(Bin),
+    <<Bin:Sz/binary, ": ", X/binary>> = Deps,
+    Ds = re:split(X, "\\s*\\\\\\R\\s*|\\s+", [{return, binary}]),
+    [D || D <- Ds, D =/= <<>>].
 
 needs_link(SoName, []) ->
     filelib:last_modified(SoName) == 0;
@@ -334,6 +354,9 @@ port_sources(Sources) ->
 port_objects(SourceFiles) ->
     [replace_extension(O, ".o") || O <- SourceFiles].
 
+port_deps(SourceFiles) ->
+    [replace_extension(O, ".d") || O <- SourceFiles].
+
 port_opts(Config, Opts) ->
     [port_opt(Config, O) || O <- Opts].
 
@@ -560,9 +583,9 @@ default_env() ->
       "$CC -c $CFLAGS $EXE_CFLAGS $PORT_IN_FILES -o $PORT_OUT_FILE"},
      {"EXE_LINK_TEMPLATE",
       "$CC $PORT_IN_FILES $LDFLAGS $EXE_LDFLAGS -o $PORT_OUT_FILE"},
-     {"DRV_CFLAGS" , "-g -Wall -fPIC $ERL_CFLAGS"},
+     {"DRV_CFLAGS" , "-g -Wall -fPIC -MMD $ERL_CFLAGS"},
      {"DRV_LDFLAGS", "-shared $ERL_LDFLAGS"},
-     {"EXE_CFLAGS" , "-g -Wall -fPIC $ERL_CFLAGS"},
+     {"EXE_CFLAGS" , "-g -Wall -fPIC -MMD $ERL_CFLAGS"},
      {"EXE_LDFLAGS", "$ERL_LDFLAGS"},
 
      {"ERL_CFLAGS", lists:concat([" -I\"", erl_interface_dir(include),