[0/8] Simple IPC Cleanups

Message

Philippe Blain via GitGitGadget March 4, 2021, 8:17 p.m. UTC

This patch series adds a few final cleanup and refactoring commits onto the
jh/simple-ipc branch. These are in response to mailing list comments that I
received on my V4 version after it was queued into next.

https://lore.kernel.org/git/[e-mail]/T/#mbd1da5ff93ef273049090f697aeab68c74f698f1

There are a couple of large changes that I'll call out here.

In the third commit, I moved the new lock-aware code out of unix-socket.c
and into its own source file. This creates a slightly misleading diff at
times (in gitk) where it looks like 51% copy of unix-socket.c rather than a
new file. On the command line and on GitHub it looks better.

In the commits prefixed with test-simple-ipc: ... I refactored the options
parsing to allow the name of the socket/named-pipe to be passed on the
command line so that the Windows version could do so (since it needs to exec
a child rather than fork). This turned into a larger cleanup/refactoring
than I had expected, but I think the result is much better. I unified all of
the option parsing into the main cmd__simple_ipc() function and got rid of
the smaller parsers inside of each subcommand. With this, all of the
subcommands now allow an alternate socket path to be used. (Just fixing the
unused arg on the Windows side would allow us to spawn a background daemon
on a different socket, but none of the client subcommands would be able to
talk to it.)

Jeff Hostetler (8):
  pkt-line: remove buffer arg from write_packetized_from_fd_no_flush()
  unix-socket: simplify initialization of unix_stream_listen_opts
  unix-stream-server: create unix-stream-server.c
  simple-ipc: move error handling up a level
  unix-stream-server: add st_dev and st_mode to socket stolen checks
  test-simple-ipc: refactor command line option processing in helper
  test-simple-ipc: add --token=<token> string option
  simple-ipc: update design documentation with more details

 Documentation/technical/api-simple-ipc.txt | 131 +++++++--
 Makefile                                   |   1 +
 compat/simple-ipc/ipc-unix-socket.c        |  49 ++--
 compat/simple-ipc/ipc-win32.c              |  14 +-
 contrib/buildsystems/CMakeLists.txt        |   2 +-
 convert.c                                  |   7 +-
 pkt-line.c                                 |  19 +-
 pkt-line.h                                 |   6 +-
 simple-ipc.h                               |   4 +
 t/helper/test-simple-ipc.c                 | 326 +++++++++++----------
 t/t0052-simple-ipc.sh                      |  10 +-
 unix-socket.c                              | 117 +-------
 unix-socket.h                              |  33 +--
 unix-stream-server.c                       | 130 ++++++++
 unix-stream-server.h                       |  35 +++
 15 files changed, 502 insertions(+), 382 deletions(-)
 create mode 100644 unix-stream-server.c
 create mode 100644 unix-stream-server.h


base-commit: edce16a37ab87513a3f0bc805e9bf372bdd02961
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-893%2Fjeffhostetler%2Fnext-simple-ipc-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-893/jeffhostetler/next-simple-ipc-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/893

Comments

Junio C Hamano March 5, 2021, 12:24 a.m. UTC | #1

sparse failed seen, so I tentatively added this on top of your
series.

Thanks.

 t/helper/test-simple-ipc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/t/helper/test-simple-ipc.c b/t/helper/test-simple-ipc.c
index 4da63fd30c..42040ef81b 100644
--- a/t/helper/test-simple-ipc.c
+++ b/t/helper/test-simple-ipc.c
@@ -227,7 +227,7 @@ struct cl_args
 	char bytevalue;
 };
 
-struct cl_args cl_args = {
+static struct cl_args cl_args = {
 	.subcommand = NULL,
 	.path = "ipc-test",
 	.token = NULL,

Jeff Hostetler March 5, 2021, 9:34 p.m. UTC | #2

On 3/4/21 7:24 PM, Junio C Hamano wrote:
> sparse failed seen, so I tentatively added this on top of your
> series.
> 
> Thanks.
> 
>   t/helper/test-simple-ipc.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/t/helper/test-simple-ipc.c b/t/helper/test-simple-ipc.c
> index 4da63fd30c..42040ef81b 100644
> --- a/t/helper/test-simple-ipc.c
> +++ b/t/helper/test-simple-ipc.c
> @@ -227,7 +227,7 @@ struct cl_args
>   	char bytevalue;
>   };
>   
> -struct cl_args cl_args = {
> +static struct cl_args cl_args = {
>   	.subcommand = NULL,
>   	.path = "ipc-test",
>   	.token = NULL,
> 

Thanks!!

I'll update my copy.

Since things are settling down on the whole simple-ipc series and
we are waiting for the current release cycle to complete before
going any further, I'm going to let this series rest for a bit
in case there are any more comments.

Then I'll combine the 2 parts and rebase/re-roll all of this into
a single series that we can re-eval for "next" post 2.31.

Jeff