Skip to contentSkip to conversation

Foreign VCS helper program for CVS repositories

RFC Superseded Johan Herland 5 participants 30 messages Jun 15, 2016

Subsystems (2)

v1 of 2 Superseded Latest is v2 →

lore
Share & tools
lore
#0 lore raw

cover Foreign VCS helper program for CVS repositories

This series is a first draft towards implementing a foreign VCS helper program for CVS.

It is based on the 'db/foreign-scm' and 'jh/notes' topics in 'pu'. As such, this patch series should apply cleanly to current 'pu'.

The first 4 patches supply the necessary parts of Daniel Barkalow's foreign-scm topic that has not yet been merged to 'pu' (i.e. everything remaining in Daniel's original patch series, except the p4-specific stuff).

The next 2 patches add some functionality to git fast-import, for facilitating the import of 'notes' objects (this is a _much_ faster way to generate/import notes than invoking "git notes edit -m MSG" for each note).

Next, there are 3 patches tweaking and expanding the git-vcs API (with corresponding implementations in the foreign transport code) to adjust for the CVS helper's needs.

The final 2 patches add the git-vcs-cvs helper program that implements the fetch/import of objects from a local or remote CVS repository, along with selftests for verifying its correctness in some simple cases. This is a first RFC/draft of this helper program, and will hopefully grow along with the foreign-scm feature itself until it provides a compelling alternative to "git cvsimport" and "git cvsexportcommit".

Daniel Barkalow (4):
Add specification of git-vcs-* helper programs Use a function to determine whether a remote is valid Allow programs to not depend on remotes having urls Add a transport implementation using git-vcs-* helpers

Johan Herland (7):
Refactor path name parsing into new function: get_path_str() Add support for mark references as path names
Preliminary clarifications to git-vcs documentation
Teach foreign transport code to perform the "capabilities" command Introduce a 'marks <filename>' feature to the foreign transport code First draft of CVS importer using the foreign-scm machinery Add simple test cases of git-vcs-cvs functionality

 Documentation/git-fast-import.txt |    9 +-
 Documentation/git-vcs-cvs.txt     |   85 ++++
 Documentation/git-vcs.txt         |   88 ++++
 Makefile                          |   47 ++
 builtin-fetch.c                   |   19 +-
 builtin-ls-remote.c               |    4 +-
 builtin-push.c                    |   54 ++-
 configure.ac                      |    3 +
 fast-import.c                     |   71 ++--
 git-vcs-cvs.py                    |  697 +++++++++++++++++++++++++++++
 git_vcs_cvs/.gitignore            |    2 +
 git_vcs_cvs/Makefile              |   27 ++
 git_vcs_cvs/changeset.py          |  114 +++++
 git_vcs_cvs/commit_states.py      |   52 +++
 git_vcs_cvs/cvs.py                |  884 +++++++++++++++++++++++++++++++++++++
 git_vcs_cvs/cvs_revision_map.py   |  367 +++++++++++++++
 git_vcs_cvs/cvs_symbol_cache.py   |  283 ++++++++++++
 git_vcs_cvs/git.py                |  588 ++++++++++++++++++++++++
 git_vcs_cvs/setup.py              |   12 +
 git_vcs_cvs/util.py               |  147 ++++++
 remote.c                          |   13 +-
 t/t9800-foreign-vcs-cvs-basic.sh  |  518 ++++++++++++++++++++++
 t/t9801-foreign-vcs-cvs-fetch.sh  |  291 ++++++++++++
 t/test-lib.sh                     |    1 +
 transport-foreign.c               |  271 ++++++++++++
 transport.c                       |    4 +
 transport.h                       |    1 +
 27 files changed, 4588 insertions(+), 64 deletions(-)
 create mode 100644 Documentation/git-vcs-cvs.txt
 create mode 100644 Documentation/git-vcs.txt
 create mode 100755 git-vcs-cvs.py
 create mode 100644 git_vcs_cvs/.gitignore
 create mode 100644 git_vcs_cvs/Makefile
 create mode 100644 git_vcs_cvs/__init__.py
 create mode 100644 git_vcs_cvs/changeset.py
 create mode 100644 git_vcs_cvs/commit_states.py
 create mode 100644 git_vcs_cvs/cvs.py
 create mode 100644 git_vcs_cvs/cvs_revision_map.py
 create mode 100644 git_vcs_cvs/cvs_symbol_cache.py
 create mode 100644 git_vcs_cvs/git.py
 create mode 100644 git_vcs_cvs/setup.py
 create mode 100644 git_vcs_cvs/util.py
 create mode 100755 t/t9800-foreign-vcs-cvs-basic.sh
 create mode 100755 t/t9801-foreign-vcs-cvs-fetch.sh
 create mode 100644 transport-foreign.c

Have fun! :)

...Johan

#1 lore raw

RFC 1/11 Add specification of git-vcs-* helper programs

From: Daniel Barkalow <[email protected]>

---
 Documentation/git-vcs.txt |   77 +++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 77 insertions(+), 0 deletions(-)
 create mode 100644 Documentation/git-vcs.txt
Documentation/git-vcs.txt new file +77 -0
new file mode 100644
index 0000000..402c927
@@ -0,0 +1,77 @@
+git-vcs(1)
+============
+
+NAME
+----
+git-vcs - Helper programs for interoperation with foreign systems
+
+SYNOPSIS
+--------
+'git vcs-<system>' <remote>
+
+DESCRIPTION
+-----------
+
+These programs are normally not used directly by end users, but are
+invoked by various git programs that interact with remote repositories
+when the repository they would operate on uses a foreign system.
+
+Each 'git vcs-<system>' is a helper for interoperating with a
+particular version control system. Different helpers have different
+capabilities (limited both by the particular helper and by the
+capabilities of the system they connect to), and they report what
+capabilities they support.
+
+These programs can store refs in refs/<system>/*, and arbitrary
+information in info/<system>.
+
+COMMANDS
+--------
+
+Commands are given by the caller on the helper's standard input, one per line.
+
+'capabilities'::
+	Outputs a single line with a list of feature names separated
+	by spaces. Each of these indicates a supported feature of the
+	helper, and the caller will only attempt operations that are
+	supported.
+
+'list'::
+	Outputs the names of refs, one per line. These may be
+	followed, after a single space, by "changed" or "unchanged",
+	indicating whether the foreign repository has changed from the
+	state in the ref. If the helper doesn't know, it doesn't have
+	to provide a value. (In particular, it shouldn't do expensive
+	operations, such as importing the content, to see whether it
+	matches.) Other information, not yet supported, may be output
+	as well, separated by single spaces.
+
+'import' ref::
+	Imports the given ref by outputting it in git-fast-import
+	format.
+
+'export' commit ref::
+	Sends the given commit to the foreign system, with the
+	location given by ref, and reimports it by outputting it in
+	git-fast-import format as the foreign system rendered it.
++
+All parents of commit must either have been created with 'import' or
+have been passed to 'export' previously. Depending on the features,
+there may be other restrictions on what may be exported.
+
+FEATURES
+--------
+
+'export'::
+	Helper supports exporting commits, at least exporting
+	non-merge commits whose parents are not the parents of any
+	other commit exported to the same branch or make in the other
+	system on the same branch.
+
+'export-branch'::
+	Helper supports creating new branches by exporting commits to
+	them.
+
+'export-merges'::
+	Helper supports exporting two-parent merges, where both
+	parents have already been exported successfully.
Signature · 1 line
1.6.4.rc3.138.ga6b98.dirty
#2 lore raw

RFC 2/11 Use a function to determine whether a remote is valid

In reply to Johan Herland

From: Daniel Barkalow <[email protected]>

Currently, it only checks url, but it will allow other things in the future.

---
 remote.c |   13 +++++++++----
 1 files changed, 9 insertions(+), 4 deletions(-)
remote.c +9 -4
index 6be9083..c5ac881 100644
@@ -48,6 +48,11 @@ static int rewrite_nr;
 #define BUF_SIZE (2048)
 static char buffer[BUF_SIZE];
 
+static int valid_remote(const struct remote *remote)
+{
+	return !!remote->url;
+}
+
 static const char *alias_url(const char *url)
 {
 	int i, j;
@@ -672,14 +677,14 @@ struct remote *remote_get(const char *name)
 
 	ret = make_remote(name, 0);
 	if (valid_remote_nick(name)) {
-		if (!ret->url)
+		if (!valid_remote(ret))
 			read_remotes_file(ret);
-		if (!ret->url)
+		if (!valid_remote(ret))
 			read_branches_file(ret);
 	}
-	if (name_given && !ret->url)
+	if (name_given && !valid_remote(ret))
 		add_url_alias(ret, name);
-	if (!ret->url)
+	if (!valid_remote(ret))
 		return NULL;
 	ret->fetch = parse_fetch_refspec(ret->fetch_refspec_nr, ret->fetch_refspec);
 	ret->push = parse_push_refspec(ret->push_refspec_nr, ret->push_refspec);
Signature · 1 line
1.6.4.rc3.138.ga6b98.dirty
#3 lore raw

RFC 3/11 Allow programs to not depend on remotes having urls

In reply to Johan Herland

From: Daniel Barkalow <[email protected]>

For fetch and ls-remote, which use the first url of a remote, have transport_get() determine this by passing a remote and passing NULL for the url. For push, which uses every url of a remote, use each url in turn if there are any, and use NULL if there are none.

This will allow the transport code to do something different if the location is not specified with a url.

Also, have the message for a fetch say "foreign" if there is no url.

---
 builtin-fetch.c     |   19 +++++++++++------
 builtin-ls-remote.c |    4 +-
 builtin-push.c      |   54 +++++++++++++++++++++++++++++++++-----------------
 transport.c         |    3 ++
 4 files changed, 52 insertions(+), 28 deletions(-)
builtin-fetch.c +12 -7
index 817dd6b..3f32db6 100644
@@ -346,12 +346,17 @@ static int store_updated_refs(const char *raw_url, const char *remote_name,
 			what = rm->name;
 		}
 
-		url_len = strlen(url);
-		for (i = url_len - 1; url[i] == '/' && 0 <= i; i--)
-			;
-		url_len = i + 1;
-		if (4 < i && !strncmp(".git", url + i - 3, 4))
-			url_len = i - 3;
+		if (url) {
+			url_len = strlen(url);
+			for (i = url_len - 1; url[i] == '/' && 0 <= i; i--)
+				;
+			url_len = i + 1;
+			if (4 < i && !strncmp(".git", url + i - 3, 4))
+				url_len = i - 3;
+		} else {
+			url = "foriegn";
+			url_len = strlen(url);
+		}
 
 		note_len = 0;
 		if (*what) {
@@ -663,7 +668,7 @@ int cmd_fetch(int argc, const char **argv, const char *prefix)
 	if (!remote)
 		die("Where do you want to fetch from today?");
 
-	transport = transport_get(remote, remote->url[0]);
+	transport = transport_get(remote, NULL);
 	if (verbosity >= 2)
 		transport->verbose = 1;
 	if (verbosity < 0)
builtin-ls-remote.c +2 -2
index 78a88f7..4c6fc58 100644
@@ -87,9 +87,9 @@ int cmd_ls_remote(int argc, const char **argv, const char *prefix)
 		}
 	}
 	remote = nongit ? NULL : remote_get(dest);
-	if (remote && !remote->url_nr)
+	if (!nongit && !remote)
 		die("remote %s has no configured URL", dest);
-	transport = transport_get(remote, remote ? remote->url[0] : dest);
+	transport = transport_get(remote, remote ? NULL : dest);
 	if (uploadpack != NULL)
 		transport_set_option(transport, TRANS_OPT_UPLOADPACK, uploadpack);
 
builtin-push.c +35 -19

67 lines not on this page · raw message

transport.c +3 -0
index 13fbca4..2864f40 100644
@@ -793,6 +793,9 @@ struct transport *transport_get(struct remote *remote, const char *url)
 	struct transport *ret = xcalloc(1, sizeof(*ret));
 
 	ret->remote = remote;
+
+	if (!url && remote && remote->url)
+		url = remote->url[0];
 	ret->url = url;
 
 	if (remote && remote->foreign_vcs) {
Signature · 1 line
1.6.4.rc3.138.ga6b98.dirty
#4 lore raw

Re: [RFC 03/11] Allow programs to not depend on remotes having urls

builtin-ls-remote.c int cmd_ls_remote(int argc, const char **argv, const char *prefix) · line 91

Quoted 11 lines · Johan Herland
builtin-ls-remote.c +1 -1
index 78a88f7..4c6fc58 100644
@@ -87,9 +87,9 @@ int cmd_ls_remote(int argc, const char **argv, const char *prefix)
 		}
 	}
 	remote = nongit ? NULL : remote_get(dest);
-	if (remote && !remote->url_nr)
+	if (!nongit && !remote)
 		die("remote %s has no configured URL", dest);

It appears to me that remote_get() calls make_remote() that never returns NULL, thus this check will never trigger. At least, the die() message needs to be updated, since remote_get() may return NULL for some reason other than "no configured URL", but the condition the message says is not what this code checks.

It is Ok if the new world order is that the URL does not have to be known before transport_get() is called, but if that is the case, it should be made clear by removing this check from here (and possibly having a new check elsewhere where it really matters, e.g. somewhere in transport code).

Quoted 4 lines
builtin-ls-remote.c +1 -1
-	transport = transport_get(remote, remote ? remote->url[0] : dest);
+	transport = transport_get(remote, remote ? NULL : dest);
 	if (uploadpack != NULL)
 		transport_set_option(transport, TRANS_OPT_UPLOADPACK, uploadpack);
#5 lore raw

Re: [RFC 03/11] Allow programs to not depend on remotes having urls

Quoted 11 lines · Junio C Hamano
Quoted 9 lines · Johan Herland
builtin-ls-remote.c +0 -1
index 78a88f7..4c6fc58 100644
@@ -87,9 +87,9 @@ int cmd_ls_remote(int argc, const char **argv, const char *prefix)
 		}
 	}
 	remote = nongit ? NULL : remote_get(dest);
-	if (remote && !remote->url_nr)
Quoted 8 lines

builtin-ls-remote.c int cmd_ls_remote(int argc, const char **argv, const char *prefix) · line 91

Quoted 2 lines
builtin-ls-remote.c +1 -0
+	if (!nongit && !remote)
 		die("remote %s has no configured URL", dest);

It appears to me that remote_get() calls make_remote() that never returns NULL, thus this check will never trigger. At least, the die() message needs to be updated, since remote_get() may return NULL for some reason other than "no configured URL", but the condition the message says is not what this code checks.

The previous patch in the series causes remote_get() to return NULL if the remote configuration is invalid. You're right, however, that the message should be something more directly accurate.

Quoted 5 lines

It is Ok if the new world order is that the URL does not have to be known before transport_get() is called, but if that is the case, it should be made clear by removing this check from here (and possibly having a new check elsewhere where it really matters, e.g. somewhere in transport code).

This spot is trying to make sure that the remote was valid, according to remote.c

Quoted 9 lines

builtin-ls-remote.c int cmd_ls_remote(int argc, const char **argv, const char *prefix) · line 94

Quoted 4 lines
builtin-ls-remote.c +1 -1
-	transport = transport_get(remote, remote ? remote->url[0] : dest);
+	transport = transport_get(remote, remote ? NULL : dest);
 	if (uploadpack != NULL)
 		transport_set_option(transport, TRANS_OPT_UPLOADPACK, uploadpack);

--
To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

#6 lore raw

Re: [RFC 03/11] Allow programs to not depend on remotes having urls

In reply to Johan Herland
Quoted 5 lines · Johan Herland
builtin-fetch.c +0 -0
index 817dd6b..3f32db6 100644

builtin-fetch.c static int store_updated_refs(const char *raw_url, const char *remote_name, · line 354

Quoted 9 lines
builtin-fetch.c +9 -0
@@ -346,12 +346,17 @@ static int store_updated_refs(const char *raw_url, const char *remote_name,
+               if (url) {
+                       url_len = strlen(url);
+                       for (i = url_len - 1; url[i] == '/' && 0 <= i; i--)
+                               ;
+                       url_len = i + 1;
+                       if (4 < i && !strncmp(".git", url + i - 3, 4))
+                               url_len = i - 3;
+               } else {
+                       url = "foriegn";

Typo. Should be "foreign".

#7 lore raw

Re: [RFC 03/11] Allow programs to not depend on remotes having urls

Quoted 17 lines · Alex Riesen
Quoted 5 lines · Johan Herland
builtin-fetch.c +0 -0
index 817dd6b..3f32db6 100644

builtin-fetch.c static int store_updated_refs(const char · line 354

Quoted 9 lines
builtin-fetch.c +7 -0
@@ -346,12 +346,17 @@ static int store_updated_refs(const char
 *raw_url, const char *remote_name, +               if (url) {
+                       url_len = strlen(url);
+                       for (i = url_len - 1; url[i] == '/' && 0 <= i;
 i--) +                               ;
+                       url_len = i + 1;
+                       if (4 < i && !strncmp(".git", url + i - 3, 4))
+                               url_len = i - 3;
+               } else {
+                       url = "foriegn";

Typo. Should be "foreign".

Thanks. Will be fixed in the next iteration of this topic.

...Johan

Signature · 2 lines
Johan Herland, <[email protected]>
www.herland.net
#8 lore raw

RFC 4/11 Add a transport implementation using git-vcs-* helpers

In reply to Johan Herland

From: Daniel Barkalow <[email protected]>

This is somewhat careless about pushes (that is, it attempts to make pushes that the helper can't necessarily handle), but actually works for fetches and simple pushes.

---
 Makefile            |    1 +
 remote.c            |    2 +-
 transport-foreign.c |  200 +++++++++++++++++++++++++++++++++++++++++++++++++++
 transport.c         |    1 +
 transport.h         |    1 +
 5 files changed, 204 insertions(+), 1 deletions(-)
 create mode 100644 transport-foreign.c
Makefile +1 -0
index f396e52..6b39d56 100644
@@ -554,6 +554,7 @@ LIB_OBJS += symlinks.o
 LIB_OBJS += tag.o
 LIB_OBJS += trace.o
 LIB_OBJS += transport.o
+LIB_OBJS += transport-foreign.o
 LIB_OBJS += transport-shim.o
 LIB_OBJS += tree-diff.o
 LIB_OBJS += tree.o
remote.c +1 -1
index c5ac881..8d1560e 100644
@@ -50,7 +50,7 @@ static char buffer[BUF_SIZE];
 
 static int valid_remote(const struct remote *remote)
 {
-	return !!remote->url;
+	return remote->url || remote->foreign_vcs;
 }
 
 static const char *alias_url(const char *url)
transport-foreign.c new file +200 -0

200 lines not on this page · raw message

transport.c +1 -0
index 2864f40..4e984ca 100644
@@ -799,6 +799,7 @@ struct transport *transport_get(struct remote *remote, const char *url)
 	ret->url = url;
 
 	if (remote && remote->foreign_vcs) {
+		transport_foreign_init(ret);
 	} else if (!prefixcmp(url, "rsync:")) {
 		ret->get_refs_list = get_refs_via_rsync;
 		ret->fetch = fetch_objs_via_rsync;
transport.h +1 -0
index 229a7b4..2e1bd6c 100644
@@ -116,6 +116,7 @@ int transport_disconnect(struct transport *transport);
 char *transport_anonymize_url(const char *url);
 
 /* Transport methods defined outside transport.c */
+void transport_foreign_init(struct transport *transport);
 void transport_shim_init(struct transport *transport, const char *name);
 
 #endif
Signature · 1 line
1.6.4.rc3.138.ga6b98.dirty
#9 lore raw

RFC 5/11 Refactor path name parsing into new function: get_path_str()

In reply to Johan Herland

This is in preparation for adding mark reference capability to path names.

---
 fast-import.c |   60 ++++++++++++++++++++++++++++++--------------------------
 1 files changed, 32 insertions(+), 28 deletions(-)
fast-import.c +32 -28
index 7ef9865..8a7cdc1 100644
@@ -1853,6 +1853,24 @@ static void load_branch(struct branch *b)
 	}
 }
 
+static const char *get_path_str(
+	struct strbuf *sb,
+	const char *p,
+	char endc,
+	const char **endp)
+{
+	strbuf_reset(sb);
+	*endp = p;
+	if (!unquote_c_style(sb, p, endp)) {
+		/* successfully unquoted C-style quoted name */
+		p = sb->buf;
+	} else {
+		/* regular path ending at endc */
+		*endp = strchr(p, endc);
+	}
+	return p;
+}
+
 static void file_change_m(struct branch *b)
 {
 	const char *p = command_buf.buf + 2;
@@ -1896,12 +1914,9 @@ static void file_change_m(struct branch *b)
 	if (*p++ != ' ')
 		die("Missing space after SHA1: %s", command_buf.buf);
 
-	strbuf_reset(&uq);
-	if (!unquote_c_style(&uq, p, &endp)) {
-		if (*endp)
-			die("Garbage after path in: %s", command_buf.buf);
-		p = uq.buf;
-	}
+	p = get_path_str(&uq, p, 0, &endp);
+	if (*endp)
+		die("Garbage after path in: %s", command_buf.buf);
 
 	if (S_ISGITLINK(mode)) {
 		if (inline_data)
@@ -1948,12 +1963,9 @@ static void file_change_d(struct branch *b)
 	static struct strbuf uq = STRBUF_INIT;
 	const char *endp;
 
-	strbuf_reset(&uq);
-	if (!unquote_c_style(&uq, p, &endp)) {
-		if (*endp)
-			die("Garbage after path in: %s", command_buf.buf);
-		p = uq.buf;
-	}
+	p = get_path_str(&uq, p, 0, &endp);
+	if (*endp)
+		die("Garbage after path in: %s", command_buf.buf);
 	tree_content_remove(&b->branch_tree, p, NULL);
 }
 
@@ -1966,29 +1978,21 @@ static void file_change_cr(struct branch *b, int rename)
 	struct tree_entry leaf;
 
 	s = command_buf.buf + 2;
-	strbuf_reset(&s_uq);
-	if (!unquote_c_style(&s_uq, s, &endp)) {
-		if (*endp != ' ')
-			die("Missing space after source: %s", command_buf.buf);
-	} else {
-		endp = strchr(s, ' ');
-		if (!endp)
-			die("Missing space after source: %s", command_buf.buf);
+	s = get_path_str(&s_uq, s, ' ', &endp);
+	if (*endp != ' ')
+		die("Missing space after source: %s", command_buf.buf);
+	if (s != s_uq.buf) {
 		strbuf_add(&s_uq, s, endp - s);
+		s = s_uq.buf;
 	}
-	s = s_uq.buf;
 
 	endp++;
 	if (!*endp)
 		die("Missing dest: %s", command_buf.buf);
 
-	d = endp;
-	strbuf_reset(&d_uq);
-	if (!unquote_c_style(&d_uq, d, &endp)) {
-		if (*endp)
-			die("Garbage after dest in: %s", command_buf.buf);
-		d = d_uq.buf;
-	}
+	d = get_path_str(&d_uq, endp, 0, &endp);
+	if (*endp)
+		die("Garbage after dest in: %s", command_buf.buf);
 
 	memset(&leaf, 0, sizeof(leaf));
 	if (rename)
Signature · 1 line
1.6.4.rc3.138.ga6b98.dirty
#10 lore raw

RFC 6/11 Add support for mark references as path names

In reply to Johan Herland

When using a mark reference as a path name, the mark reference will be expanded to the 40-byte hex version of the object name associated with the mark. This is useful e.g. when importing notes objects (where the filenames in a notes tree are the object names of the annotated objects).

---
 Documentation/git-fast-import.txt |    9 +++++++--
 fast-import.c                     |   11 +++++++++--
 2 files changed, 16 insertions(+), 4 deletions(-)
Documentation/git-fast-import.txt +7 -2
index c2f483a..bbc8b78 100644
@@ -487,12 +487,17 @@ in octal.  Git only supports the following modes:
 
 In both formats `<path>` is the complete path of the file to be added
 (if not already existing) or modified (if already existing).
+`<path>` may also be a mark reference (`:<idnum>`) set by a prior
+command, which will expand to a full 40-byte SHA-1 of the Git object
+associated with the mark. This is useful e.g. when importing commit
+notes (the filenames in a notes commit are the object names of the
+annotated commits).
 
 A `<path>` string must use UNIX-style directory separators (forward
 slash `/`), may contain any byte other than `LF`, and must not
-start with double quote (`"`).
+start with double quote (`"`) or colon (`:`).
 
-If an `LF` or double quote must be encoded into `<path>` shell-style
+If an `LF`, double quote or colon must be encoded into `<path>` shell-style
 quoting should be used, e.g. `"path/with\n and \" in it"`.
 
 The value of `<path>` must be in canonical form. That is it must not:
fast-import.c +9 -2
index 8a7cdc1..3edfcf0 100644
@@ -85,13 +85,13 @@ Format of STDIN stream:
      # common escapes of 'c' (e..g \n, \t, \\, \") or \nnn where nnn
      # is the signed byte value in octal.  Note that the only
      # characters which must actually be escaped to protect the
-     # stream formatting is: \, " and LF.  Otherwise these values
+     # stream formatting is: \, ", : and LF.  Otherwise these values
      # are UTF8.
      #
   ref_str     ::= ref;
   sha1exp_str ::= sha1exp;
   tag_str     ::= tag;
-  path_str    ::= path    | '"' quoted(path)    '"' ;
+  path_str    ::= path | '"' quoted(path) '"' | idnum;
   mode        ::= '100644' | '644'
                 | '100755' | '755'
                 | '120000'
@@ -1864,6 +1864,13 @@ static const char *get_path_str(
 	if (!unquote_c_style(sb, p, endp)) {
 		/* successfully unquoted C-style quoted name */
 		p = sb->buf;
+	} else if (*p == ':') {
+		/* resolve mark reference */
+		char *x;
+		struct object_entry *pe = find_mark(strtoumax(p + 1, &x, 10));
+		*endp = x;
+		strbuf_add(sb, sha1_to_hex(pe->sha1), 40);
+		p = sb->buf;
 	} else {
 		/* regular path ending at endc */
 		*endp = strchr(p, endc);
Signature · 1 line
1.6.4.rc3.138.ga6b98.dirty
#11 lore raw

Re: [RFC 06/11] Add support for mark references as path names

Quoted 19 lines · Johan Herland

When using a mark reference as a path name, the mark reference will be expanded to the 40-byte hex version of the object name associated with the mark. This is useful e.g. when importing notes objects (where the filenames in a notes tree are the object names of the annotated objects).

---
 Documentation/git-fast-import.txt |    9 +++++++--
 fast-import.c                     |   11 +++++++++--
 2 files changed, 16 insertions(+), 4 deletions(-)
Documentation/git-fast-import.txt +0 -0
index c2f483a..bbc8b78 100644
@@ -487,12 +487,17 @@ in octal.  Git only supports the following modes:
 
 In both formats `<path>` is the complete path of the file to be added
 (if not already existing) or modified (if already existing).

Documentation/git-fast-import.txt in octal. Git only supports the following modes: · line 498

Quoted 10 lines
Documentation/git-fast-import.txt +6 -1
+`<path>` may also be a mark reference (`:<idnum>`) set by a prior
+command, which will expand to a full 40-byte SHA-1 of the Git object
+associated with the mark. This is useful e.g. when importing commit
+notes (the filenames in a notes commit are the object names of the
+annotated commits).
 
 A `<path>` string must use UNIX-style directory separators (forward
 slash `/`), may contain any byte other than `LF`, and must not
-start with double quote (`"`).
+start with double quote (`"`) or colon (`:`).

I'm worried about changing the path rules here. Previously writing a path as :1 was legal and produced a file named ":1" in the top level directory of the repository. Now it will create a file that matches a mark.

I think you need to find another back door, something that the language wouldn't have considered as valid previously.

Signature · 1 line
Shawn.
#12 lore raw

Re: [RFC 06/11] Add support for mark references as path names

Quoted 9 lines · Shawn O. Pearce
Quoted 7 lines · Johan Herland

When using a mark reference as a path name, the mark reference will be expanded to the 40-byte hex version of the object name associated with the mark. This is useful e.g. when importing notes objects (where the filenames in a notes tree are the object names of the annotated objects).

[...cut...]

Quoted 7 lines

I'm worried about changing the path rules here. Previously writing a path as :1 was legal and produced a file named ":1" in the top level directory of the repository. Now it will create a file that matches a mark.

I think you need to find another back door, something that the language wouldn't have considered as valid previously.

Ok. One (moderately hideous) option is to introduce new commands m/d/r/c that works just like M/D/R/C, except that they also expand mark references (leaving M/D/R/C with the old behaviour). I don't know how you'd feel about that...

However, I'm also pondering Dscho's idea of organizing note trees into hierarchies (to limit #entries in tree objects), and (although I have yet to try to implement this) it feels like this transformation should be done in the notes.c code, which means that it must happen on the "git side" of fast-import (at least the transformation should not happen on the "external side" of fast-import). This ultimately means that we cannot handle notes as "regular" trees and commits (which is what I'm trying to do with the help of this patch), and suggests that instead of using 'commit' with 'M' for adding notes, we should probably have a dedicated 'note' fast-import command that collects note objects, and does the tree transformation and subsequent commit upon checkpoint/exit.

Have fun! :)

...Johan

Signature · 2 lines
Johan Herland, <[email protected]>
www.herland.net
#13 lore raw

Re: [RFC 06/11] Add support for mark references as path names

Quoted 8 lines · Johan Herland
Quoted 2 lines · Shawn O. Pearce

I think you need to find another back door, something that the language wouldn't have considered as valid previously.

Ok. One (moderately hideous) option is to introduce new commands m/d/r/c that works just like M/D/R/C, except that they also expand mark references (leaving M/D/R/C with the old behaviour). I don't know how you'd feel about that...

Bleh. Because I think this has more truth to it:

Quoted 4 lines

However, I'm also pondering Dscho's idea of organizing note trees into hierarchies (to limit #entries in tree objects), and (although I have yet to try to implement this) it feels like this transformation should be done in the notes.c code, which means that it must happen on

Quoted 8 lines

the "git side" of fast-import (at least the transformation should not happen on the "external side" of fast-import). This ultimately means that we cannot handle notes as "regular" trees and commits (which is what I'm trying to do with the help of this patch), and suggests that instead of using 'commit' with 'M' for adding notes, we should probably have a dedicated 'note' fast-import command that collects note objects, and does the tree transformation and subsequent commit upon checkpoint/exit.

Exactly. We probably want to do this instead.

Though you might want the "note" command to be a subcommand of commit, like M/D/R/C are, because notes are wrapped up inside of a commit and you can modify multiple notes in a single commit.

Signature · 1 line
Shawn.
#14 lore raw

RFC v2 6/11 fast-import: Add support for importing commit notes

Introduce a 'notemodify' subcommand of the 'commit' command. This subcommand is similar to 'filemodify', except that no mode is supplied (all notes have mode 0644), and the path is set to the hex SHA1 of the given "comittish".

This enables fast import of note objects along with their associated commits, since the notes can now be named using the mark references of their corresponding commits.

The patch also includes a test case of the added functionality.

---

Quoted 3 lines · Shawn O. Pearce

Though you might want the "note" command to be a subcommand of commit, like M/D/R/C are, because notes are wrapped up inside of a commit and you can modify multiple notes in a single commit.

Thanks for the suggestion. Here's a replacement patch introducing a new 'notemodify' ("N") command.

Note that this patch also makes the preceding patch superfluous:

[RFC 05/11] Refactor path name parsing into new function: get_path_str()

If you think that patch is useful in its own right, I can keep it in the series, although it has no connection to the rest of the series at this point.

Finally, this patch should probably migrate into the jh/notes topic...

Have fun! :)

...Johan

 Documentation/git-fast-import.txt |   45 +++++++++--
 fast-import.c                     |   88 +++++++++++++++++++-
 t/t9300-fast-import.sh            |  166 +++++++++++++++++++++++++++++++++++++
 3 files changed, 289 insertions(+), 10 deletions(-)
Documentation/git-fast-import.txt +39 -6
index c2f483a..288032c 100644
@@ -316,7 +316,7 @@ change to the project.
 	data
 	('from' SP <committish> LF)?
 	('merge' SP <committish> LF)?
-	(filemodify | filedelete | filecopy | filerename | filedeleteall)*
+	(filemodify | filedelete | filecopy | filerename | filedeleteall | notemodify)*
 	LF?
 ....
 
@@ -339,14 +339,13 @@ commit message use a 0 length data.  Commit messages are free-form
 and are not interpreted by Git.  Currently they must be encoded in
 UTF-8, as fast-import does not permit other encodings to be specified.
 
-Zero or more `filemodify`, `filedelete`, `filecopy`, `filerename`
-and `filedeleteall` commands
+Zero or more `filemodify`, `filedelete`, `filecopy`, `filerename`,
+`filedeleteall` and `notemodify` commands
 may be included to update the contents of the branch prior to
 creating the commit.  These commands may be supplied in any order.
 However it is recommended that a `filedeleteall` command precede
-all `filemodify`, `filecopy` and `filerename` commands in the same
-commit, as `filedeleteall`
-wipes the branch clean (see below).
+all `filemodify`, `filecopy`, `filerename` and `notemodify` commands in
+the same commit, as `filedeleteall` wipes the branch clean (see below).
 
 The `LF` after the command is optional (it used to be required).
 
@@ -595,6 +594,40 @@ more memory per active branch (less than 1 MiB for even most large
 projects); so frontends that can easily obtain only the affected
 paths for a commit are encouraged to do so.
 
+`notemodify`
+^^^^^^^^^^^^
+Included in a `commit` command to add a new note (annotating a given
+commit) or change the content of an existing note.  This command has
+two different means of specifying the content of the note.
+
+External data format::
+	The data content for the note was already supplied by a prior
+	`blob` command.  The frontend just needs to connect it to the
+	commit that is to be annotated.
++
+....
+	'N' SP <dataref> SP <committish> LF
+....
++
+Here `<dataref>` can be either a mark reference (`:<idnum>`)
+set by a prior `blob` command, or a full 40-byte SHA-1 of an
+existing Git blob object.
+
+Inline data format::
+	The data content for the note has not been supplied yet.
+	The frontend wants to supply it as part of this modify
+	command.
++
+....
+	'N' SP 'inline' SP <committish> LF
+	data
+....
++
+See below for a detailed description of the `data` command.
+
+In both formats `<committish>` is any of the commit specification
+expressions also accepted by `from` (see above).
+
 `mark`
 ~~~~~~
 Arranges for fast-import to save a reference to the current object, allowing
fast-import.c +84 -4

125 lines not on this page · raw message

t/t9300-fast-import.sh +166 -0

170 lines not on this page · raw message

Signature · 1 line
1.6.4.rc3.138.ga6b98.dirty
#15 lore raw

Re: [RFC 06/11 v2] fast-import: Add support for importing commit notes

Quoted 11 lines · Johan Herland

Introduce a 'notemodify' subcommand of the 'commit' command. This subcommand is similar to 'filemodify', except that no mode is supplied (all notes have mode 0644), and the path is set to the hex SHA1 of the given "comittish".

This enables fast import of note objects along with their associated commits, since the notes can now be named using the mark references of their corresponding commits.

The patch also includes a test case of the added functionality.

Thanks.

Replacing this will cascade to the rest of the series, right?

#16 lore raw

Re: [RFC 06/11 v2] fast-import: Add support for importing commit notes

Quoted 10 lines · Junio C Hamano
Quoted 9 lines · Johan Herland

Introduce a 'notemodify' subcommand of the 'commit' command. This subcommand is similar to 'filemodify', except that no mode is supplied (all notes have mode 0644), and the path is set to the hex SHA1 of the given "comittish".

This enables fast import of note objects along with their associated commits, since the notes can now be named using the mark references of their corresponding commits.

Quoted 7 lines
Quoted 3 lines

The patch also includes a test case of the added functionality.

Thanks.

Replacing this will cascade to the rest of the series, right?

Not sure what you mean by "cascade to the rest of the series". If you just replace 06/11_v1 with 06/11_v2, things will stop working (since 10/11 and 11/11 needs a corresponding update of their fast-import commands).

On the other hand, I have moved this patch into the jh/notes series (which I updated/resent last night).

I will update/resend this patch series tomorrow, with suggested fixes, and rebased onto the updated jh/notes series.

...Johan

Signature · 2 lines
Johan Herland, <[email protected]>
www.herland.net
#17 lore raw

Re: [RFC 06/11 v2] fast-import: Add support for importing commit notes

In reply to Johan Herland
Quoted 17 lines · Johan Herland
Documentation/git-fast-import.txt +17 -0
+External data format::
+	The data content for the note was already supplied by a prior
+	`blob` command.  The frontend just needs to connect it to the
+	commit that is to be annotated.
++
+....
+	'N' SP <dataref> SP <committish> LF
+....
++
+Here `<dataref>` can be either a mark reference (`:<idnum>`)
+set by a prior `blob` command, or a full 40-byte SHA-1 of an
+existing Git blob object.
+
+Inline data format::
+	The data content for the note has not been supplied yet.
+	The frontend wants to supply it as part of this modify
+	command.

Documentation/git-fast-import.txt more memory per active branch (less than 1 MiB for even most large · line 629

Quoted 10 lines
Documentation/git-fast-import.txt +10 -0
++
+....
+	'N' SP 'inline' SP <committish> LF
+	data
+....
++
+See below for a detailed description of the `data` command.
+
+In both formats `<committish>` is any of the commit specification
+expressions also accepted by `from` (see above).

Doesn't this make fast-import language incapable of add notes to anything other than commits? As far as I remember, there is no such limitation in the underlying data structure on git notes, even though the git-notes sample Porcelain might have such a restriction.

We recently hit a similar unintended limitation that we regret in the fast-import language, didn't we?

Although personally I do not think it is a big deal if we cannot tag or add notes to trees, I am pointing it out in case other people care.

#18 lore raw

Re: [RFC 06/11 v2] fast-import: Add support for importing commit notes

Quoted 25 lines · Junio C Hamano
Quoted 24 lines · Johan Herland
Documentation/git-fast-import.txt +24 -0
+External data format::
+	The data content for the note was already supplied by a prior
+	`blob` command.  The frontend just needs to connect it to the
+	commit that is to be annotated.
++
+....
+	'N' SP <dataref> SP <committish> LF
+....
++
+Here `<dataref>` can be either a mark reference (`:<idnum>`)
+set by a prior `blob` command, or a full 40-byte SHA-1 of an
+existing Git blob object.
+
+Inline data format::
+	The data content for the note has not been supplied yet.
+	The frontend wants to supply it as part of this modify
+	command.
++
+....
+	'N' SP 'inline' SP <committish> LF
+	data
+....
++
+See below for a detailed description of the `data` command.
Quoted 8 lines

Documentation/git-fast-import.txt more memory per active branch (less than 1 MiB for even most large · line 629

Quoted 3 lines
Documentation/git-fast-import.txt +3 -0
+
+In both formats `<committish>` is any of the commit specification
+expressions also accepted by `from` (see above).

Doesn't this make fast-import language incapable of add notes to anything other than commits? As far as I remember, there is no such limitation in the underlying data structure on git notes, even though the git-notes sample Porcelain might have such a restriction.

It does (probably because the default notes tree is "refs/notes/commits").

Quoted 2 lines

We recently hit a similar unintended limitation that we regret in the fast-import language, didn't we?

I don't know. Must have slipped past my mailbox.

Quoted 2 lines

Although personally I do not think it is a big deal if we cannot tag or add notes to trees, I am pointing it out in case other people care.

I copied the semantics from the 'tag' command, for no particular reason (except following the git-notes procelain). Expanding 'notemodify' (and 'tag') to cover all types of objects is fine by me, unless there are good arguments otherwise. Shawn?

Have fun!

...Johan

Signature · 2 lines
Johan Herland, <[email protected]>
www.herland.net
#19 lore raw

Re: [RFC 06/11 v2] fast-import: Add support for importing commit notes

Quoted 11 lines · Johan Herland
Quoted 9 lines · Junio C Hamano

Documentation/git-fast-import.txt more memory per active branch (less than 1 MiB for even most large · line 629

Quoted 3 lines · Johan Herland
Documentation/git-fast-import.txt +3 -0
+
+In both formats `<committish>` is any of the commit specification
+expressions also accepted by `from` (see above).

Doesn't this make fast-import language incapable of add notes to anything other than commits? As far as I remember, there is no such limitation in the underlying data structure on git notes, even though the git-notes sample Porcelain might have such a restriction.

Quoted 1 line

It does (probably because the default notes tree is "refs/notes/commits").

Yea, it does have that limitation right now. That limitation could be relaxed in the code by just allowing the <committish> to be any object and simply don't check its type.

But I've already stated with regards to the notes that I think we should only allow noting commits and annotated tags, where we have a timestamp we can use to split the notes in the note tree by time, so that we can index recent notes much more quickly and can answer git log -20 much more efficiently.

I just don't see a lot of value in noting a blob or a tree, there is too little context information on such things for it to really be all that useful.

Quoted 4 lines
Quoted 2 lines

We recently hit a similar unintended limitation that we regret in the fast-import language, didn't we?

I don't know. Must have slipped past my mailbox.

I remember something being raised, but I can't remember exactly what it was either.

It might have had to do with the effects of rename commands, e.g. a file rename takes place immediately when issued, and some frontends wanted it to take place only after the commit was completed.

Quoted 7 lines
Quoted 2 lines

Although personally I do not think it is a big deal if we cannot tag or add notes to trees, I am pointing it out in case other people care.

I copied the semantics from the 'tag' command, for no particular reason (except following the git-notes procelain). Expanding 'notemodify' (and 'tag') to cover all types of objects is fine by me, unless there are good arguments otherwise. Shawn?

tag, there might be arguments for tagging trees, e.g. so you can export the linux kernel repository with git fast-export and reload it with fast-import. But that's unrelated to this change.

See above about notes.

Signature · 1 line
Shawn.
#20 lore raw

Re: [RFC 06/11 v2] fast-import: Add support for importing commit notes

Quoted 7 lines · Shawn O. Pearce
Quoted 4 lines
Quoted 2 lines

We recently hit a similar unintended limitation that we regret in the fast-import language, didn't we?

I don't know. Must have slipped past my mailbox.

I remember something being raised, but I can't remember exactly what it was either.

Exporting a tag that points at a non commit was what I had in mind.

Quoted 5 lines

tag, there might be arguments for tagging trees, e.g. so you can export the linux kernel repository with git fast-export and reload it with fast-import. But that's unrelated to this change.

See above about notes.

I think we are in agreement then. Good.

#21 lore raw

Re: [RFC 06/11 v2] fast-import: Add support for importing commit notes

Quoted 11 lines · Junio C Hamano
Quoted 7 lines · Shawn O. Pearce
Quoted 4 lines
Quoted 2 lines

We recently hit a similar unintended limitation that we regret in the fast-import language, didn't we?

I don't know. Must have slipped past my mailbox.

I remember something being raised, but I can't remember exactly what it was either.

Exporting a tag that points at a non commit was what I had in mind.

Quoted 7 lines
Quoted 5 lines

tag, there might be arguments for tagging trees, e.g. so you can export the linux kernel repository with git fast-export and reload it with fast-import. But that's unrelated to this change.

See above about notes.

I think we are in agreement then. Good.

We do? on what? That 'notemodify' should work on <committish> + <tag object>? Or that 'notemodify' should work on <any object>?

...Johan

Signature · 2 lines
Johan Herland, <[email protected]>
www.herland.net
#22 lore raw

Re: [RFC 06/11 v2] fast-import: Add support for importing commit notes

Quoted 8 lines · Johan Herland
Quoted 1 line · Junio C Hamano

"Shawn O. Pearce" <[email protected]> writes:

...

Quoted 3 lines
Quoted 1 line

See above about notes.

I think we are in agreement then. Good.

We do? on what?

We might mind if fast-import does not handle tags to non-commits, but we would want to restrict notes to apply only to commits for other reasons anyway, and it is perfectly fine if your patch handled only notes that are attached to commits and not arbitrary objects.

#23 lore raw

RFC 7/11 Preliminary clarifications to git-vcs documentation

In reply to Johan Herland
---
 Documentation/git-vcs.txt |    7 +++++--
 1 files changed, 5 insertions(+), 2 deletions(-)
Documentation/git-vcs.txt +5 -2
index 402c927..febe415 100644
@@ -22,13 +22,15 @@ capabilities (limited both by the particular helper and by the
 capabilities of the system they connect to), and they report what
 capabilities they support.
 
-These programs can store refs in refs/<system>/*, and arbitrary
-information in info/<system>.
+These programs can store refs in refs/<system>/*, note refs in
+refs/notes/<system>/*, and arbitrary information in info/<system>/*.
 
 COMMANDS
 --------
 
 Commands are given by the caller on the helper's standard input, one per line.
+The output of each command must be produced on the helper's standard output.
+The helper's standard error stream can be used for status/progress messages.
 
 'capabilities'::
 	Outputs a single line with a list of feature names separated
@@ -45,6 +47,7 @@ Commands are given by the caller on the helper's standard input, one per line.
 	operations, such as importing the content, to see whether it
 	matches.) Other information, not yet supported, may be output
 	as well, separated by single spaces.
+	The output list shall be terminated with a blank line.
 
 'import' ref::
 	Imports the given ref by outputting it in git-fast-import
Signature · 1 line
1.6.4.rc3.138.ga6b98.dirty
#24 lore raw

RFC 8/11 Teach foreign transport code to perform the "capabilities" command

In reply to Johan Herland

The features reported by the vcs helper are stored in the foreign_data struct which is expanded for this purpose.

In the process, we also change the capabilities command slightly to expect one feature per line (instead of all features on a single line). This enables us to add features in the future that take one or more arguments. To terminate the list of features, a blank line is output at the end.

---
 Documentation/git-vcs.txt |    8 ++--
 transport-foreign.c       |   83 ++++++++++++++++++++++++++++++++++-----------
 2 files changed, 67 insertions(+), 24 deletions(-)
Documentation/git-vcs.txt +4 -4
index febe415..85656d4 100644
@@ -33,10 +33,10 @@ The output of each command must be produced on the helper's standard output.
 The helper's standard error stream can be used for status/progress messages.
 
 'capabilities'::
-	Outputs a single line with a list of feature names separated
-	by spaces. Each of these indicates a supported feature of the
-	helper, and the caller will only attempt operations that are
-	supported.
+	Outputs a list of feature names, one per line. Each of these
+	indicates a supported feature of the helper, and the caller
+	will only attempt operations that are supported.
+	The output list shall be terminated with a blank line.
 
 'list'::
 	Outputs the names of refs, one per line. These may be
transport-foreign.c +63 -20

101 lines not on this page · raw message

Signature · 1 line
1.6.4.rc3.138.ga6b98.dirty
#25 lore raw

RFC 9/11 Introduce a 'marks <filename>' feature to the foreign transport code

In reply to Johan Herland

The 'marks' feature is reported by the vcs helper when it requires the fast-import marks database to loaded/saved by the git-fast-import process that is executed by the foreign transport machinery. The feature is advertised along with exactly one argument: the location of the file containing the marks database.

---
 Documentation/git-vcs.txt |    8 ++++++++
 transport-foreign.c       |   32 ++++++++++++++++++++++++++++++--
 2 files changed, 38 insertions(+), 2 deletions(-)
Documentation/git-vcs.txt +8 -0
index 85656d4..75815ba 100644
@@ -65,6 +65,14 @@ there may be other restrictions on what may be exported.
 FEATURES
 --------
 
+'marks' filename::
+	Helper requires the marks from a git-fast-import run to be loaded
+	from, and saved to, the given filename. When this "feature" is
+	advertised, each git-fast-import run must load and save the
+	internal marks database (see the --import-marks and --export-marks
+	option to git-fast-import for more details) located at the given
+	filename.
+
 'export'::
 	Helper supports exporting commits, at least exporting
 	non-merge commits whose parents are not the parents of any
transport-foreign.c +30 -2
index be0a587..ae9cb9a 100644
@@ -11,6 +11,7 @@ struct foreign_data
 	struct child_process importer;
 
 	/* capabilities */
+	char *marks_filename;
 	unsigned export:1;
 	unsigned export_branch:1;
 	unsigned export_merges:1;
@@ -38,7 +39,12 @@ static void get_foreign_capabilities(struct foreign_data *fdata)
 			*eon = '\0';
 
 		/* parse features */
-		if (!strcmp(buf.buf, "export"))
+		if (!strcmp(buf.buf, "marks")) {
+			if (!eon)
+				die("Feature 'marks' requires an argument");
+			fdata->marks_filename = xstrdup(eon + 1);
+		}
+		else if (!strcmp(buf.buf, "export"))
 			fdata->export = 1;
 		else if (!strcmp(buf.buf, "export-branch"))
 			fdata->export_branch = 1;
@@ -82,6 +88,7 @@ static int disconnect_foreign(struct transport *transport)
 		write(fdata->importer.in, "\n", 1);
 		close(fdata->importer.in);
 		finish_command(&fdata->importer);
+		free(fdata->marks_filename);
 		free(fdata);
 		transport->data = NULL;
 	}
@@ -91,10 +98,13 @@ static int disconnect_foreign(struct transport *transport)
 static int fetch_refs_via_foreign(struct transport *transport,
 				  int nr_heads, struct ref **to_fetch)
 {
+	struct foreign_data *fdata;
 	struct child_process *importer;
 	struct child_process fastimport;
 	struct ref *posn;
+	struct strbuf export_marks, import_marks;
 	int i, count;
+	FILE *f;
 
 	count = 0;
 	for (i = 0; i < nr_heads; i++) {
@@ -105,12 +115,28 @@ static int fetch_refs_via_foreign(struct transport *transport,
 	}
 	if (count) {
 		importer = get_importer(transport);
+		fdata = (struct foreign_data *) transport->data;
+		strbuf_init(&export_marks, 0);
+		strbuf_init(&import_marks, 0);
 
 		memset(&fastimport, 0, sizeof(fastimport));
 		fastimport.in = importer->out;
-		fastimport.argv = xcalloc(3, sizeof(*fastimport.argv));
+		fastimport.argv = xcalloc(5, sizeof(*fastimport.argv));
 		fastimport.argv[0] = "fast-import";
 		fastimport.argv[1] = "--quiet";
+		if (fdata->marks_filename) {
+			strbuf_addf(&export_marks, "--export-marks=%s",
+				fdata->marks_filename);
+			fastimport.argv[2] = export_marks.buf;
+
+			f = fopen(fdata->marks_filename, "r");
+			if (f) {
+				strbuf_addf(&import_marks, "--import-marks=%s",
+					fdata->marks_filename);
+				fastimport.argv[3] = import_marks.buf;
+				fclose(f);
+			}
+		}
 		fastimport.git_cmd = 1;
 		start_command(&fastimport);
 
@@ -124,6 +150,8 @@ static int fetch_refs_via_foreign(struct transport *transport,
 		}
 		disconnect_foreign(transport);
 		finish_command(&fastimport);
+		strbuf_release(&export_marks);
+		strbuf_release(&import_marks);
 	}
 	for (i = 0; i < nr_heads; i++) {
 		posn = to_fetch[i];
Signature · 1 line
1.6.4.rc3.138.ga6b98.dirty
#26 lore raw

RFC 11/11 Add simple test cases of git-vcs-cvs functionality

In reply to Johan Herland
---
 t/t9800-foreign-vcs-cvs-basic.sh |  518 ++++++++++++++++++++++++++++++++++++++
 t/t9801-foreign-vcs-cvs-fetch.sh |  291 +++++++++++++++++++++
 t/test-lib.sh                    |    1 +
 3 files changed, 810 insertions(+), 0 deletions(-)
 create mode 100755 t/t9800-foreign-vcs-cvs-basic.sh
 create mode 100755 t/t9801-foreign-vcs-cvs-fetch.sh
t/t9800-foreign-vcs-cvs-basic.sh new file +518 -0
new file mode 100755
index 0000000..4e29e62
@@ -0,0 +1,518 @@
+#!/bin/sh
+
+test_description='git vcs-cvs basic tests'
+. ./test-lib.sh
+
+if ! test_have_prereq PYTHON; then
+	say 'skipping CVS foreign-vcs helper tests, python not available'
+	test_done
+fi
+
+CVS_EXEC=cvs
+CVS_OPTS="-f -q"
+CVS="$CVS_EXEC $CVS_OPTS"
+
+CVSROOT=$(pwd)/cvsroot
+export CVSROOT
+unset CVS_SERVER
+
+CVSMODULE=cvsmodule
+GITREMOTE=cvsremote
+
+if ! type $CVS_EXEC >/dev/null 2>&1
+then
+	say 'skipping vcs-cvs tests, $CVS_EXEC not found'
+	test_done
+fi
+
+test_expect_success 'setup cvsroot' '$CVS init'
+
+test_expect_success '#1: setup a cvs module' '
+
+	mkdir "$CVSROOT/$CVSMODULE" &&
+	$CVS co -d module-cvs $CVSMODULE &&
+	(
+		cd module-cvs &&
+		cat <<EOF >o_fortuna &&
+O Fortuna
+velut luna
+statu variabilis,
+
+semper crescis
+aut decrescis;
+vita detestabilis
+
+nunc obdurat
+et tunc curat
+ludo mentis aciem,
+
+egestatem,
+potestatem
+dissolvit ut glaciem.
+EOF
+		$CVS add o_fortuna &&
+		cat <<EOF >message &&
+add "O Fortuna" lyrics
+
+These public domain lyrics make an excellent sample text.
+EOF
+		$CVS commit -f -F message o_fortuna
+	)
+'
+
+test_expect_success 'set up CVS repo as a foreign remote' '
+
+	git config "user.name" "Test User"
+	git config "user.email" "[email protected]"
+	git config "remote.$GITREMOTE.vcs" cvs
+	git config "remote.$GITREMOTE.cvsRoot" "$CVSROOT"
+	git config "remote.$GITREMOTE.cvsModule" "$CVSMODULE"
+	git config "remote.$GITREMOTE.fetch" \
+		"+refs/cvs/$GITREMOTE/*:refs/remotes/$GITREMOTE/*"
+
+'
+
+test_expect_success '#1: git-vcs-cvs "capabilities" command' '
+
+	echo "capabilities" | git vcs-cvs "$GITREMOTE" > actual &&
+	cat <<EOF >expect &&
+marks .git/info/cvs/$GITREMOTE/marks
+
+EOF
+	test_cmp expect actual
+
+'
+
+test_expect_success '#1: git-vcs-cvs "list" command' '
+
+	echo "list" | git vcs-cvs "$GITREMOTE" > actual &&
+	cat <<EOF >expect &&
+refs/cvs/$GITREMOTE/HEAD changed
+
+EOF
+	test_cmp expect actual
+
+'
+
+test_expect_success '#1: git-vcs-cvs "import" command' '
+
+	echo "import refs/cvs/$GITREMOTE/HEAD" | git vcs-cvs "$GITREMOTE" > actual &&
+	cat <<EOF >expect &&
+# Importing CVS revision o_fortuna:1.1
+blob
+mark :1
+data 180
+O Fortuna
+velut luna
+statu variabilis,
+
+semper crescis
+aut decrescis;
+vita detestabilis
+
+nunc obdurat
+et tunc curat
+ludo mentis aciem,
+
+egestatem,
+potestatem
+dissolvit ut glaciem.
+
+commit refs/cvs/$GITREMOTE/HEAD
+mark :2
+data 82
+add "O Fortuna" lyrics
+
+These public domain lyrics make an excellent sample text.
+
+M 644 :1 o_fortuna
+
+# Importing note for object 2
+blob
+mark :3
+data 14
+o_fortuna:1.1
+
+commit refs/notes/cvs/$GITREMOTE
+mark :4
+data 43
+Annotate commits imported by "git vcs-cvs"
+
+M 644 :3 :2
+
+blob
+mark :5
+data 32
+1 o_fortuna:1.1
+2 o_fortuna:1.1
+
+blob
+mark :6
+data 16
+blob 1
+commit 2
+
+commit refs/cvs/$GITREMOTE/_metadata
+mark :7
+data 39
+Updated metadata used by "git vcs-cvs"
+
+M 644 :5 CVS/marks
+M 644 :6 o_fortuna/1.1
+
+EOF
+	grep -v "^committer " actual > actual.filtered &&
+	test_cmp expect actual.filtered
+
+'
+
+test_expect_success '#1: Passing git-vcs-cvs output to git-fast-import' '
+
+	git fast-import --quiet \
+		--export-marks=".git/info/cvs/$GITREMOTE/marks" \
+		< actual &&
+	git gc
+
+'
+
+test_expect_success '#1: Verifying correctness of import' '
+
+	echo "verify HEAD" | git vcs-cvs "$GITREMOTE"
+
+'
+
+test_expect_success '#2: update cvs module' '
+
+	(
+		cd module-cvs &&
+		cat <<EOF >o_fortuna &&
+O Fortune,
+like the moon
+you are changeable,
+
+ever waxing
+and waning;
+hateful life
+
+first oppresses
+and then soothes
+as fancy takes it;
+
+poverty
+and power
+it melts them like ice.
+EOF
+		cat <<EOF >message &&
+translate to English
+
+My Latin is terrible.
+EOF
+		$CVS commit -f -F message o_fortuna
+	)
+'
+
+test_expect_success '#2: git-vcs-cvs "capabilities" command' '
+
+	echo "capabilities" | git vcs-cvs "$GITREMOTE" > actual &&
+	cat <<EOF >expect &&
+marks .git/info/cvs/$GITREMOTE/marks
+
+EOF
+	test_cmp expect actual
+
+'
+
+test_expect_success '#2: git-vcs-cvs "list" command' '
+
+	echo "list" | git vcs-cvs "$GITREMOTE" > actual &&
+	cat <<EOF >expect &&
+refs/cvs/$GITREMOTE/HEAD changed
+
+EOF
+	test_cmp expect actual
+
+'
+
+test_expect_success '#2: git-vcs-cvs "import" command' '
+
+	echo "import refs/cvs/$GITREMOTE/HEAD" | git vcs-cvs "$GITREMOTE" > actual &&
+	cat <<EOF >expect &&
+# Importing CVS revision o_fortuna:1.2
+blob
+mark :8
+data 179
+O Fortune,
+like the moon
+you are changeable,
+
+ever waxing
+and waning;
+hateful life
+
+first oppresses
+and then soothes
+as fancy takes it;
+
+poverty
+and power
+it melts them like ice.
+
+commit refs/cvs/$GITREMOTE/HEAD
+mark :9
+data 44
+translate to English
+
+My Latin is terrible.
+
+from refs/cvs/$GITREMOTE/HEAD^0
+M 644 :8 o_fortuna
+
+# Importing note for object 9
+blob
+mark :10
+data 14
+o_fortuna:1.2
+
+commit refs/notes/cvs/$GITREMOTE
+mark :11
+data 43
+Annotate commits imported by "git vcs-cvs"
+
+from refs/notes/cvs/$GITREMOTE^0
+M 644 :10 :9
+
+blob
+mark :12
+data 32
+8 o_fortuna:1.2
+9 o_fortuna:1.2
+
+blob
+mark :13
+data 94
+
+blob
+mark :14
+data 16
+blob 8
+commit 9
+
+commit refs/cvs/$GITREMOTE/_metadata
+mark :15
+data 39
+Updated metadata used by "git vcs-cvs"
+
+from refs/cvs/$GITREMOTE/_metadata^0
+M 644 :12 CVS/marks
+M 644 :13 o_fortuna/1.1
+M 644 :14 o_fortuna/1.2
+
+EOF
+	grep -v -e "^committer " -e "\b[0-9a-f]\{40\}\b" actual > actual.filtered &&
+	test_cmp expect actual.filtered
+
+'
+
+test_expect_success '#2: Passing git-vcs-cvs output to git-fast-import' '
+
+	git fast-import --quiet \
+		--import-marks=".git/info/cvs/$GITREMOTE/marks" \
+		--export-marks=".git/info/cvs/$GITREMOTE/marks" \
+		< actual &&
+	git gc
+
+'
+
+test_expect_success '#2: Verifying correctness of import' '
+
+	echo "verify HEAD" | git vcs-cvs "$GITREMOTE"
+
+'
+
+test_expect_success '#3: update cvs module' '
+
+	(
+		cd module-cvs &&
+		echo 1 >tick &&
+		$CVS add tick &&
+		$CVS commit -f -m 1 tick
+	)
+
+'
+
+test_expect_success '#3: git-vcs-cvs "capabilities" command' '
+
+	echo "capabilities" | git vcs-cvs "$GITREMOTE" > actual &&
+	cat <<EOF >expect &&
+marks .git/info/cvs/$GITREMOTE/marks
+
+EOF
+	test_cmp expect actual
+
+'
+
+test_expect_success '#3: git-vcs-cvs "list" command' '
+
+	echo "list" | git vcs-cvs "$GITREMOTE" > actual &&
+	cat <<EOF >expect &&
+refs/cvs/$GITREMOTE/HEAD changed
+
+EOF
+	test_cmp expect actual
+
+'
+
+test_expect_success '#3: git-vcs-cvs "import" command' '
+
+	echo "import refs/cvs/$GITREMOTE/HEAD" | git vcs-cvs "$GITREMOTE" > actual &&
+	cat <<EOF >expect &&
+# Importing CVS revision tick:1.1
+blob
+mark :16
+data 2
+1
+
+commit refs/cvs/$GITREMOTE/HEAD
+mark :17
+data 2
+1
+
+from refs/cvs/$GITREMOTE/HEAD^0
+M 644 :16 tick
+
+# Importing note for object 17
+blob
+mark :18
+data 23
+o_fortuna:1.2
+tick:1.1
+
+commit refs/notes/cvs/$GITREMOTE
+mark :19
+data 43
+Annotate commits imported by "git vcs-cvs"
+
+from refs/notes/cvs/$GITREMOTE^0
+M 644 :18 :17
+
+blob
+mark :20
+data 41
+16 tick:1.1
+17 tick:1.1
+17 o_fortuna:1.2
+
+blob
+mark :21
+data 104
+commit 17
+
+blob
+mark :22
+data 18
+blob 16
+commit 17
+
+commit refs/cvs/$GITREMOTE/_metadata
+mark :23
+data 39
+Updated metadata used by "git vcs-cvs"
+
+from refs/cvs/$GITREMOTE/_metadata^0
+M 644 :20 CVS/marks
+M 644 :21 o_fortuna/1.2
+M 644 :22 tick/1.1
+
+EOF
+	grep -v -e "^committer " -e "\b[0-9a-f]\{40\}\b" actual > actual.filtered &&
+	test_cmp expect actual.filtered
+
+'
+
+test_expect_success '#3: Passing git-vcs-cvs output to git-fast-import' '
+
+	git fast-import --quiet \
+		--import-marks=".git/info/cvs/$GITREMOTE/marks" \
+		--export-marks=".git/info/cvs/$GITREMOTE/marks" \
+		< actual &&
+	git gc
+
+'
+
+test_expect_success '#3: Verifying correctness of import' '
+
+	echo "verify HEAD" | git vcs-cvs "$GITREMOTE"
+
+'
+
+test_expect_success '#4: git-vcs-cvs "capabilities" command' '
+
+	echo "capabilities" | git vcs-cvs "$GITREMOTE" > actual &&
+	cat <<EOF >expect &&
+marks .git/info/cvs/$GITREMOTE/marks
+
+EOF
+	test_cmp expect actual
+
+'
+
+test_expect_success '#4: git-vcs-cvs "list" command' '
+
+	echo "list" | git vcs-cvs "$GITREMOTE" > actual &&
+	cat <<EOF >expect &&
+refs/cvs/$GITREMOTE/HEAD unchanged
+
+EOF
+	test_cmp expect actual
+
+'
+
+test_expect_success '#4: git-vcs-cvs "import" command' '
+
+	echo "import refs/cvs/$GITREMOTE/HEAD" | git vcs-cvs "$GITREMOTE" > actual &&
+	cat <<EOF >expect &&
+blob
+mark :24
+data 0
+
+blob
+mark :25
+data 142
+
+blob
+mark :26
+data 94
+
+commit refs/cvs/$GITREMOTE/_metadata
+mark :27
+data 39
+Updated metadata used by "git vcs-cvs"
+
+from refs/cvs/$GITREMOTE/_metadata^0
+M 644 :24 CVS/marks
+M 644 :25 o_fortuna/1.2
+M 644 :26 tick/1.1
+
+EOF
+	grep -v -e "^committer " -e "\b[0-9a-f]\{40\}\b" actual > actual.filtered &&
+	test_cmp expect actual.filtered
+
+'
+
+test_expect_success '#4: Passing git-vcs-cvs output to git-fast-import' '
+
+	git fast-import --quiet \
+		--import-marks=".git/info/cvs/$GITREMOTE/marks" \
+		--export-marks=".git/info/cvs/$GITREMOTE/marks" \
+		< actual &&
+	git gc
+
+'
+
+test_expect_success '#4: Verifying correctness of import' '
+
+	echo "verify HEAD" | git vcs-cvs "$GITREMOTE"
+
+'
+
+test_done
t/t9801-foreign-vcs-cvs-fetch.sh new file +291 -0

291 lines not on this page · raw message

t/test-lib.sh +1 -0
index 5fdc5d9..8eb8b95 100644
@@ -706,6 +706,7 @@ case $(uname -s) in
 esac
 
 test -z "$NO_PERL" && test_set_prereq PERL
+test -z "$NO_PYTHON" && test_set_prereq PYTHON
 
 # test whether the filesystem supports symbolic links
 ln -s x y 2>/dev/null && test -h y 2>/dev/null && test_set_prereq SYMLINKS
Signature · 1 line
1.6.4.rc3.138.ga6b98.dirty
#27 lore raw
In reply to Johan Herland
Quoted 9 lines · Johan Herland

This series is a first draft towards implementing a foreign VCS helper program for CVS.

It is based on the 'db/foreign-scm' and 'jh/notes' topics in 'pu'. As such, this patch series should apply cleanly to current 'pu'.

The first 4 patches supply the necessary parts of Daniel Barkalow's foreign-scm topic that has not yet been merged to 'pu' (i.e. everything remaining in Daniel's original patch series, except the p4-specific stuff).

I've got some changes to this series in the works; I just made a few changes to share the transport-side code for interacting with the helper with the transport-side code for interacting with native-object helpers. Mainly, this means starting each line in the "list" response with "?" to indicate that you don't know the hash, since the protocol is trying to be compatible with cases where you do know.

Quoted 7 lines

The next 2 patches add some functionality to git fast-import, for facilitating the import of 'notes' objects (this is a _much_ faster way to generate/import notes than invoking "git notes edit -m MSG" for each note).

Next, there are 3 patches tweaking and expanding the git-vcs API (with corresponding implementations in the foreign transport code) to adjust for the CVS helper's needs.

These make sense to me. I assume you're planning to support exporting in some ways, but haven't got it working yet? I largely ignored the capabilities stuff because I didn't have two systems with differing requirements to actually use it.

-Daniel
*This .sig left intentionally blank*

#28 lore raw
Quoted 12 lines · Daniel Barkalow
Quoted 10 lines · Johan Herland

This series is a first draft towards implementing a foreign VCS helper program for CVS.

It is based on the 'db/foreign-scm' and 'jh/notes' topics in 'pu'. As such, this patch series should apply cleanly to current 'pu'.

The first 4 patches supply the necessary parts of Daniel Barkalow's foreign-scm topic that has not yet been merged to 'pu' (i.e. everything remaining in Daniel's original patch series, except the p4-specific stuff).

Quoted 7 lines

I've got some changes to this series in the works; I just made a few changes to share the transport-side code for interacting with the helper with the transport-side code for interacting with native-object helpers. Mainly, this means starting each line in the "list" response with "?" to indicate that you don't know the hash, since the protocol is trying to be compatible with cases where you do know.

No problem. I'm actually considering whether I should rewrite the CVS helper to a full-fledged git-shim (btw, thanks for the shim work; it looks really promising). I would still probably have to use git-fast-import, but I would have more control over the fast-import process (e.g. by closing the fast-import process myself, I could simplify the code maintaining the marks database).

With the git-shim feature already having found an excellent use case (the HTTP fetcher), how do you see the future for the foreign-scm topic? I like the idea of rewriting foreign-scm on top of git-shim, but is there anything substantial _left_ in foreign-scm after such a rewrite?

Quoted 6 lines
Quoted 3 lines

Next, there are 3 patches tweaking and expanding the git-vcs API (with corresponding implementations in the foreign transport code) to adjust for the CVS helper's needs.

These make sense to me. I assume you're planning to support exporting in some ways, but haven't got it working yet?

Indeed. The current version is only a first draft that fetches successfully from simple toy repositories (but probably not many real-world CVS repos). I hope to support both fetching and pushing to real-world CVS repos at some point.

Have fun! :)

...Johan

Signature · 2 lines
Johan Herland, <[email protected]>
www.herland.net
#29 lore raw
Quoted 28 lines · Johan Herland
Quoted 12 lines · Daniel Barkalow
Quoted 10 lines · Johan Herland

This series is a first draft towards implementing a foreign VCS helper program for CVS.

It is based on the 'db/foreign-scm' and 'jh/notes' topics in 'pu'. As such, this patch series should apply cleanly to current 'pu'.

The first 4 patches supply the necessary parts of Daniel Barkalow's foreign-scm topic that has not yet been merged to 'pu' (i.e. everything remaining in Daniel's original patch series, except the p4-specific stuff).

Quoted 7 lines

I've got some changes to this series in the works; I just made a few changes to share the transport-side code for interacting with the helper with the transport-side code for interacting with native-object helpers. Mainly, this means starting each line in the "list" response with "?" to indicate that you don't know the hash, since the protocol is trying to be compatible with cases where you do know.

No problem. I'm actually considering whether I should rewrite the CVS helper to a full-fledged git-shim (btw, thanks for the shim work; it looks really promising). I would still probably have to use git-fast-import, but I would have more control over the fast-import process (e.g. by closing the fast-import process myself, I could simplify the code maintaining the marks database).

Quoted 5 lines

With the git-shim feature already having found an excellent use case (the HTTP fetcher), how do you see the future for the foreign-scm topic? I like the idea of rewriting foreign-scm on top of git-shim, but is there anything substantial _left_ in foreign-scm after such a rewrite?

Things that are still there:

- vcs helpers make active use of the ability to state that there is a remote ref by a particular name, without having any idea what hash it currently holds (until they're told to import it).

- vcs helper is specified directly by the user's config, possibly to invoke a helper distributed separately; shim helpers are determined by transport_get() when it identifies that the url can be handled by that shim.

- vcs helpers don't necessarily use URLs, or anything that works in the same way, to identify the remote repo.

- vcs export wants to deal with the remote server pretty much always rewriting exported commits.

- For export, the vcs helper is going to want to get the commits ordered in some way that makes each export a single-commit export operation, so that each helper doesn't have to deal with ordering the commits suitably; this is particularly significant because the transport-side code should make sure that the history is something that can be represented in the foreign vcs. The native shim helper wants just the endpoint.

Quoted 11 lines
Quoted 6 lines
Quoted 3 lines

Next, there are 3 patches tweaking and expanding the git-vcs API (with corresponding implementations in the foreign transport code) to adjust for the CVS helper's needs.

These make sense to me. I assume you're planning to support exporting in some ways, but haven't got it working yet?

Indeed. The current version is only a first draft that fetches successfully from simple toy repositories (but probably not many real-world CVS repos). I hope to support both fetching and pushing to real-world CVS repos at some point.

Great.
-Daniel
*This .sig left intentionally blank*