[PATCH 0/4] vcs-svn: remove repo_tree library

[PATCH 0/4] vcs-svn: remove repo_tree library

[Jonathan Nieder]

Hi,

Stefan noticed that repo_init from vcs-svn/repo_tree.h conflicts with
repository.h[1].  Earlier brian m. carlson noticed the same thing[2].

Originally repo_tree.h was used to manage an in-memory representation
of the state of the svn tree being imported.  When that in-memory
representation was retired, we were lazy and left some utility
functions there.  Here is a patch series to finish cleaning up and
remove vcs-svn/repo_tree.h completely.

This is an alternative to bc/vcs-svn-cleanup from 'next'.  Those
patches weren't cc-ed to me and I missed them --- sorry about that.  I
can rebase on top of them if that is more convenient.

Thoughts of all kinds welcome, as always.

Thanks,
Jonathan

Jonathan Nieder (4):
  vcs-svn: remove prototypes for missing functions
  vcs-svn: remove custom mode constants
  vcs-svn: remove repo_delete wrapper function
  vcs-svn: move remaining repo_tree functions to fast_export.h

 Makefile              |  1 -
 vcs-svn/fast_export.c | 41 +++++++++++++++++++++++++++++++++++++----
 vcs-svn/fast_export.h |  3 +++
 vcs-svn/repo_tree.c   | 48 ------------------------------------------------
 vcs-svn/repo_tree.h   | 23 -----------------------
 vcs-svn/svndump.c     | 33 ++++++++++++++++-----------------
 6 files changed, 56 insertions(+), 93 deletions(-)
 delete mode 100644 vcs-svn/repo_tree.c
 delete mode 100644 vcs-svn/repo_tree.h

[1] https://public-inbox.org/git/20170822213501.5928-1-sbeller@google.com
[2] https://public-inbox.org/git/20170821000022.26729-3-sandals@crustytoothpaste.net

[PATCH 1/4] vcs-svn: remove prototypes for missing functions

[Jonathan Nieder]

I forgot to remove these prototypes when removing these functions in
v1.7.10-rc0~118^2~4^2~5^2~4 (vcs-svn: eliminate repo_tree structure,
2010-12-10).

Noticed by building a new file that included both repo_tree.h and
repository.h ("error: conflicting types for 'repo_init'").

Reported-by: brian m. carlson <sandals@crustytoothpaste.net>
Reported-by: Stefan Beller <sbeller@google.com>
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
 vcs-svn/repo_tree.h | 10 ----------
 1 file changed, 10 deletions(-)

diff --git a/vcs-svn/repo_tree.h b/vcs-svn/repo_tree.h
index 889c6a3c95..6c2f5f8a00 100644
--- a/vcs-svn/repo_tree.h
+++ b/vcs-svn/repo_tree.h
@@ -1,23 +1,13 @@
 #ifndef REPO_TREE_H_
 #define REPO_TREE_H_
 
-struct strbuf;
-
 #define REPO_MODE_DIR 0040000
 #define REPO_MODE_BLB 0100644
 #define REPO_MODE_EXE 0100755
 #define REPO_MODE_LNK 0120000
 
-uint32_t next_blob_mark(void);
 void repo_copy(uint32_t revision, const char *src, const char *dst);
-void repo_add(const char *path, uint32_t mode, uint32_t blob_mark);
 const char *repo_read_path(const char *path, uint32_t *mode_out);
 void repo_delete(const char *path);
-void repo_commit(uint32_t revision, const char *author,
-		const struct strbuf *log, const char *uuid, const char *url,
-		long unsigned timestamp);
-void repo_diff(uint32_t r1, uint32_t r2);
-void repo_init(void);
-void repo_reset(void);
 
 #endif
-- 
2.14.1.342.g6490525c54

[PATCH 2/4] vcs-svn: remove custom mode constants

[Jonathan Nieder]

In the rest of Git, these modes are spelled as S_IFDIR,
S_IFREG | 0644, S_IFREG | 0755, and S_IFLNK.  Use the same constants
in svn-fe for simplicity and consistency.

No functional change intended.

Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
 vcs-svn/fast_export.c |  6 +++---
 vcs-svn/repo_tree.c   |  2 +-
 vcs-svn/repo_tree.h   |  5 -----
 vcs-svn/svndump.c     | 24 ++++++++++++------------
 4 files changed, 16 insertions(+), 21 deletions(-)

diff --git a/vcs-svn/fast_export.c b/vcs-svn/fast_export.c
index 5a89db30e3..7790f8e1d1 100644
--- a/vcs-svn/fast_export.c
+++ b/vcs-svn/fast_export.c
@@ -210,7 +210,7 @@ static long apply_delta(off_t len, struct line_buffer *input,
 			die("invalid cat-blob response: %s", response);
 		check_preimage_overflow(preimage.max_off, 1);
 	}
-	if (old_mode == REPO_MODE_LNK) {
+	if (old_mode == S_IFLNK) {
 		strbuf_addstr(&preimage.buf, "link ");
 		check_preimage_overflow(preimage.max_off, strlen("link "));
 		preimage.max_off += strlen("link ");
@@ -244,7 +244,7 @@ void fast_export_buf_to_data(const struct strbuf *data)
 void fast_export_data(uint32_t mode, off_t len, struct line_buffer *input)
 {
 	assert(len >= 0);
-	if (mode == REPO_MODE_LNK) {
+	if (mode == S_IFLNK) {
 		/* svn symlink blobs start with "link " */
 		if (len < 5)
 			die("invalid dump: symlink too short for \"link\" prefix");
@@ -320,7 +320,7 @@ void fast_export_blob_delta(uint32_t mode,
 
 	assert(len >= 0);
 	postimage_len = apply_delta(len, input, old_data, old_mode);
-	if (mode == REPO_MODE_LNK) {
+	if (mode == S_IFLNK) {
 		buffer_skip_bytes(&postimage, strlen("link "));
 		postimage_len -= strlen("link ");
 	}
diff --git a/vcs-svn/repo_tree.c b/vcs-svn/repo_tree.c
index 67d27f0b6c..9107f9663d 100644
--- a/vcs-svn/repo_tree.c
+++ b/vcs-svn/repo_tree.c
@@ -19,7 +19,7 @@ const char *repo_read_path(const char *path, uint32_t *mode_out)
 		if (errno != ENOENT)
 			die_errno("BUG: unexpected fast_export_ls error");
 		/* Treat missing paths as directories. */
-		*mode_out = REPO_MODE_DIR;
+		*mode_out = S_IFDIR;
 		return NULL;
 	}
 	return buf.buf;
diff --git a/vcs-svn/repo_tree.h b/vcs-svn/repo_tree.h
index 6c2f5f8a00..7f59fd9148 100644
--- a/vcs-svn/repo_tree.h
+++ b/vcs-svn/repo_tree.h
@@ -1,11 +1,6 @@
 #ifndef REPO_TREE_H_
 #define REPO_TREE_H_
 
-#define REPO_MODE_DIR 0040000
-#define REPO_MODE_BLB 0100644
-#define REPO_MODE_EXE 0100755
-#define REPO_MODE_LNK 0120000
-
 void repo_copy(uint32_t revision, const char *src, const char *dst);
 const char *repo_read_path(const char *path, uint32_t *mode_out);
 void repo_delete(const char *path);
diff --git a/vcs-svn/svndump.c b/vcs-svn/svndump.c
index 1846685a21..dc42ae3316 100644
--- a/vcs-svn/svndump.c
+++ b/vcs-svn/svndump.c
@@ -134,13 +134,13 @@ static void handle_property(const struct strbuf *key_buf,
 			die("invalid dump: sets type twice");
 		}
 		if (!val) {
-			node_ctx.type = REPO_MODE_BLB;
+			node_ctx.type = S_IFREG | 0644;
 			return;
 		}
 		*type_set = 1;
 		node_ctx.type = keylen == strlen("svn:executable") ?
-				REPO_MODE_EXE :
-				REPO_MODE_LNK;
+				(S_IFREG | 0755) :
+				S_IFLNK;
 	}
 }
 
@@ -219,7 +219,7 @@ static void handle_node(void)
 	 */
 	static const char *const empty_blob = "::empty::";
 	const char *old_data = NULL;
-	uint32_t old_mode = REPO_MODE_BLB;
+	uint32_t old_mode = S_IFREG | 0644;
 
 	if (node_ctx.action == NODEACT_DELETE) {
 		if (have_text || have_props || node_ctx.srcRev)
@@ -237,27 +237,27 @@ static void handle_node(void)
 		if (node_ctx.action == NODEACT_ADD)
 			node_ctx.action = NODEACT_CHANGE;
 	}
-	if (have_text && type == REPO_MODE_DIR)
+	if (have_text && type == S_IFDIR)
 		die("invalid dump: directories cannot have text attached");
 
 	/*
 	 * Find old content (old_data) and decide on the new mode.
 	 */
 	if (node_ctx.action == NODEACT_CHANGE && !*node_ctx.dst.buf) {
-		if (type != REPO_MODE_DIR)
+		if (type != S_IFDIR)
 			die("invalid dump: root of tree is not a regular file");
 		old_data = NULL;
 	} else if (node_ctx.action == NODEACT_CHANGE) {
 		uint32_t mode;
 		old_data = repo_read_path(node_ctx.dst.buf, &mode);
-		if (mode == REPO_MODE_DIR && type != REPO_MODE_DIR)
+		if (mode == S_IFDIR && type != S_IFDIR)
 			die("invalid dump: cannot modify a directory into a file");
-		if (mode != REPO_MODE_DIR && type == REPO_MODE_DIR)
+		if (mode != S_IFDIR && type == S_IFDIR)
 			die("invalid dump: cannot modify a file into a directory");
 		node_ctx.type = mode;
 		old_mode = mode;
 	} else if (node_ctx.action == NODEACT_ADD) {
-		if (type == REPO_MODE_DIR)
+		if (type == S_IFDIR)
 			old_data = NULL;
 		else if (have_text)
 			old_data = empty_blob;
@@ -280,7 +280,7 @@ static void handle_node(void)
 	/*
 	 * Save the result.
 	 */
-	if (type == REPO_MODE_DIR)	/* directories are not tracked. */
+	if (type == S_IFDIR)	/* directories are not tracked. */
 		return;
 	assert(old_data);
 	if (old_data == empty_blob)
@@ -385,9 +385,9 @@ void svndump_read(const char *url, const char *local_ref, const char *notes_ref)
 				continue;
 			strbuf_addf(&rev_ctx.note, "%s\n", t);
 			if (!strcmp(val, "dir"))
-				node_ctx.type = REPO_MODE_DIR;
+				node_ctx.type = S_IFDIR;
 			else if (!strcmp(val, "file"))
-				node_ctx.type = REPO_MODE_BLB;
+				node_ctx.type = S_IFREG | 0644;
 			else
 				fprintf(stderr, "Unknown node-kind: %s\n", val);
 			break;
-- 
2.14.1.342.g6490525c54

[PATCH 3/4] vcs-svn: remove repo_delete wrapper function

[Jonathan Nieder]

Since v1.7.10-rc0~118^2~4^2~4^2~3 (vcs-svn: pass paths through to
fast-import, 2010-12-13) this is an alias for fast_export_delete.
Remove the unnecessary layer of indirection.

No functional change intended.

Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
 vcs-svn/repo_tree.c | 5 -----
 vcs-svn/repo_tree.h | 1 -
 vcs-svn/svndump.c   | 4 ++--
 3 files changed, 2 insertions(+), 8 deletions(-)

diff --git a/vcs-svn/repo_tree.c b/vcs-svn/repo_tree.c
index 9107f9663d..99747e373a 100644
--- a/vcs-svn/repo_tree.c
+++ b/vcs-svn/repo_tree.c
@@ -41,8 +41,3 @@ void repo_copy(uint32_t revision, const char *src, const char *dst)
 	}
 	fast_export_modify(dst, mode, data.buf);
 }
-
-void repo_delete(const char *path)
-{
-	fast_export_delete(path);
-}
diff --git a/vcs-svn/repo_tree.h b/vcs-svn/repo_tree.h
index 7f59fd9148..56a3209d01 100644
--- a/vcs-svn/repo_tree.h
+++ b/vcs-svn/repo_tree.h
@@ -3,6 +3,5 @@
 
 void repo_copy(uint32_t revision, const char *src, const char *dst);
 const char *repo_read_path(const char *path, uint32_t *mode_out);
-void repo_delete(const char *path);
 
 #endif
diff --git a/vcs-svn/svndump.c b/vcs-svn/svndump.c
index dc42ae3316..d51136fac5 100644
--- a/vcs-svn/svndump.c
+++ b/vcs-svn/svndump.c
@@ -225,11 +225,11 @@ static void handle_node(void)
 		if (have_text || have_props || node_ctx.srcRev)
 			die("invalid dump: deletion node has "
 				"copyfrom info, text, or properties");
-		repo_delete(node_ctx.dst.buf);
+		fast_export_delete(node_ctx.dst.buf);
 		return;
 	}
 	if (node_ctx.action == NODEACT_REPLACE) {
-		repo_delete(node_ctx.dst.buf);
+		fast_export_delete(node_ctx.dst.buf);
 		node_ctx.action = NODEACT_ADD;
 	}
 	if (node_ctx.srcRev) {
-- 
2.14.1.342.g6490525c54

[PATCH 4/4] vcs-svn: move remaining repo_tree functions to fast_export.h

[Jonathan Nieder]

These used to be for manipulating the in-memory repo_tree structure,
but nowadays they are convenience wrappers to handle a few git-vs-svn
mismatches:

 1. Git does not track empty directories but Subversion does.  When
    looking up a path in git that Subversion thinks exists and finding
    nothing, we can safely assume that the path represents a
    directory.  This is needed when a later Subversion revision
    modifies that directory.

 2. Subversion allows deleting a file by copying.  In Git fast-import
    we have to handle that more explicitly as a deletion.

These are details of the tool's interaction with git fast-import.
Move them to fast_export.c, where other such details are handled.

This way the functions do not start with a repo_ prefix that would
clash with the repository object introduced in v2.14.0-rc0~38^2~16
(repository: introduce the repository object, 2017-06-22).

Reported-by: brian m. carlson <sandals@crustytoothpaste.net>
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
 Makefile              |  1 -
 vcs-svn/fast_export.c | 35 ++++++++++++++++++++++++++++++++++-
 vcs-svn/fast_export.h |  3 +++
 vcs-svn/repo_tree.c   | 43 -------------------------------------------
 vcs-svn/repo_tree.h   |  7 -------
 vcs-svn/svndump.c     |  5 ++---
 6 files changed, 39 insertions(+), 55 deletions(-)
 delete mode 100644 vcs-svn/repo_tree.c
 delete mode 100644 vcs-svn/repo_tree.h

diff --git a/Makefile b/Makefile
index 86ec29202b..493b8f5d2d 100644
--- a/Makefile
+++ b/Makefile
@@ -2038,7 +2038,6 @@ XDIFF_OBJS += xdiff/xhistogram.o
 
 VCSSVN_OBJS += vcs-svn/line_buffer.o
 VCSSVN_OBJS += vcs-svn/sliding_window.o
-VCSSVN_OBJS += vcs-svn/repo_tree.o
 VCSSVN_OBJS += vcs-svn/fast_export.o
 VCSSVN_OBJS += vcs-svn/svndiff.o
 VCSSVN_OBJS += vcs-svn/svndump.o
diff --git a/vcs-svn/fast_export.c b/vcs-svn/fast_export.c
index 7790f8e1d1..3fd047a8b8 100644
--- a/vcs-svn/fast_export.c
+++ b/vcs-svn/fast_export.c
@@ -6,7 +6,6 @@
 #include "cache.h"
 #include "quote.h"
 #include "fast_export.h"
-#include "repo_tree.h"
 #include "strbuf.h"
 #include "svndiff.h"
 #include "sliding_window.h"
@@ -312,6 +311,40 @@ int fast_export_ls(const char *path, uint32_t *mode, struct strbuf *dataref)
 	return parse_ls_response(get_response_line(), mode, dataref);
 }
 
+const char *fast_export_read_path(const char *path, uint32_t *mode_out)
+{
+	int err;
+	static struct strbuf buf = STRBUF_INIT;
+
+	strbuf_reset(&buf);
+	err = fast_export_ls(path, mode_out, &buf);
+	if (err) {
+		if (errno != ENOENT)
+			die_errno("BUG: unexpected fast_export_ls error");
+		/* Treat missing paths as directories. */
+		*mode_out = S_IFDIR;
+		return NULL;
+	}
+	return buf.buf;
+}
+
+void fast_export_copy(uint32_t revision, const char *src, const char *dst)
+{
+	int err;
+	uint32_t mode;
+	static struct strbuf data = STRBUF_INIT;
+
+	strbuf_reset(&data);
+	err = fast_export_ls_rev(revision, src, &mode, &data);
+	if (err) {
+		if (errno != ENOENT)
+			die_errno("BUG: unexpected fast_export_ls_rev error");
+		fast_export_delete(dst);
+		return;
+	}
+	fast_export_modify(dst, mode, data.buf);
+}
+
 void fast_export_blob_delta(uint32_t mode,
 				uint32_t old_mode, const char *old_data,
 				off_t len, struct line_buffer *input)
diff --git a/vcs-svn/fast_export.h b/vcs-svn/fast_export.h
index b9a3b71c99..60b79c35b9 100644
--- a/vcs-svn/fast_export.h
+++ b/vcs-svn/fast_export.h
@@ -28,4 +28,7 @@ int fast_export_ls_rev(uint32_t rev, const char *path,
 int fast_export_ls(const char *path,
 			uint32_t *mode_out, struct strbuf *dataref_out);
 
+void fast_export_copy(uint32_t revision, const char *src, const char *dst);
+const char *fast_export_read_path(const char *path, uint32_t *mode_out);
+
 #endif
diff --git a/vcs-svn/repo_tree.c b/vcs-svn/repo_tree.c
deleted file mode 100644
index 99747e373a..0000000000
--- a/vcs-svn/repo_tree.c
+++ /dev/null
@@ -1,43 +0,0 @@
-/*
- * Licensed under a two-clause BSD-style license.
- * See LICENSE for details.
- */
-
-#include "git-compat-util.h"
-#include "strbuf.h"
-#include "repo_tree.h"
-#include "fast_export.h"
-
-const char *repo_read_path(const char *path, uint32_t *mode_out)
-{
-	int err;
-	static struct strbuf buf = STRBUF_INIT;
-
-	strbuf_reset(&buf);
-	err = fast_export_ls(path, mode_out, &buf);
-	if (err) {
-		if (errno != ENOENT)
-			die_errno("BUG: unexpected fast_export_ls error");
-		/* Treat missing paths as directories. */
-		*mode_out = S_IFDIR;
-		return NULL;
-	}
-	return buf.buf;
-}
-
-void repo_copy(uint32_t revision, const char *src, const char *dst)
-{
-	int err;
-	uint32_t mode;
-	static struct strbuf data = STRBUF_INIT;
-
-	strbuf_reset(&data);
-	err = fast_export_ls_rev(revision, src, &mode, &data);
-	if (err) {
-		if (errno != ENOENT)
-			die_errno("BUG: unexpected fast_export_ls_rev error");
-		fast_export_delete(dst);
-		return;
-	}
-	fast_export_modify(dst, mode, data.buf);
-}
diff --git a/vcs-svn/repo_tree.h b/vcs-svn/repo_tree.h
deleted file mode 100644
index 56a3209d01..0000000000
--- a/vcs-svn/repo_tree.h
+++ /dev/null
@@ -1,7 +0,0 @@
-#ifndef REPO_TREE_H_
-#define REPO_TREE_H_
-
-void repo_copy(uint32_t revision, const char *src, const char *dst);
-const char *repo_read_path(const char *path, uint32_t *mode_out);
-
-#endif
diff --git a/vcs-svn/svndump.c b/vcs-svn/svndump.c
index d51136fac5..ec6b350611 100644
--- a/vcs-svn/svndump.c
+++ b/vcs-svn/svndump.c
@@ -8,7 +8,6 @@
  */
 
 #include "cache.h"
-#include "repo_tree.h"
 #include "fast_export.h"
 #include "line_buffer.h"
 #include "strbuf.h"
@@ -233,7 +232,7 @@ static void handle_node(void)
 		node_ctx.action = NODEACT_ADD;
 	}
 	if (node_ctx.srcRev) {
-		repo_copy(node_ctx.srcRev, node_ctx.src.buf, node_ctx.dst.buf);
+		fast_export_copy(node_ctx.srcRev, node_ctx.src.buf, node_ctx.dst.buf);
 		if (node_ctx.action == NODEACT_ADD)
 			node_ctx.action = NODEACT_CHANGE;
 	}
@@ -249,7 +248,7 @@ static void handle_node(void)
 		old_data = NULL;
 	} else if (node_ctx.action == NODEACT_CHANGE) {
 		uint32_t mode;
-		old_data = repo_read_path(node_ctx.dst.buf, &mode);
+		old_data = fast_export_read_path(node_ctx.dst.buf, &mode);
 		if (mode == S_IFDIR && type != S_IFDIR)
 			die("invalid dump: cannot modify a directory into a file");
 		if (mode != S_IFDIR && type == S_IFDIR)
-- 
2.14.1.342.g6490525c54

Re: [PATCH 0/4] vcs-svn: remove repo_tree library

[Stefan Beller]

On Tue, Aug 22, 2017 at 4:37 PM, Jonathan Nieder <jrnieder@gmail.com> wrote:
> Hi,
>
> Stefan noticed that repo_init from vcs-svn/repo_tree.h conflicts with
> repository.h[1].  Earlier brian m. carlson noticed the same thing[2].
>
> Originally repo_tree.h was used to manage an in-memory representation
> of the state of the svn tree being imported.  When that in-memory
> representation was retired, we were lazy and left some utility
> functions there.  Here is a patch series to finish cleaning up and
> remove vcs-svn/repo_tree.h completely.
>
> This is an alternative to bc/vcs-svn-cleanup from 'next'.  Those
> patches weren't cc-ed to me and I missed them --- sorry about that.  I
> can rebase on top of them if that is more convenient.

A rebased version would be easier IIUC Junios reply to
the one-off that I sent.

The patches look good.

Thanks,
Stefan

>
> Thoughts of all kinds welcome, as always.
>
> Thanks,
> Jonathan
>
> Jonathan Nieder (4):
>   vcs-svn: remove prototypes for missing functions
>   vcs-svn: remove custom mode constants
>   vcs-svn: remove repo_delete wrapper function
>   vcs-svn: move remaining repo_tree functions to fast_export.h
>
>  Makefile              |  1 -
>  vcs-svn/fast_export.c | 41 +++++++++++++++++++++++++++++++++++++----
>  vcs-svn/fast_export.h |  3 +++
>  vcs-svn/repo_tree.c   | 48 ------------------------------------------------
>  vcs-svn/repo_tree.h   | 23 -----------------------
>  vcs-svn/svndump.c     | 33 ++++++++++++++++-----------------
>  6 files changed, 56 insertions(+), 93 deletions(-)
>  delete mode 100644 vcs-svn/repo_tree.c
>  delete mode 100644 vcs-svn/repo_tree.h
>
> [1] https://public-inbox.org/git/20170822213501.5928-1-sbeller@google.com
> [2] https://public-inbox.org/git/20170821000022.26729-3-sandals@crustytoothpaste.net

[PATCH v2 bc/vcs-svn-cleanup] vcs-svn: remove repo_tree library

[Jonathan Nieder]

Hi again,

Jonathan Nieder wrote:

> This is an alternative to bc/vcs-svn-cleanup from 'next'.  Those
> patches weren't cc-ed to me and I missed them --- sorry about that.  I
> can rebase on top of them if that is more convenient.

Here is the same series rebased on top of bc/vcs-svn-cleanup.

Thoughts welcome, as always.

Jonathan Nieder (4):
  vcs-svn: remove more unused prototypes and declarations
  vcs-svn: remove custom mode constants
  vcs-svn: remove repo_delete wrapper function
  vcs-svn: move remaining repo_tree functions to fast_export.h

 Makefile              |  1 -
 vcs-svn/fast_export.c | 41 +++++++++++++++++++++++++++++++++++++----
 vcs-svn/fast_export.h |  3 +++
 vcs-svn/repo_tree.c   | 48 ------------------------------------------------
 vcs-svn/repo_tree.h   | 16 ----------------
 vcs-svn/svndump.c     | 33 ++++++++++++++++-----------------
 6 files changed, 56 insertions(+), 86 deletions(-)
 delete mode 100644 vcs-svn/repo_tree.c
 delete mode 100644 vcs-svn/repo_tree.h

> [1] https://public-inbox.org/git/20170822213501.5928-1-sbeller@google.com
> [2] https://public-inbox.org/git/20170821000022.26729-3-sandals@crustytoothpaste.net

[PATCH v2 1/4] vcs-svn: remove more unused prototypes and declarations

[Jonathan Nieder]

I forgot to remove these in v1.7.10-rc0~118^2~4^2~5^2~4 (vcs-svn:
eliminate repo_tree structure, 2010-12-10).

This finishes what was started in commit 36f63b50 (vcs-svn: remove
unused prototypes, 2017-08-21).

Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
 vcs-svn/repo_tree.h | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/vcs-svn/repo_tree.h b/vcs-svn/repo_tree.h
index 555b64bbb6..0d3bbb677d 100644
--- a/vcs-svn/repo_tree.h
+++ b/vcs-svn/repo_tree.h
@@ -1,14 +1,11 @@
 #ifndef REPO_TREE_H_
 #define REPO_TREE_H_
 
-struct strbuf;
-
 #define REPO_MODE_DIR 0040000
 #define REPO_MODE_BLB 0100644
 #define REPO_MODE_EXE 0100755
 #define REPO_MODE_LNK 0120000
 
-uint32_t next_blob_mark(void);
 void svn_repo_copy(uint32_t revision, const char *src, const char *dst);
 const char *svn_repo_read_path(const char *path, uint32_t *mode_out);
 void svn_repo_delete(const char *path);
-- 
2.14.1.342.g6490525c54

[PATCH v2 2/4] vcs-svn: remove custom mode constants

[Jonathan Nieder]

In the rest of Git, these modes are spelled as S_IFDIR,
S_IFREG | 0644, S_IFREG | 0755, and S_IFLNK.  Use the same constants
in svn-fe for simplicity and consistency.

No functional change intended.

Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
Unchanged.

 vcs-svn/fast_export.c |  6 +++---
 vcs-svn/repo_tree.c   |  2 +-
 vcs-svn/repo_tree.h   |  5 -----
 vcs-svn/svndump.c     | 24 ++++++++++++------------
 4 files changed, 16 insertions(+), 21 deletions(-)

diff --git a/vcs-svn/fast_export.c b/vcs-svn/fast_export.c
index 97cba39cdf..6d133ed6bc 100644
--- a/vcs-svn/fast_export.c
+++ b/vcs-svn/fast_export.c
@@ -210,7 +210,7 @@ static long apply_delta(off_t len, struct line_buffer *input,
 			die("invalid cat-blob response: %s", response);
 		check_preimage_overflow(preimage.max_off, 1);
 	}
-	if (old_mode == REPO_MODE_LNK) {
+	if (old_mode == S_IFLNK) {
 		strbuf_addstr(&preimage.buf, "link ");
 		check_preimage_overflow(preimage.max_off, strlen("link "));
 		preimage.max_off += strlen("link ");
@@ -244,7 +244,7 @@ void fast_export_buf_to_data(const struct strbuf *data)
 void fast_export_data(uint32_t mode, off_t len, struct line_buffer *input)
 {
 	assert(len >= 0);
-	if (mode == REPO_MODE_LNK) {
+	if (mode == S_IFLNK) {
 		/* svn symlink blobs start with "link " */
 		if (len < 5)
 			die("invalid dump: symlink too short for \"link\" prefix");
@@ -320,7 +320,7 @@ void fast_export_blob_delta(uint32_t mode,
 
 	assert(len >= 0);
 	postimage_len = apply_delta(len, input, old_data, old_mode);
-	if (mode == REPO_MODE_LNK) {
+	if (mode == S_IFLNK) {
 		buffer_skip_bytes(&postimage, strlen("link "));
 		postimage_len -= strlen("link ");
 	}
diff --git a/vcs-svn/repo_tree.c b/vcs-svn/repo_tree.c
index d77cb0ada7..1a6f32d7cb 100644
--- a/vcs-svn/repo_tree.c
+++ b/vcs-svn/repo_tree.c
@@ -19,7 +19,7 @@ const char *svn_repo_read_path(const char *path, uint32_t *mode_out)
 		if (errno != ENOENT)
 			die_errno("BUG: unexpected fast_export_ls error");
 		/* Treat missing paths as directories. */
-		*mode_out = REPO_MODE_DIR;
+		*mode_out = S_IFDIR;
 		return NULL;
 	}
 	return buf.buf;
diff --git a/vcs-svn/repo_tree.h b/vcs-svn/repo_tree.h
index 0d3bbb677d..c840bc9bae 100644
--- a/vcs-svn/repo_tree.h
+++ b/vcs-svn/repo_tree.h
@@ -1,11 +1,6 @@
 #ifndef REPO_TREE_H_
 #define REPO_TREE_H_
 
-#define REPO_MODE_DIR 0040000
-#define REPO_MODE_BLB 0100644
-#define REPO_MODE_EXE 0100755
-#define REPO_MODE_LNK 0120000
-
 void svn_repo_copy(uint32_t revision, const char *src, const char *dst);
 const char *svn_repo_read_path(const char *path, uint32_t *mode_out);
 void svn_repo_delete(const char *path);
diff --git a/vcs-svn/svndump.c b/vcs-svn/svndump.c
index 7da84b2aab..c0fa4eb723 100644
--- a/vcs-svn/svndump.c
+++ b/vcs-svn/svndump.c
@@ -134,13 +134,13 @@ static void handle_property(const struct strbuf *key_buf,
 			die("invalid dump: sets type twice");
 		}
 		if (!val) {
-			node_ctx.type = REPO_MODE_BLB;
+			node_ctx.type = S_IFREG | 0644;
 			return;
 		}
 		*type_set = 1;
 		node_ctx.type = keylen == strlen("svn:executable") ?
-				REPO_MODE_EXE :
-				REPO_MODE_LNK;
+				(S_IFREG | 0755) :
+				S_IFLNK;
 	}
 }
 
@@ -219,7 +219,7 @@ static void handle_node(void)
 	 */
 	static const char *const empty_blob = "::empty::";
 	const char *old_data = NULL;
-	uint32_t old_mode = REPO_MODE_BLB;
+	uint32_t old_mode = S_IFREG | 0644;
 
 	if (node_ctx.action == NODEACT_DELETE) {
 		if (have_text || have_props || node_ctx.srcRev)
@@ -237,27 +237,27 @@ static void handle_node(void)
 		if (node_ctx.action == NODEACT_ADD)
 			node_ctx.action = NODEACT_CHANGE;
 	}
-	if (have_text && type == REPO_MODE_DIR)
+	if (have_text && type == S_IFDIR)
 		die("invalid dump: directories cannot have text attached");
 
 	/*
 	 * Find old content (old_data) and decide on the new mode.
 	 */
 	if (node_ctx.action == NODEACT_CHANGE && !*node_ctx.dst.buf) {
-		if (type != REPO_MODE_DIR)
+		if (type != S_IFDIR)
 			die("invalid dump: root of tree is not a regular file");
 		old_data = NULL;
 	} else if (node_ctx.action == NODEACT_CHANGE) {
 		uint32_t mode;
 		old_data = svn_repo_read_path(node_ctx.dst.buf, &mode);
-		if (mode == REPO_MODE_DIR && type != REPO_MODE_DIR)
+		if (mode == S_IFDIR && type != S_IFDIR)
 			die("invalid dump: cannot modify a directory into a file");
-		if (mode != REPO_MODE_DIR && type == REPO_MODE_DIR)
+		if (mode != S_IFDIR && type == S_IFDIR)
 			die("invalid dump: cannot modify a file into a directory");
 		node_ctx.type = mode;
 		old_mode = mode;
 	} else if (node_ctx.action == NODEACT_ADD) {
-		if (type == REPO_MODE_DIR)
+		if (type == S_IFDIR)
 			old_data = NULL;
 		else if (have_text)
 			old_data = empty_blob;
@@ -280,7 +280,7 @@ static void handle_node(void)
 	/*
 	 * Save the result.
 	 */
-	if (type == REPO_MODE_DIR)	/* directories are not tracked. */
+	if (type == S_IFDIR)	/* directories are not tracked. */
 		return;
 	assert(old_data);
 	if (old_data == empty_blob)
@@ -385,9 +385,9 @@ void svndump_read(const char *url, const char *local_ref, const char *notes_ref)
 				continue;
 			strbuf_addf(&rev_ctx.note, "%s\n", t);
 			if (!strcmp(val, "dir"))
-				node_ctx.type = REPO_MODE_DIR;
+				node_ctx.type = S_IFDIR;
 			else if (!strcmp(val, "file"))
-				node_ctx.type = REPO_MODE_BLB;
+				node_ctx.type = S_IFREG | 0644;
 			else
 				fprintf(stderr, "Unknown node-kind: %s\n", val);
 			break;
-- 
2.14.1.342.g6490525c54

[PATCH v2 3/4] vcs-svn: remove repo_delete wrapper function

[Jonathan Nieder]

Since v1.7.10-rc0~118^2~4^2~4^2~3 (vcs-svn: pass paths through to
fast-import, 2010-12-13) this is an alias for fast_export_delete.
Remove the unnecessary layer of indirection.

No functional change intended.

Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
Unchanged.

 vcs-svn/repo_tree.c | 5 -----
 vcs-svn/repo_tree.h | 1 -
 vcs-svn/svndump.c   | 4 ++--
 3 files changed, 2 insertions(+), 8 deletions(-)

diff --git a/vcs-svn/repo_tree.c b/vcs-svn/repo_tree.c
index 1a6f32d7cb..5bd4977cb6 100644
--- a/vcs-svn/repo_tree.c
+++ b/vcs-svn/repo_tree.c
@@ -41,8 +41,3 @@ void svn_repo_copy(uint32_t revision, const char *src, const char *dst)
 	}
 	fast_export_modify(dst, mode, data.buf);
 }
-
-void svn_repo_delete(const char *path)
-{
-	fast_export_delete(path);
-}
diff --git a/vcs-svn/repo_tree.h b/vcs-svn/repo_tree.h
index c840bc9bae..0cd2761183 100644
--- a/vcs-svn/repo_tree.h
+++ b/vcs-svn/repo_tree.h
@@ -3,6 +3,5 @@
 
 void svn_repo_copy(uint32_t revision, const char *src, const char *dst);
 const char *svn_repo_read_path(const char *path, uint32_t *mode_out);
-void svn_repo_delete(const char *path);
 
 #endif
diff --git a/vcs-svn/svndump.c b/vcs-svn/svndump.c
index c0fa4eb723..41113119bd 100644
--- a/vcs-svn/svndump.c
+++ b/vcs-svn/svndump.c
@@ -225,11 +225,11 @@ static void handle_node(void)
 		if (have_text || have_props || node_ctx.srcRev)
 			die("invalid dump: deletion node has "
 				"copyfrom info, text, or properties");
-		svn_repo_delete(node_ctx.dst.buf);
+		fast_export_delete(node_ctx.dst.buf);
 		return;
 	}
 	if (node_ctx.action == NODEACT_REPLACE) {
-		svn_repo_delete(node_ctx.dst.buf);
+		fast_export_delete(node_ctx.dst.buf);
 		node_ctx.action = NODEACT_ADD;
 	}
 	if (node_ctx.srcRev) {
-- 
2.14.1.342.g6490525c54

[PATCH v2 4/4] vcs-svn: move remaining repo_tree functions to fast_export.h

[Jonathan Nieder]

These used to be for manipulating the in-memory repo_tree structure,
but nowadays they are convenience wrappers to handle a few git-vs-svn
mismatches:

 1. Git does not track empty directories but Subversion does.  When
    looking up a path in git that Subversion thinks exists and finding
    nothing, we can safely assume that the path represents a
    directory.  This is needed when a later Subversion revision
    modifies that directory.

 2. Subversion allows deleting a file by copying.  In Git fast-import
    we have to handle that more explicitly as a deletion.

These are details of the tool's interaction with git fast-import.
Move them to fast_export.c, where other such details are handled.

This way the function names do not start with a repo_ prefix that
would clash with the repository object introduced in
v2.14.0-rc0~38^2~16 (repository: introduce the repository object,
2017-06-22) or an svn_ prefix that would clash with libsvn (in case
someone wants to link this code with libsvn some day).

Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
The only change is the commit message.  These functions are already
namespaced on the bc/vcs-svn-cleanup, so added a note about that.

That's the end of the series.  Thanks for reading.

 Makefile              |  1 -
 vcs-svn/fast_export.c | 35 ++++++++++++++++++++++++++++++++++-
 vcs-svn/fast_export.h |  3 +++
 vcs-svn/repo_tree.c   | 43 -------------------------------------------
 vcs-svn/repo_tree.h   |  7 -------
 vcs-svn/svndump.c     |  5 ++---
 6 files changed, 39 insertions(+), 55 deletions(-)
 delete mode 100644 vcs-svn/repo_tree.c
 delete mode 100644 vcs-svn/repo_tree.h

diff --git a/Makefile b/Makefile
index 46ad908ec5..9b00d5b219 100644
--- a/Makefile
+++ b/Makefile
@@ -1942,7 +1942,6 @@ XDIFF_OBJS += xdiff/xhistogram.o
 
 VCSSVN_OBJS += vcs-svn/line_buffer.o
 VCSSVN_OBJS += vcs-svn/sliding_window.o
-VCSSVN_OBJS += vcs-svn/repo_tree.o
 VCSSVN_OBJS += vcs-svn/fast_export.o
 VCSSVN_OBJS += vcs-svn/svndiff.o
 VCSSVN_OBJS += vcs-svn/svndump.o
diff --git a/vcs-svn/fast_export.c b/vcs-svn/fast_export.c
index 6d133ed6bc..5bd455b8c8 100644
--- a/vcs-svn/fast_export.c
+++ b/vcs-svn/fast_export.c
@@ -6,7 +6,6 @@
 #include "cache.h"
 #include "quote.h"
 #include "fast_export.h"
-#include "repo_tree.h"
 #include "strbuf.h"
 #include "svndiff.h"
 #include "sliding_window.h"
@@ -312,6 +311,40 @@ int fast_export_ls(const char *path, uint32_t *mode, struct strbuf *dataref)
 	return parse_ls_response(get_response_line(), mode, dataref);
 }
 
+const char *fast_export_read_path(const char *path, uint32_t *mode_out)
+{
+	int err;
+	static struct strbuf buf = STRBUF_INIT;
+
+	strbuf_reset(&buf);
+	err = fast_export_ls(path, mode_out, &buf);
+	if (err) {
+		if (errno != ENOENT)
+			die_errno("BUG: unexpected fast_export_ls error");
+		/* Treat missing paths as directories. */
+		*mode_out = S_IFDIR;
+		return NULL;
+	}
+	return buf.buf;
+}
+
+void fast_export_copy(uint32_t revision, const char *src, const char *dst)
+{
+	int err;
+	uint32_t mode;
+	static struct strbuf data = STRBUF_INIT;
+
+	strbuf_reset(&data);
+	err = fast_export_ls_rev(revision, src, &mode, &data);
+	if (err) {
+		if (errno != ENOENT)
+			die_errno("BUG: unexpected fast_export_ls_rev error");
+		fast_export_delete(dst);
+		return;
+	}
+	fast_export_modify(dst, mode, data.buf);
+}
+
 void fast_export_blob_delta(uint32_t mode,
 				uint32_t old_mode, const char *old_data,
 				off_t len, struct line_buffer *input)
diff --git a/vcs-svn/fast_export.h b/vcs-svn/fast_export.h
index c8b5adb811..ae8ab7e589 100644
--- a/vcs-svn/fast_export.h
+++ b/vcs-svn/fast_export.h
@@ -28,4 +28,7 @@ int fast_export_ls_rev(uint32_t rev, const char *path,
 int fast_export_ls(const char *path,
 			uint32_t *mode_out, struct strbuf *dataref_out);
 
+void fast_export_copy(uint32_t revision, const char *src, const char *dst);
+const char *fast_export_read_path(const char *path, uint32_t *mode_out);
+
 #endif
diff --git a/vcs-svn/repo_tree.c b/vcs-svn/repo_tree.c
deleted file mode 100644
index 5bd4977cb6..0000000000
--- a/vcs-svn/repo_tree.c
+++ /dev/null
@@ -1,43 +0,0 @@
-/*
- * Licensed under a two-clause BSD-style license.
- * See LICENSE for details.
- */
-
-#include "git-compat-util.h"
-#include "strbuf.h"
-#include "repo_tree.h"
-#include "fast_export.h"
-
-const char *svn_repo_read_path(const char *path, uint32_t *mode_out)
-{
-	int err;
-	static struct strbuf buf = STRBUF_INIT;
-
-	strbuf_reset(&buf);
-	err = fast_export_ls(path, mode_out, &buf);
-	if (err) {
-		if (errno != ENOENT)
-			die_errno("BUG: unexpected fast_export_ls error");
-		/* Treat missing paths as directories. */
-		*mode_out = S_IFDIR;
-		return NULL;
-	}
-	return buf.buf;
-}
-
-void svn_repo_copy(uint32_t revision, const char *src, const char *dst)
-{
-	int err;
-	uint32_t mode;
-	static struct strbuf data = STRBUF_INIT;
-
-	strbuf_reset(&data);
-	err = fast_export_ls_rev(revision, src, &mode, &data);
-	if (err) {
-		if (errno != ENOENT)
-			die_errno("BUG: unexpected fast_export_ls_rev error");
-		fast_export_delete(dst);
-		return;
-	}
-	fast_export_modify(dst, mode, data.buf);
-}
diff --git a/vcs-svn/repo_tree.h b/vcs-svn/repo_tree.h
deleted file mode 100644
index 0cd2761183..0000000000
--- a/vcs-svn/repo_tree.h
+++ /dev/null
@@ -1,7 +0,0 @@
-#ifndef REPO_TREE_H_
-#define REPO_TREE_H_
-
-void svn_repo_copy(uint32_t revision, const char *src, const char *dst);
-const char *svn_repo_read_path(const char *path, uint32_t *mode_out);
-
-#endif
diff --git a/vcs-svn/svndump.c b/vcs-svn/svndump.c
index 41113119bd..01b6ded400 100644
--- a/vcs-svn/svndump.c
+++ b/vcs-svn/svndump.c
@@ -8,7 +8,6 @@
  */
 
 #include "cache.h"
-#include "repo_tree.h"
 #include "fast_export.h"
 #include "line_buffer.h"
 #include "strbuf.h"
@@ -233,7 +232,7 @@ static void handle_node(void)
 		node_ctx.action = NODEACT_ADD;
 	}
 	if (node_ctx.srcRev) {
-		svn_repo_copy(node_ctx.srcRev, node_ctx.src.buf, node_ctx.dst.buf);
+		fast_export_copy(node_ctx.srcRev, node_ctx.src.buf, node_ctx.dst.buf);
 		if (node_ctx.action == NODEACT_ADD)
 			node_ctx.action = NODEACT_CHANGE;
 	}
@@ -249,7 +248,7 @@ static void handle_node(void)
 		old_data = NULL;
 	} else if (node_ctx.action == NODEACT_CHANGE) {
 		uint32_t mode;
-		old_data = svn_repo_read_path(node_ctx.dst.buf, &mode);
+		old_data = fast_export_read_path(node_ctx.dst.buf, &mode);
 		if (mode == S_IFDIR && type != S_IFDIR)
 			die("invalid dump: cannot modify a directory into a file");
 		if (mode != S_IFDIR && type == S_IFDIR)
-- 
2.14.1.342.g6490525c54