Re: [RFC PATCH] Support gitlinks in fast-import/export.
- From
Johannes Schindelin <johannes.schindelin@gmx.de>
- Date
- Jul 18, 2008, 17:36 UTC
- Message-ID
- <alpine.DEB.1.00.0807181810400.8986@racer>
- In-Reply-To
- <200807182103.37272.angavrilov@gmail.com>
Hi,
On Fri, 18 Jul 2008, Alexander Gavrilov wrote:
> What I'm unsure of is, should fast-export try to reuse commit > marks for gitlinks where it happened to recognize the object, > or always output the SHA as it is stored in the tree?
Are they commit marks? No. So they should be handled as marks, just as those for blobs and trees.
(They are commit marks in the _submodule_, but that does not matter here.)
Show 11 quoted lines
> diff --git a/Documentation/git-fast-import.txt b/Documentation/git-fast-import.txt > index 395c055..80c591a 100644 > --- a/Documentation/git-fast-import.txt > +++ b/Documentation/git-fast-import.txt > @@ -481,6 +481,9 @@ in octal. Git only supports the following modes: > what you want. > * `100755` or `755`: A normal, but executable, file. > * `120000`: A symlink, the content of the file will be the link target. > +* `160000`: A gitlink, SHA-1 of the object refers to a commit in > + another repository. Git links can only be specified by SHA or through > + a commit mark. They are used to implements submodules.
s/\(implement\)s/\1/
Show 11 quoted lines
> diff --git a/builtin-fast-export.c b/builtin-fast-export.c
> index d0a462f..14b1549 100644
> --- a/builtin-fast-export.c
> +++ b/builtin-fast-export.c
> @@ -123,8 +123,19 @@ static void show_filemodify(struct diff_queue_struct *q,
> printf("D %s\n", spec->path);
> else {
> struct object *object = lookup_object(spec->sha1);
> - printf("M %06o :%d %s\n", spec->mode,
> - get_object_mark(object), spec->path);
> + int mark = object ? get_object_mark(object) : 0;As I said, that looks wrong. Maybe we have to fake objects for the gitlinks.
Show 7 quoted lines
> @@ -183,7 +194,8 @@ static void handle_commit(struct commit *commit, struct rev_info *rev) > "", &rev->diffopt); > > for (i = 0; i < diff_queued_diff.nr; i++) > - handle_object(diff_queued_diff.queue[i]->two->sha1); > + if (!S_ISGITLINK(diff_queued_diff.queue[i]->two->mode)) > + handle_object(diff_queued_diff.queue[i]->two->sha1);
Why? You do not want to export changes in the submodules?
> diff --git a/fast-import.c b/fast-import.c > index e72b286..e7977c1 100644 > --- a/fast-import.c > +++ b/fast-import.c
I'll let Shawn comment on that. Oh, wait, it's his last day in work today. Maybe I have something useful to say about this part of the patch, then, to save Shawn some work.
Show 13 quoted lines
> @@ -1900,7 +1901,16 @@ static void file_change_m(struct branch *b)
> p = uq.buf;
> }
>
> - if (inline_data) {
> + if (S_ISGITLINK(mode)) {
> + if (inline_data)
> + die("Git links cannot be specified 'inline': %s",
> + command_buf.buf);
> + else if (oe) {
> + if (oe->type != OBJ_COMMIT)
> + die("Not a commit (actually a %s): %s",
> + typename(oe->type), command_buf.buf);How is that supposed to work? Do I understand correctly that you require the user to construct a commit object for the gitlink? That would be actively wrong.
Oh, and your patch lacks test cases that demonstrate how you envisage the whole thing to work.
Ciao, Dscho