Skip to content

Commit 51b34a6

Browse files
tomas-abrahamssontomas-abrahamsson
committed
Recompile .proto files with gpb also with prefix/suffix
For protocol buffer files, when there were gpb options to alter the module name with prefix or suffix, recompilation was not properly detected. This is now fixed. (Issue basho#384). Properly detecting change meant moving from rebar_base_compiler, so it also meant processing protocol buffer files in sequence instead of in parallel.
1 parent 6e6b0fd commit 51b34a6

File tree

2 files changed

+106
-27
lines changed

2 files changed

+106
-27
lines changed

‎inttest/proto_gpb/proto_gpb_rt.erl

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,8 @@
2929
run/1]).
3030

3131
-include_lib("eunit/include/eunit.hrl").
32+
-include_lib("kernel/include/file.hrl").
33+
-include_lib("deps/retest/include/retest.hrl").
3234

3335
-define(MODULES,
3436
[foo,
@@ -42,6 +44,13 @@
4244
test4_gpb,
4345
test5_gpb]).
4446

47+
-define(SOURCE_PROTO_FILES,
48+
["test.proto",
49+
"a/test2.proto",
50+
"a/b/test3.proto",
51+
"c/test4.proto",
52+
"c/d/test5.proto"]).
53+
4554
files() ->
4655
[
4756
{copy, "../../rebar", "rebar"},
@@ -60,6 +69,17 @@ run(_Dir) ->
6069
%% generating the test_gpb.hrl file, and also that it generated
6170
%% the .hrl file was generated before foo was compiled.
6271
ok = check_beams_generated(),
72+
73+
?DEBUG("Verifying recompilation~n", []),
74+
TestErl = hd(generated_erl_files()),
75+
TestProto = hd(source_proto_files()),
76+
make_proto_newer_than_erl(TestProto, TestErl),
77+
TestMTime1 = read_mtime(TestErl),
78+
?assertMatch({ok, _}, retest_sh:run("./rebar compile", [])),
79+
TestMTime2 = read_mtime(TestErl),
80+
?assert(TestMTime2 > TestMTime1),
81+
82+
?DEBUG("Verify cleanup~n", []),
6383
?assertMatch({ok, _}, retest_sh:run("./rebar clean", [])),
6484
ok = check_files_deleted(),
6585
ok.
@@ -81,6 +101,12 @@ generated_erl_files() ->
81101
generated_hrl_files() ->
82102
add_dir("include", add_ext(?GENERATED_MODULES, ".hrl")).
83103

104+
generated_beam_files() ->
105+
add_dir("ebin", add_ext(?GENERATED_MODULES, ".beam")).
106+
107+
source_proto_files() ->
108+
add_dir("src", ?SOURCE_PROTO_FILES).
109+
84110
file_does_not_exist(F) ->
85111
not filelib:is_regular(F).
86112

@@ -90,6 +116,30 @@ add_ext(Modules, Ext) ->
90116
add_dir(Dir, Files) ->
91117
[filename:join(Dir, File) || File <- Files].
92118

119+
read_mtime(File) ->
120+
{ok, #file_info{mtime=MTime}} = file:read_file_info(File),
121+
MTime.
122+
123+
124+
make_proto_newer_than_erl(Proto, Erl) ->
125+
%% Do this by back-dating the erl file instead of touching the
126+
%% proto file. Do this instead of sleeping for a second to get a
127+
%% reliable test. Sleeping would have been needed sin ce the
128+
%% #file_info{} (used by eg. filelib:last_modified) does not have
129+
%% sub-second resolution (even though most file systems have).
130+
{ok, #file_info{mtime=ProtoMTime}} = file:read_file_info(Proto),
131+
{ok, ErlInfo} = file:read_file_info(Erl),
132+
OlderMTime = update_seconds_to_datetime(ProtoMTime, -2),
133+
OlderErlInfo = ErlInfo#file_info{mtime = OlderMTime},
134+
ok = file:write_file_info(Erl, OlderErlInfo).
135+
136+
update_seconds_to_datetime(DT, ToAdd) ->
137+
calendar:gregorian_seconds_to_datetime(
138+
calendar:datetime_to_gregorian_seconds(DT) + ToAdd).
139+
140+
touch_file(File) ->
141+
?assertMatch({ok, _}, retest_sh:run("touch " ++ File, [])).
142+
93143
check(Check, Files) ->
94144
lists:foreach(
95145
fun(F) ->

‎src/rebar_proto_gpb_compiler.erl

Lines changed: 56 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -42,29 +42,36 @@
4242
key() ->
4343
gpb.
4444

45-
proto_compile(Config, _AppFile, _ProtoFiles) ->
45+
proto_compile(Config, _AppFile, ProtoFiles) ->
4646
%% Check for gpb library -- if it's not present, fail
4747
%% since we have.proto files that need building
4848
case gpb_is_present() of
4949
true ->
50-
rebar_base_compiler:run(Config, [],
51-
"src", ".proto",
52-
"src", ".erl",
53-
fun compile_gpb/3,
54-
[{check_last_mod, true}]);
50+
UserGpbOpts = user_gpb_opts(Config),
51+
lists:foreach(
52+
fun(ProtoFile) ->
53+
GpbOpts = UserGpbOpts ++ default_dest_opts()
54+
++ default_include_opts(ProtoFile),
55+
56+
case needs_compile(ProtoFile, GpbOpts) of
57+
true ->
58+
compile_gpb(ProtoFile, GpbOpts);
59+
false ->
60+
ok
61+
end
62+
end,
63+
ProtoFiles);
5564
false ->
5665
?ERROR("The gpb library is not present in code path!\n", []),
5766
?FAIL
5867
end.
5968

6069
proto_clean(Config, _AppFile, ProtoFiles) ->
61-
GpbOpts = gpb_opts(Config),
62-
MPrefix = proplists:get_value(module_name_prefix, GpbOpts, ""),
63-
MSuffix = proplists:get_value(module_name_suffix, GpbOpts, ""),
70+
GpbOpts = user_gpb_opts(Config) ++ default_dest_opts(),
6471
rebar_file_utils:delete_each(
65-
[beam_relpath(MPrefix, F, MSuffix) || F <- ProtoFiles]
66-
++ [erl_relpath(MPrefix, F, MSuffix) || F <- ProtoFiles]
67-
++ [hrl_relpath(MPrefix, F, MSuffix) || F <- ProtoFiles]),
72+
[beam_file(F, GpbOpts) || F <- ProtoFiles]
73+
++ [erl_file(F, GpbOpts) || F <- ProtoFiles]
74+
++ [hrl_file(F, GpbOpts) || F <- ProtoFiles]),
6875
ok.
6976

7077
%% ===================================================================
@@ -82,17 +89,27 @@ proto_info(help, compile) ->
8289
proto_info(help, clean) ->
8390
?CONSOLE("", []).
8491

85-
gpb_opts(Config) ->
86-
rebar_config:get_local(Config, gpb_opts, []).
87-
8892
gpb_is_present() ->
8993
code:which(gpb) =/= non_existing.
9094

91-
compile_gpb(Source, _Target, Config) ->
95+
user_gpb_opts(Config) ->
96+
rebar_config:get_local(Config, gpb_opts, []).
97+
98+
default_dest_opts() ->
99+
[{o_erl, "src"}, {o_hrl, "include"}].
100+
101+
default_include_opts(Source) ->
102+
SourceFullPath = filename:absname(Source),
103+
[{i,filename:dirname(SourceFullPath)}].
104+
105+
needs_compile(ProtoFile, GpbOpts) ->
106+
Erl = erl_file(ProtoFile, GpbOpts),
107+
Hrl = hrl_file(ProtoFile, GpbOpts),
108+
filelib:last_modified(Erl) < filelib:last_modified(ProtoFile) orelse
109+
filelib:last_modified(Hrl) < filelib:last_modified(ProtoFile).
110+
111+
compile_gpb(Source, GpbOpts) ->
92112
SourceFullPath = filename:absname(Source),
93-
DefaultDestOpts = [{o_erl, "src"}, {o_hrl, "include"}],
94-
SelfIncludeOpt = [{i,filename:dirname(SourceFullPath)}],
95-
GpbOpts = gpb_opts(Config) ++ DefaultDestOpts ++ SelfIncludeOpt,
96113
ok = filelib:ensure_dir(filename:join("ebin", "dummy")),
97114
ok = filelib:ensure_dir(filename:join("include", "dummy")),
98115
?CONSOLE("Compiling ~s\n", [Source]),
@@ -104,16 +121,28 @@ compile_gpb(Source, _Target, Config) ->
104121
?FAIL
105122
end.
106123

107-
beam_relpath(Prefix, Proto, Suffix) ->
108-
proto_filename_to_relpath("ebin", Prefix, Proto, Suffix, ".beam").
124+
beam_file(ProtoFile, GpbOpts) ->
125+
proto_filename_to_path("ebin", ProtoFile, ".beam", GpbOpts).
109126

110-
erl_relpath(Prefix, Proto, Suffix) ->
111-
proto_filename_to_relpath("src", Prefix, Proto, Suffix, ".erl").
127+
erl_file(ProtoFile, GpbOpts) ->
128+
ErlOutDir = get_erl_outdir(GpbOpts),
129+
proto_filename_to_path(ErlOutDir, ProtoFile, ".erl", GpbOpts).
112130

113-
hrl_relpath(Prefix, Proto, Suffix) ->
114-
proto_filename_to_relpath("include", Prefix, Proto, Suffix, ".hrl").
131+
hrl_file(ProtoFile, GpbOpts) ->
132+
HrlOutDir = get_hrl_outdir(GpbOpts),
133+
proto_filename_to_path(HrlOutDir, ProtoFile, ".hrl", GpbOpts).
115134

116-
proto_filename_to_relpath(Dir, Prefix, Proto, Suffix, NewExt) ->
117-
BaseNoExt = filename:basename(Proto, ".proto"),
135+
proto_filename_to_path(Dir, ProtoFile, NewExt, GpbOpts) ->
136+
BaseNoExt = filename:basename(ProtoFile, ".proto"),
137+
Prefix = proplists:get_value(module_name_prefix, GpbOpts, ""),
138+
Suffix = proplists:get_value(module_name_suffix, GpbOpts, ""),
118139
filename:join([Dir, Prefix ++ BaseNoExt ++ Suffix ++ NewExt]).
119140

141+
get_erl_outdir(Opts) ->
142+
proplists:get_value(o_erl, Opts, get_outdir(Opts)).
143+
144+
get_hrl_outdir(Opts) ->
145+
proplists:get_value(o_hrl, Opts, get_outdir(Opts)).
146+
147+
get_outdir(Opts) ->
148+
proplists:get_value(o, Opts, ".").

0 commit comments

Comments
 (0)