threads / rfc / 17095

RFC patch, 3 partsSupport fetching from foreign VCSes

Subject: [RFC PATCH 3/3] Support fetching from foreign VCSes

## tl;dr

9 messages between Jan 11, 2009 and Feb 1, 2009. Diffs are folded; open one to read it.

replies: 8people: 5as markdown or json

Daniel Barkalow· Jan 11, 2009, 20:12 UTC · lore

This supports a useful subset of the usual fetch logic, mostly in the config file.

Signed-off-by: Daniel Barkalow <barkalow@iabervon.org>
---
 builtin-fetch.c |  135 +++++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 files changed, 132 insertions(+), 3 deletions(-)
Show changes to builtin-fetch.c +132 −3
diff --git a/builtin-fetch.c b/builtin-fetch.c
index 7b46f8f..14e037e 100644
--- a/builtin-fetch.c
+++ b/builtin-fetch.c
@@ -614,6 +614,136 @@ static void set_option(const char *name, const char *value)
 			name, transport->url);
 }
 
+static struct ref *list_foreign(struct remote *remote)
+{
+	struct child_process importer;
+	struct ref *ret = NULL;
+	struct ref **end = &ret;
+	struct strbuf buf;
+	memset(&importer, 0, sizeof(importer));
+	importer.in = 0;
+	importer.no_stdin = 1;
+	importer.out = -1;
+	importer.err = 0;
+	importer.argv = xcalloc(5, sizeof(*importer.argv));
+	strbuf_init(&buf, 80);
+	strbuf_addf(&buf, "vcs-%s", remote->foreign_vcs);
+	importer.argv[0] = buf.buf;
+	importer.argv[1] = "list";
+	importer.argv[2] = remote->name;
+	importer.git_cmd = 1;
+	start_command(&importer);
+
+	strbuf_reset(&buf);
+	while (1) {
+		char *eol, *eon;
+		if (strbuf_read(&buf, importer.out, 80) <= 0)
+			break;
+		while (1) {
+			eol = strchr(buf.buf, '\n');
+			if (!eol)
+				break;
+			*eol = '\0';
+			eon = strchr(buf.buf, ' ');
+			if (eon)
+				*eon = '\0';
+			*end = alloc_ref(buf.buf);
+			end = &((*end)->next);
+			strbuf_remove(&buf, 0, eol - buf.buf + 1);
+		}
+	}
+
+	finish_command(&importer);
+	strbuf_release(&buf);
+	return ret;
+}
+
+static int import_foreign(struct remote *remote, struct ref *refs)
+{
+	struct child_process importer;
+	struct child_process fastimport;
+	struct ref *posn;
+	int count = 0;
+	struct strbuf buf;
+
+	for (posn = refs; posn; posn = posn->next)
+		count++;
+
+	memset(&importer, 0, sizeof(importer));
+	importer.in = 0;
+	importer.no_stdin = 1;
+	importer.out = -1;
+	importer.err = 0;
+	importer.argv = xcalloc(5 + count, sizeof(*importer.argv));
+	strbuf_init(&buf, 80);
+	strbuf_addf(&buf, "vcs-%s", remote->foreign_vcs);
+	importer.argv[0] = buf.buf;
+	importer.argv[1] = "import";
+	importer.argv[2] = remote->name;
+	count = 0;
+	for (posn = refs; posn; posn = posn->next) {
+		importer.argv[3 + count] = posn->name;
+		count++;
+	}
+	importer.git_cmd = 1;
+	start_command(&importer);
+
+	memset(&fastimport, 0, sizeof(fastimport));
+	fastimport.in = importer.out;
+	fastimport.argv = xcalloc(2, sizeof(*fastimport.argv));
+	fastimport.argv[0] = "fast-import";
+	fastimport.argv[1] = "--quiet";
+	fastimport.git_cmd = 1;
+	start_command(&fastimport);
+
+	finish_command(&importer);
+	finish_command(&fastimport);
+	strbuf_release(&buf);
+	return 0;
+}
+
+static int fetch_foreign(struct remote *remote)
+{
+	struct ref *remote_refs = list_foreign(remote);
+	struct ref *ref_map = NULL;
+	struct ref *rm;
+	struct ref **tail = &ref_map;
+	struct branch *branch;
+	int i;
+
+	int exit_code = import_foreign(remote, remote_refs);
+	if (exit_code)
+		return exit_code;
+
+	/* if not appending, truncate FETCH_HEAD */
+	if (!append) {
+		char *filename = git_path("FETCH_HEAD");
+		FILE *fp = fopen(filename, "w");
+		if (!fp)
+			return error("cannot open %s: %s\n", filename, strerror(errno));
+		fclose(fp);
+	}
+
+	for (rm = remote_refs; rm; rm = rm->next)
+		read_ref(rm->name, rm->old_sha1);
+
+	branch = branch_get(NULL);
+
+	for (i = 0; i < remote->fetch_refspec_nr; i++) {
+		get_fetch_map(remote_refs, &remote->fetch[i], &tail, 0);
+		if (!strcmp(branch->remote_name, remote->name))
+			add_merge_config(&ref_map, remote_refs, branch, &tail);
+	}
+
+	for (rm = ref_map; rm; rm = rm->next)
+		if (rm->peer_ref)
+			read_ref(rm->peer_ref->name, rm->peer_ref->old_sha1);
+
+	store_updated_refs("foreign", remote->name, ref_map);
+
+	return exit_code;
+}
+
 int cmd_fetch(int argc, const char **argv, const char *prefix)
 {
 	struct remote *remote;
@@ -635,9 +765,8 @@ int cmd_fetch(int argc, const char **argv, const char *prefix)
 	else
 		remote = remote_get(argv[0]);
 
-	if (remote->foreign_vcs) {
-		die("Using foreign VCSes for fetch is not yet supported.");
-	}
+	if (remote->foreign_vcs)
+		return fetch_foreign(remote);
 
 	transport = transport_get(remote, remote->url[0]);
 	if (verbosity >= 2)
-- 
1.6.0.6
Junio C Hamano· Jan 12, 2009, 01:33 UTC · re: Daniel Barkalow · lore

Re: [RFC PATCH 3/3] Support fetching from foreign VCSes

Daniel Barkalow <barkalow@iabervon.org> writes:
> This supports a useful subset of the usual fetch logic, mostly in the
> config file.
>
> Signed-off-by: Daniel Barkalow <barkalow@iabervon.org>

I like the direction this series is going. In the longer term, it would be nice if we can have git-svn and git-cvsimport replaced with something like this.

Is the current foreign vcs interface sufficiently rich to support git as a foreign scm by using fast-import and fast-export?

Thanks.
Daniel Barkalow· Jan 12, 2009, 05:48 UTC · re: Junio C Hamano · lore

Re: [RFC PATCH 3/3] Support fetching from foreign VCSes

On Sun, 11 Jan 2009, Junio C Hamano wrote:
Show 10 quoted lines
> Daniel Barkalow <barkalow@iabervon.org> writes:
> 
> > This supports a useful subset of the usual fetch logic, mostly in the
> > config file.
> >
> > Signed-off-by: Daniel Barkalow <barkalow@iabervon.org>
> 
> I like the direction this series is going.  In the longer term, it would
> be nice if we can have git-svn and git-cvsimport replaced with something
> like this.

That was my goal, although the only foreign system I currently use is Perforce.

> Is the current foreign vcs interface sufficiently rich to support git as a
> foreign scm by using fast-import and fast-export?

I think so, although it would be pretty strange, since it would generally have a whole lot of local data (a complete clone of any remote repository).

	-Daniel
*This .sig left intentionally blank*
Shawn O. Pearce· Jan 12, 2009, 15:42 UTC · re: Daniel Barkalow · lore

Re: [RFC PATCH 3/3] Support fetching from foreign VCSes

Daniel Barkalow <barkalow@iabervon.org> wrote:
Show 8 quoted lines
> On Sun, 11 Jan 2009, Junio C Hamano wrote:
> > Daniel Barkalow <barkalow@iabervon.org> writes:
> > > This supports a useful subset of the usual fetch logic, mostly in the
> > > config file.
> > 
> > I like the direction this series is going.  In the longer term, it would
> > be nice if we can have git-svn and git-cvsimport replaced with something
> > like this.

The series seems to require that the foreign tool speak fast-import. I've tried to get git-svn to use it, but its currently not possible because the git-svn process needs to be able to read objects it has written during this session. Those objects are stored in the output pack, where only the active fast-import process can get to them. Thus git-svn can't use fast-import.

As import as the git-p4 stuff is, git-svn is our "killer feature" when it
comes to foreign system integration.  I think we need to make SVN work for
this foreign vcs thing to work.
 
Show 6 quoted lines
> > Is the current foreign vcs interface sufficiently rich to support git as a
> > foreign scm by using fast-import and fast-export?
> 
> I think so, although it would be pretty strange, since it would generally 
> have a whole lot of local data (a complete clone of any remote 
> repository).

It might not be that bad. If the foreign system is git and the local system is git, we should have a massive amount of object reuse. At least all of the blobs from the foreign system should be reused by the local system, and that's like 80-90% of most project's object state.

If the import is flawless (no mangling of commit messages or history) then you should get 100% object reuse and the git-git import would give you the same SHA-1, but just slower than going over git://.

Its strange only because it would be sucking more CPU time on the client than is necessary to do the fetch. But if something like a filter-branch tool existed to massage a fast-import stream, it might be useful to fetch someone else's tree, massage it a bit, and then import it with a subtree merge. :-)

-- 
Shawn.
Daniel Barkalow· Jan 12, 2009, 18:19 UTC · re: Shawn O. Pearce · lore

Re: [RFC PATCH 3/3] Support fetching from foreign VCSes

On Mon, 12 Jan 2009, Shawn O. Pearce wrote:
Show 19 quoted lines
> Daniel Barkalow <barkalow@iabervon.org> wrote:
> > On Sun, 11 Jan 2009, Junio C Hamano wrote:
> > > Daniel Barkalow <barkalow@iabervon.org> writes:
> > > > This supports a useful subset of the usual fetch logic, mostly in the
> > > > config file.
> > > 
> > > I like the direction this series is going.  In the longer term, it would
> > > be nice if we can have git-svn and git-cvsimport replaced with something
> > > like this.
> 
> The series seems to require that the foreign tool speak fast-import.  I've
> tried to get git-svn to use it, but its currently not possible because the
> git-svn process needs to be able to read objects it has written during this
> session.  Those objects are stored in the output pack, where only the active
> fast-import process can get to them.  Thus git-svn can't use fast-import.
>
> As import as the git-p4 stuff is, git-svn is our "killer feature" when it
> comes to foreign system integration.  I think we need to make SVN work for
> this foreign vcs thing to work.

I think fast-import should be made sufficient for git-svn, rather than having the integration about to speak other things. As you say, git-svn is the killer feature, and I'd like to have it using (and therefore contributing to the testing of) all of the applicable tools.

Maybe fast-import should be able to run with a bi-directional connection to its input, so it can acknowledge checkpoints? When the pipeline is set up in the main git code, it should be easy enough to do this sort of complicated stuff.

Show 12 quoted lines
> > > Is the current foreign vcs interface sufficiently rich to support git as a
> > > foreign scm by using fast-import and fast-export?
> > 
> > I think so, although it would be pretty strange, since it would generally 
> > have a whole lot of local data (a complete clone of any remote 
> > repository).
> 
> It might not be that bad.  If the foreign system is git and the
> local system is git, we should have a massive amount of object reuse.
> At least all of the blobs from the foreign system should be reused
> by the local system, and that's like 80-90% of most project's
> object state.

The part I think is weird is that the helper would be first going to the network to get all of the data, and then extracting the history from it locally in a separate process (that is, passing data to it only through the filesystem). Other systems generally get the data on demand, which sort of makes more sense for the case where you will then run exactly one command (fast-export) to it. I suppose sticking the data into the destination object store would make this not so strange.

Show 9 quoted lines
> If the import is flawless (no mangling of commit messages or history)
> then you should get 100% object reuse and the git-git import would
> give you the same SHA-1, but just slower than going over git://.
> 
> Its strange only because it would be sucking more CPU time on the
> client than is necessary to do the fetch.  But if something like
> a filter-branch tool existed to massage a fast-import stream, it
> might be useful to fetch someone else's tree, massage it a bit,
> and then import it with a subtree merge.  :-)

That is true, and would be a good actual use for this. It would make it possible for Wine to have a remote for the upstream Kconfig repository (that being a filtering of the linux-2.6 repository).

That is, a git repository can be "foreign" in that it disagrees with you over the project layout or contents.

	-Daniel
*This .sig left intentionally blank*
Johannes Schindelin· Jan 12, 2009, 19:09 UTC · re: Daniel Barkalow · lore

Re: [RFC PATCH 3/3] Support fetching from foreign VCSes

Hi,
On Mon, 12 Jan 2009, Daniel Barkalow wrote:
> Maybe fast-import should be able to run with a bi-directional connection 
> to its input, so it can acknowledge checkpoints?
Whoa.

fast-import was first and foremost a very simple way to get stuff done. Is it absolutely necessary to complicate that?

I mean, I don't know about git-svn, but shouldn't it be relatively easy to use fast-import to import from Subversion once the information is extracted from Subversion?

Ciao, Dscho

Daniel Barkalow· Jan 12, 2009, 19:38 UTC · re: Johannes Schindelin · lore

Re: [RFC PATCH 3/3] Support fetching from foreign VCSes

On Mon, 12 Jan 2009, Johannes Schindelin wrote:
Show 15 quoted lines
> Hi,
> 
> On Mon, 12 Jan 2009, Daniel Barkalow wrote:
> 
> > Maybe fast-import should be able to run with a bi-directional connection 
> > to its input, so it can acknowledge checkpoints?
> 
> Whoa.
> 
> fast-import was first and foremost a very simple way to get stuff done.  
> Is it absolutely necessary to complicate that?
> 
> I mean, I don't know about git-svn, but shouldn't it be 
> relatively easy to use fast-import to import from Subversion once the 
> information is extracted from Subversion?

I assume that git-svn's problem is that it would need to store too much of the information (rather than being able to discard it) if it couldn't get the information back from the git object database.

Actually, I think it might be actually simplify programs using fast-import if they could ask it for data from before, and I think that would have to be necessary in order to have a reasonable non-git-specific incremental importer without duplicating the storage of a lot of information that previously went into fast-import.

I know I found it helpful in my p4 one to be able to pick up the incremental state by looking at what refs exist and the information in commits they point to, rather than trying to get the information from the perforce client state or custom storage.

	-Daniel
*This .sig left intentionally blank*
Johan Herland· Feb 1, 2009, 02:20 UTC · re: Daniel Barkalow · lore

Re: [RFC PATCH 3/3] Support fetching from foreign VCSes

On Sunday 11 January 2009, Daniel Barkalow wrote:
> This supports a useful subset of the usual fetch logic, mostly in the
> config file.
Hi,

I love the idea of this patch series, and have started working on a CVS backend for this. I have a question though...

Show 16 quoted lines
> Signed-off-by: Daniel Barkalow <barkalow@iabervon.org>
> ---
>  builtin-fetch.c |  135
> +++++++++++++++++++++++++++++++++++++++++++++++++++++- 1 files changed,
> 132 insertions(+), 3 deletions(-)
>
> diff --git a/builtin-fetch.c b/builtin-fetch.c
> index 7b46f8f..14e037e 100644
> --- a/builtin-fetch.c
> +++ b/builtin-fetch.c
> @@ -614,6 +614,136 @@ static void set_option(const char *name, const char
> *value) name, transport->url);
>  }
>
> +static struct ref *list_foreign(struct remote *remote)
> +{
[...]
> +}
> +
> +static int import_foreign(struct remote *remote, struct ref *refs)
> +{
[...]
Show 5 quoted lines
> +}
> +
> +static int fetch_foreign(struct remote *remote)
> +{
> +	struct ref *remote_refs = list_foreign(remote);
We retrieve a list of all refs available at the given remote...
Show 7 quoted lines
> +	struct ref *ref_map = NULL;
> +	struct ref *rm;
> +	struct ref **tail = &ref_map;
> +	struct branch *branch;
> +	int i;
> +
> +	int exit_code = import_foreign(remote, remote_refs);
...and then we start fetching _all_ the refs returned by list_foreign().

When I call "git fetch <vcs-remote> <ref>", I expect _only_ <ref> to be fetched from the remote, but it seems the above code doesn't even concern itself with the ref(s) specified on the "git fetch" command-line

I'm not even sure why list_foreign() should be called in this case (except, maybe, to verify the existence of <ref> before attempting to fetch it).

AFAICS list_foreign() is only needed by "git fetch" if <ref> contains a wildcard (like the default refspec: +refs/heads/*:refs/remotes/<remote>/*).

...or have I misunderstood something fundamental about how this is going to work?

Have fun! :)
...Johan
-- 
Johan Herland, <johan@herland.net>
www.herland.net
Daniel Barkalow· Feb 1, 2009, 02:35 UTC · re: Johan Herland · lore

Re: [RFC PATCH 3/3] Support fetching from foreign VCSes

On Sun, 1 Feb 2009, Johan Herland wrote:
Show 65 quoted lines
> On Sunday 11 January 2009, Daniel Barkalow wrote:
> > This supports a useful subset of the usual fetch logic, mostly in the
> > config file.
> 
> Hi,
> 
> I love the idea of this patch series, and have started working on a CVS 
> backend for this. I have a question though...
> 
> > Signed-off-by: Daniel Barkalow <barkalow@iabervon.org>
> > ---
> >  builtin-fetch.c |  135
> > +++++++++++++++++++++++++++++++++++++++++++++++++++++- 1 files changed,
> > 132 insertions(+), 3 deletions(-)
> >
> > diff --git a/builtin-fetch.c b/builtin-fetch.c
> > index 7b46f8f..14e037e 100644
> > --- a/builtin-fetch.c
> > +++ b/builtin-fetch.c
> > @@ -614,6 +614,136 @@ static void set_option(const char *name, const char
> > *value) name, transport->url);
> >  }
> >
> > +static struct ref *list_foreign(struct remote *remote)
> > +{
> 
> [...]
> 
> > +}
> > +
> > +static int import_foreign(struct remote *remote, struct ref *refs)
> > +{
> 
> [...]
> 
> > +}
> > +
> > +static int fetch_foreign(struct remote *remote)
> > +{
> > +	struct ref *remote_refs = list_foreign(remote);
> 
> We retrieve a list of all refs available at the given remote...
> 
> > +	struct ref *ref_map = NULL;
> > +	struct ref *rm;
> > +	struct ref **tail = &ref_map;
> > +	struct branch *branch;
> > +	int i;
> > +
> > +	int exit_code = import_foreign(remote, remote_refs);
> 
> ...and then we start fetching _all_ the refs returned by list_foreign().
> 
> When I call "git fetch <vcs-remote> <ref>", I expect _only_ <ref> to be 
> fetched from the remote, but it seems the above code doesn't even concern 
> itself with the ref(s) specified on the "git fetch" command-line
> 
> I'm not even sure why list_foreign() should be called in this case (except, 
> maybe, to verify the existence of <ref> before attempting to fetch it).
> 
> AFAICS list_foreign() is only needed by "git fetch" if <ref> contains a 
> wildcard (like the default refspec: +refs/heads/*:refs/remotes/<remote>/*).
> 
> ...or have I misunderstood something fundamental about how this is going to 
> work?

Nope, I was just a bit lazy; I just didn't implement looking at the command line (I just use "git fetch" when I'm using this), and my goal is to use wildcards (although I haven't gotten wildcards implemented that can match the branch names than come naturally out of the particular foreign system I'm using).

I was also staying as close as possible to the git design, where it always fetches the complete list and then looks at what matches. It would probably be a good optimization to only ask for the complete list if it's needed to determine what specific refs to ask for.

Incidently, you probably want this patch so it doesn't crash if you don't have a merge config:

commit 71266fe87dae351c0f77b0ecf8802dab2909db05
Author: Daniel Barkalow <barkalow@iabervon.org>
Date:   Fri Jan 30 20:30:10 2009 -0500
    Fix bug with not having merge config
    
    Signed-off-by: Daniel Barkalow <barkalow@iabervon.org>
Show changes to builtin-fetch.c +4 −1
diff --git a/builtin-fetch.c b/builtin-fetch.c
index 14e037e..63e0637 100644
--- a/builtin-fetch.c
+++ b/builtin-fetch.c
@@ -710,6 +710,7 @@ static int fetch_foreign(struct remote *remote)
 	struct ref **tail = &ref_map;
 	struct branch *branch;
 	int i;
+	int has_merge;
 
 	int exit_code = import_foreign(remote, remote_refs);
 	if (exit_code)
@@ -729,9 +730,11 @@ static int fetch_foreign(struct remote *remote)
 
 	branch = branch_get(NULL);
 
+	has_merge = branch_has_merge_config(branch);
+
 	for (i = 0; i < remote->fetch_refspec_nr; i++) {
 		get_fetch_map(remote_refs, &remote->fetch[i], &tail, 0);
-		if (!strcmp(branch->remote_name, remote->name))
+		if (has_merge && !strcmp(branch->remote_name, remote->name))
 			add_merge_config(&ref_map, remote_refs, branch, &tail);
 	}
 

	-Daniel
*This .sig left intentionally blank*

← back to recent threads