# [WIP PATCH] Write stubby remote helper for SVN 7 messages from 2010-05-10 to 2010-05-10. Participants: Ramkumar Ramachandra, Tay Ray Chuan, Jonathan Nieder. Thread: https://gitlist.dev/t/23764 ## Ramkumar Ramachandra, 2010-05-10 11:39 Subject: [WIP PATCH] Write stubby remote helper for SVN Message-ID: <1273491574-31870-1-git-send-email-artagnon@gmail.com> URL: https://gitlist.dev/e/1273491574-31870-1-git-send-email-artagnon%40gmail.com ``` 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 ``` ## Ramkumar Ramachandra, 2010-05-10 11:39 Subject: [WIP PATCH] Add skeleton remote helper for SVN Message-ID: <1273491574-31870-2-git-send-email-artagnon@gmail.com> URL: https://gitlist.dev/e/1273491574-31870-2-git-send-email-artagnon%40gmail.com In-Reply-To: <1273491574-31870-1-git-send-email-artagnon@gmail.com> ``` 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 --- 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 ``` ## Tay Ray Chuan, 2010-05-10 12:00 Subject: Re: [WIP PATCH] Add skeleton remote helper for SVN Message-ID: URL: https://gitlist.dev/e/AANLkTilzn2GLbxz3vLD4SoNIrYDU9_YzFfPI5Wv_WFs2%40mail.gmail.com In-Reply-To: <1273491574-31870-2-git-send-email-artagnon@gmail.com> ``` Hi, On Mon, May 10, 2010 at 7:39 PM, Ramkumar Ramachandra 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 ``` ## Ramkumar Ramachandra, 2010-05-10 12:31 Subject: Re: [WIP PATCH] Add skeleton remote helper for SVN Message-ID: URL: https://gitlist.dev/e/AANLkTikFl25Hay7UEvsz5bsdg8fdT8485SxqgAZXCYqd%40mail.gmail.com In-Reply-To: ``` Hi Tay, Thanks for the style nitpicks. On Mon, May 10, 2010 at 2:00 PM, Tay Ray Chuan 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 ``` ## Jonathan Nieder, 2010-05-10 12:59 Subject: Re: [WIP PATCH] Add skeleton remote helper for SVN Message-ID: <20100510125953.GA5642@progeny.tock> URL: https://gitlist.dev/e/20100510125953.GA5642%40progeny.tock In-Reply-To: ``` 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 ``` ## Ramkumar Ramachandra, 2010-05-10 13:08 Subject: Re: [WIP PATCH] Add skeleton remote helper for SVN Message-ID: URL: https://gitlist.dev/e/AANLkTil8Ezw3rE-q6pQHr4o-aUNXr-HYbFwbX597VlT1%40mail.gmail.com In-Reply-To: <20100510125953.GA5642@progeny.tock> ``` Hi Jonathan, On Mon, May 10, 2010 at 2:59 PM, Jonathan Nieder 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 ``` ## Jonathan Nieder, 2010-05-10 13:21 Subject: Re: [WIP PATCH] Add skeleton remote helper for SVN Message-ID: <20100510132117.GA5746@progeny.tock> URL: https://gitlist.dev/e/20100510132117.GA5746%40progeny.tock In-Reply-To: ``` 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 ```