Closed DavidMikeSimon closed 6 years ago
I think not. One important feature of this library is compiling using protoc, which is the official and right way to do that.
The idea would be that mix
would call protoc
for you.
@DavidMikeSimon Then the things will become more complex. We have to support all options protoc support. I can't see what's the benefit from implementing that.
The benefits are:
.proto
files every time you change them.pb.ex
files to your repo. These files are not true source code, but purely derivable artifacts of the build system.For protoc options, we wouldn't have to specifically support everything it does. We can just provide a protoc_arguments
option which accepts a list of strings for any additional command-line arguments.
.pb.ex
files were one of main reasons why I replaced exprotobuf with this lib as soon as it was possible... magic modules/structs from nowhere are terrible
Checking in on if you've changed your mind @tony612
I would seriously consider using the Mix compiler behaviour. If you use it right, users get a lot of benefits.
https://hexdocs.pm/mix/Mix.Task.Compiler.html
Just check it out.
Checking in here, I'm planning on implementing a protoc
Mix compiler for a couple reasons:
protoc
without a Mix compiler causes friction here.pb.ex
files is error-prone because I have to remember to run protoc
locally and commit the generated file(s). This also causes headaches in local developmentI think @DavidMikeSimon is right that this would be a pretty low-effort compiler to write, since protoc
should do most if not all of the work. But one can only know once they've done it, so I'll see how it goes and report back here
This compiler intends to be a drop-in for compiling protobuf files stored in a project directory:
defmodule Mix.Tasks.Compiler.ProtocGen do
use Mix.Task.Compiler
def run(_args) do
protobuf_dir = Application.get_env(:bootstrap, :protobuf_path, "priv/protobuf")
if is_binary(protobuf_dir) and File.dir?(protobuf_dir) and
not Enum.empty?(File.ls!(protobuf_dir)) do
with {:ok, protoc_path} <- find_executable("protoc"),
{:ok, _elixir_protoc_path} <- find_executable("protoc-gen-elixir") do
protoc_files = File.ls!(protobuf_dir) |> Enum.map(&Path.join(protobuf_dir, &1))
IO.puts("Compiling protoc files: #{inspect(protoc_files)}")
# Create the directory where we'll store protobuf generated files, if it doesn't exist
File.mkdir_p!("./lib/protoc_gen")
compile_while_successful(protoc_files, protoc_path)
else
{:error, {name, :not_found}} ->
warn_without_stacktrace("Failed to find the #{name} exectuable on the path")
{:error, []}
end
else
warn_without_stacktrace(
"Expected the #{inspect(protobuf_dir)} directory to contain at least one file, skipping Elixir protoc generation."
)
end
end
def clean do
IO.puts("Cleaned up protoc module generation")
end
defp compile_while_successful(protoc_files, protoc_executable_path)
when is_list(protoc_files) and is_binary(protoc_executable_path) do
Enum.reduce_while(protoc_files, :ok, fn file_path, _acc ->
case System.cmd(protoc_executable_path, ["--elixir_out=./lib/protoc_gen", file_path]) do
{_, 0} ->
{:cont, :ok}
# Usually the message is printed by the command anyway
{_message, nonzero_exit_code} ->
warn_without_stacktrace(
"Failed to generate Elixir protobuf modules for #{file_path} with exit code: #{
nonzero_exit_code
}"
)
{:halt, {:error, []}}
end
end)
end
defp warn_without_stacktrace(message) do
IO.warn(message, [])
end
defp find_executable(name) do
case System.find_executable(name) do
nil -> {:error, {name, :not_found}}
path -> {:ok, path}
end
end
end
It'll exit when the first protobuf file fails to compile, and stop the compilation. You'd register it in your mix.exs like:
def project do
[
# Be sure to add the compiler before the standard Mix compilers
# This ensures that Protobuf-generated code is available before
# the project's Elixir source is compiled.
compilers: [:protoc_gen] ++ Mix.compilers()
# ... Other project configuration
]
end
I've been playing around with trying to make protobuf compilation happen during a project's
mix compile
, and it seems to be pretty feasible. Would this be an appropriate feature for this repo? Would you be up to review a PR to implement this?