From b5c4d6530fadfd8ada830b8906c92e99b5f601b3 Mon Sep 17 00:00:00 2001 From: Mario Flach Date: Fri, 15 Dec 2017 20:24:46 +0100 Subject: [PATCH] Refactor GitRekt.WireProtocol See issues #1 and #8 fore more details. --- apps/gitgud/lib/gitgud/ssh_server.ex | 9 ++- apps/gitrekt/lib/gitrekt/packfile.ex | 33 +++++--- apps/gitrekt/lib/gitrekt/wire_protocol.ex | 4 +- .../lib/gitrekt/wire_protocol/receive_pack.ex | 25 ++++-- .../lib/gitrekt/wire_protocol/service.ex | 2 +- .../lib/gitrekt/wire_protocol/upload_pack.ex | 80 ++++++++++++++++--- 6 files changed, 124 insertions(+), 29 deletions(-) diff --git a/apps/gitgud/lib/gitgud/ssh_server.ex b/apps/gitgud/lib/gitgud/ssh_server.ex index 568c3a5b..71c40453 100644 --- a/apps/gitgud/lib/gitgud/ssh_server.ex +++ b/apps/gitgud/lib/gitgud/ssh_server.ex @@ -89,8 +89,13 @@ defmodule GitGud.SSHServer do @impl true def handle_ssh_msg({:ssh_cm, conn, {:data, chan, _type, data}}, %__MODULE__{conn: conn, chan: chan, exec: exec} = state) do {exec, io} = Service.next(exec, data) - if io, do: :ssh_connection.send(conn, chan, io) - if Service.done?(exec), do: :ssh_connection.close(conn, chan) + :ssh_connection.send(conn, chan, io) + if Service.done?(exec) do + :ssh_connection.send_eof(conn, chan) + :ssh_connection.exit_status(conn, chan, 0) + :ssh_connection.close(conn, chan) + end + {:ok, %{state|exec: exec}} end diff --git a/apps/gitrekt/lib/gitrekt/packfile.ex b/apps/gitrekt/lib/gitrekt/packfile.ex index 5a8aa1cc..c5d10cc1 100644 --- a/apps/gitrekt/lib/gitrekt/packfile.ex +++ b/apps/gitrekt/lib/gitrekt/packfile.ex @@ -13,11 +13,11 @@ defmodule GitRekt.Packfile do @doc """ Returns a *PACK* file for the given `oids` list. """ - @spec create(Git.repo, [Git.oid]) :: binary + @spec create(Git.repo, [Git.oid|{Git.oid, boolean}]) :: binary def create(repo, oids) when is_list(oids) do with {:ok, pack} <- Git.pack_new(repo), {:ok, walk} <- Git.revwalk_new(repo), - :ok <- pack_insert(pack, walk, oids), + :ok <- pack_insert(pack, walk, oid_mask(oids)), {:ok, data} <- Git.pack_data(pack), do: data end @@ -32,15 +32,30 @@ defmodule GitRekt.Packfile do # Helpers # - defp unpack(2 = _version, count, data) do - unpack_obj_next(0, count, data, []) + defp pack_insert(pack, walk, oids) do + case walk_insert(walk, oids) do + :ok -> Git.pack_insert_walk(pack, walk) + {:error, reason} -> {:error, reason} + end + end + + defp walk_insert(_walk, []), do: :ok + defp walk_insert(walk, [{oid, hide}|oids]) do + case Git.revwalk_push(walk, oid, hide) do + :ok -> walk_insert(walk, oids) + {:error, reason} -> {:error, reason} + end + end + + defp oid_mask(oids) do + Enum.map(oids, fn + {oid, hidden} when is_binary(oid) -> {oid, hidden} + oid when is_binary(oid) -> {oid, false} + end) end - defp pack_insert(_pack, _walk, []), do: :ok - defp pack_insert(pack, walk, [oid|oids]) do - with :ok <- Git.revwalk_push(walk, oid), - :ok <- Git.pack_insert_walk(pack, walk), - :ok <- Git.revwalk_reset(walk), do: pack_insert(pack, walk, oids) + defp unpack(2 = _version, count, data) do + unpack_obj_next(0, count, data, []) end defp unpack_obj_next(i, max, rest, acc) when i < max do diff --git a/apps/gitrekt/lib/gitrekt/wire_protocol.ex b/apps/gitrekt/lib/gitrekt/wire_protocol.ex index 32d0d8fe..27c03080 100644 --- a/apps/gitrekt/lib/gitrekt/wire_protocol.ex +++ b/apps/gitrekt/lib/gitrekt/wire_protocol.ex @@ -6,7 +6,7 @@ defmodule GitRekt.WireProtocol do alias GitRekt.Git alias GitRekt.Packfile - @upload_caps ~w() + @upload_caps ~w(multi_ack multi_ack_detailed) @receive_caps ~w(report-status delete-refs) @doc """ @@ -43,6 +43,8 @@ defmodule GitRekt.WireProtocol do @spec pkt_line(binary|:flush) :: binary def pkt_line(data \\ :flush) def pkt_line(:flush), do: "0000" + def pkt_line({:ack, oid}), do: pkt_line("ACK #{Git.oid_fmt(oid)}") + def pkt_line({:ack, oid, status}), do: pkt_line("ACK #{Git.oid_fmt(oid)} #{status}") def pkt_line(:nak), do: pkt_line("NAK") def pkt_line(<<"PACK", _rest::binary>> = pack), do: pack def pkt_line(data) when is_binary(data) do diff --git a/apps/gitrekt/lib/gitrekt/wire_protocol/receive_pack.ex b/apps/gitrekt/lib/gitrekt/wire_protocol/receive_pack.ex index 2c07822f..6393997c 100644 --- a/apps/gitrekt/lib/gitrekt/wire_protocol/receive_pack.ex +++ b/apps/gitrekt/lib/gitrekt/wire_protocol/receive_pack.ex @@ -11,6 +11,8 @@ defmodule GitRekt.WireProtocol.ReceivePack do defstruct [:repo, state: :disco, caps: [], cmds: [], pack: []] + @null_oid String.duplicate("0", 40) + @type cmd :: { :create | :update | :delete, Git.oid, @@ -73,23 +75,26 @@ defmodule GitRekt.WireProtocol.ReceivePack do def run(%__MODULE__{state: :pack} = handle) do :ok = apply_pack(handle.repo, handle.pack) :ok = apply_cmds(handle.repo, handle.cmds) - handle = struct(handle, state: :done) - if "report-status" in handle.caps, - do: {handle, report_status(handle.cmds)}, - else: {handle, []} + run(struct(handle, state: :done)) end @impl true - def run(%__MODULE__{state: :done} = handle) do + def run(%__MODULE__{state: :done, cmds: []} = handle) do {handle, []} end + def run(%__MODULE__{state: :done} = handle) do + if "report-status" in handle.caps, + do: {handle, report_status(handle.cmds)}, + else: {handle, []} + end + # # Helpers # defp report_status(cmds) do - List.flatten(["unpack ok", Enum.map(cmds, &"ok #{elem(&1, 3)}"), :flush]) + List.flatten(["unpack ok", Enum.map(cmds, &"ok #{elem(&1, :erlang.tuple_size(&1)-1)}"), :flush]) end defp obj_match?({type, _oid}, type), do: true @@ -97,7 +102,11 @@ defmodule GitRekt.WireProtocol.ReceivePack do defp parse_cmds(cmds) do Enum.map(cmds, fn cmd -> - case String.split(cmd) do + case String.split(cmd, " ", parts: 3) do + [@null_oid, new, name] -> + {:create, Git.oid_parse(new), name} + [old, @null_oid, name] -> + {:delete, Git.oid_parse(old), name} [old, new, name] -> {:update, Git.oid_parse(old), Git.oid_parse(new), name} end @@ -114,7 +123,9 @@ defmodule GitRekt.WireProtocol.ReceivePack do defp apply_cmds(repo, cmds) do Enum.each(cmds, fn + {:create, new_oid, refname} -> Git.reference_create(repo, refname, :oid, new_oid, false) {:update, _old_oid, new_oid, refname} -> Git.reference_create(repo, refname, :oid, new_oid, true) + # {:delete, old_oid, refname} -> Git.reference_delete(repo, refname, :oid, old_oid) end) end diff --git a/apps/gitrekt/lib/gitrekt/wire_protocol/service.ex b/apps/gitrekt/lib/gitrekt/wire_protocol/service.ex index 9ddc70bd..1b06be4d 100644 --- a/apps/gitrekt/lib/gitrekt/wire_protocol/service.ex +++ b/apps/gitrekt/lib/gitrekt/wire_protocol/service.ex @@ -39,7 +39,7 @@ defmodule GitRekt.WireProtocol.Service do @spec flush(Module.t) :: {Module.t, iolist} def flush(service) do case apply(service.__struct__, :run, [service]) do - {handle, []} -> {handle, nil} + {handle, []} -> {handle, []} {handle, io} -> {handle, encode(io)} end end diff --git a/apps/gitrekt/lib/gitrekt/wire_protocol/upload_pack.ex b/apps/gitrekt/lib/gitrekt/wire_protocol/upload_pack.ex index 2aa80301..740aab9d 100644 --- a/apps/gitrekt/lib/gitrekt/wire_protocol/upload_pack.ex +++ b/apps/gitrekt/lib/gitrekt/wire_protocol/upload_pack.ex @@ -39,14 +39,21 @@ defmodule GitRekt.WireProtocol.UploadPack do end @impl true - def next(%__MODULE__{state: :upload_wants} = handle, lines) do - {haves, lines} = Enum.split_while(lines, &obj_match?(&1, :have)) - {struct(handle, state: :upload_haves, haves: parse_cmds(haves)), lines} + def next(%__MODULE__{state: :upload_wants} = handle, [:done]) do + {struct(handle, state: :done), []} end @impl true - def next(%__MODULE__{state: :upload_haves} = handle, [:done]) do - {handle, []} + def next(%__MODULE__{state: :upload_wants} = handle, lines) do + {:ok, odb} = Git.repository_get_odb(handle.repo) + {haves, lines} = Enum.split_while(lines, &obj_match?(&1, :have)) + acks = Enum.filter(parse_cmds(haves), &Git.odb_object_exists?(odb, &1)) + case lines do + [:flush|lines] -> + {struct(handle, haves: [acks|handle.haves]), lines} + [:done|lines] -> + {struct(handle, state: :upload_haves, haves: [acks|handle.haves]), lines} + end end @impl true @@ -61,20 +68,61 @@ defmodule GitRekt.WireProtocol.UploadPack do end @impl true - def run(%__MODULE__{state: :upload_wants} = handle) do + def run(%__MODULE__{state: :upload_wants, haves: []} = handle) do {handle, []} end + @impl true + def run(%__MODULE__{state: :upload_wants} = handle) do + acks = ack_haves(handle.haves, handle.caps) + cond do + "multi_ack_detailed" in handle.caps -> + {handle, acks ++ [:nak]} + "multi_ack" in handle.caps -> + {handle, acks ++ [:nak]} + Enum.empty?(handle.haves) -> + {handle, [:nak]} + Enum.empty?(acks) -> + {handle, []} + true -> + {handle, [List.first(acks)]} + end + end + @impl true def run(%__MODULE__{state: :upload_haves} = handle) do - {struct(handle, state: :done), [:nak, create(handle.repo, handle.wants)]} + {next, lines} = run(struct(handle, state: :done)) + cond do + "multi_ack_detailed" in handle.caps -> + {next, ack_haves(handle.haves, handle.caps) ++ lines} + "multi_ack" in handle.caps -> + {next, ack_haves(handle.haves, handle.caps) ++ lines} + true -> + {next, lines} + end end @impl true - def run(%__MODULE__{state: :done} = handle) do + def run(%__MODULE__{state: :done, wants: [], haves: []} = handle) do {handle, []} end + def run(%__MODULE__{state: :done, haves: []} = handle) do + {handle, [:nak, create(handle.repo, handle.wants)]} + end + + def run(%__MODULE__{state: :done} = handle) do + [have|_] = List.flatten(Enum.reverse(handle.haves)) # TODO + cond do + "multi_ack_detailed" in handle.caps -> + {handle, [{:ack, have}, create(handle.repo, handle.wants ++ [{have, true}])]} + "multi_ack" in handle.caps -> + {handle, [{:ack, have}, create(handle.repo, handle.wants ++ [{have, true}])]} + true -> + {handle, [create(handle.repo, handle.wants ++ [{have, true}])]} + end + end + # # Helpers # @@ -86,9 +134,23 @@ defmodule GitRekt.WireProtocol.UploadPack do defp parse_caps([]), do: {[], []} defp parse_caps([{obj_type, first_ref}|wants]) do - case String.split(first_ref, "\0", parts: 2) do + case String.split(first_ref, " ", parts: 2) do [first_ref] -> {[], [{obj_type, first_ref}|wants]} [first_ref, caps] -> {String.split(caps, " ", trim: true), [{obj_type, first_ref}|wants]} end end + + defp ack_haves([], _caps), do: [] + defp ack_haves([haves|_], caps) do + cond do + "multi_ack_detailed" in caps -> + Enum.map(haves, &{:ack, &1, :common}) + "multi_ack" in caps -> + Enum.map(haves, &{:ack, &1, :continue}) + [have|_] = haves -> + [{:ack, have}] + true -> + [] + end + end end