[PATCH v7 00/16] GSOC remote-svn

[PATCH v7 00/16] GSOC remote-svn

[Florian Achleitner]

Reroll includes fixups by Ramsey. Thanks!
Diff:
- Add missing dependency to rule in Makefile.
- improve compatibility of integer types.
- t9020-*.sh: remove excess slash in urls that makes python on windows 
  interpret it as a network path.
- t9020-*.sh: skip if python isn't available.
- replace getline() in remote-testsvn.c. There are platforms that don't provide
  this function.

[PATCH v7 01/16] Implement a remote helper for svn in C
[PATCH v7 02/16] Add git-remote-testsvn to Makefile
[PATCH v7 03/16] Add svndump_init_fd to allow reading dumps from
[PATCH v7 04/16] Add argv_array_detach and argv_array_free_detached
[PATCH v7 05/16] Connect fast-import to the remote-helper via pipe,
[PATCH v7 06/16] Add documentation for the 'bidi-import' capability
[PATCH v7 07/16] When debug==1, start fast-import with "--stats"
[PATCH v7 08/16] remote-svn, vcs-svn: Enable fetching to private
[PATCH v7 09/16] Allow reading svn dumps from files via file:// urls
[PATCH v7 10/16] vcs-svn: add fast_export_note to create notes
[PATCH v7 11/16] Create a note for every imported commit containing
[PATCH v7 12/16] remote-svn: Activate import/export-marks for
[PATCH v7 13/16] remote-svn: add incremental import
[PATCH v7 14/16] Add a svnrdump-simulator replaying a dump file for
[PATCH v7 15/16] remote-svn: add marks-file regeneration
[PATCH v7 16/16] Add a test script for remote-svn

[PATCH v7 01/16] Implement a remote helper for svn in C

[Florian Achleitner]

Enable basic fetching from subversion repositories. When processing
remote URLs starting with testsvn::, git invokes this remote-helper.
It starts svnrdump to extract revisions from the subversion repository
in the 'dump file format', and converts them to a git-fast-import stream
using the functions of vcs-svn/.

Imported refs are created in a private namespace at
refs/svn/<remote-name>/master.  The revision history is imported
linearly (no branch detection) and completely, i.e. from revision 0 to
HEAD.

The 'bidi-import' capability is used. The remote-helper expects data
from fast-import on its stdin. It buffers a batch of 'import' command
lines in a string_list before starting to process them.

Signed-off-by: Florian Achleitner <florian.achleitner.2.6.31@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 remote-testsvn.c |  174 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 174 insertions(+)
 create mode 100644 remote-testsvn.c

diff --git a/remote-testsvn.c b/remote-testsvn.c
new file mode 100644
index 0000000..ebe803b
--- /dev/null
+++ b/remote-testsvn.c
@@ -0,0 +1,174 @@
+#include "cache.h"
+#include "remote.h"
+#include "strbuf.h"
+#include "url.h"
+#include "exec_cmd.h"
+#include "run-command.h"
+#include "vcs-svn/svndump.h"
+#include "notes.h"
+#include "argv-array.h"
+
+static const char *url;
+static const char *private_ref;
+static const char *remote_ref = "refs/heads/master";
+
+static int cmd_capabilities(const char *line);
+static int cmd_import(const char *line);
+static int cmd_list(const char *line);
+
+typedef int (*input_command_handler)(const char *);
+struct input_command_entry {
+	const char *name;
+	input_command_handler fn;
+	unsigned char batchable;	/* whether the command starts or is part of a batch */
+};
+
+static const struct input_command_entry input_command_list[] = {
+	{ "capabilities", cmd_capabilities, 0 },
+	{ "import", cmd_import, 1 },
+	{ "list", cmd_list, 0 },
+	{ NULL, NULL }
+};
+
+static int cmd_capabilities(const char *line) {
+	printf("import\n");
+	printf("bidi-import\n");
+	printf("refspec %s:%s\n\n", remote_ref, private_ref);
+	fflush(stdout);
+	return 0;
+}
+
+static void terminate_batch(void)
+{
+	/* terminate a current batch's fast-import stream */
+	printf("done\n");
+	fflush(stdout);
+}
+
+static int cmd_import(const char *line)
+{
+	int code;
+	int dumpin_fd;
+	unsigned int startrev = 0;
+	struct argv_array svndump_argv = ARGV_ARRAY_INIT;
+	struct child_process svndump_proc;
+
+	memset(&svndump_proc, 0, sizeof(struct child_process));
+	svndump_proc.out = -1;
+	argv_array_push(&svndump_argv, "svnrdump");
+	argv_array_push(&svndump_argv, "dump");
+	argv_array_push(&svndump_argv, url);
+	argv_array_pushf(&svndump_argv, "-r%u:HEAD", startrev);
+	svndump_proc.argv = svndump_argv.argv;
+
+	code = start_command(&svndump_proc);
+	if (code)
+		die("Unable to start %s, code %d", svndump_proc.argv[0], code);
+	dumpin_fd = svndump_proc.out;
+
+	svndump_init_fd(dumpin_fd, STDIN_FILENO);
+	svndump_read(url, private_ref);
+	svndump_deinit();
+	svndump_reset();
+
+	close(dumpin_fd);
+	code = finish_command(&svndump_proc);
+	if (code)
+		warning("%s, returned %d", svndump_proc.argv[0], code);
+	argv_array_clear(&svndump_argv);
+
+	return 0;
+}
+
+static int cmd_list(const char *line)
+{
+	printf("? %s\n\n", remote_ref);
+	fflush(stdout);
+	return 0;
+}
+
+static int do_command(struct strbuf *line)
+{
+	const struct input_command_entry *p = input_command_list;
+	static struct string_list batchlines = STRING_LIST_INIT_DUP;
+	static const struct input_command_entry *batch_cmd;
+	/*
+	 * commands can be grouped together in a batch.
+	 * Batches are ended by \n. If no batch is active the program ends.
+	 * During a batch all lines are buffered and passed to the handler function
+	 * when the batch is terminated.
+	 */
+	if (line->len == 0) {
+		if (batch_cmd) {
+			struct string_list_item *item;
+			for_each_string_list_item(item, &batchlines)
+				batch_cmd->fn(item->string);
+			terminate_batch();
+			batch_cmd = NULL;
+			string_list_clear(&batchlines, 0);
+			return 0;	/* end of the batch, continue reading other commands. */
+		}
+		return 1;	/* end of command stream, quit */
+	}
+	if (batch_cmd) {
+		if (prefixcmp(batch_cmd->name, line->buf))
+			die("Active %s batch interrupted by %s", batch_cmd->name, line->buf);
+		/* buffer batch lines */
+		string_list_append(&batchlines, line->buf);
+		return 0;
+	}
+
+	for (p = input_command_list; p->name; p++) {
+		if (!prefixcmp(line->buf, p->name) && (strlen(p->name) == line->len ||
+				line->buf[strlen(p->name)] == ' ')) {
+			if (p->batchable) {
+				batch_cmd = p;
+				string_list_append(&batchlines, line->buf);
+				return 0;
+			}
+			return p->fn(line->buf);
+		}
+	}
+	die("Unknown command '%s'\n", line->buf);
+	return 0;
+}
+
+int main(int argc, const char **argv)
+{
+	struct strbuf buf = STRBUF_INIT;
+	static struct remote *remote;
+	const char *url_in;
+
+	git_extract_argv0_path(argv[0]);
+	setup_git_directory();
+	if (argc < 2 || argc > 3) {
+		usage("git-remote-svn <remote-name> [<url>]");
+		return 1;
+	}
+
+	remote = remote_get(argv[1]);
+	url_in = (argc == 3) ? argv[2] : remote->url[0];
+
+	end_url_with_slash(&buf, url_in);
+	url = strbuf_detach(&buf, NULL);
+
+	strbuf_addf(&buf, "refs/svn/%s/master", remote->name);
+	private_ref = strbuf_detach(&buf, NULL);
+
+	while(1) {
+		if (strbuf_getline(&buf, stdin, '\n') == EOF) {
+			if (ferror(stdin))
+				die("Error reading command stream");
+			else
+				die("Unexpected end of command stream");
+		}
+		if (do_command(&buf))
+			break;
+		strbuf_reset(&buf);
+	}
+
+	strbuf_release(&buf);
+	free((void*)url);
+	free((void*)private_ref);
+	return 0;
+}
-- 
1.7.9.5

[PATCH v7 02/16] Add git-remote-testsvn to Makefile

[Florian Achleitner]

The link-rule is a copy of the standard git$X rule but adds VCSSVN_LIB.

Signed-off-by: Florian Achleitner <florian.achleitner.2.6.31@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 Makefile |    5 +++++
 1 file changed, 5 insertions(+)

diff --git a/Makefile b/Makefile
index 66e8216..1b09454 100644
--- a/Makefile
+++ b/Makefile
@@ -477,6 +477,7 @@ PROGRAM_OBJS += sh-i18n--envsubst.o
 PROGRAM_OBJS += shell.o
 PROGRAM_OBJS += show-index.o
 PROGRAM_OBJS += upload-pack.o
+PROGRAM_OBJS += remote-testsvn.o
 
 # Binary suffix, set to .exe for Windows builds
 X =
@@ -2352,6 +2353,10 @@ git-http-push$X: revision.o http.o http-push.o GIT-LDFLAGS $(GITLIBS)
 	$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) \
 		$(LIBS) $(CURL_LIBCURL) $(EXPAT_LIBEXPAT)
 
+git-remote-testsvn$X: remote-testsvn.o GIT-LDFLAGS $(GITLIBS) $(VCSSVN_LIB)
+	$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) $(LIBS) \
+	$(VCSSVN_LIB)
+
 $(REMOTE_CURL_ALIASES): $(REMOTE_CURL_PRIMARY)
 	$(QUIET_LNCP)$(RM) $@ && \
 	ln $< $@ 2>/dev/null || \
-- 
1.7.9.5

[PATCH v7 03/16] Add svndump_init_fd to allow reading dumps from arbitrary FDs

[Florian Achleitner]

The existing function only allows reading from a filename or from
stdin. Allow passing of a FD and an additional FD for the back report
pipe. This allows us to retrieve the name of the pipe in the caller.

Signed-off-by: Florian Achleitner <florian.achleitner.2.6.31@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 vcs-svn/svndump.c |   22 ++++++++++++++++++----
 vcs-svn/svndump.h |    1 +
 2 files changed, 19 insertions(+), 4 deletions(-)

diff --git a/vcs-svn/svndump.c b/vcs-svn/svndump.c
index 2b168ae..d81a078 100644
--- a/vcs-svn/svndump.c
+++ b/vcs-svn/svndump.c
@@ -468,11 +468,9 @@ void svndump_read(const char *url)
 		end_revision();
 }
 
-int svndump_init(const char *filename)
+static void init(int report_fd)
 {
-	if (buffer_init(&input, filename))
-		return error("cannot open %s: %s", filename, strerror(errno));
-	fast_export_init(REPORT_FILENO);
+	fast_export_init(report_fd);
 	strbuf_init(&dump_ctx.uuid, 4096);
 	strbuf_init(&dump_ctx.url, 4096);
 	strbuf_init(&rev_ctx.log, 4096);
@@ -482,6 +480,22 @@ int svndump_init(const char *filename)
 	reset_dump_ctx(NULL);
 	reset_rev_ctx(0);
 	reset_node_ctx(NULL);
+	return;
+}
+
+int svndump_init(const char *filename)
+{
+	if (buffer_init(&input, filename))
+		return error("cannot open %s: %s", filename ? filename : "NULL", strerror(errno));
+	init(REPORT_FILENO);
+	return 0;
+}
+
+int svndump_init_fd(int in_fd, int back_fd)
+{
+	if(buffer_fdinit(&input, xdup(in_fd)))
+		return error("cannot open fd %d: %s", in_fd, strerror(errno));
+	init(xdup(back_fd));
 	return 0;
 }
 
diff --git a/vcs-svn/svndump.h b/vcs-svn/svndump.h
index df9ceb0..acb5b47 100644
--- a/vcs-svn/svndump.h
+++ b/vcs-svn/svndump.h
@@ -2,6 +2,7 @@
 #define SVNDUMP_H_
 
 int svndump_init(const char *filename);
+int svndump_init_fd(int in_fd, int back_fd);
 void svndump_read(const char *url);
 void svndump_deinit(void);
 void svndump_reset(void);
-- 
1.7.9.5

[PATCH v7 04/16] Add argv_array_detach and argv_array_free_detached

[Florian Achleitner]

Allow detaching of ownership of the argv_array's contents and add a
function to free those detached argv_arrays later.

This makes it possible to use argv_array efficiently with the exiting
struct child_process which only contains a member char **argv.

Add to documentation.

Signed-off-by: Florian Achleitner <florian.achleitner.2.6.31@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 Documentation/technical/api-argv-array.txt |    8 ++++++++
 argv-array.c                               |   20 ++++++++++++++++++++
 argv-array.h                               |    2 ++
 3 files changed, 30 insertions(+)

diff --git a/Documentation/technical/api-argv-array.txt b/Documentation/technical/api-argv-array.txt
index 1b7d8f1..6b97d6d 100644
--- a/Documentation/technical/api-argv-array.txt
+++ b/Documentation/technical/api-argv-array.txt
@@ -49,3 +49,11 @@ Functions
 `argv_array_clear`::
 	Free all memory associated with the array and return it to the
 	initial, empty state.
+
+`argv_array_detach`::
+	Detach the argv array from the `struct argv_array`, transfering
+	ownership of the allocated array and strings.
+
+`argv_array_free_detached`::
+	Free the memory allocated by a `struct argv_array` that was later
+	detached and is now no longer needed.
diff --git a/argv-array.c b/argv-array.c
index 0b5f889..aab50d6 100644
--- a/argv-array.c
+++ b/argv-array.c
@@ -59,3 +59,23 @@ void argv_array_clear(struct argv_array *array)
 	}
 	argv_array_init(array);
 }
+
+const char **argv_array_detach(struct argv_array *array, int *argc)
+{
+	const char **argv =
+		array->argv == empty_argv || array->argc == 0 ? NULL : array->argv;
+	if (argc)
+		*argc = array->argc;
+	argv_array_init(array);
+	return argv;
+}
+
+void argv_array_free_detached(const char **argv)
+{
+	if (argv) {
+		int i;
+		for (i = 0; argv[i]; i++)
+			free((char **)argv[i]);
+		free(argv);
+	}
+}
diff --git a/argv-array.h b/argv-array.h
index b93a69c..b3ef351 100644
--- a/argv-array.h
+++ b/argv-array.h
@@ -17,5 +17,7 @@ __attribute__((format (printf,2,3)))
 void argv_array_pushf(struct argv_array *, const char *fmt, ...);
 void argv_array_pushl(struct argv_array *, ...);
 void argv_array_clear(struct argv_array *);
+const char **argv_array_detach(struct argv_array *array, int *argc);
+void argv_array_free_detached(const char **argv);
 
 #endif /* ARGV_ARRAY_H */
-- 
1.7.9.5

[PATCH v7 05/16] Connect fast-import to the remote-helper via pipe, adding 'bidi-import' capability

[Florian Achleitner]

The fast-import commands 'cat-blob' and 'ls' can be used by
remote-helpers to retrieve information about blobs and trees that
already exist in fast-import's memory. This requires a channel from
fast-import to the remote-helper.

remote-helpers that use these features shall advertise the new
'bidi-import' capability to signal that they require the communication
channel.  When forking fast-import in transport-helper.c connect it to
a dup of the remote-helper's stdin-pipe. The additional file
descriptor is passed to fast-import via its command line
(--cat-blob-fd).  It follows that git and fast-import are connected to
the remote-helpers's stdin.

Because git can send multiple commands to the remote-helper on it's
stdin, it is required that helpers that advertise 'bidi-import' buffer
all input commands until the batch of 'import' commands is ended by a
newline before sending data to fast-import.  This is to prevent mixing
commands and fast-import responses on the helper's stdin.

Signed-off-by: Florian Achleitner <florian.achleitner.2.6.31@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 transport-helper.c |   44 +++++++++++++++++++++++++++++++-------------
 1 file changed, 31 insertions(+), 13 deletions(-)

diff --git a/transport-helper.c b/transport-helper.c
index cfe0988..3523f1f 100644
--- a/transport-helper.c
+++ b/transport-helper.c
@@ -10,6 +10,7 @@
 #include "string-list.h"
 #include "thread-utils.h"
 #include "sigchain.h"
+#include "argv-array.h"
 
 static int debug;
 
@@ -19,6 +20,7 @@ struct helper_data {
 	FILE *out;
 	unsigned fetch : 1,
 		import : 1,
+		bidi_import : 1,
 		export : 1,
 		option : 1,
 		push : 1,
@@ -101,6 +103,7 @@ static void do_take_over(struct transport *transport)
 static struct child_process *get_helper(struct transport *transport)
 {
 	struct helper_data *data = transport->data;
+	struct argv_array argv = ARGV_ARRAY_INIT;
 	struct strbuf buf = STRBUF_INIT;
 	struct child_process *helper;
 	const char **refspecs = NULL;
@@ -122,11 +125,10 @@ static struct child_process *get_helper(struct transport *transport)
 	helper->in = -1;
 	helper->out = -1;
 	helper->err = 0;
-	helper->argv = xcalloc(4, sizeof(*helper->argv));
-	strbuf_addf(&buf, "git-remote-%s", data->name);
-	helper->argv[0] = strbuf_detach(&buf, NULL);
-	helper->argv[1] = transport->remote->name;
-	helper->argv[2] = remove_ext_force(transport->url);
+	argv_array_pushf(&argv, "git-remote-%s", data->name);
+	argv_array_push(&argv, transport->remote->name);
+	argv_array_push(&argv, remove_ext_force(transport->url));
+	helper->argv = argv_array_detach(&argv, NULL);
 	helper->git_cmd = 0;
 	helper->silent_exec_failure = 1;
 
@@ -178,6 +180,8 @@ static struct child_process *get_helper(struct transport *transport)
 			data->push = 1;
 		else if (!strcmp(capname, "import"))
 			data->import = 1;
+		else if (!strcmp(capname, "bidi-import"))
+			data->bidi_import = 1;
 		else if (!strcmp(capname, "export"))
 			data->export = 1;
 		else if (!data->refspecs && !prefixcmp(capname, "refspec ")) {
@@ -241,8 +245,7 @@ static int disconnect_helper(struct transport *transport)
 		close(data->helper->out);
 		fclose(data->out);
 		res = finish_command(data->helper);
-		free((char *)data->helper->argv[0]);
-		free(data->helper->argv);
+		argv_array_free_detached(data->helper->argv);
 		free(data->helper);
 		data->helper = NULL;
 	}
@@ -376,14 +379,23 @@ static int fetch_with_fetch(struct transport *transport,
 static int get_importer(struct transport *transport, struct child_process *fastimport)
 {
 	struct child_process *helper = get_helper(transport);
+	struct helper_data *data = transport->data;
+	struct argv_array argv = ARGV_ARRAY_INIT;
+	int cat_blob_fd, code;
 	memset(fastimport, 0, sizeof(*fastimport));
 	fastimport->in = helper->out;
-	fastimport->argv = xcalloc(5, sizeof(*fastimport->argv));
-	fastimport->argv[0] = "fast-import";
-	fastimport->argv[1] = "--quiet";
+	argv_array_push(&argv, "fast-import");
+	argv_array_push(&argv, "--quiet");
 
+	if (data->bidi_import) {
+		cat_blob_fd = xdup(helper->in);
+		argv_array_pushf(&argv, "--cat-blob-fd=%d", cat_blob_fd);
+	}
+	fastimport->argv = argv.argv;
 	fastimport->git_cmd = 1;
-	return start_command(fastimport);
+
+	code = start_command(fastimport);
+	return code;
 }
 
 static int get_exporter(struct transport *transport,
@@ -438,11 +450,17 @@ static int fetch_with_import(struct transport *transport,
 	}
 
 	write_constant(data->helper->in, "\n");
+	/*
+	 * remote-helpers that advertise the bidi-import capability are required to
+	 * buffer the complete batch of import commands until this newline before
+	 * sending data to fast-import.
+	 * These helpers read back data from fast-import on their stdin, which could
+	 * be mixed with import commands, otherwise.
+	 */
 
 	if (finish_command(&fastimport))
 		die("Error while running fast-import");
-	free(fastimport.argv);
-	fastimport.argv = NULL;
+	argv_array_free_detached(fastimport.argv);
 
 	/*
 	 * The fast-import stream of a remote helper that advertises
-- 
1.7.9.5

[PATCH v7 06/16] Add documentation for the 'bidi-import' capability of remote-helpers

[Florian Achleitner]

Signed-off-by: Florian Achleitner <florian.achleitner.2.6.31@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 Documentation/git-remote-helpers.txt |   21 ++++++++++++++++++++-
 1 file changed, 20 insertions(+), 1 deletion(-)

diff --git a/Documentation/git-remote-helpers.txt b/Documentation/git-remote-helpers.txt
index f5836e4..5ce4cda 100644
--- a/Documentation/git-remote-helpers.txt
+++ b/Documentation/git-remote-helpers.txt
@@ -98,6 +98,20 @@ advertised with this capability must cover all refs reported by
 the list command.  If no 'refspec' capability is advertised,
 there is an implied `refspec *:*`.
 
+'bidi-import'::
+	The fast-import commands 'cat-blob' and 'ls' can be used by remote-helpers
+	to retrieve information about blobs and trees that already exist in
+	fast-import's memory. This requires a channel from fast-import to the
+	remote-helper.
+	If it is advertised in addition to "import", git establishes a pipe from
+	fast-import to the remote-helper's stdin.
+	It follows that git and fast-import are both connected to the
+	remote-helper's stdin. Because git can send multiple commands to
+	the remote-helper it is required that helpers that use 'bidi-import'
+	buffer all 'import' commands of a batch before sending data to fast-import.
+	This is to prevent mixing commands and fast-import responses on the
+	helper's stdin.
+
 Capabilities for Pushing
 ~~~~~~~~~~~~~~~~~~~~~~~~
 'connect'::
@@ -286,7 +300,12 @@ terminated with a blank line. For each batch of 'import', the remote
 helper should produce a fast-import stream terminated by a 'done'
 command.
 +
-Supported if the helper has the "import" capability.
+Note that if the 'bidi-import' capability is used the complete batch
+sequence has to be buffered before starting to send data to fast-import
+to prevent mixing of commands and fast-import responses on the helper's
+stdin.
++
+Supported if the helper has the 'import' capability.
 
 'connect' <service>::
 	Connects to given service. Standard input and standard output
-- 
1.7.9.5

[PATCH v7 07/16] When debug==1, start fast-import with "--stats" instead of "--quiet"

[Florian Achleitner]

fast-import prints statistics that could be interesting to the
developer of remote helpers.

Signed-off-by: Florian Achleitner <florian.achleitner.2.6.31@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 transport-helper.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/transport-helper.c b/transport-helper.c
index 3523f1f..4713b69 100644
--- a/transport-helper.c
+++ b/transport-helper.c
@@ -385,7 +385,7 @@ static int get_importer(struct transport *transport, struct child_process *fasti
 	memset(fastimport, 0, sizeof(*fastimport));
 	fastimport->in = helper->out;
 	argv_array_push(&argv, "fast-import");
-	argv_array_push(&argv, "--quiet");
+	argv_array_push(&argv, debug ? "--stats" : "--quiet");
 
 	if (data->bidi_import) {
 		cat_blob_fd = xdup(helper->in);
-- 
1.7.9.5

[PATCH v7 08/16] remote-svn, vcs-svn: Enable fetching to private refs

[Florian Achleitner]

The reference to update by the fast-import stream is hard-coded.  When
fetching from a remote the remote-helper shall update refs in a
private namespace, i.e. a private subdir of refs/.  This namespace is
defined by the 'refspec' capability, that the remote-helper advertises
as a reply to the 'capabilities' command.

Extend svndump and fast-export to allow passing the target ref.
Update svn-fe to be compatible.

Signed-off-by: Florian Achleitner <florian.achleitner.2.6.31@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 contrib/svn-fe/svn-fe.c |    2 +-
 test-svn-fe.c           |    2 +-
 vcs-svn/fast_export.c   |    4 ++--
 vcs-svn/fast_export.h   |    2 +-
 vcs-svn/svndump.c       |   14 +++++++-------
 vcs-svn/svndump.h       |    2 +-
 6 files changed, 13 insertions(+), 13 deletions(-)

diff --git a/contrib/svn-fe/svn-fe.c b/contrib/svn-fe/svn-fe.c
index 35db24f..c796cc0 100644
--- a/contrib/svn-fe/svn-fe.c
+++ b/contrib/svn-fe/svn-fe.c
@@ -10,7 +10,7 @@ int main(int argc, char **argv)
 {
 	if (svndump_init(NULL))
 		return 1;
-	svndump_read((argc > 1) ? argv[1] : NULL);
+	svndump_read((argc > 1) ? argv[1] : NULL, "refs/heads/master");
 	svndump_deinit();
 	svndump_reset();
 	return 0;
diff --git a/test-svn-fe.c b/test-svn-fe.c
index 83633a2..cb0d80f 100644
--- a/test-svn-fe.c
+++ b/test-svn-fe.c
@@ -40,7 +40,7 @@ int main(int argc, char *argv[])
 	if (argc == 2) {
 		if (svndump_init(argv[1]))
 			return 1;
-		svndump_read(NULL);
+		svndump_read(NULL, "refs/heads/master");
 		svndump_deinit();
 		svndump_reset();
 		return 0;
diff --git a/vcs-svn/fast_export.c b/vcs-svn/fast_export.c
index 1f04697..11f8f94 100644
--- a/vcs-svn/fast_export.c
+++ b/vcs-svn/fast_export.c
@@ -72,7 +72,7 @@ static char gitsvnline[MAX_GITSVN_LINE_LEN];
 void fast_export_begin_commit(uint32_t revision, const char *author,
 			const struct strbuf *log,
 			const char *uuid, const char *url,
-			unsigned long timestamp)
+			unsigned long timestamp, const char *local_ref)
 {
 	static const struct strbuf empty = STRBUF_INIT;
 	if (!log)
@@ -84,7 +84,7 @@ void fast_export_begin_commit(uint32_t revision, const char *author,
 	} else {
 		*gitsvnline = '\0';
 	}
-	printf("commit refs/heads/master\n");
+	printf("commit %s\n", local_ref);
 	printf("mark :%"PRIu32"\n", revision);
 	printf("committer %s <%s@%s> %ld +0000\n",
 		   *author ? author : "nobody",
diff --git a/vcs-svn/fast_export.h b/vcs-svn/fast_export.h
index 8823aca..17eb13b 100644
--- a/vcs-svn/fast_export.h
+++ b/vcs-svn/fast_export.h
@@ -11,7 +11,7 @@ void fast_export_delete(const char *path);
 void fast_export_modify(const char *path, uint32_t mode, const char *dataref);
 void fast_export_begin_commit(uint32_t revision, const char *author,
 			const struct strbuf *log, const char *uuid,
-			const char *url, unsigned long timestamp);
+			const char *url, unsigned long timestamp, const char *local_ref);
 void fast_export_end_commit(uint32_t revision);
 void fast_export_data(uint32_t mode, off_t len, struct line_buffer *input);
 void fast_export_blob_delta(uint32_t mode,
diff --git a/vcs-svn/svndump.c b/vcs-svn/svndump.c
index d81a078..288bb42 100644
--- a/vcs-svn/svndump.c
+++ b/vcs-svn/svndump.c
@@ -299,22 +299,22 @@ static void handle_node(void)
 				node_ctx.text_length, &input);
 }
 
-static void begin_revision(void)
+static void begin_revision(const char *remote_ref)
 {
 	if (!rev_ctx.revision)	/* revision 0 gets no git commit. */
 		return;
 	fast_export_begin_commit(rev_ctx.revision, rev_ctx.author.buf,
 		&rev_ctx.log, dump_ctx.uuid.buf, dump_ctx.url.buf,
-		rev_ctx.timestamp);
+		rev_ctx.timestamp, remote_ref);
 }
 
-static void end_revision(void)
+static void end_revision()
 {
 	if (rev_ctx.revision)
 		fast_export_end_commit(rev_ctx.revision);
 }
 
-void svndump_read(const char *url)
+void svndump_read(const char *url, const char *local_ref)
 {
 	char *val;
 	char *t;
@@ -353,7 +353,7 @@ void svndump_read(const char *url)
 			if (active_ctx == NODE_CTX)
 				handle_node();
 			if (active_ctx == REV_CTX)
-				begin_revision();
+				begin_revision(local_ref);
 			if (active_ctx != DUMP_CTX)
 				end_revision();
 			active_ctx = REV_CTX;
@@ -366,7 +366,7 @@ void svndump_read(const char *url)
 				if (active_ctx == NODE_CTX)
 					handle_node();
 				if (active_ctx == REV_CTX)
-					begin_revision();
+					begin_revision(local_ref);
 				active_ctx = NODE_CTX;
 				reset_node_ctx(val);
 				break;
@@ -463,7 +463,7 @@ void svndump_read(const char *url)
 	if (active_ctx == NODE_CTX)
 		handle_node();
 	if (active_ctx == REV_CTX)
-		begin_revision();
+		begin_revision(local_ref);
 	if (active_ctx != DUMP_CTX)
 		end_revision();
 }
diff --git a/vcs-svn/svndump.h b/vcs-svn/svndump.h
index acb5b47..febeecb 100644
--- a/vcs-svn/svndump.h
+++ b/vcs-svn/svndump.h
@@ -3,7 +3,7 @@
 
 int svndump_init(const char *filename);
 int svndump_init_fd(int in_fd, int back_fd);
-void svndump_read(const char *url);
+void svndump_read(const char *url, const char *local_ref);
 void svndump_deinit(void);
 void svndump_reset(void);
 
-- 
1.7.9.5

[PATCH v7 09/16] Allow reading svn dumps from files via file:// urls

[Florian Achleitner]

For testing as well as for importing large, already available dumps,
it's useful to bypass svnrdump and replay the svndump from a file
directly.

Add support for file:// urls in the remote url, e.g.

  svn::file:///path/to/dump

When the remote helper finds an url starting with file:// it tries to
open that file instead of invoking svnrdump.

Signed-off-by: Florian Achleitner <florian.achleitner.2.6.31@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 remote-testsvn.c |   55 +++++++++++++++++++++++++++++++++++-------------------
 1 file changed, 36 insertions(+), 19 deletions(-)

diff --git a/remote-testsvn.c b/remote-testsvn.c
index ebe803b..2b9d151 100644
--- a/remote-testsvn.c
+++ b/remote-testsvn.c
@@ -9,6 +9,7 @@
 #include "argv-array.h"
 
 static const char *url;
+static int dump_from_file;
 static const char *private_ref;
 static const char *remote_ref = "refs/heads/master";
 
@@ -53,29 +54,38 @@ static int cmd_import(const char *line)
 	struct argv_array svndump_argv = ARGV_ARRAY_INIT;
 	struct child_process svndump_proc;
 
-	memset(&svndump_proc, 0, sizeof(struct child_process));
-	svndump_proc.out = -1;
-	argv_array_push(&svndump_argv, "svnrdump");
-	argv_array_push(&svndump_argv, "dump");
-	argv_array_push(&svndump_argv, url);
-	argv_array_pushf(&svndump_argv, "-r%u:HEAD", startrev);
-	svndump_proc.argv = svndump_argv.argv;
-
-	code = start_command(&svndump_proc);
-	if (code)
-		die("Unable to start %s, code %d", svndump_proc.argv[0], code);
-	dumpin_fd = svndump_proc.out;
-
+	if (dump_from_file) {
+		dumpin_fd = open(url, O_RDONLY);
+		if(dumpin_fd < 0) {
+			die_errno("Couldn't open svn dump file %s.", url);
+		}
+	}
+	else {
+		memset(&svndump_proc, 0, sizeof(struct child_process));
+		svndump_proc.out = -1;
+		argv_array_push(&svndump_argv, "svnrdump");
+		argv_array_push(&svndump_argv, "dump");
+		argv_array_push(&svndump_argv, url);
+		argv_array_pushf(&svndump_argv, "-r%u:HEAD", startrev);
+		svndump_proc.argv = svndump_argv.argv;
+
+		code = start_command(&svndump_proc);
+		if (code)
+			die("Unable to start %s, code %d", svndump_proc.argv[0], code);
+		dumpin_fd = svndump_proc.out;
+	}
 	svndump_init_fd(dumpin_fd, STDIN_FILENO);
 	svndump_read(url, private_ref);
 	svndump_deinit();
 	svndump_reset();
 
 	close(dumpin_fd);
-	code = finish_command(&svndump_proc);
-	if (code)
-		warning("%s, returned %d", svndump_proc.argv[0], code);
-	argv_array_clear(&svndump_argv);
+	if(!dump_from_file) {
+		code = finish_command(&svndump_proc);
+		if (code)
+			warning("%s, returned %d", svndump_proc.argv[0], code);
+		argv_array_clear(&svndump_argv);
+	}
 
 	return 0;
 }
@@ -149,8 +159,15 @@ int main(int argc, const char **argv)
 	remote = remote_get(argv[1]);
 	url_in = (argc == 3) ? argv[2] : remote->url[0];
 
-	end_url_with_slash(&buf, url_in);
-	url = strbuf_detach(&buf, NULL);
+	if (!prefixcmp(url_in, "file://")) {
+		dump_from_file = 1;
+		url = url_decode(url_in + sizeof("file://")-1);
+	}
+	else {
+		dump_from_file = 0;
+		end_url_with_slash(&buf, url_in);
+		url = strbuf_detach(&buf, NULL);
+	}
 
 	strbuf_addf(&buf, "refs/svn/%s/master", remote->name);
 	private_ref = strbuf_detach(&buf, NULL);
-- 
1.7.9.5

[PATCH v7 10/16] vcs-svn: add fast_export_note to create notes

[Florian Achleitner]

From: Dmitry Ivankov <divanorama@gmail.com>

fast_export lacked a method to writes notes to fast-import stream.
Add two new functions fast_export_note which is similar to
fast_export_modify. And also add fast_export_buf_to_data to be able to
write inline blobs that don't come from a line_buffer or from delta
application.

To be used like this:

  fast_export_begin_commit("refs/notes/somenotes", ...)
  fast_export_note("refs/heads/master", "inline")
  fast_export_buf_to_data(&data)

or maybe

  fast_export_note("refs/heads/master", sha1)

Signed-off-by: Dmitry Ivankov <divanorama@gmail.com>
Signed-off-by: Florian Achleitner <florian.achleitner.2.6.31@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 vcs-svn/fast_export.c |   12 ++++++++++++
 vcs-svn/fast_export.h |    2 ++
 2 files changed, 14 insertions(+)

diff --git a/vcs-svn/fast_export.c b/vcs-svn/fast_export.c
index 11f8f94..1ecae4b 100644
--- a/vcs-svn/fast_export.c
+++ b/vcs-svn/fast_export.c
@@ -68,6 +68,11 @@ void fast_export_modify(const char *path, uint32_t mode, const char *dataref)
 	putchar('\n');
 }
 
+void fast_export_note(const char *committish, const char *dataref)
+{
+	printf("N %s %s\n", dataref, committish);
+}
+
 static char gitsvnline[MAX_GITSVN_LINE_LEN];
 void fast_export_begin_commit(uint32_t revision, const char *author,
 			const struct strbuf *log,
@@ -222,6 +227,13 @@ static long apply_delta(off_t len, struct line_buffer *input,
 	return ret;
 }
 
+void fast_export_buf_to_data(const struct strbuf *data)
+{
+	printf("data %"PRIuMAX"\n", (uintmax_t)data->len);
+	fwrite(data->buf, data->len, 1, stdout);
+	fputc('\n', stdout);
+}
+
 void fast_export_data(uint32_t mode, off_t len, struct line_buffer *input)
 {
 	assert(len >= 0);
diff --git a/vcs-svn/fast_export.h b/vcs-svn/fast_export.h
index 17eb13b..9b32f1e 100644
--- a/vcs-svn/fast_export.h
+++ b/vcs-svn/fast_export.h
@@ -9,11 +9,13 @@ void fast_export_deinit(void);
 
 void fast_export_delete(const char *path);
 void fast_export_modify(const char *path, uint32_t mode, const char *dataref);
+void fast_export_note(const char *committish, const char *dataref);
 void fast_export_begin_commit(uint32_t revision, const char *author,
 			const struct strbuf *log, const char *uuid,
 			const char *url, unsigned long timestamp, const char *local_ref);
 void fast_export_end_commit(uint32_t revision);
 void fast_export_data(uint32_t mode, off_t len, struct line_buffer *input);
+void fast_export_buf_to_data(const struct strbuf *data);
 void fast_export_blob_delta(uint32_t mode,
 			uint32_t old_mode, const char *old_data,
 			off_t len, struct line_buffer *input);
-- 
1.7.9.5

[PATCH v7 11/16] Create a note for every imported commit containing svn metadata

[Florian Achleitner]

To provide metadata from svn dumps for further processing, e.g.
branch detection, attach a note to each imported commit that stores
additional information.  The notes are currently hard-coded in
refs/notes/svn/revs.  Currently the following lines from the svn dump
are directly accumulated in the note. This can be refined as needed.

 - "Revision-number"
 - "Node-path"
 - "Node-kind"
 - "Node-action"
 - "Node-copyfrom-path"
 - "Node-copyfrom-rev"

Signed-off-by: Florian Achleitner <florian.achleitner.2.6.31@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 vcs-svn/fast_export.c |   14 ++++++++++++--
 vcs-svn/fast_export.h |    2 ++
 vcs-svn/svndump.c     |   21 +++++++++++++++++++--
 3 files changed, 33 insertions(+), 4 deletions(-)

diff --git a/vcs-svn/fast_export.c b/vcs-svn/fast_export.c
index 1ecae4b..df51c59 100644
--- a/vcs-svn/fast_export.c
+++ b/vcs-svn/fast_export.c
@@ -3,8 +3,7 @@
  * See LICENSE for details.
  */
 
-#include "git-compat-util.h"
-#include "strbuf.h"
+#include "cache.h"
 #include "quote.h"
 #include "fast_export.h"
 #include "repo_tree.h"
@@ -68,6 +67,17 @@ void fast_export_modify(const char *path, uint32_t mode, const char *dataref)
 	putchar('\n');
 }
 
+void fast_export_begin_note(uint32_t revision, const char *author,
+		const char *log, unsigned long timestamp)
+{
+	size_t loglen = strlen(log);
+	printf("commit refs/notes/svn/revs\n");
+	printf("committer %s <%s@%s> %ld +0000\n", author, author, "local", timestamp);
+	printf("data %"PRIuMAX"\n", (uintmax_t)loglen);
+	fwrite(log, loglen, 1, stdout);
+	fputc('\n', stdout);
+}
+
 void fast_export_note(const char *committish, const char *dataref)
 {
 	printf("N %s %s\n", dataref, committish);
diff --git a/vcs-svn/fast_export.h b/vcs-svn/fast_export.h
index 9b32f1e..c2f6f11 100644
--- a/vcs-svn/fast_export.h
+++ b/vcs-svn/fast_export.h
@@ -10,6 +10,8 @@ void fast_export_deinit(void);
 void fast_export_delete(const char *path);
 void fast_export_modify(const char *path, uint32_t mode, const char *dataref);
 void fast_export_note(const char *committish, const char *dataref);
+void fast_export_begin_note(uint32_t revision, const char *author,
+		const char *log, unsigned long timestamp);
 void fast_export_begin_commit(uint32_t revision, const char *author,
 			const struct strbuf *log, const char *uuid,
 			const char *url, unsigned long timestamp, const char *local_ref);
diff --git a/vcs-svn/svndump.c b/vcs-svn/svndump.c
index 288bb42..cd65b51 100644
--- a/vcs-svn/svndump.c
+++ b/vcs-svn/svndump.c
@@ -48,7 +48,7 @@ static struct {
 static struct {
 	uint32_t revision;
 	unsigned long timestamp;
-	struct strbuf log, author;
+	struct strbuf log, author, note;
 } rev_ctx;
 
 static struct {
@@ -77,6 +77,7 @@ static void reset_rev_ctx(uint32_t revision)
 	rev_ctx.timestamp = 0;
 	strbuf_reset(&rev_ctx.log);
 	strbuf_reset(&rev_ctx.author);
+	strbuf_reset(&rev_ctx.note);
 }
 
 static void reset_dump_ctx(const char *url)
@@ -310,8 +311,15 @@ static void begin_revision(const char *remote_ref)
 
 static void end_revision()
 {
-	if (rev_ctx.revision)
+	struct strbuf mark = STRBUF_INIT;
+	if (rev_ctx.revision) {
 		fast_export_end_commit(rev_ctx.revision);
+		fast_export_begin_note(rev_ctx.revision, "remote-svn",
+				"Note created by remote-svn.", rev_ctx.timestamp);
+		strbuf_addf(&mark, ":%"PRIu32, rev_ctx.revision);
+		fast_export_note(mark.buf, "inline");
+		fast_export_buf_to_data(&rev_ctx.note);
+	}
 }
 
 void svndump_read(const char *url, const char *local_ref)
@@ -358,6 +366,7 @@ void svndump_read(const char *url, const char *local_ref)
 				end_revision();
 			active_ctx = REV_CTX;
 			reset_rev_ctx(atoi(val));
+			strbuf_addf(&rev_ctx.note, "%s\n", t);
 			break;
 		case sizeof("Node-path"):
 			if (constcmp(t, "Node-"))
@@ -369,10 +378,12 @@ void svndump_read(const char *url, const char *local_ref)
 					begin_revision(local_ref);
 				active_ctx = NODE_CTX;
 				reset_node_ctx(val);
+				strbuf_addf(&rev_ctx.note, "%s\n", t);
 				break;
 			}
 			if (constcmp(t + strlen("Node-"), "kind"))
 				continue;
+			strbuf_addf(&rev_ctx.note, "%s\n", t);
 			if (!strcmp(val, "dir"))
 				node_ctx.type = REPO_MODE_DIR;
 			else if (!strcmp(val, "file"))
@@ -383,6 +394,7 @@ void svndump_read(const char *url, const char *local_ref)
 		case sizeof("Node-action"):
 			if (constcmp(t, "Node-action"))
 				continue;
+			strbuf_addf(&rev_ctx.note, "%s\n", t);
 			if (!strcmp(val, "delete")) {
 				node_ctx.action = NODEACT_DELETE;
 			} else if (!strcmp(val, "add")) {
@@ -401,11 +413,13 @@ void svndump_read(const char *url, const char *local_ref)
 				continue;
 			strbuf_reset(&node_ctx.src);
 			strbuf_addstr(&node_ctx.src, val);
+			strbuf_addf(&rev_ctx.note, "%s\n", t);
 			break;
 		case sizeof("Node-copyfrom-rev"):
 			if (constcmp(t, "Node-copyfrom-rev"))
 				continue;
 			node_ctx.srcRev = atoi(val);
+			strbuf_addf(&rev_ctx.note, "%s\n", t);
 			break;
 		case sizeof("Text-content-length"):
 			if (constcmp(t, "Text") && constcmp(t, "Prop"))
@@ -475,6 +489,7 @@ static void init(int report_fd)
 	strbuf_init(&dump_ctx.url, 4096);
 	strbuf_init(&rev_ctx.log, 4096);
 	strbuf_init(&rev_ctx.author, 4096);
+	strbuf_init(&rev_ctx.note, 4096);
 	strbuf_init(&node_ctx.src, 4096);
 	strbuf_init(&node_ctx.dst, 4096);
 	reset_dump_ctx(NULL);
@@ -506,6 +521,8 @@ void svndump_deinit(void)
 	reset_rev_ctx(0);
 	reset_node_ctx(NULL);
 	strbuf_release(&rev_ctx.log);
+	strbuf_release(&rev_ctx.author);
+	strbuf_release(&rev_ctx.note);
 	strbuf_release(&node_ctx.src);
 	strbuf_release(&node_ctx.dst);
 	if (buffer_deinit(&input))
-- 
1.7.9.5

[PATCH v7 12/16] remote-svn: Activate import/export-marks for fast-import

[Florian Achleitner]

Enable import and export of a marks file by sending the appropriate
feature commands to fast-import before sending data.

Signed-off-by: Florian Achleitner <florian.achleitner.2.6.31@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 remote-testsvn.c |   10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/remote-testsvn.c b/remote-testsvn.c
index 2b9d151..b6e7968 100644
--- a/remote-testsvn.c
+++ b/remote-testsvn.c
@@ -12,6 +12,7 @@ static const char *url;
 static int dump_from_file;
 static const char *private_ref;
 static const char *remote_ref = "refs/heads/master";
+static const char *marksfilename;
 
 static int cmd_capabilities(const char *line);
 static int cmd_import(const char *line);
@@ -74,6 +75,10 @@ static int cmd_import(const char *line)
 			die("Unable to start %s, code %d", svndump_proc.argv[0], code);
 		dumpin_fd = svndump_proc.out;
 	}
+	/* setup marks file import/export */
+	printf("feature import-marks-if-exists=%s\n"
+			"feature export-marks=%s\n", marksfilename, marksfilename);
+
 	svndump_init_fd(dumpin_fd, STDIN_FILENO);
 	svndump_read(url, private_ref);
 	svndump_deinit();
@@ -172,6 +177,10 @@ int main(int argc, const char **argv)
 	strbuf_addf(&buf, "refs/svn/%s/master", remote->name);
 	private_ref = strbuf_detach(&buf, NULL);
 
+	strbuf_addf(&buf, "%s/info/fast-import/remote-svn/%s.marks",
+		get_git_dir(), remote->name);
+	marksfilename = strbuf_detach(&buf, NULL);
+
 	while(1) {
 		if (strbuf_getline(&buf, stdin, '\n') == EOF) {
 			if (ferror(stdin))
@@ -187,5 +196,6 @@ int main(int argc, const char **argv)
 	strbuf_release(&buf);
 	free((void*)url);
 	free((void*)private_ref);
+	free((void*)marksfilename);
 	return 0;
 }
-- 
1.7.9.5

[PATCH v7 13/16] remote-svn: add incremental import

[Florian Achleitner]

Search for a note attached to the ref to update and read it's
'Revision-number:'-line. Start import from the next svn revision.

If there is no next revision in the svn repo, svnrdump terminates with
a message on stderr an non-zero return value. This looks a little
weird, but there is no other way to know whether there is a new
revision in the svn repo.

On the start of an incremental import, the parent of the first commit
in the fast-import stream is set to the branch name to update. All
following commits specify their parent by a mark number. Previous mark
files are currently not reused.

Signed-off-by: Florian Achleitner <florian.achleitner.2.6.31@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 contrib/svn-fe/svn-fe.c |    3 ++-
 remote-testsvn.c        |   67 ++++++++++++++++++++++++++++++++++++++++++++---
 test-svn-fe.c           |    2 +-
 vcs-svn/fast_export.c   |   10 +++++--
 vcs-svn/fast_export.h   |    6 ++---
 vcs-svn/svndump.c       |   10 +++----
 vcs-svn/svndump.h       |    2 +-
 7 files changed, 84 insertions(+), 16 deletions(-)

diff --git a/contrib/svn-fe/svn-fe.c b/contrib/svn-fe/svn-fe.c
index c796cc0..f363505 100644
--- a/contrib/svn-fe/svn-fe.c
+++ b/contrib/svn-fe/svn-fe.c
@@ -10,7 +10,8 @@ int main(int argc, char **argv)
 {
 	if (svndump_init(NULL))
 		return 1;
-	svndump_read((argc > 1) ? argv[1] : NULL, "refs/heads/master");
+	svndump_read((argc > 1) ? argv[1] : NULL, "refs/heads/master",
+			"refs/notes/svn/revs");
 	svndump_deinit();
 	svndump_reset();
 	return 0;
diff --git a/remote-testsvn.c b/remote-testsvn.c
index b6e7968..e90d221 100644
--- a/remote-testsvn.c
+++ b/remote-testsvn.c
@@ -12,7 +12,8 @@ static const char *url;
 static int dump_from_file;
 static const char *private_ref;
 static const char *remote_ref = "refs/heads/master";
-static const char *marksfilename;
+static const char *marksfilename, *notes_ref;
+struct rev_note { unsigned int rev_nr; };
 
 static int cmd_capabilities(const char *line);
 static int cmd_import(const char *line);
@@ -47,14 +48,70 @@ static void terminate_batch(void)
 	fflush(stdout);
 }
 
+/* NOTE: 'ref' refers to a git reference, while 'rev' refers to a svn revision. */
+static char *read_ref_note(const unsigned char sha1[20]) {
+	const unsigned char *note_sha1;
+	char *msg = NULL;
+	unsigned long msglen;
+	enum object_type type;
+	init_notes(NULL, notes_ref, NULL, 0);
+	if(	(note_sha1 = get_note(NULL, sha1)) == NULL ||
+			!(msg = read_sha1_file(note_sha1, &type, &msglen)) ||
+			!msglen || type != OBJ_BLOB) {
+		free(msg);
+		return NULL;
+	}
+	free_notes(NULL);
+	return msg;
+}
+
+static int parse_rev_note(const char *msg, struct rev_note *res) {
+	const char *key, *value, *end;
+	size_t len;
+	while(*msg) {
+		end = strchr(msg, '\n');
+		len = end ? end - msg : strlen(msg);
+
+		key = "Revision-number: ";
+		if(!prefixcmp(msg, key)) {
+			long i;
+			value = msg + strlen(key);
+			i = atol(value);
+			if(i < 0 || i > UINT32_MAX)
+				return 1;
+			res->rev_nr = i;
+		}
+		msg += len + 1;
+	}
+	return 0;
+}
+
 static int cmd_import(const char *line)
 {
 	int code;
 	int dumpin_fd;
-	unsigned int startrev = 0;
+	char *note_msg;
+	unsigned char head_sha1[20];
+	unsigned int startrev;
 	struct argv_array svndump_argv = ARGV_ARRAY_INIT;
 	struct child_process svndump_proc;
 
+	if(read_ref(private_ref, head_sha1))
+		startrev = 0;
+	else {
+		note_msg = read_ref_note(head_sha1);
+		if(note_msg == NULL) {
+			warning("No note found for %s.", private_ref);
+			startrev = 0;
+		}
+		else {
+			struct rev_note note = { 0 };
+			parse_rev_note(note_msg, &note);
+			startrev = note.rev_nr + 1;
+			free(note_msg);
+		}
+	}
+
 	if (dump_from_file) {
 		dumpin_fd = open(url, O_RDONLY);
 		if(dumpin_fd < 0) {
@@ -80,7 +137,7 @@ static int cmd_import(const char *line)
 			"feature export-marks=%s\n", marksfilename, marksfilename);
 
 	svndump_init_fd(dumpin_fd, STDIN_FILENO);
-	svndump_read(url, private_ref);
+	svndump_read(url, private_ref, notes_ref);
 	svndump_deinit();
 	svndump_reset();
 
@@ -177,6 +234,9 @@ int main(int argc, const char **argv)
 	strbuf_addf(&buf, "refs/svn/%s/master", remote->name);
 	private_ref = strbuf_detach(&buf, NULL);
 
+	strbuf_addf(&buf, "refs/notes/%s/revs", remote->name);
+	notes_ref = strbuf_detach(&buf, NULL);
+
 	strbuf_addf(&buf, "%s/info/fast-import/remote-svn/%s.marks",
 		get_git_dir(), remote->name);
 	marksfilename = strbuf_detach(&buf, NULL);
@@ -196,6 +256,7 @@ int main(int argc, const char **argv)
 	strbuf_release(&buf);
 	free((void*)url);
 	free((void*)private_ref);
+	free((void*)notes_ref);
 	free((void*)marksfilename);
 	return 0;
 }
diff --git a/test-svn-fe.c b/test-svn-fe.c
index cb0d80f..0f2d9a4 100644
--- a/test-svn-fe.c
+++ b/test-svn-fe.c
@@ -40,7 +40,7 @@ int main(int argc, char *argv[])
 	if (argc == 2) {
 		if (svndump_init(argv[1]))
 			return 1;
-		svndump_read(NULL, "refs/heads/master");
+		svndump_read(NULL, "refs/heads/master", "refs/notes/svn/revs");
 		svndump_deinit();
 		svndump_reset();
 		return 0;
diff --git a/vcs-svn/fast_export.c b/vcs-svn/fast_export.c
index df51c59..f2b23c8 100644
--- a/vcs-svn/fast_export.c
+++ b/vcs-svn/fast_export.c
@@ -68,13 +68,19 @@ void fast_export_modify(const char *path, uint32_t mode, const char *dataref)
 }
 
 void fast_export_begin_note(uint32_t revision, const char *author,
-		const char *log, unsigned long timestamp)
+		const char *log, unsigned long timestamp, const char *note_ref)
 {
+	static int firstnote = 1;
 	size_t loglen = strlen(log);
-	printf("commit refs/notes/svn/revs\n");
+	printf("commit %s\n", note_ref);
 	printf("committer %s <%s@%s> %ld +0000\n", author, author, "local", timestamp);
 	printf("data %"PRIuMAX"\n", (uintmax_t)loglen);
 	fwrite(log, loglen, 1, stdout);
+	if (firstnote) {
+		if (revision > 1)
+			printf("from %s^0", note_ref);
+		firstnote = 0;
+	}
 	fputc('\n', stdout);
 }
 
diff --git a/vcs-svn/fast_export.h b/vcs-svn/fast_export.h
index c2f6f11..c8b5adb 100644
--- a/vcs-svn/fast_export.h
+++ b/vcs-svn/fast_export.h
@@ -11,10 +11,10 @@ void fast_export_delete(const char *path);
 void fast_export_modify(const char *path, uint32_t mode, const char *dataref);
 void fast_export_note(const char *committish, const char *dataref);
 void fast_export_begin_note(uint32_t revision, const char *author,
-		const char *log, unsigned long timestamp);
+		const char *log, unsigned long timestamp, const char *note_ref);
 void fast_export_begin_commit(uint32_t revision, const char *author,
-			const struct strbuf *log, const char *uuid,
-			const char *url, unsigned long timestamp, const char *local_ref);
+			const struct strbuf *log, const char *uuid,const char *url,
+			unsigned long timestamp, const char *local_ref);
 void fast_export_end_commit(uint32_t revision);
 void fast_export_data(uint32_t mode, off_t len, struct line_buffer *input);
 void fast_export_buf_to_data(const struct strbuf *data);
diff --git a/vcs-svn/svndump.c b/vcs-svn/svndump.c
index cd65b51..31d1d83 100644
--- a/vcs-svn/svndump.c
+++ b/vcs-svn/svndump.c
@@ -309,20 +309,20 @@ static void begin_revision(const char *remote_ref)
 		rev_ctx.timestamp, remote_ref);
 }
 
-static void end_revision()
+static void end_revision(const char *note_ref)
 {
 	struct strbuf mark = STRBUF_INIT;
 	if (rev_ctx.revision) {
 		fast_export_end_commit(rev_ctx.revision);
 		fast_export_begin_note(rev_ctx.revision, "remote-svn",
-				"Note created by remote-svn.", rev_ctx.timestamp);
+				"Note created by remote-svn.", rev_ctx.timestamp, note_ref);
 		strbuf_addf(&mark, ":%"PRIu32, rev_ctx.revision);
 		fast_export_note(mark.buf, "inline");
 		fast_export_buf_to_data(&rev_ctx.note);
 	}
 }
 
-void svndump_read(const char *url, const char *local_ref)
+void svndump_read(const char *url, const char *local_ref, const char *notes_ref)
 {
 	char *val;
 	char *t;
@@ -363,7 +363,7 @@ void svndump_read(const char *url, const char *local_ref)
 			if (active_ctx == REV_CTX)
 				begin_revision(local_ref);
 			if (active_ctx != DUMP_CTX)
-				end_revision();
+				end_revision(notes_ref);
 			active_ctx = REV_CTX;
 			reset_rev_ctx(atoi(val));
 			strbuf_addf(&rev_ctx.note, "%s\n", t);
@@ -479,7 +479,7 @@ void svndump_read(const char *url, const char *local_ref)
 	if (active_ctx == REV_CTX)
 		begin_revision(local_ref);
 	if (active_ctx != DUMP_CTX)
-		end_revision();
+		end_revision(notes_ref);
 }
 
 static void init(int report_fd)
diff --git a/vcs-svn/svndump.h b/vcs-svn/svndump.h
index febeecb..b8eb129 100644
--- a/vcs-svn/svndump.h
+++ b/vcs-svn/svndump.h
@@ -3,7 +3,7 @@
 
 int svndump_init(const char *filename);
 int svndump_init_fd(int in_fd, int back_fd);
-void svndump_read(const char *url, const char *local_ref);
+void svndump_read(const char *url, const char *local_ref, const char *notes_ref);
 void svndump_deinit(void);
 void svndump_reset(void);
 
-- 
1.7.9.5

[PATCH v7 14/16] Add a svnrdump-simulator replaying a dump file for testing

[Florian Achleitner]

To ease testing without depending on a reachable svn server, this
compact python script mimics parts of svnrdumps behaviour.  It
requires the remote url to start with sim://.

Start and end revisions are evaluated.  If the requested revision
doesn't exist, as it is the case with incremental imports, if no new
commit was added, it returns 1 (like svnrdump).

To allow using the same dump file for simulating multiple incremental
imports, the highest revision can be limited by setting the environment
variable SVNRMAX to that value. This simulates the situation where
higher revs don't exist yet.

Signed-off-by: Florian Achleitner <florian.achleitner.2.6.31@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 contrib/svn-fe/svnrdump_sim.py |   53 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 53 insertions(+)
 create mode 100755 contrib/svn-fe/svnrdump_sim.py

diff --git a/contrib/svn-fe/svnrdump_sim.py b/contrib/svn-fe/svnrdump_sim.py
new file mode 100755
index 0000000..1cfac4a
--- /dev/null
+++ b/contrib/svn-fe/svnrdump_sim.py
@@ -0,0 +1,53 @@
+#!/usr/bin/python
+"""
+Simulates svnrdump by replaying an existing dump from a file, taking care
+of the specified revision range.
+To simulate incremental imports the environment variable SVNRMAX can be set
+to the highest revision that should be available.
+"""
+import sys, os
+
+
+def getrevlimit():
+        var = 'SVNRMAX'
+        if os.environ.has_key(var):
+                return os.environ[var]
+        return None
+
+def writedump(url, lower, upper):
+        if url.startswith('sim://'):
+                filename = url[6:]
+                if filename[-1] == '/': filename = filename[:-1] #remove terminating slash
+        else:
+                raise ValueError('sim:// url required')
+        f = open(filename, 'r');
+        state = 'header'
+        wroterev = False
+        while(True):
+                l = f.readline()
+                if l == '': break
+                if state == 'header' and l.startswith('Revision-number: '):
+                        state = 'prefix'
+                if state == 'prefix' and l == 'Revision-number: %s\n' % lower:
+                        state = 'selection'
+                if not upper == 'HEAD' and state == 'selection' and l == 'Revision-number: %s\n' % upper:
+                        break;
+
+                if state == 'header' or state == 'selection':
+                        if state == 'selection': wroterev = True
+                        sys.stdout.write(l)
+        return wroterev
+
+if __name__ == "__main__":
+        if not (len(sys.argv) in (3, 4, 5)):
+                print "usage: %s dump URL -rLOWER:UPPER"
+                sys.exit(1)
+        if not sys.argv[1] == 'dump': raise NotImplementedError('only "dump" is suppported.')
+        url = sys.argv[2]
+        r = ('0', 'HEAD')
+        if len(sys.argv) == 4 and sys.argv[3][0:2] == '-r':
+                r = sys.argv[3][2:].lstrip().split(':')
+        if not getrevlimit() is None: r[1] = getrevlimit()
+        if writedump(url, r[0], r[1]): ret = 0
+        else: ret = 1
+        sys.exit(ret)
-- 
1.7.9.5

[PATCH v7 15/16] remote-svn: add marks-file regeneration

[Florian Achleitner]

fast-import mark files are stored outside the object database and are
therefore not fetched and can be lost somehow else.  marks provide a
svn revision --> git sha1 mapping, while the notes that are attached
to each commit when it is imported provide a git sha1 --> svn revision
mapping.

If the marks file is not available or not plausible, regenerate it by
walking through the notes tree.  , i.e.  The plausibility check tests
if the highest revision in the marks file matches the revision of the
top ref. It doesn't ensure that the mark file is completely correct.
This could only be done with an effort equal to unconditional
regeneration.

Signed-off-by: Florian Achleitner <florian.achleitner.2.6.31@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 remote-testsvn.c |   68 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 68 insertions(+)

diff --git a/remote-testsvn.c b/remote-testsvn.c
index e90d221..d0b81d5 100644
--- a/remote-testsvn.c
+++ b/remote-testsvn.c
@@ -86,6 +86,73 @@ static int parse_rev_note(const char *msg, struct rev_note *res) {
 	return 0;
 }
 
+static int note2mark_cb(const unsigned char *object_sha1,
+		const unsigned char *note_sha1, char *note_path,
+		void *cb_data) {
+	FILE *file = (FILE *)cb_data;
+	char *msg;
+	unsigned long msglen;
+	enum object_type type;
+	struct rev_note note;
+	if (!(msg = read_sha1_file(note_sha1, &type, &msglen)) ||
+			!msglen || type != OBJ_BLOB) {
+		free(msg);
+		return 1;
+	}
+	if (parse_rev_note(msg, &note))
+		return 2;
+	if (fprintf(file, ":%d %s\n", note.rev_nr, sha1_to_hex(object_sha1)) < 1)
+		return 3;
+	return 0;
+}
+
+static void regenerate_marks(void)
+{
+	int ret;
+	FILE *marksfile;
+	marksfile = fopen(marksfilename, "w+");
+	if (!marksfile)
+		die_errno("Couldn't create mark file %s.", marksfilename);
+	ret = for_each_note(NULL, 0, note2mark_cb, marksfile);
+	if (ret)
+		die("Regeneration of marks failed, returned %d.", ret);
+	fclose(marksfile);
+}
+
+static void check_or_regenerate_marks(int latestrev) {
+	FILE *marksfile;
+	struct strbuf sb = STRBUF_INIT;
+	struct strbuf line = STRBUF_INIT;
+	int found = 0;
+
+	if (latestrev < 1)
+		return;
+
+	init_notes(NULL, notes_ref, NULL, 0);
+	marksfile = fopen(marksfilename, "r");
+	if (!marksfile) {
+		regenerate_marks();
+		marksfile = fopen(marksfilename, "r");
+		if (!marksfile)
+			die_errno("cannot read marks file %s!", marksfilename);
+		fclose(marksfile);
+	} else {
+		strbuf_addf(&sb, ":%d ", latestrev);
+		while (strbuf_getline(&line, marksfile, '\n') != EOF) {
+			if (!prefixcmp(line.buf, sb.buf)) {
+				found++;
+				break;
+			}
+		}
+		fclose(marksfile);
+		if (!found)
+			regenerate_marks();
+	}
+	free_notes(NULL);
+	strbuf_release(&sb);
+	strbuf_release(&line);
+}
+
 static int cmd_import(const char *line)
 {
 	int code;
@@ -111,6 +178,7 @@ static int cmd_import(const char *line)
 			free(note_msg);
 		}
 	}
+	check_or_regenerate_marks(startrev - 1);
 
 	if (dump_from_file) {
 		dumpin_fd = open(url, O_RDONLY);
-- 
1.7.9.5

[PATCH v7 16/16] Add a test script for remote-svn

[Florian Achleitner]

Use svnrdump_sim.py to emulate svnrdump without an svn server.
Tests fetching, incremental fetching, fetching from file://,
and the regeneration of fast-import's marks file.

Signed-off-by: Florian Achleitner <florian.achleitner.2.6.31@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 t/t9020-remote-svn.sh |   82 +++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 82 insertions(+)
 create mode 100755 t/t9020-remote-svn.sh

diff --git a/t/t9020-remote-svn.sh b/t/t9020-remote-svn.sh
new file mode 100755
index 0000000..e6ed4ca
--- /dev/null
+++ b/t/t9020-remote-svn.sh
@@ -0,0 +1,82 @@
+#!/bin/sh
+
+test_description='tests remote-svn'
+
+. ./test-lib.sh
+
+if ! test_have_prereq PYTHON
+then
+	skip_all='skipping remote-svn tests, python not available'
+	test_done
+fi
+
+# We override svnrdump by placing a symlink to the svnrdump-emulator in .
+export PATH="$HOME:$PATH"
+ln -sf $GIT_BUILD_DIR/contrib/svn-fe/svnrdump_sim.py "$HOME/svnrdump"
+
+init_git () {
+	rm -fr .git &&
+	git init &&
+	#git remote add svnsim testsvn::sim:///$TEST_DIRECTORY/t9020/example.svnrdump
+	# let's reuse an exisiting dump file!?
+	git remote add svnsim testsvn::sim://$TEST_DIRECTORY/t9154/svn.dump
+	git remote add svnfile testsvn::file://$TEST_DIRECTORY/t9154/svn.dump
+}
+
+if test -e "$GIT_BUILD_DIR/git-remote-testsvn"
+then
+	test_set_prereq REMOTE_SVN
+fi
+
+test_debug '
+	git --version
+	which git
+	which svnrdump
+'
+
+test_expect_success REMOTE_SVN 'simple fetch' '
+	init_git &&
+	git fetch svnsim &&
+	test_cmp .git/refs/svn/svnsim/master .git/refs/remotes/svnsim/master  &&
+	cp .git/refs/remotes/svnsim/master master.good
+'
+
+test_debug '
+	cat .git/refs/svn/svnsim/master
+	cat .git/refs/remotes/svnsim/master
+'
+
+test_expect_success REMOTE_SVN 'repeated fetch, nothing shall change' '
+	git fetch svnsim &&
+	test_cmp master.good .git/refs/remotes/svnsim/master
+'
+
+test_expect_success REMOTE_SVN 'fetch from a file:// url gives the same result' '
+	git fetch svnfile
+'
+
+test_expect_failure REMOTE_SVN 'the sha1 differ because the git-svn-id line in the commit msg contains the url' '
+	test_cmp .git/refs/remotes/svnfile/master .git/refs/remotes/svnsim/master
+'
+
+test_expect_success REMOTE_SVN 'mark-file regeneration' '
+	# filter out any other marks, that can not be regenerated. Only up to 3 digit revisions are allowed here
+	grep ":[0-9]\{1,3\} " .git/info/fast-import/remote-svn/svnsim.marks > .git/info/fast-import/remote-svn/svnsim.marks.old &&
+	rm .git/info/fast-import/remote-svn/svnsim.marks &&
+	git fetch svnsim &&
+	test_cmp .git/info/fast-import/remote-svn/svnsim.marks.old .git/info/fast-import/remote-svn/svnsim.marks
+'
+
+test_expect_success REMOTE_SVN 'incremental imports must lead to the same head' '
+	export SVNRMAX=3 &&
+	init_git &&
+	git fetch svnsim &&
+	test_cmp .git/refs/svn/svnsim/master .git/refs/remotes/svnsim/master  &&
+	unset SVNRMAX &&
+	git fetch svnsim &&
+	test_cmp master.good .git/refs/remotes/svnsim/master
+'
+
+test_debug 'git branch -a'
+
+test_done
-- 
1.7.9.5

Re: [PATCH v7 16/16] Add a test script for remote-svn

[Junio C Hamano]

Florian Achleitner <florian.achleitner.2.6.31@gmail.com> writes:

> Use svnrdump_sim.py to emulate svnrdump without an svn server.
> Tests fetching, incremental fetching, fetching from file://,
> and the regeneration of fast-import's marks file.
>
> Signed-off-by: Florian Achleitner <florian.achleitner.2.6.31@gmail.com>
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
> ---
>  t/t9020-remote-svn.sh |   82 +++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 82 insertions(+)
>  create mode 100755 t/t9020-remote-svn.sh
>
> diff --git a/t/t9020-remote-svn.sh b/t/t9020-remote-svn.sh
> new file mode 100755
> index 0000000..e6ed4ca
> --- /dev/null
> +++ b/t/t9020-remote-svn.sh
> @@ -0,0 +1,82 @@
> +#!/bin/sh
> +
> +test_description='tests remote-svn'
> +
> +. ./test-lib.sh
> +
> +if ! test_have_prereq PYTHON
> +then
> +	skip_all='skipping remote-svn tests, python not available'
> +	test_done
> +fi
> +
> +# We override svnrdump by placing a symlink to the svnrdump-emulator in .
> +export PATH="$HOME:$PATH"
> +ln -sf $GIT_BUILD_DIR/contrib/svn-fe/svnrdump_sim.py "$HOME/svnrdump"
> +
> +init_git () {
> +	rm -fr .git &&
> +	git init &&
> +	#git remote add svnsim testsvn::sim:///$TEST_DIRECTORY/t9020/example.svnrdump
> +	# let's reuse an exisiting dump file!?
> +	git remote add svnsim testsvn::sim://$TEST_DIRECTORY/t9154/svn.dump
> +	git remote add svnfile testsvn::file://$TEST_DIRECTORY/t9154/svn.dump
> +}
> +
> +if test -e "$GIT_BUILD_DIR/git-remote-testsvn"
> +then
> +	test_set_prereq REMOTE_SVN
> +fi
> +
> +test_debug '
> +	git --version
> +	which git
> +	which svnrdump
> +'
> +
> +test_expect_success REMOTE_SVN 'simple fetch' '
> +	init_git &&
> +	git fetch svnsim &&
> +	test_cmp .git/refs/svn/svnsim/master .git/refs/remotes/svnsim/master  &&
> +	cp .git/refs/remotes/svnsim/master master.good
> +'
> +
> +test_debug '
> +	cat .git/refs/svn/svnsim/master
> +	cat .git/refs/remotes/svnsim/master
> +'
> +
> +test_expect_success REMOTE_SVN 'repeated fetch, nothing shall change' '
> +	git fetch svnsim &&
> +	test_cmp master.good .git/refs/remotes/svnsim/master
> +'
> +
> +test_expect_success REMOTE_SVN 'fetch from a file:// url gives the same result' '
> +	git fetch svnfile
> +'
> +
> +test_expect_failure REMOTE_SVN 'the sha1 differ because the git-svn-id line in the commit msg contains the url' '
> +	test_cmp .git/refs/remotes/svnfile/master .git/refs/remotes/svnsim/master
> +'
> +
> +test_expect_success REMOTE_SVN 'mark-file regeneration' '
> +	# filter out any other marks, that can not be regenerated. Only up to 3 digit revisions are allowed here
> +	grep ":[0-9]\{1,3\} " .git/info/fast-import/remote-svn/svnsim.marks > .git/info/fast-import/remote-svn/svnsim.marks.old &&
> +	rm .git/info/fast-import/remote-svn/svnsim.marks &&
> +	git fetch svnsim &&
> +	test_cmp .git/info/fast-import/remote-svn/svnsim.marks.old .git/info/fast-import/remote-svn/svnsim.marks
> +'

Could these loooooong lines be made a bit more manageable?  It is
getting extremely hard to follow.


> +test_expect_success REMOTE_SVN 'incremental imports must lead to the same head' '
> +	export SVNRMAX=3 &&
> +	init_git &&
> +	git fetch svnsim &&
> +	test_cmp .git/refs/svn/svnsim/master .git/refs/remotes/svnsim/master  &&
> +	unset SVNRMAX &&
> +	git fetch svnsim &&
> +	test_cmp master.good .git/refs/remotes/svnsim/master
> +'
> +
> +test_debug 'git branch -a'
> +
> +test_done
 

Re: [PATCH v7 15/16] remote-svn: add marks-file regeneration

[Junio C Hamano]

Florian Achleitner <florian.achleitner.2.6.31@gmail.com> writes:

> fast-import mark files are stored outside the object database and are
> therefore not fetched and can be lost somehow else.  marks provide a
> svn revision --> git sha1 mapping, while the notes that are attached
> to each commit when it is imported provide a git sha1 --> svn revision
> mapping.
>
> If the marks file is not available or not plausible, regenerate it by
> walking through the notes tree.  , i.e.  The plausibility check tests
> if the highest revision in the marks file matches the revision of the
> top ref. It doesn't ensure that the mark file is completely correct.
> This could only be done with an effort equal to unconditional
> regeneration.
>
> Signed-off-by: Florian Achleitner <florian.achleitner.2.6.31@gmail.com>
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
> ---
>  remote-testsvn.c |   68 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 68 insertions(+)
>
> diff --git a/remote-testsvn.c b/remote-testsvn.c
> index e90d221..d0b81d5 100644
> --- a/remote-testsvn.c
> +++ b/remote-testsvn.c
> @@ -86,6 +86,73 @@ static int parse_rev_note(const char *msg, struct rev_note *res) {
>  	return 0;
>  }
>  
> +static int note2mark_cb(const unsigned char *object_sha1,
> +		const unsigned char *note_sha1, char *note_path,
> +		void *cb_data) {
> +	FILE *file = (FILE *)cb_data;
> +	char *msg;
> +	unsigned long msglen;
> +	enum object_type type;
> +	struct rev_note note;
> +	if (!(msg = read_sha1_file(note_sha1, &type, &msglen)) ||
> +			!msglen || type != OBJ_BLOB) {
> +		free(msg);
> +		return 1;
> +	}

The same comments as an earlier patch in the series applies here,
regarding chained assignments in coditional, whether each case is an
error that needs to be reported, and the sign of the error return
value.

> +	if (parse_rev_note(msg, &note))
> +		return 2;
> +	if (fprintf(file, ":%d %s\n", note.rev_nr, sha1_to_hex(object_sha1)) < 1)
> +		return 3;
> +	return 0;
> +}
> +
> +static void regenerate_marks(void)
> +{
> +	int ret;
> +	FILE *marksfile;
> +	marksfile = fopen(marksfilename, "w+");
> +	if (!marksfile)
> +		die_errno("Couldn't create mark file %s.", marksfilename);
> +	ret = for_each_note(NULL, 0, note2mark_cb, marksfile);
> +	if (ret)
> +		die("Regeneration of marks failed, returned %d.", ret);
> +	fclose(marksfile);
> +}
> +
> +static void check_or_regenerate_marks(int latestrev) {

Style.

> +	FILE *marksfile;
> +	struct strbuf sb = STRBUF_INIT;
> +	struct strbuf line = STRBUF_INIT;
> +	int found = 0;
> +
> +	if (latestrev < 1)
> +		return;

It's more pleasant to read to have a blank line between the end of
decls and the first statement, like this function does.  Please fix
two functions that appear before this function in this file to match.

> +	init_notes(NULL, notes_ref, NULL, 0);
> +	marksfile = fopen(marksfilename, "r");
> +	if (!marksfile) {
> +		regenerate_marks();
> +		marksfile = fopen(marksfilename, "r");
> +		if (!marksfile)
> +			die_errno("cannot read marks file %s!", marksfilename);
> +		fclose(marksfile);
> +	} else {
> +		strbuf_addf(&sb, ":%d ", latestrev);
> +		while (strbuf_getline(&line, marksfile, '\n') != EOF) {
> +			if (!prefixcmp(line.buf, sb.buf)) {
> +				found++;
> +				break;
> +			}
> +		}
> +		fclose(marksfile);
> +		if (!found)
> +			regenerate_marks();
> +	}
> +	free_notes(NULL);
> +	strbuf_release(&sb);
> +	strbuf_release(&line);
> +}
> +
>  static int cmd_import(const char *line)
>  {
>  	int code;
> @@ -111,6 +178,7 @@ static int cmd_import(const char *line)
>  			free(note_msg);
>  		}
>  	}
> +	check_or_regenerate_marks(startrev - 1);
>  
>  	if (dump_from_file) {
>  		dumpin_fd = open(url, O_RDONLY);

Re: [PATCH v7 13/16] remote-svn: add incremental import

[Junio C Hamano]

Florian Achleitner <florian.achleitner.2.6.31@gmail.com> writes:

> Search for a note attached to the ref to update and read it's
> 'Revision-number:'-line. Start import from the next svn revision.
>
> If there is no next revision in the svn repo, svnrdump terminates with
> a message on stderr an non-zero return value. This looks a little
> weird, but there is no other way to know whether there is a new
> revision in the svn repo.
>
> On the start of an incremental import, the parent of the first commit
> in the fast-import stream is set to the branch name to update. All
> following commits specify their parent by a mark number. Previous mark
> files are currently not reused.
>
> Signed-off-by: Florian Achleitner <florian.achleitner.2.6.31@gmail.com>
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
> ---
>  contrib/svn-fe/svn-fe.c |    3 ++-
>  remote-testsvn.c        |   67 ++++++++++++++++++++++++++++++++++++++++++++---
>  test-svn-fe.c           |    2 +-
>  vcs-svn/fast_export.c   |   10 +++++--
>  vcs-svn/fast_export.h   |    6 ++---
>  vcs-svn/svndump.c       |   10 +++----
>  vcs-svn/svndump.h       |    2 +-
>  7 files changed, 84 insertions(+), 16 deletions(-)
>
> diff --git a/contrib/svn-fe/svn-fe.c b/contrib/svn-fe/svn-fe.c
> index c796cc0..f363505 100644
> --- a/contrib/svn-fe/svn-fe.c
> +++ b/contrib/svn-fe/svn-fe.c
> @@ -10,7 +10,8 @@ int main(int argc, char **argv)
>  {
>  	if (svndump_init(NULL))
>  		return 1;
> -	svndump_read((argc > 1) ? argv[1] : NULL, "refs/heads/master");
> +	svndump_read((argc > 1) ? argv[1] : NULL, "refs/heads/master",
> +			"refs/notes/svn/revs");
>  	svndump_deinit();
>  	svndump_reset();
>  	return 0;
> diff --git a/remote-testsvn.c b/remote-testsvn.c
> index b6e7968..e90d221 100644
> --- a/remote-testsvn.c
> +++ b/remote-testsvn.c
> @@ -12,7 +12,8 @@ static const char *url;
>  static int dump_from_file;
>  static const char *private_ref;
>  static const char *remote_ref = "refs/heads/master";
> -static const char *marksfilename;
> +static const char *marksfilename, *notes_ref;
> +struct rev_note { unsigned int rev_nr; };
>  
>  static int cmd_capabilities(const char *line);
>  static int cmd_import(const char *line);
> @@ -47,14 +48,70 @@ static void terminate_batch(void)
>  	fflush(stdout);
>  }
>  
> +/* NOTE: 'ref' refers to a git reference, while 'rev' refers to a svn revision. */
> +static char *read_ref_note(const unsigned char sha1[20]) {

Style:

	static char *read_ref_note(const unsigned char sha1[20])
        {

> +	const unsigned char *note_sha1;
> +	char *msg = NULL;
> +	unsigned long msglen;
> +	enum object_type type;
> +	init_notes(NULL, notes_ref, NULL, 0);
> +	if(	(note_sha1 = get_note(NULL, sha1)) == NULL ||
> +			!(msg = read_sha1_file(note_sha1, &type, &msglen)) ||
> +			!msglen || type != OBJ_BLOB) {
> +		free(msg);
> +		return NULL;
> +	}
> +	free_notes(NULL);
> +	return msg;
> +}

Style:

	if (!(note_sha1 = get_note(NULL, sha1)) ||
	    !(msg = read_sha1_file(note_sha1, &type, &msglen)) ||
	    !msglen ||
            type != OBJ_BLOB) {
		...

But a bigger question is if any of these cases is a non-error.

It may be perfectly normal so get_note() that returns NULL may be a
normal condition, but is it something you want to silently ignore if
read_sha1_file() did not give you anything when called with a
note_sha1 that ought to be valid?  How about the case where you got
msg but msglen is zero?  Is it an error?  Is it normal and the
caller wants to see a note that happens to be an empty string?  How
about the case where the note returned was ot a blob?  Is it
something you want to silently ignore, or is it an error?

Having multiple assingments inside a conditional, and chaining them
together with "||", lets you code lazily, and the resulting code
like the above _appear_ concise, but in order to prepare the code to
answer these questions sensibly, it is often a good habit to avoid
the appearance of conciseness that hides the lack of thought (e.g.
the code is hiding the reason why it does not call free_notes() when
you do have note_sah1 but found a note that is of undesired object
type, and the reader cannot tell if it is done deliberately).

It is far more preferrable to see this written like:

	if (!(note_sha1 = get_note(NULL, sha1)))
        	return NULL; /* no notes - nothing to return */
	msg = read_sha1_file(note_sha1, &type, &msglen);
	if (!msg) {
		error("cannot read notes for ...");
        } else if (type != OBJ_BLOB) {
		free(msg); /* something we cannot use */
		msg = NULL;
	} ... you may have more else if clauses here ...
        free_notes(NULL);
        return msg;

Also, I am not sure if you want to silently ignore OBJ_BLOB here.
If I understand correctly, you are not reading from a random notes
tree, but from a notes tree that was populated by an earlier
incarnation of your process, no?  If asking for a note in that notes
tree yields a note that you do not recognize, shouldn't you treat it
as an error and raise a big red flag?  The same discussion goes for
ignoring an empty msg.  If you never produce an empty msg, and if
nobody else is supposed to add random stuff to that notes tree,
shouldn't you treat it as an indication that something fishy is
going on if you read an empty msg?

> +static int parse_rev_note(const char *msg, struct rev_note *res) {

Style.

	static int parse_rev_note(const char *msg, struct rev_note *res)
	{

> +	const char *key, *value, *end;
> +	size_t len;
> +	while(*msg) {

Style.

        while (*msg) {

> +		end = strchr(msg, '\n');
> +		len = end ? end - msg : strlen(msg);
> +
> +		key = "Revision-number: ";
> +		if(!prefixcmp(msg, key)) {

Style.

		if (!prefixcmp(msg, key)) {

> +			long i;
> +			value = msg + strlen(key);
> +			i = atol(value);
> +			if(i < 0 || i > UINT32_MAX)

Style.
		if (i < 0 || ...)

More importantly, if you are parsing text that is supposed to be a
format known to you and not human generated, you should avoid using
atoi & atol when parsing numbers; use strtol or strtoul instead, as
they allow you much better error handling.

> +				return 1;

Is it signaling an error to the caller?  The usual convention is to
use negative value for such a purpose.

> +			res->rev_nr = i;
> +		}
> +		msg += len + 1;
> +	}
> +	return 0;
> +}
> +
>  static int cmd_import(const char *line)
>  {
>  	int code;
>  	int dumpin_fd;
> -	unsigned int startrev = 0;
> +	char *note_msg;
> +	unsigned char head_sha1[20];
> +	unsigned int startrev;
>  	struct argv_array svndump_argv = ARGV_ARRAY_INIT;
>  	struct child_process svndump_proc;
>  
> +	if(read_ref(private_ref, head_sha1))

Style.

> +		startrev = 0;
> +	else {
> +		note_msg = read_ref_note(head_sha1);
> +		if(note_msg == NULL) {
> +			warning("No note found for %s.", private_ref);
> +			startrev = 0;
> +		}
> +		else {

Style.

> +			struct rev_note note = { 0 };
> +			parse_rev_note(note_msg, &note);
> +			startrev = note.rev_nr + 1;
> +			free(note_msg);
> +		}
> +	}
> +
>  	if (dump_from_file) {
>  		dumpin_fd = open(url, O_RDONLY);
>  		if(dumpin_fd < 0) {
> @@ -80,7 +137,7 @@ static int cmd_import(const char *line)
>  			"feature export-marks=%s\n", marksfilename, marksfilename);
>  
>  	svndump_init_fd(dumpin_fd, STDIN_FILENO);
> -	svndump_read(url, private_ref);
> +	svndump_read(url, private_ref, notes_ref);
>  	svndump_deinit();
>  	svndump_reset();
>  
> @@ -177,6 +234,9 @@ int main(int argc, const char **argv)
>  	strbuf_addf(&buf, "refs/svn/%s/master", remote->name);
>  	private_ref = strbuf_detach(&buf, NULL);
>  
> +	strbuf_addf(&buf, "refs/notes/%s/revs", remote->name);
> +	notes_ref = strbuf_detach(&buf, NULL);
> +
>  	strbuf_addf(&buf, "%s/info/fast-import/remote-svn/%s.marks",
>  		get_git_dir(), remote->name);
>  	marksfilename = strbuf_detach(&buf, NULL);
> @@ -196,6 +256,7 @@ int main(int argc, const char **argv)
>  	strbuf_release(&buf);
>  	free((void*)url);
>  	free((void*)private_ref);
> +	free((void*)notes_ref);
>  	free((void*)marksfilename);
>  	return 0;
>  }
> diff --git a/test-svn-fe.c b/test-svn-fe.c
> index cb0d80f..0f2d9a4 100644
> --- a/test-svn-fe.c
> +++ b/test-svn-fe.c
> @@ -40,7 +40,7 @@ int main(int argc, char *argv[])
>  	if (argc == 2) {
>  		if (svndump_init(argv[1]))
>  			return 1;
> -		svndump_read(NULL, "refs/heads/master");
> +		svndump_read(NULL, "refs/heads/master", "refs/notes/svn/revs");
>  		svndump_deinit();
>  		svndump_reset();
>  		return 0;
> diff --git a/vcs-svn/fast_export.c b/vcs-svn/fast_export.c
> index df51c59..f2b23c8 100644
> --- a/vcs-svn/fast_export.c
> +++ b/vcs-svn/fast_export.c
> @@ -68,13 +68,19 @@ void fast_export_modify(const char *path, uint32_t mode, const char *dataref)
>  }
>  
>  void fast_export_begin_note(uint32_t revision, const char *author,
> -		const char *log, unsigned long timestamp)
> +		const char *log, unsigned long timestamp, const char *note_ref)
>  {
> +	static int firstnote = 1;
>  	size_t loglen = strlen(log);
> -	printf("commit refs/notes/svn/revs\n");
> +	printf("commit %s\n", note_ref);
>  	printf("committer %s <%s@%s> %ld +0000\n", author, author, "local", timestamp);
>  	printf("data %"PRIuMAX"\n", (uintmax_t)loglen);
>  	fwrite(log, loglen, 1, stdout);
> +	if (firstnote) {
> +		if (revision > 1)
> +			printf("from %s^0", note_ref);
> +		firstnote = 0;
> +	}
>  	fputc('\n', stdout);
>  }
>  
> diff --git a/vcs-svn/fast_export.h b/vcs-svn/fast_export.h
> index c2f6f11..c8b5adb 100644
> --- a/vcs-svn/fast_export.h
> +++ b/vcs-svn/fast_export.h
> @@ -11,10 +11,10 @@ void fast_export_delete(const char *path);
>  void fast_export_modify(const char *path, uint32_t mode, const char *dataref);
>  void fast_export_note(const char *committish, const char *dataref);
>  void fast_export_begin_note(uint32_t revision, const char *author,
> -		const char *log, unsigned long timestamp);
> +		const char *log, unsigned long timestamp, const char *note_ref);
>  void fast_export_begin_commit(uint32_t revision, const char *author,
> -			const struct strbuf *log, const char *uuid,
> -			const char *url, unsigned long timestamp, const char *local_ref);
> +			const struct strbuf *log, const char *uuid,const char *url,
> +			unsigned long timestamp, const char *local_ref);
>  void fast_export_end_commit(uint32_t revision);
>  void fast_export_data(uint32_t mode, off_t len, struct line_buffer *input);
>  void fast_export_buf_to_data(const struct strbuf *data);
> diff --git a/vcs-svn/svndump.c b/vcs-svn/svndump.c
> index cd65b51..31d1d83 100644
> --- a/vcs-svn/svndump.c
> +++ b/vcs-svn/svndump.c
> @@ -309,20 +309,20 @@ static void begin_revision(const char *remote_ref)
>  		rev_ctx.timestamp, remote_ref);
>  }
>  
> -static void end_revision()

7e11902 (vcs-svn: add a comment before each commit, 2011-01-04)
added this as

	static void end_revision(void)

but it degenerated to pre-ANSI definition at "[PATCH 8/16] Enable
fetching to private refs", which needs to be fixed by losing that
hunk.

> +static void end_revision(const char *note_ref)
>  {
>  	struct strbuf mark = STRBUF_INIT;
>  	if (rev_ctx.revision) {
>  		fast_export_end_commit(rev_ctx.revision);
>  		fast_export_begin_note(rev_ctx.revision, "remote-svn",
> -				"Note created by remote-svn.", rev_ctx.timestamp);
> +				"Note created by remote-svn.", rev_ctx.timestamp, note_ref);
>  		strbuf_addf(&mark, ":%"PRIu32, rev_ctx.revision);
>  		fast_export_note(mark.buf, "inline");
>  		fast_export_buf_to_data(&rev_ctx.note);
>  	}
>  }
>  
> -void svndump_read(const char *url, const char *local_ref)
> +void svndump_read(const char *url, const char *local_ref, const char *notes_ref)
>  {
>  	char *val;
>  	char *t;
> @@ -363,7 +363,7 @@ void svndump_read(const char *url, const char *local_ref)
>  			if (active_ctx == REV_CTX)
>  				begin_revision(local_ref);
>  			if (active_ctx != DUMP_CTX)
> -				end_revision();
> +				end_revision(notes_ref);
>  			active_ctx = REV_CTX;
>  			reset_rev_ctx(atoi(val));
>  			strbuf_addf(&rev_ctx.note, "%s\n", t);
> @@ -479,7 +479,7 @@ void svndump_read(const char *url, const char *local_ref)
>  	if (active_ctx == REV_CTX)
>  		begin_revision(local_ref);
>  	if (active_ctx != DUMP_CTX)
> -		end_revision();
> +		end_revision(notes_ref);
>  }
>  
>  static void init(int report_fd)
> diff --git a/vcs-svn/svndump.h b/vcs-svn/svndump.h
> index febeecb..b8eb129 100644
> --- a/vcs-svn/svndump.h
> +++ b/vcs-svn/svndump.h
> @@ -3,7 +3,7 @@
>  
>  int svndump_init(const char *filename);
>  int svndump_init_fd(int in_fd, int back_fd);
> -void svndump_read(const char *url, const char *local_ref);
> +void svndump_read(const char *url, const char *local_ref, const char *notes_ref);
>  void svndump_deinit(void);
>  void svndump_reset(void);

Re: [PATCH v7 09/16] Allow reading svn dumps from files via file:// urls

[Junio C Hamano]

Florian Achleitner <florian.achleitner.2.6.31@gmail.com> writes:

> For testing as well as for importing large, already available dumps,
> it's useful to bypass svnrdump and replay the svndump from a file
> directly.
>
> Add support for file:// urls in the remote url, e.g.
>
>   svn::file:///path/to/dump
>
> When the remote helper finds an url starting with file:// it tries to
> open that file instead of invoking svnrdump.
>
> Signed-off-by: Florian Achleitner <florian.achleitner.2.6.31@gmail.com>
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
> ---
>  remote-testsvn.c |   55 +++++++++++++++++++++++++++++++++++-------------------
>  1 file changed, 36 insertions(+), 19 deletions(-)
>
> diff --git a/remote-testsvn.c b/remote-testsvn.c
> index ebe803b..2b9d151 100644
> --- a/remote-testsvn.c
> +++ b/remote-testsvn.c
> @@ -9,6 +9,7 @@
>  #include "argv-array.h"
>  
>  static const char *url;
> +static int dump_from_file;
>  static const char *private_ref;
>  static const char *remote_ref = "refs/heads/master";
>  
> @@ -53,29 +54,38 @@ static int cmd_import(const char *line)
>  	struct argv_array svndump_argv = ARGV_ARRAY_INIT;
>  	struct child_process svndump_proc;
>  
> -	memset(&svndump_proc, 0, sizeof(struct child_process));
> -	svndump_proc.out = -1;
> -	argv_array_push(&svndump_argv, "svnrdump");
> -	argv_array_push(&svndump_argv, "dump");
> -	argv_array_push(&svndump_argv, url);
> -	argv_array_pushf(&svndump_argv, "-r%u:HEAD", startrev);
> -	svndump_proc.argv = svndump_argv.argv;
> -
> -	code = start_command(&svndump_proc);
> -	if (code)
> -		die("Unable to start %s, code %d", svndump_proc.argv[0], code);
> -	dumpin_fd = svndump_proc.out;
> -
> +	if (dump_from_file) {
> +		dumpin_fd = open(url, O_RDONLY);
> +		if(dumpin_fd < 0) {
> +			die_errno("Couldn't open svn dump file %s.", url);
> +		}
> +	}
> +	else {
> +		memset(&svndump_proc, 0, sizeof(struct child_process));


Style:

	if (dump_from_file) {
		dumpin_fd = open(url, O_RDONLY);
		if (dumpin_fd < 0)
                	die_errno("Couldn't...", url);
	} else {
        	memset(&svndump_proc, ...);
		...
	}

> +		svndump_proc.out = -1;
> +		argv_array_push(&svndump_argv, "svnrdump");
> +		argv_array_push(&svndump_argv, "dump");
> +		argv_array_push(&svndump_argv, url);
> +		argv_array_pushf(&svndump_argv, "-r%u:HEAD", startrev);
> +		svndump_proc.argv = svndump_argv.argv;
> +
> +		code = start_command(&svndump_proc);
> +		if (code)
> +			die("Unable to start %s, code %d", svndump_proc.argv[0], code);
> +		dumpin_fd = svndump_proc.out;
> +	}
>  	svndump_init_fd(dumpin_fd, STDIN_FILENO);
>  	svndump_read(url, private_ref);
>  	svndump_deinit();
>  	svndump_reset();
>  
>  	close(dumpin_fd);
> -	code = finish_command(&svndump_proc);
> -	if (code)
> -		warning("%s, returned %d", svndump_proc.argv[0], code);
> -	argv_array_clear(&svndump_argv);
> +	if(!dump_from_file) {

Style:

	if (!dump_from_file) {

> +		code = finish_command(&svndump_proc);
> +		if (code)
> +			warning("%s, returned %d", svndump_proc.argv[0], code);
> +		argv_array_clear(&svndump_argv);
> +	}
>  
>  	return 0;
>  }
> @@ -149,8 +159,15 @@ int main(int argc, const char **argv)
>  	remote = remote_get(argv[1]);
>  	url_in = (argc == 3) ? argv[2] : remote->url[0];
>  
> -	end_url_with_slash(&buf, url_in);
> -	url = strbuf_detach(&buf, NULL);
> +	if (!prefixcmp(url_in, "file://")) {
> +		dump_from_file = 1;
> +		url = url_decode(url_in + sizeof("file://")-1);
> +	}
> +	else {

Style:

	if (...) {
		...
        } else {
		...
	}

> +		dump_from_file = 0;
> +		end_url_with_slash(&buf, url_in);
> +		url = strbuf_detach(&buf, NULL);
> +	}
>  
>  	strbuf_addf(&buf, "refs/svn/%s/master", remote->name);
>  	private_ref = strbuf_detach(&buf, NULL);

Re: [PATCH v7 08/16] remote-svn, vcs-svn: Enable fetching to private refs

[Junio C Hamano]

Florian Achleitner <florian.achleitner.2.6.31@gmail.com> writes:

> The reference to update by the fast-import stream is hard-coded.  When
> fetching from a remote the remote-helper shall update refs in a
> private namespace, i.e. a private subdir of refs/.  This namespace is
> defined by the 'refspec' capability, that the remote-helper advertises
> as a reply to the 'capabilities' command.
>
> Extend svndump and fast-export to allow passing the target ref.
> Update svn-fe to be compatible.
>
> Signed-off-by: Florian Achleitner <florian.achleitner.2.6.31@gmail.com>
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
> ---
> ...
> diff --git a/vcs-svn/svndump.c b/vcs-svn/svndump.c
> index d81a078..288bb42 100644
> --- a/vcs-svn/svndump.c
> +++ b/vcs-svn/svndump.c
> @@ -299,22 +299,22 @@ static void handle_node(void)
>  				node_ctx.text_length, &input);
>  }
>  
> -static void begin_revision(void)
> +static void begin_revision(const char *remote_ref)
>  {
>  	if (!rev_ctx.revision)	/* revision 0 gets no git commit. */
>  		return;
>  	fast_export_begin_commit(rev_ctx.revision, rev_ctx.author.buf,
>  		&rev_ctx.log, dump_ctx.uuid.buf, dump_ctx.url.buf,
> -		rev_ctx.timestamp);
> +		rev_ctx.timestamp, remote_ref);
>  }
>  
> -static void end_revision(void)
> +static void end_revision()

Don't.

>  {
>  	if (rev_ctx.revision)
>  		fast_export_end_commit(rev_ctx.revision);
>  }

Re: [PATCH v7 03/16] Add svndump_init_fd to allow reading dumps from arbitrary FDs

[Junio C Hamano]

Florian Achleitner <florian.achleitner.2.6.31@gmail.com> writes:

> The existing function only allows reading from a filename or from
> stdin. Allow passing of a FD and an additional FD for the back report
> pipe. This allows us to retrieve the name of the pipe in the caller.
>
> Signed-off-by: Florian Achleitner <florian.achleitner.2.6.31@gmail.com>
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
> ---
>  vcs-svn/svndump.c |   22 ++++++++++++++++++----
>  vcs-svn/svndump.h |    1 +
>  2 files changed, 19 insertions(+), 4 deletions(-)
>
> diff --git a/vcs-svn/svndump.c b/vcs-svn/svndump.c
> index 2b168ae..d81a078 100644
> --- a/vcs-svn/svndump.c
> +++ b/vcs-svn/svndump.c
> @@ -468,11 +468,9 @@ void svndump_read(const char *url)
>  		end_revision();
>  }
>  
> -int svndump_init(const char *filename)
> +static void init(int report_fd)
>  {
> -	if (buffer_init(&input, filename))
> -		return error("cannot open %s: %s", filename, strerror(errno));
> -	fast_export_init(REPORT_FILENO);
> +	fast_export_init(report_fd);
>  	strbuf_init(&dump_ctx.uuid, 4096);
>  	strbuf_init(&dump_ctx.url, 4096);
>  	strbuf_init(&rev_ctx.log, 4096);
> @@ -482,6 +480,22 @@ int svndump_init(const char *filename)
>  	reset_dump_ctx(NULL);
>  	reset_rev_ctx(0);
>  	reset_node_ctx(NULL);
> +	return;
> +}
> +
> +int svndump_init(const char *filename)
> +{
> +	if (buffer_init(&input, filename))
> +		return error("cannot open %s: %s", filename ? filename : "NULL", strerror(errno));
> +	init(REPORT_FILENO);
> +	return 0;
> +}
> +
> +int svndump_init_fd(int in_fd, int back_fd)
> +{
> +	if(buffer_fdinit(&input, xdup(in_fd)))

Style:

	if (buffer_fdinit(&input, xdup(in_fd))

> +		return error("cannot open fd %d: %s", in_fd, strerror(errno));
> +	init(xdup(back_fd));
>  	return 0;
>  }
>  
> diff --git a/vcs-svn/svndump.h b/vcs-svn/svndump.h
> index df9ceb0..acb5b47 100644
> --- a/vcs-svn/svndump.h
> +++ b/vcs-svn/svndump.h
> @@ -2,6 +2,7 @@
>  #define SVNDUMP_H_
>  
>  int svndump_init(const char *filename);
> +int svndump_init_fd(int in_fd, int back_fd);
>  void svndump_read(const char *url);
>  void svndump_deinit(void);
>  void svndump_reset(void);

Re: [PATCH v7 02/16] Add git-remote-testsvn to Makefile

[Junio C Hamano]

Florian Achleitner <florian.achleitner.2.6.31@gmail.com> writes:

> The link-rule is a copy of the standard git$X rule but adds VCSSVN_LIB.
>
> Signed-off-by: Florian Achleitner <florian.achleitner.2.6.31@gmail.com>
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
> ---
>  Makefile |    5 +++++
>  1 file changed, 5 insertions(+)
>
> diff --git a/Makefile b/Makefile
> index 66e8216..1b09454 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -477,6 +477,7 @@ PROGRAM_OBJS += sh-i18n--envsubst.o
>  PROGRAM_OBJS += shell.o
>  PROGRAM_OBJS += show-index.o
>  PROGRAM_OBJS += upload-pack.o
> +PROGRAM_OBJS += remote-testsvn.o
>  
>  # Binary suffix, set to .exe for Windows builds
>  X =
> @@ -2352,6 +2353,10 @@ git-http-push$X: revision.o http.o http-push.o GIT-LDFLAGS $(GITLIBS)
>  	$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) \
>  		$(LIBS) $(CURL_LIBCURL) $(EXPAT_LIBEXPAT)
>  
> +git-remote-testsvn$X: remote-testsvn.o GIT-LDFLAGS $(GITLIBS) $(VCSSVN_LIB)
> +	$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) $(LIBS) \
> +	$(VCSSVN_LIB)
> +
>  $(REMOTE_CURL_ALIASES): $(REMOTE_CURL_PRIMARY)
>  	$(QUIET_LNCP)$(RM) $@ && \
>  	ln $< $@ 2>/dev/null || \

I'd squash in a change to add this executable to .gitignore
(attached) to this patch.  Unless you have objections to it, or
suggestions to do it better, there is no need to resend.

Thanks.

 .gitignore | 1 +
 1 file changed, 1 insertion(+)

diff --git i/.gitignore w/.gitignore
index bb5c91e..51294d6 100644
--- i/.gitignore
+++ w/.gitignore
@@ -125,6 +125,7 @@
 /git-remote-fd
 /git-remote-ext
 /git-remote-testgit
+/git-remote-testsvn
 /git-repack
 /git-replace
 /git-repo-config

Re: [PATCH v7 01/16] Implement a remote helper for svn in C

[Junio C Hamano]

Florian Achleitner <florian.achleitner.2.6.31@gmail.com> writes:

> Enable basic fetching from subversion repositories. When processing
> remote URLs starting with testsvn::, git invokes this remote-helper.
> It starts svnrdump to extract revisions from the subversion repository
> in the 'dump file format', and converts them to a git-fast-import stream
> using the functions of vcs-svn/.
>
> Imported refs are created in a private namespace at
> refs/svn/<remote-name>/master.  The revision history is imported
> linearly (no branch detection) and completely, i.e. from revision 0 to
> HEAD.
>
> The 'bidi-import' capability is used. The remote-helper expects data
> from fast-import on its stdin. It buffers a batch of 'import' command
> lines in a string_list before starting to process them.
>
> Signed-off-by: Florian Achleitner <florian.achleitner.2.6.31@gmail.com>
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
> ---
>  remote-testsvn.c |  174 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 174 insertions(+)
>  create mode 100644 remote-testsvn.c
>
> diff --git a/remote-testsvn.c b/remote-testsvn.c
> new file mode 100644
> index 0000000..ebe803b
> --- /dev/null
> +++ b/remote-testsvn.c
> @@ -0,0 +1,174 @@
> +#include "cache.h"
> +#include "remote.h"
> +#include "strbuf.h"
> +#include "url.h"
> +#include "exec_cmd.h"
> +#include "run-command.h"
> +#include "vcs-svn/svndump.h"
> +#include "notes.h"
> +#include "argv-array.h"
> +
> +static const char *url;
> +static const char *private_ref;
> +static const char *remote_ref = "refs/heads/master";
> +
> +static int cmd_capabilities(const char *line);
> +static int cmd_import(const char *line);
> +static int cmd_list(const char *line);
> +
> +typedef int (*input_command_handler)(const char *);
> +struct input_command_entry {
> +	const char *name;
> +	input_command_handler fn;
> +	unsigned char batchable;	/* whether the command starts or is part of a batch */
> +};
> +
> +static const struct input_command_entry input_command_list[] = {
> +	{ "capabilities", cmd_capabilities, 0 },
> +	{ "import", cmd_import, 1 },
> +	{ "list", cmd_list, 0 },
> +	{ NULL, NULL }
> +};
> +
> +static int cmd_capabilities(const char *line) {

Style:

	static int cmd_capabilities(const char *line)
        {

> +	printf("import\n");
> +	printf("bidi-import\n");
> +	printf("refspec %s:%s\n\n", remote_ref, private_ref);
> +	fflush(stdout);
> +	return 0;
> +}
> +
> +static void terminate_batch(void)
> +{
> +	/* terminate a current batch's fast-import stream */
> +	printf("done\n");
> +	fflush(stdout);
> +}
> +
> +static int cmd_import(const char *line)
> +{
> +	int code;
> +	int dumpin_fd;
> +	unsigned int startrev = 0;
> +	struct argv_array svndump_argv = ARGV_ARRAY_INIT;
> +	struct child_process svndump_proc;
> +
> +	memset(&svndump_proc, 0, sizeof(struct child_process));
> +	svndump_proc.out = -1;
> +	argv_array_push(&svndump_argv, "svnrdump");
> +	argv_array_push(&svndump_argv, "dump");
> +	argv_array_push(&svndump_argv, url);
> +	argv_array_pushf(&svndump_argv, "-r%u:HEAD", startrev);
> +	svndump_proc.argv = svndump_argv.argv;
> +
> +	code = start_command(&svndump_proc);
> +	if (code)
> +		die("Unable to start %s, code %d", svndump_proc.argv[0], code);
> +	dumpin_fd = svndump_proc.out;
> +
> +	svndump_init_fd(dumpin_fd, STDIN_FILENO);
> +	svndump_read(url, private_ref);
> +	svndump_deinit();
> +	svndump_reset();
> +
> +	close(dumpin_fd);
> +	code = finish_command(&svndump_proc);
> +	if (code)
> +		warning("%s, returned %d", svndump_proc.argv[0], code);
> +	argv_array_clear(&svndump_argv);
> +
> +	return 0;
> +}
> +
> +static int cmd_list(const char *line)
> +{
> +	printf("? %s\n\n", remote_ref);
> +	fflush(stdout);
> +	return 0;
> +}
> +
> +static int do_command(struct strbuf *line)
> +{
> +	const struct input_command_entry *p = input_command_list;
> +	static struct string_list batchlines = STRING_LIST_INIT_DUP;
> +	static const struct input_command_entry *batch_cmd;
> +	/*
> +	 * commands can be grouped together in a batch.
> +	 * Batches are ended by \n. If no batch is active the program ends.
> +	 * During a batch all lines are buffered and passed to the handler function
> +	 * when the batch is terminated.
> +	 */
> +	if (line->len == 0) {
> +		if (batch_cmd) {
> +			struct string_list_item *item;
> +			for_each_string_list_item(item, &batchlines)
> +				batch_cmd->fn(item->string);
> +			terminate_batch();
> +			batch_cmd = NULL;
> +			string_list_clear(&batchlines, 0);
> +			return 0;	/* end of the batch, continue reading other commands. */
> +		}
> +		return 1;	/* end of command stream, quit */
> +	}
> +	if (batch_cmd) {
> +		if (prefixcmp(batch_cmd->name, line->buf))
> +			die("Active %s batch interrupted by %s", batch_cmd->name, line->buf);
> +		/* buffer batch lines */
> +		string_list_append(&batchlines, line->buf);
> +		return 0;
> +	}
> +
> +	for (p = input_command_list; p->name; p++) {
> +		if (!prefixcmp(line->buf, p->name) && (strlen(p->name) == line->len ||
> +				line->buf[strlen(p->name)] == ' ')) {
> +			if (p->batchable) {
> +				batch_cmd = p;
> +				string_list_append(&batchlines, line->buf);
> +				return 0;
> +			}
> +			return p->fn(line->buf);
> +		}
> +	}
> +	die("Unknown command '%s'\n", line->buf);
> +	return 0;
> +}
> +
> +int main(int argc, const char **argv)
> +{
> +	struct strbuf buf = STRBUF_INIT;
> +	static struct remote *remote;
> +	const char *url_in;
> +
> +	git_extract_argv0_path(argv[0]);
> +	setup_git_directory();
> +	if (argc < 2 || argc > 3) {
> +		usage("git-remote-svn <remote-name> [<url>]");
> +		return 1;
> +	}
> +
> +	remote = remote_get(argv[1]);
> +	url_in = (argc == 3) ? argv[2] : remote->url[0];
> +
> +	end_url_with_slash(&buf, url_in);
> +	url = strbuf_detach(&buf, NULL);
> +
> +	strbuf_addf(&buf, "refs/svn/%s/master", remote->name);
> +	private_ref = strbuf_detach(&buf, NULL);
> +
> +	while(1) {

Style:

	while (1) {

> +		if (strbuf_getline(&buf, stdin, '\n') == EOF) {
> +			if (ferror(stdin))
> +				die("Error reading command stream");
> +			else
> +				die("Unexpected end of command stream");
> +		}
> +		if (do_command(&buf))
> +			break;
> +		strbuf_reset(&buf);
> +	}
> +
> +	strbuf_release(&buf);
> +	free((void*)url);
> +	free((void*)private_ref);

Style:

	free((void *)url);
	free((void *)private_ref);

[PATCH 12/16] adds another instance of this.

Having said that, it might be cleaner to prepare two strbuf
dedicated for these variables, borrow url and private_ref out of
them while they are used as the program level globals, and use
strbuf_release() at the end.

> +	return 0;
> +}

Re: [PATCH v7 00/16] GSOC remote-svn

[Florian Achleitner]

On Tuesday 28 August 2012 10:49:34 Florian Achleitner wrote:
> Reroll includes fixups by Ramsey. Thanks!
> Diff:
> [..]
> - improve compatibility of integer types.
> [..]

This line is wrong in this series. Just delete it. Sorry.