threads / discuss / 37990

Getting a commit sha1 from fast-import in a remote-helper

Subject: Getting a commit sha1 from fast-import in a remote-helper

## tl;dr

15 messages between Nov 18, 2014 and Nov 19, 2014.

replies: 14people: 2as markdown or json

Mike Hommey· Nov 18, 2014, 00:34 UTC · lore
Hi,
For some reason, I need to know the sha1 corresponding to some marks
I'm putting in a fast-import stream. Unfortunately, this does not appear
to be possible.
- I'd rather not require a checkpoint to export marks each time I need
  such a sha1, and I'd rather not do that work that requires them after
  all the marks have been created (although currently, I'm forced to).
- fast-import's cat-blob only allows to cat blobs, which, well, is kind
  of in its name; how about adding an equivalent to the git-cat-file
  command?
- fast-import's `ls` command documentation about its output format
  mentions that the output may contain commits, so I tried the trick of
  creating a tree with commits, but fast-import then fails with:
    fatal: Not a blob (actually a commit)
  which I totally understand, but then I wonder why the documentation
  mentions it and how one would get a tree containing references to
  commits. I guess the documentation should be fixed.

So, while there's a possible solution with an equivalent to the git-cat-file command if that'd be accepted, that's also overkill for my need, which is to simply get the sha1 corresponding to a mark. Would you consider a patch adding a command that allows such a resolution?

Cheers,
Mike
Jonathan Nieder· Nov 18, 2014, 01:40 UTC · re: Mike Hommey · lore

Re: Getting a commit sha1 from fast-import in a remote-helper

Mike Hommey wrote:
Show 7 quoted lines
> - fast-import's `ls` command documentation about its output format
>   mentions that the output may contain commits, so I tried the trick of
>   creating a tree with commits, but fast-import then fails with:
>     fatal: Not a blob (actually a commit)
>   which I totally understand, but then I wonder why the documentation
>   mentions it and how one would get a tree containing references to
>   commits. I guess the documentation should be fixed.
Odd.  Here's what happens when I try:
 $ echo "ls $(git rev-parse HEAD)" | git fast-import --quiet
 fatal: Missing space after tree-ish: ls a4a226a366ab0a173ed9e5f70f2a95d0d21e54c5
 fast-import: dumping crash report to .git/fast_import_crash_14080
 $ echo "ls $(git rev-parse HEAD) " | git fast-import --quiet
 040000 tree d3d38e7d71cb40ebbaf2798b01837b3de43fd4a1
How did you get that "Not a blob" message?

I think a good fix would be to teach parse_ls a mode with no <path> parameter. Something like this (untested; needs cleanup and tests):

Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
diff --git i/fast-import.c w/fast-import.c
index d0bd285..a9a46be 100644
--- i/fast-import.c
+++ w/fast-import.c
@@ -278,6 +278,8 @@ struct recent_command {
 	char *buf;
 };
 
+extern const char *tag_type;
+
 /* Configured limits on output */
 static unsigned long max_depth = 10;
 static off_t max_packsize;
@@ -3047,6 +3049,49 @@ static void parse_ls(const char *p, struct branch *b)
 	struct tree_entry *root = NULL;
 	struct tree_entry leaf = {NULL};
 
+	/* ls SP <tree-ish> */
+	if (*p != '"' && !strchr(p, ' ')) {
+		unsigned char sha1[20];
+		struct object_entry *e;
+		static struct strbuf line = STRBUF_INIT;
+		const char *type;
+
+		if (*p == ':') {	/* <mark> */
+			e = find_mark(parse_mark_ref_eol(p));
+			if (!e)
+				die("Unknown mark: %s", command_buf.buf);
+			hashcpy(sha1, e->idx.sha1);
+		} else {	/* <sha1> */
+			if (get_sha1_hex(p, sha1))
+				die("Invalid dataref: %s", command_buf.buf);
+			e = find_object(sha1);
+			p += 40;
+			if (*p)
+				die("Garbage after dataref: %s", command_buf.buf);
+		}
+
+		switch (e->type) {
+		case OBJ_COMMIT:
+			type = commit_type;
+			break;
+		case OBJ_TREE:
+			type = tree_type;
+			break;
+		case OBJ_BLOB:
+			type = blob_type;
+			break;
+		case OBJ_TAG:
+			type = tag_type;
+			break;
+		default:
+			die("Not a tree-ish: %s", command_buf.buf);
+		}
+
+		strbuf_reset(&line);
+		strbuf_addf(&line, "%s %s\n", type, sha1_to_hex(sha1));
+		cat_blob_write(line.buf, line.len);
+	}
+
 	/* ls SP (<tree-ish> SP)? <path> */
 	if (*p == '"') {
 		if (!b)
Mike Hommey· Nov 18, 2014, 02:31 UTC · re: Jonathan Nieder · lore

Re: Getting a commit sha1 from fast-import in a remote-helper

On Mon, Nov 17, 2014 at 05:40:28PM -0800, Jonathan Nieder wrote:
Show 19 quoted lines
> Mike Hommey wrote:
> 
> > - fast-import's `ls` command documentation about its output format
> >   mentions that the output may contain commits, so I tried the trick of
> >   creating a tree with commits, but fast-import then fails with:
> >     fatal: Not a blob (actually a commit)
> >   which I totally understand, but then I wonder why the documentation
> >   mentions it and how one would get a tree containing references to
> >   commits. I guess the documentation should be fixed.
> 
> Odd.  Here's what happens when I try:
> 
>  $ echo "ls $(git rev-parse HEAD)" | git fast-import --quiet
>  fatal: Missing space after tree-ish: ls a4a226a366ab0a173ed9e5f70f2a95d0d21e54c5
>  fast-import: dumping crash report to .git/fast_import_crash_14080
>  $ echo "ls $(git rev-parse HEAD) " | git fast-import --quiet
>  040000 tree d3d38e7d71cb40ebbaf2798b01837b3de43fd4a1
> 
> How did you get that "Not a blob" message?

When trying to *create* a tree with a commit in it, so instead of giving the mark for a blob to a filemodify command, giving a mark for a commit. That is what fails with "Not a blob". So it's not possible to create a tree with a reference to a commit, at least with fast-import. But, the documentation for the ls command says this:

   Output uses the same format as git ls-tree <tree> -- <path>:
        <mode> SP ('blob' | 'tree' | 'commit') SP <dataref> HT <path> LF
The 'commit' string certainly seems it cannot be there.
> I think a good fix would be to teach parse_ls a mode with no <path>
> parameter.  Something like this (untested; needs cleanup and tests):

This would make both your commands output the same thing, right? It wouldn't help my case :)

Mike
Jonathan Nieder· Nov 18, 2014, 02:51 UTC · re: Mike Hommey · lore

Re: Getting a commit sha1 from fast-import in a remote-helper

Mike Hommey wrote:
> On Mon, Nov 17, 2014 at 05:40:28PM -0800, Jonathan Nieder wrote:
Show 5 quoted lines
>> How did you get that "Not a blob" message?
>
> When trying to *create* a tree with a commit in it, so instead of giving
> the mark for a blob to a filemodify command, giving a mark for a commit.
> That is what fails with "Not a blob".

Ah, I see what you were trying now. It's complaining that the data and mode don't match up. See <mode> under 'filemodify' in the manual.

Something like
	M 160000 :1 mycommit

should work fine, though that's a pretty ugly workaround for the inability to do

	ls :1
[...]
Show 5 quoted lines
>> I think a good fix would be to teach parse_ls a mode with no <path>
>> parameter.  Something like this (untested; needs cleanup and tests):
>
> This would make both your commands output the same thing, right? It
> wouldn't help my case :)

It's easily possible my patch has a typo somewhere, but the expected output format would be

	commit 6066a7eac4b2bcdb86971783b583e4e408b32e81
That wouldn't help?

Puzzled, Jonathan

Mike Hommey· Nov 18, 2014, 03:11 UTC · re: Jonathan Nieder · lore

Re: Getting a commit sha1 from fast-import in a remote-helper

On Mon, Nov 17, 2014 at 06:51:31PM -0800, Jonathan Nieder wrote:
Show 20 quoted lines
> Mike Hommey wrote:
> > On Mon, Nov 17, 2014 at 05:40:28PM -0800, Jonathan Nieder wrote:
> 
> >> How did you get that "Not a blob" message?
> >
> > When trying to *create* a tree with a commit in it, so instead of giving
> > the mark for a blob to a filemodify command, giving a mark for a commit.
> > That is what fails with "Not a blob".
> 
> Ah, I see what you were trying now.  It's complaining that the data
> and mode don't match up.  See <mode> under 'filemodify' in the manual.
> 
> Something like
> 
> 	M 160000 :1 mycommit
> 
> should work fine, though that's a pretty ugly workaround for the
> inability to do
> 
> 	ls :1

Actually, for my use, that ugly workaround actually improves things for me, avoiding to use blobs in some of the stuff I want to store :) How did I miss that? Thanks a lot for the enlightenment.

Show 13 quoted lines
> [...]
> >> I think a good fix would be to teach parse_ls a mode with no <path>
> >> parameter.  Something like this (untested; needs cleanup and tests):
> >
> > This would make both your commands output the same thing, right? It
> > wouldn't help my case :)
> 
> It's easily possible my patch has a typo somewhere, but the expected
> output format would be
> 
> 	commit 6066a7eac4b2bcdb86971783b583e4e408b32e81
> 
> That wouldn't help?

Oh, so `ls <dataref>` would print out what <dataref> is? That would definitely help, although with the trick above, I probably wouldn't actually need it anymore.

Mike
Mike Hommey· Nov 19, 2014, 02:18 UTC · re: Mike Hommey · lore

Re: Getting a commit sha1 from fast-import in a remote-helper

On Tue, Nov 18, 2014 at 12:11:47PM +0900, Mike Hommey wrote:
> Oh, so `ls <dataref>` would print out what <dataref> is? That would
> definitely help, although with the trick above, I probably wouldn't
> actually need it anymore.

So, in the end, I was able to do everything with what's currently provided by git fast-import, but one thing would probably make life easier for me: being able to initialize a commit tree from a commit that's not one of the direct parents.

Because the data I'm using gives diffs against possibly unrelated commits, and because starting a tree from scratch is actually slow when you have thousands of subdirectories, it would be easier if I could just start from that tree I have a diff against and apply the changes. Without this, there would be a lot of `ls` command emitting involved, and I'm actually not sure that wouldn't be as slow as starting from scratch (haven't implemented yet, so I can't tell). Also, I'm not sure how I'm supposed to know how much to read back from `ls`.

Mike
Jonathan Nieder· Nov 19, 2014, 02:21 UTC · re: Mike Hommey · lore

Re: Getting a commit sha1 from fast-import in a remote-helper

Mike Hommey wrote:
> So, in the end, I was able to do everything with what's currently
> provided by git fast-import, but one thing would probably make life
> easier for me: being able to initialize a commit tree from a commit
> that's not one of the direct parents.

IIRC then 'M 040000' wants a tree object, not a commit object, so you'd have to do

	ls <commit> ""
	M 040000 <tree> ""
Mike Hommey· Nov 19, 2014, 02:27 UTC · re: Jonathan Nieder · lore

Re: Getting a commit sha1 from fast-import in a remote-helper

On Tue, Nov 18, 2014 at 06:21:22PM -0800, Jonathan Nieder wrote:
Show 12 quoted lines
> Mike Hommey wrote:
> 
> > So, in the end, I was able to do everything with what's currently
> > provided by git fast-import, but one thing would probably make life
> > easier for me: being able to initialize a commit tree from a commit
> > that's not one of the direct parents.
> 
> IIRC then 'M 040000' wants a tree object, not a commit object, so
> you'd have to do
> 
> 	ls <commit> ""
> 	M 040000 <tree> ""

That's what I'm planning to try ; Would doing: M 040000 <tree> "" M 0644 <blob> some/path D other/path

work? Or do I have to actually build a tree from the combination of the output from various ls and those filedelete/filemodify?

Mike
Jonathan Nieder· Nov 19, 2014, 02:36 UTC · re: Mike Hommey · lore

Re: Getting a commit sha1 from fast-import in a remote-helper

On Wed, Nov 19, 2014 at 11:27:59AM +0900, Mike Hommey wrote:
> On Tue, Nov 18, 2014 at 06:21:22PM -0800, Jonathan Nieder wrote:
Show 12 quoted lines
>> IIRC then 'M 040000' wants a tree object, not a commit object, so
>> you'd have to do
>>
>> 	ls <commit> ""
>> 	M 040000 <tree> ""
>
> That's what I'm planning to try ; Would doing:
> M 040000 <tree> ""
> M 0644 <blob> some/path
> D other/path
>
> work?

Yes, that should work fine. That's how contrib/svn-fe/svn-fe.c deals with 'Node-copyfrom-rev' in Subversion dumpfiles, for what it's worth.

Mike Hommey· Nov 18, 2014, 02:21 UTC · re: Mike Hommey · lore

Re: Getting a commit sha1 from fast-import in a remote-helper

On Tue, Nov 18, 2014 at 09:34:26AM +0900, Mike Hommey wrote:
Show 8 quoted lines
> Hi,
> 
> For some reason, I need to know the sha1 corresponding to some marks
> I'm putting in a fast-import stream. Unfortunately, this does not appear
> to be possible.
> - I'd rather not require a checkpoint to export marks each time I need
>   such a sha1, and I'd rather not do that work that requires them after
>   all the marks have been created (although currently, I'm forced to).

BTW, if it so happens that all the operations that were done end up creating objects that already existed for some reason, checkpoint doesn't do anything, which is fine for the pack and tags, but not necessarily so for export-marks.

Mike
Mike Hommey· Nov 18, 2014, 02:35 UTC · re: Mike Hommey · lore

Re: Getting a commit sha1 from fast-import in a remote-helper

On Tue, Nov 18, 2014 at 11:21:37AM +0900, Mike Hommey wrote:
Show 14 quoted lines
> On Tue, Nov 18, 2014 at 09:34:26AM +0900, Mike Hommey wrote:
> > Hi,
> > 
> > For some reason, I need to know the sha1 corresponding to some marks
> > I'm putting in a fast-import stream. Unfortunately, this does not appear
> > to be possible.
> > - I'd rather not require a checkpoint to export marks each time I need
> >   such a sha1, and I'd rather not do that work that requires them after
> >   all the marks have been created (although currently, I'm forced to).
> 
> BTW, if it so happens that all the operations that were done end up
> creating objects that already existed for some reason, checkpoint
> doesn't do anything, which is fine for the pack and tags, but not
> necessarily so for export-marks.

And while I'm here, it's sad that one needs to emit a dummy cat-blob or ls command to wait for a checkpoint to be finished because they are the only commands that trigger something written on the cat-blob fd that can be waited for.

Mike.
Jonathan Nieder· Nov 18, 2014, 03:27 UTC · re: Mike Hommey · lore

Re: Getting a commit sha1 from fast-import in a remote-helper

Mike Hommey wrote:
> And while I'm here, it's sad that one needs to emit a dummy cat-blob or
> ls command to wait for a checkpoint to be finished

That's a good point. (Though relying on checkpoints to read back information is an ugly trick, so if we can get other commands to provide the information you need then that would be better.)

The old-fashioned way is to do "progress checkpoint done". Alas, that writes to the progress fd, which doesn't go to the remote helper's stdin. How about something like this?

Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
diff --git i/Documentation/git-fast-import.txt w/Documentation/git-fast-import.txt
index 377eeaa..c0dedac 100644
--- i/Documentation/git-fast-import.txt
+++ w/Documentation/git-fast-import.txt
@@ -1055,6 +1055,12 @@ ls::
 	rather than wasting time on the early part of an import
 	before the unsupported command is detected.
 
+notify-on-checkpoint::
+	Cause the backend to write `checkpoint complete` to the file
+	descriptor set with `--cat-blob-fd`, or to `stdout` if
+	`--cat-blob-fd` was unspecified, when finishing a checkpoint
+	requested via a `checkpoint` command or SIGUSR1.
+
 notes::
 	Require that the backend support the 'notemodify' (N)
 	subcommand to the 'commit' command.
diff --git i/fast-import.c w/fast-import.c
index d0bd285..ccf4768 100644
--- i/fast-import.c
+++ w/fast-import.c
@@ -329,6 +329,7 @@ static const char *import_marks_file;
 static int import_marks_file_from_stream;
 static int import_marks_file_ignore_missing;
 static int relative_marks_paths;
+static int notify_on_checkpoint;
 
 /* Our last blob */
 static struct last_object last_blob = { STRBUF_INIT, 0, 0, 0 };
@@ -3094,6 +3095,9 @@ static void checkpoint(void)
 		dump_tags();
 		dump_marks();
 	}
+	if (notify_on_checkpoint)
+		cat_blob_write("checkpoint complete\n",
+			       strlen("checkpoint complete\n"));
 }
 
 static void parse_checkpoint(void)
@@ -3244,6 +3248,8 @@ static int parse_one_feature(const char *feature, int from_stream)
 		option_export_marks(arg);
 	} else if (!strcmp(feature, "cat-blob")) {
 		; /* Don't die - this feature is supported */
+	} else if (!strcmp(feature, "notify-on-checkpoint")) {
+		notify_on_checkpoint = 1;
 	} else if (!strcmp(feature, "relative-marks")) {
 		relative_marks_paths = 1;
 	} else if (!strcmp(feature, "no-relative-marks")) {
Mike Hommey· Nov 18, 2014, 04:17 UTC · re: Jonathan Nieder · lore

Re: Getting a commit sha1 from fast-import in a remote-helper

On Mon, Nov 17, 2014 at 07:27:41PM -0800, Jonathan Nieder wrote:
Show 12 quoted lines
> Mike Hommey wrote:
> 
> > And while I'm here, it's sad that one needs to emit a dummy cat-blob or
> > ls command to wait for a checkpoint to be finished
> 
> That's a good point.  (Though relying on checkpoints to read back
> information is an ugly trick, so if we can get other commands to
> provide the information you need then that would be better.)
> 
> The old-fashioned way is to do "progress checkpoint done".  Alas, that
> writes to the progress fd, which doesn't go to the remote helper's
> stdin.  How about something like this?

How about something more generic, like "sync", which purpose would be to request synchronisation with fast-import, which would respond "sync" on the cat-blob fd?

Or "ping"<->"pong".

Although... I wonder if that would really be useful for anything else than checkpoint...

That said, I, for one, would be fine if this is not "fixed" as long as marks can be resolved some other way (and, in fact, it may turn out that I don't need to resolve marks to sha1s at all)

Thanks for you help
Mike
Jonathan Nieder· Nov 18, 2014, 02:53 UTC · re: Mike Hommey · lore

Re: Getting a commit sha1 from fast-import in a remote-helper

Mike Hommey wrote:
> BTW, if it so happens that all the operations that were done end up
> creating objects that already existed for some reason, checkpoint
> doesn't do anything, which is fine for the pack and tags, but not
> necessarily so for export-marks.
Does something like this help?
Do you have a short script that can demonstrate the failure?

Lazily, Jonathan

Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
diff --git i/fast-import.c w/fast-import.c
index d0bd285..c3d53c8 100644
--- i/fast-import.c
+++ w/fast-import.c
@@ -3088,12 +3088,11 @@ static void parse_ls(const char *p, struct branch *b)
 static void checkpoint(void)
 {
 	checkpoint_requested = 0;
-	if (object_count) {
+	if (object_count)
 		cycle_packfile();
-		dump_branches();
-		dump_tags();
-		dump_marks();
-	}
+	dump_branches();
+	dump_tags();
+	dump_marks();
 }
 
 static void parse_checkpoint(void)
Mike Hommey· Nov 18, 2014, 03:14 UTC · re: Jonathan Nieder · lore

Re: Getting a commit sha1 from fast-import in a remote-helper

On Mon, Nov 17, 2014 at 06:53:59PM -0800, Jonathan Nieder wrote:
Show 8 quoted lines
> Mike Hommey wrote:
> 
> > BTW, if it so happens that all the operations that were done end up
> > creating objects that already existed for some reason, checkpoint
> > doesn't do anything, which is fine for the pack and tags, but not
> > necessarily so for export-marks.
> 
> Does something like this help?

I'm not sure about branches and tags, as they would mostly be noops, but dump_marks definitely needs to happen. I did try to move dump_marks out of the if already, and that did what I wanted.

Mike

← back to recent threads