[WIP PATCH] Write stubby remote helper for SVN

[WIP PATCH] Write stubby remote helper for SVN

[Ramkumar Ramachandra]

This is a WIP patch based on master, and is part of my GSoC project
git-remote-svn. It is not intended for inclusion- the purpose of the
patch is to get feedback on aspects such as style and overall
design. I plan to put an SVN client, fast-import and fast-export
helpers for SVN inside a directory vcs-svn. Only this file will be in
the root directory.

I've also included a Makefile target git-remote-svn, which should
build just fine, but do nothing except list capabilities.

Ramkumar Ramachandra (1):
  Add skeleton remote helper for SVN

 Makefile     |    6 ++-
 remote-svn.c |  202 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 207 insertions(+), 1 deletions(-)
 create mode 100644 remote-svn.c

[WIP PATCH] Add skeleton remote helper for SVN

[Ramkumar Ramachandra]

Create remote-svn.c, which is essentially a stripped-down version of
remote-curl.c to build the SVN remote helper upon. Also add a Makefile
rule to build it.

Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
---
 Makefile     |    6 ++-
 remote-svn.c |  202 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 207 insertions(+), 1 deletions(-)
 create mode 100644 remote-svn.c

diff --git a/Makefile b/Makefile
index 4f7224a..06b969f 100644
--- a/Makefile
+++ b/Makefile
@@ -1658,7 +1658,7 @@ git.o git.spec \
 
 TEST_OBJS := $(patsubst test-%$X,test-%.o,$(TEST_PROGRAMS))
 GIT_OBJS := $(LIB_OBJS) $(BUILTIN_OBJS) $(PROGRAM_OBJS) $(TEST_OBJS) \
-	git.o http.o http-walker.o remote-curl.o
+	git.o http.o http-walker.o remote-curl.o remote-svn.o
 XDIFF_OBJS = xdiff/xdiffi.o xdiff/xprepare.o xdiff/xutils.o xdiff/xemit.o \
 	xdiff/xmerge.o xdiff/xpatience.o
 OBJECTS := $(GIT_OBJS) $(XDIFF_OBJS)
@@ -1824,6 +1824,10 @@ $(REMOTE_CURL_PRIMARY): remote-curl.o http.o http-walker.o $(GITLIBS)
 	$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) \
 		$(LIBS) $(CURL_LIBCURL) $(EXPAT_LIBEXPAT)
 
+git-remote-svn$X: remote-svn.o $(GITLIBS)
+	$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@	\
+		$(ALL_LDFLAGS) $(filter %.o,$^) $(LIBS)
+
 $(LIB_FILE): $(LIB_OBJS)
 	$(QUIET_AR)$(RM) $@ && $(AR) rcs $@ $(LIB_OBJS)
 
diff --git a/remote-svn.c b/remote-svn.c
new file mode 100644
index 0000000..dafbd6a
--- /dev/null
+++ b/remote-svn.c
@@ -0,0 +1,202 @@
+#include "cache.h"
+#include "strbuf.h"
+#include "remote.h"
+#include "strbuf.h"
+
+static struct remote *remote;
+static const char *url;
+
+struct options {
+	int verbosity;
+	unsigned long depth;
+	unsigned progress : 1,
+		dry_run : 1;
+};
+static struct options options;
+
+static int set_option(const char *name, const char *value)
+{
+	if (!strcmp(name, "verbosity")) {
+		char *end;
+		int v = strtol(value, &end, 10);
+		if (value == end || *end)
+			return -1;
+		options.verbosity = v;
+		return 0;
+	}
+	else if (!strcmp(name, "progress")) {
+		if (!strcmp(value, "true"))
+			options.progress = 1;
+		else if (!strcmp(value, "false"))
+			options.progress = 0;
+		else
+			return -1;
+		return 0;
+	}
+	else if (!strcmp(name, "dry-run")) {
+		if (!strcmp(value, "true"))
+			options.dry_run = 1;
+		else if (!strcmp(value, "false"))
+			options.dry_run = 0;
+		else
+			return -1;
+		return 0;
+	}
+	else {
+		return 1;
+	}
+}
+
+static int export_handler(int nr_spec, char **specs)
+{
+	int err = 0;
+
+	// TODO: The real exporting
+        // TODO: Write an importer for SVN
+	
+	return err;
+}
+
+static int import_handler(int nr_spec, char **specs)
+{
+	int err = 0;
+	
+	// TODO: The real importing
+	// TODO: Hook up an SVN exporter's fast-export stream
+
+	return err;
+}
+
+static void parse_import(struct strbuf *buf)
+{
+	char **specs = NULL;
+	int alloc_spec = 0, nr_spec = 0, i;
+
+	do {
+		if (!prefixcmp(buf->buf, "import ")) {
+			ALLOC_GROW(specs, nr_spec + 1, alloc_spec);
+			specs[nr_spec++] = xstrdup(buf->buf + 5);
+		}
+		else
+			die("remote helper does not support %s", buf->buf);
+
+		strbuf_reset(buf);
+		if (strbuf_getline(buf, stdin, '\n') == EOF)
+			return;
+		if (!*buf->buf)
+			break;
+	} while (1);
+
+	if (import_handler(nr_spec, specs))
+		exit(128); /* error already reported */
+	for (i = 0; i < nr_spec; i++)
+		free(specs[i]);
+	free(specs);
+
+	printf("\n");
+	fflush(stdout);
+}
+
+static void parse_export(struct strbuf *buf)
+{
+	char **specs = NULL;
+	int alloc_spec = 0, nr_spec = 0, i;
+
+	do {
+		if (!prefixcmp(buf->buf, "export ")) {
+			ALLOC_GROW(specs, nr_spec + 1, alloc_spec);
+			specs[nr_spec++] = xstrdup(buf->buf + 5);
+		}
+		else
+			die("remote helper does not support %s", buf->buf);
+
+		strbuf_reset(buf);
+		if (strbuf_getline(buf, stdin, '\n') == EOF)
+			return;
+		if (!*buf->buf)
+			break;
+	} while (1);
+
+	if (export_handler(nr_spec, specs))
+		exit(128); /* error already reported */
+	for (i = 0; i < nr_spec; i++)
+		free(specs[i]);
+	free(specs);
+
+	printf("\n");
+	fflush(stdout);
+}
+
+int main(int argc, const char **argv)
+{
+	struct strbuf buf = STRBUF_INIT;
+	int nongit;
+
+	// git_extract_argv0_path(argv[0]);
+	setup_git_directory_gently(&nongit);
+	if (argc < 2) {
+		fprintf(stderr, "Remote needed\n");
+		return 1;
+	}
+
+	options.verbosity = 1;
+	options.progress = !!isatty(2);
+
+	remote = remote_get(argv[1]);
+
+	if (argc > 2) {
+		url = argv[2];
+	} else {
+		url = remote->url[0];
+	}
+
+	// open_svn_connection(remote);
+	do {
+		if (strbuf_getline(&buf, stdin, '\n') == EOF)
+			break;
+
+		else if (!strcmp(buf.buf, "list")) {
+			// TODO: Code to list refs
+			break;
+
+		} else if (!prefixcmp(buf.buf, "import ")) {
+			if (nongit)
+				die("Import attempted without a local repo");
+
+			parse_import(&buf);
+
+		} else if (!prefixcmp(buf.buf, "export ")) {
+			parse_export(&buf);
+
+		} else if (!prefixcmp(buf.buf, "option ")) {
+			char *name = buf.buf + strlen("option ");
+			char *value = strchr(name, ' ');
+			int result;
+			if (value)
+				*value++ = '\0';
+			else
+				value = "true";
+			result = set_option(name, value);
+			if (!result)
+				printf("ok\n");
+			else if (result < 0)
+				printf("error invalid value\n");
+			else
+				printf("unsupported\n");
+			fflush(stdout);
+
+		} else if (!strcmp(buf.buf, "capabilities")) {
+			printf("option\n");
+			printf("import\n");
+			printf("export\n");
+			printf("\n");
+			fflush(stdout);
+		} else
+			return 1;
+
+		strbuf_reset(&buf);
+	} while (1);
+
+	// close_svn_connection(remote);
+	return 0;
+}
-- 
1.7.1

Re: [WIP PATCH] Add skeleton remote helper for SVN

[Tay Ray Chuan]

Hi,

On Mon, May 10, 2010 at 7:39 PM, Ramkumar Ramachandra
<artagnon@gmail.com> wrote:
> diff --git a/Makefile b/Makefile
> index 4f7224a..06b969f 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -1658,7 +1658,7 @@ git.o git.spec \
>
>  TEST_OBJS := $(patsubst test-%$X,test-%.o,$(TEST_PROGRAMS))
>  GIT_OBJS := $(LIB_OBJS) $(BUILTIN_OBJS) $(PROGRAM_OBJS) $(TEST_OBJS) \
> -       git.o http.o http-walker.o remote-curl.o
> +       git.o http.o http-walker.o remote-curl.o remote-svn.o
>  XDIFF_OBJS = xdiff/xdiffi.o xdiff/xprepare.o xdiff/xutils.o xdiff/xemit.o \
>        xdiff/xmerge.o xdiff/xpatience.o
>  OBJECTS := $(GIT_OBJS) $(XDIFF_OBJS)

Johnathan (Cc'ed) had a recent patch
(<20100509035728.GA8198@progeny.tock>) that changed this area, and I
think it would be good to build on top of that.

> [snip]
> diff --git a/remote-svn.c b/remote-svn.c
> new file mode 100644
> index 0000000..dafbd6a
> --- /dev/null
> +++ b/remote-svn.c
> @@ -0,0 +1,202 @@
> [snip]
> +static int set_option(const char *name, const char *value)
> +{
> +       if (!strcmp(name, "verbosity")) {
> +               char *end;
> +               int v = strtol(value, &end, 10);
> +               if (value == end || *end)
> +                       return -1;
> +               options.verbosity = v;
> +               return 0;
> +       }
> +       else if (!strcmp(name, "progress")) {

We usually put the brace on the same line as the "else":

    if (...) {
      /* stuff */
    } else if (...) {
      /* stuff */
    }

(of course, tabs should be used - gmail doesn't allow me to do it here.)

> [snip]
> +static int export_handler(int nr_spec, char **specs)
> +{
> +       int err = 0;
> +
> +       // TODO: The real exporting
> +        // TODO: Write an importer for SVN

Here and elsewhere - no C++-style comments please.

> [snip]
> +static void parse_import(struct strbuf *buf)
> +{
> +       char **specs = NULL;
> +       int alloc_spec = 0, nr_spec = 0, i;
> +
> +       do {
> +               if (!prefixcmp(buf->buf, "import ")) {
> +                       ALLOC_GROW(specs, nr_spec + 1, alloc_spec);
> +                       specs[nr_spec++] = xstrdup(buf->buf + 5);
> +               }
> +               else
> +                       die("remote helper does not support %s", buf->buf);

See note on else-brace style.

-- 
Cheers,
Ray Chuan

Re: [WIP PATCH] Add skeleton remote helper for SVN

[Ramkumar Ramachandra]

Hi Tay,

Thanks for the style nitpicks.

On Mon, May 10, 2010 at 2:00 PM, Tay Ray Chuan <rctay89@gmail.com> wrote:
> Johnathan (Cc'ed) had a recent patch
> (<20100509035728.GA8198@progeny.tock>) that changed this area, and I
> think it would be good to build on top of that.

This'll probably be ready for inclusion atleast into `pu` in a few
weeks- I'll rebase it on top of the `master` then before sending it
out anyway. It'll be good if Jonathan's patch makes it to `master` by
then, but I don't think I should prematurely rebase on patches that
aren't in `master` yet.

-- Ram

Re: [WIP PATCH] Add skeleton remote helper for SVN

[Jonathan Nieder]

Hi Ram,

Ramkumar Ramachandra wrote:

> It'll be good if Jonathan's patch makes it to `master` by
> then, but I don't think I should prematurely rebase on patches that
> aren't in `master` yet.

Since the conflict resolution is trivial, I agree.  When there is an
actual dependency between patches, I think there is nothing wrong
with basing one patch on another; as long as you make it clear
what your patch applies to, humans and Junio’s scripts can cope with
the task of applying one series on top of the topic branch for
another.

Re your skeleton code, it looks reasonable to me.  I could nitpick the
names of the import_handler() and export_handler() functions and the
needless use of do ... while, but I will refrain. :)

Much more importantly, there is something to play with now.  On that
note, is David’s code out there somewhere for people to fool around
with?  At this point, I am not interested in a stable interface or
permanent history, just a way to try out the code and start to get
used to it.

Thanks,
Jonathan

[1] git://repo.or.cz/svn-all-fast-export.git

Re: [WIP PATCH] Add skeleton remote helper for SVN

[Ramkumar Ramachandra]

Hi Jonathan,

On Mon, May 10, 2010 at 2:59 PM, Jonathan Nieder <jrnieder@gmail.com> wrote:
> Since the conflict resolution is trivial, I agree.  When there is an
> actual dependency between patches, I think there is nothing wrong
> with basing one patch on another; as long as you make it clear
> what your patch applies to, humans and Junio’s scripts can cope with
> the task of applying one series on top of the topic branch for
> another.

Okay, got it.

> Re your skeleton code, it looks reasonable to me.  I could nitpick the
> names of the import_handler() and export_handler() functions and the
> needless use of do ... while, but I will refrain. :)

Yeah, I suppose that can wait for a couple of weeks- atleast until I
have something minimally working :)

> Much more importantly, there is something to play with now.  On that
> note, is David’s code out there somewhere for people to fool around
> with?  At this point, I am not interested in a stable interface or
> permanent history, just a way to try out the code and start to get
> used to it.

Yes. You can see it on David's GitHub [1] and my fork [2]. I send
patches to David by email, so both should be in sync.

-- Ram

[1] http://github.com/barrbrain/svn-dump-fast-export.git
[2] http://github.com/artagnon/svn-dump-fast-export.git

Re: [WIP PATCH] Add skeleton remote helper for SVN

[Jonathan Nieder]

Ramkumar Ramachandra wrote:

> [1] http://github.com/barrbrain/svn-dump-fast-export.git
> [2] http://github.com/artagnon/svn-dump-fast-export.git

>From README.textile:

| h1. A tool to convert a subversion dump for input to git-fast-import
|
| This project is now largely functionally complete. There is still
| quite a bit of testing and code cleanup to be done before the first
| major release.

Awesome. :) Thank you.

Cheers,
Jonathan