Skip to content

Commit 1ce7059

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). Use the rebar_base_compiler's ability to specify both source and target file names, to be able to also support prefixes. This also introduces a call to gpb_compile:format_error, so the xref recipe needs to be updated to ignore it, to avoid false errors.
1 parent 2e9706f commit 1ce7059

File tree

4 files changed

+102
-26
lines changed

4 files changed

+102
-26
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) ->

‎rebar.config‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626
- (\"neotoma\":\"file\"/\"2\")
2727
- (\"protobuffs_compile\":\"scan_file\"/\"2\")
2828
- (\"gpb_compile\":\"file\"/\"2\")
29+
- (\"gpb_compile\":\"format_error\"/\"1\")
2930
- (\"diameter_codegen\":\"from_dict\"/\"4\")
3031
- (\"diameter_dict_util\":\"format_error\"/\"1\")
3132
- (\"diameter_dict_util\":\"parse\"/\"2\"))",

‎src/rebar_proto_compiler.erl‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@
4444
%% ===================================================================
4545

4646
compile(Config, AppFile) ->
47-
case rebar_utils:find_files_by_ext("src", ".proto", true) of
47+
case rebar_utils:find_files_by_ext("src", ".proto") of
4848
[] ->
4949
ok;
5050
Protos ->
@@ -56,7 +56,7 @@ compile(Config, AppFile) ->
5656

5757
clean(Config, AppFile) ->
5858
%% Get a list of generated .beam and .hrl files and then delete them
59-
Protos = rebar_utils:find_files_by_ext("src", ".proto", true),
59+
Protos = rebar_utils:find_files_by_ext("src", ".proto"),
6060
case Protos of
6161
[] ->
6262
ok;

‎src/rebar_proto_gpb_compiler.erl‎

Lines changed: 49 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -47,24 +47,31 @@ proto_compile(Config, _AppFile, _ProtoFiles) ->
4747
%% since we have.proto files that need building
4848
case gpb_is_present() of
4949
true ->
50+
GpbOpts = user_gpb_opts(Config),
51+
Files = rebar_utils:find_files_by_ext("src", ".proto"),
52+
Targets = [filename:join("src", target_filename(F, GpbOpts))
53+
|| F <- Files],
5054
rebar_base_compiler:run(Config, [],
51-
"src", ".proto",
52-
"src", ".erl",
55+
lists:zip(Files, Targets),
5356
fun compile_gpb/3,
5457
[{check_last_mod, true}]);
5558
false ->
5659
?ERROR("The gpb library is not present in code path!\n", []),
5760
?FAIL
5861
end.
5962

63+
target_filename(ProtoFileName, GpbOpts) ->
64+
ModulePrefix = proplists:get_value(module_name_prefix, GpbOpts, ""),
65+
ModuleSuffix = proplists:get_value(module_name_suffix, GpbOpts, ""),
66+
Base = filename:basename(ProtoFileName, ".proto"),
67+
ModulePrefix ++ Base ++ ModuleSuffix ++ ".erl".
68+
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,37 +89,55 @@ 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

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+
91101
compile_gpb(Source, _Target, Config) ->
92102
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,
103+
GpbOpts = user_gpb_opts(Config) ++ default_dest_opts()
104+
++ default_include_opts(SourceFullPath),
96105
ok = filelib:ensure_dir(filename:join("ebin", "dummy")),
97106
ok = filelib:ensure_dir(filename:join("include", "dummy")),
98107
case gpb_compile:file(SourceFullPath, GpbOpts) of
99108
ok ->
100109
ok;
101-
{error, _Reason} ->
102-
?ERROR("Failed to compile ~s~n", [Source]),
110+
{error, Reason} ->
111+
ReasonStr = gpb_compile:format_error(Reason),
112+
?ERROR("Failed to compile ~s: ~s~n", [SourceFullPath, ReasonStr]),
103113
?FAIL
104114
end.
105115

106-
beam_relpath(Prefix, Proto, Suffix) ->
107-
proto_filename_to_relpath("ebin", Prefix, Proto, Suffix, ".beam").
116+
default_include_opts(SourceFullPath) ->
117+
[{i,filename:dirname(SourceFullPath)}].
118+
119+
beam_file(ProtoFile, GpbOpts) ->
120+
proto_filename_to_path("ebin", ProtoFile, ".beam", GpbOpts).
108121

109-
erl_relpath(Prefix, Proto, Suffix) ->
110-
proto_filename_to_relpath("src", Prefix, Proto, Suffix, ".erl").
122+
erl_file(ProtoFile, GpbOpts) ->
123+
ErlOutDir = get_erl_outdir(GpbOpts),
124+
proto_filename_to_path(ErlOutDir, ProtoFile, ".erl", GpbOpts).
111125

112-
hrl_relpath(Prefix, Proto, Suffix) ->
113-
proto_filename_to_relpath("include", Prefix, Proto, Suffix, ".hrl").
126+
hrl_file(ProtoFile, GpbOpts) ->
127+
HrlOutDir = get_hrl_outdir(GpbOpts),
128+
proto_filename_to_path(HrlOutDir, ProtoFile, ".hrl", GpbOpts).
114129

115-
proto_filename_to_relpath(Dir, Prefix, Proto, Suffix, NewExt) ->
116-
BaseNoExt = filename:basename(Proto, ".proto"),
130+
proto_filename_to_path(Dir, ProtoFile, NewExt, GpbOpts) ->
131+
BaseNoExt = filename:basename(ProtoFile, ".proto"),
132+
Prefix = proplists:get_value(module_name_prefix, GpbOpts, ""),
133+
Suffix = proplists:get_value(module_name_suffix, GpbOpts, ""),
117134
filename:join([Dir, Prefix ++ BaseNoExt ++ Suffix ++ NewExt]).
118135

136+
get_erl_outdir(Opts) ->
137+
proplists:get_value(o_erl, Opts, get_outdir(Opts)).
138+
139+
get_hrl_outdir(Opts) ->
140+
proplists:get_value(o_hrl, Opts, get_outdir(Opts)).
141+
142+
get_outdir(Opts) ->
143+
proplists:get_value(o, Opts, ".").

0 commit comments

Comments
 (0)