All of lore.kernel.org
 help / color / mirror / Atom feed
* A few fast-export fixups
@ 2009-06-20  4:36 newren
  2009-06-20  4:36 ` [PATCH 1/7] fast-export: Omit tags that tag trees newren
                   ` (6 more replies)
  0 siblings, 7 replies; 14+ messages in thread
From: newren @ 2009-06-20  4:36 UTC (permalink / raw)
  To: git; +Cc: Johannes.Schindelin, kusambite

This patch series fixes a few fast-export bugs I have come across,
plus some new testcases to verify the fixes, and finally a small
addition to the documentation.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH 1/7] fast-export: Omit tags that tag trees
  2009-06-20  4:36 A few fast-export fixups newren
@ 2009-06-20  4:36 ` newren
  2009-06-20 17:31   ` Jeff King
  2009-06-20  4:36 ` [PATCH 2/7] Modify fast-export testcase to check that we correctly omit tags of trees newren
                   ` (5 subsequent siblings)
  6 siblings, 1 reply; 14+ messages in thread
From: newren @ 2009-06-20  4:36 UTC (permalink / raw)
  To: git; +Cc: Johannes.Schindelin, kusambite, Elijah Newren

From: Elijah Newren <newren@gmail.com>

Commit c0582c53bcf4e83bba70e1ad23abbad31f96ebc8 introduced logic to just
omit tags that point to tree objects.  However, a case was missed
resulting in a tag being output which pointed at "mark :0", which would
cause fast-import to crash.

Signed-off-by: Elijah Newren <newren@gmail.com>
---
 builtin-fast-export.c |    8 +++++++-
 1 files changed, 7 insertions(+), 1 deletions(-)

diff --git a/builtin-fast-export.c b/builtin-fast-export.c
index 6cef810..891e2d4 100644
--- a/builtin-fast-export.c
+++ b/builtin-fast-export.c
@@ -407,9 +407,15 @@ static void handle_tags_and_duplicates(struct string_list *extra_refs)
 	for (i = extra_refs->nr - 1; i >= 0; i--) {
 		const char *name = extra_refs->items[i].string;
 		struct object *object = extra_refs->items[i].util;
+		struct tag *tag;
 		switch (object->type) {
 		case OBJ_TAG:
-			handle_tag(name, (struct tag *)object);
+			tag = (struct tag *)object;
+			if (tag->tagged->type == OBJ_TREE) {
+				/* Ignore this tag altogether */
+				return;
+			}
+			handle_tag(name, tag);
 			break;
 		case OBJ_COMMIT:
 			/* create refs pointing to already seen commits */
-- 
1.6.3.2.323.gfb84f

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 2/7] Modify fast-export testcase to check that we correctly omit tags of trees
  2009-06-20  4:36 A few fast-export fixups newren
  2009-06-20  4:36 ` [PATCH 1/7] fast-export: Omit tags that tag trees newren
@ 2009-06-20  4:36 ` newren
  2009-06-21  5:53   ` Stephen Boyd
  2009-06-21  6:17   ` Johannes Sixt
  2009-06-20  4:36 ` [PATCH 3/7] fast-export: Make sure we show actual ref names instead of "(null)" newren
                   ` (4 subsequent siblings)
  6 siblings, 2 replies; 14+ messages in thread
From: newren @ 2009-06-20  4:36 UTC (permalink / raw)
  To: git; +Cc: Johannes.Schindelin, kusambite, Elijah Newren

From: Elijah Newren <newren@gmail.com>


Signed-off-by: Elijah Newren <newren@gmail.com>
---
Should I have just squashed this with the previous patch?  Or with the
other testcase patch?

 t/t9301-fast-export.sh |    8 +++++++-
 1 files changed, 7 insertions(+), 1 deletions(-)

diff --git a/t/t9301-fast-export.sh b/t/t9301-fast-export.sh
index 8c8a9e6..d17f0e4 100755
--- a/t/t9301-fast-export.sh
+++ b/t/t9301-fast-export.sh
@@ -272,7 +272,13 @@ test_expect_success 'set-up a few more tags for tag export tests' '
 '
 
 # NEEDSWORK: not just check return status, but validate the output
-test_expect_success 'tree_tag'        'git fast-export tree_tag'
+test_expect_success 'tree_tag'        '
+	mkdir result &&
+	cd result &&
+	git init &&
+	cd ..
+	git fast-export tree_tag | (cd result && git fast-import)
+'
 test_expect_success 'tree_tag-obj'    'git fast-export tree_tag-obj'
 test_expect_success 'tag-obj_tag'     'git fast-export tag-obj_tag'
 test_expect_success 'tag-obj_tag-obj' 'git fast-export tag-obj_tag-obj'
-- 
1.6.3.2.323.gfb84f

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 3/7] fast-export: Make sure we show actual ref names instead of "(null)"
  2009-06-20  4:36 A few fast-export fixups newren
  2009-06-20  4:36 ` [PATCH 1/7] fast-export: Omit tags that tag trees newren
  2009-06-20  4:36 ` [PATCH 2/7] Modify fast-export testcase to check that we correctly omit tags of trees newren
@ 2009-06-20  4:36 ` newren
  2009-06-20  4:37 ` [PATCH 4/7] fast-export: Do parent rewriting to avoid dropping relevant commits newren
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 14+ messages in thread
From: newren @ 2009-06-20  4:36 UTC (permalink / raw)
  To: git; +Cc: Johannes.Schindelin, kusambite, Elijah Newren

From: Elijah Newren <newren@gmail.com>

Under some cases (e.g. 'git fast-export --parents master -- COPYING' run
in git.git), the output included the lines
  reset (null)
  commit (null)
instead of
  reset refs/heads/master
  commit refs/heads/master
This simple change fixes that.

Signed-off-by: Elijah Newren <newren@gmail.com>
---
As noted above, run 'git fast-export --parents master -- COPYING' in
git.git to see this bug triggered.

 builtin-fast-export.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/builtin-fast-export.c b/builtin-fast-export.c
index 891e2d4..9aa409b 100644
--- a/builtin-fast-export.c
+++ b/builtin-fast-export.c
@@ -531,6 +531,7 @@ int cmd_fast_export(int argc, const char **argv, const char *prefix)
 	get_tags_and_duplicates(&revs.pending, &extra_refs);
 
 	revs.topo_order = 1;
+	revs.show_source = 1;
 	if (prepare_revision_walk(&revs))
 		die("revision walk setup failed");
 	revs.diffopt.format_callback = show_filemodify;
-- 
1.6.3.2.323.gfb84f

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 4/7] fast-export: Do parent rewriting to avoid dropping relevant commits
  2009-06-20  4:36 A few fast-export fixups newren
                   ` (2 preceding siblings ...)
  2009-06-20  4:36 ` [PATCH 3/7] fast-export: Make sure we show actual ref names instead of "(null)" newren
@ 2009-06-20  4:37 ` newren
  2009-06-20  4:37 ` [PATCH 5/7] fast-export: Add a --tag-of-filtered-object option for newly dangling tags newren
                   ` (2 subsequent siblings)
  6 siblings, 0 replies; 14+ messages in thread
From: newren @ 2009-06-20  4:37 UTC (permalink / raw)
  To: git; +Cc: Johannes.Schindelin, kusambite, Elijah Newren

From: Elijah Newren <newren@gmail.com>

When specifying paths to export, parent rewriting must be turned on for
fast-export to output anything at all.

Signed-off-by: Elijah Newren <newren@gmail.com>
---
Try 'git fast-export master -- COPYING' in git.git and notice how there
is no output at all (whereas 'git rev-list master -- COPYING' does show
some existing revisions).  This simple patch restores the missing file
and commit in the fast-export output.

 builtin-fast-export.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/builtin-fast-export.c b/builtin-fast-export.c
index 9aa409b..b60a97e 100644
--- a/builtin-fast-export.c
+++ b/builtin-fast-export.c
@@ -532,6 +532,7 @@ int cmd_fast_export(int argc, const char **argv, const char *prefix)
 
 	revs.topo_order = 1;
 	revs.show_source = 1;
+	revs.rewrite_parents = 1;
 	if (prepare_revision_walk(&revs))
 		die("revision walk setup failed");
 	revs.diffopt.format_callback = show_filemodify;
-- 
1.6.3.2.323.gfb84f

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 5/7] fast-export: Add a --tag-of-filtered-object option for newly dangling tags
  2009-06-20  4:36 A few fast-export fixups newren
                   ` (3 preceding siblings ...)
  2009-06-20  4:37 ` [PATCH 4/7] fast-export: Do parent rewriting to avoid dropping relevant commits newren
@ 2009-06-20  4:37 ` newren
  2009-06-20  4:37 ` [PATCH 6/7] Add new fast-export testcases newren
  2009-06-20  4:37 ` [PATCH 7/7] fast-export: Document the fact that git-rev-list arguments are accepted newren
  6 siblings, 0 replies; 14+ messages in thread
From: newren @ 2009-06-20  4:37 UTC (permalink / raw)
  To: git; +Cc: Johannes.Schindelin, kusambite, Elijah Newren

From: Elijah Newren <newren@gmail.com>

When providing a list of paths to limit what is exported, the object that
a tag points to can be filtered out entirely.  This new switch allows
the user to specify what should happen to the tag in such a case.  The
default action, 'abort' will exit with an error message.  With 'drop', the
tag will simply be omitted from the output.  With 'rewrite', if the object
tagged was a commit, the tag will be modified to tag an ancestor of the
removed commit.

Signed-off-by: Elijah Newren <newren@gmail.com>
---
I had a hard time coming up with a decent option name.  If anyone has a
better suggestion for the name, I'm all ears.

To see a case where this bug is triggered, run
  git fast-export --signed-tag=strip v1.6.3.2 -- COPYING
in git.git and note that the tag that is output points to 'mark :0' (a
non-existent mark), which will cause fast-import to crash.

 Documentation/git-fast-export.txt |   11 ++++++
 builtin-fast-export.c             |   64 +++++++++++++++++++++++++++++++++---
 revision.c                        |   10 +-----
 revision.h                        |    8 +++++
 4 files changed, 79 insertions(+), 14 deletions(-)

diff --git a/Documentation/git-fast-export.txt b/Documentation/git-fast-export.txt
index 0c9eb56..194abde 100644
--- a/Documentation/git-fast-export.txt
+++ b/Documentation/git-fast-export.txt
@@ -36,6 +36,17 @@ when encountering a signed tag.  With 'strip', the tags will be made
 unsigned, with 'verbatim', they will be silently exported
 and with 'warn', they will be exported, but you will see a warning.
 
+--tag-of-filtered-object=(abort|drop|rewrite)::
+	Specify how to handle tags whose tagged objectis filtered out.
+	Since revisions and files to export can be limited by path,
+	tagged objects may be filtered completely.
++
+When asking to 'abort' (which is the default), this program will die
+when encountering such a tag.  With 'drop' it will omit such tags from
+the output.  With 'rewrite', if the tagged object is a commit, it will
+rewrite the tag to tag an ancestor commit (via parent rewriting; see
+linkgit:git-rev-list[1])
+
 -M::
 -C::
 	Perform move and/or copy detection, as described in the
diff --git a/builtin-fast-export.c b/builtin-fast-export.c
index b60a97e..917ac05 100644
--- a/builtin-fast-export.c
+++ b/builtin-fast-export.c
@@ -23,7 +23,8 @@ static const char *fast_export_usage[] = {
 };
 
 static int progress;
-static enum { VERBATIM, WARN, STRIP, ABORT } signed_tag_mode = ABORT;
+static enum { ABORT, VERBATIM, WARN, STRIP } signed_tag_mode = ABORT;
+static enum { ERROR, DROP, REWRITE } tag_of_filtered_mode = ABORT;
 static int fake_missing_tagger;
 
 static int parse_opt_signed_tag_mode(const struct option *opt,
@@ -42,6 +43,20 @@ static int parse_opt_signed_tag_mode(const struct option *opt,
 	return 0;
 }
 
+static int parse_opt_tag_of_filtered_mode(const struct option *opt,
+					  const char *arg, int unset)
+{
+	if (unset || !strcmp(arg, "abort"))
+		tag_of_filtered_mode = ABORT;
+	else if (!strcmp(arg, "drop"))
+		tag_of_filtered_mode = DROP;
+	else if (!strcmp(arg, "rewrite"))
+		tag_of_filtered_mode = REWRITE;
+	else
+		return error("Unknown tag-of-filtered mode: %s", arg);
+	return 0;
+}
+
 static struct decoration idnums;
 static uint32_t last_idnum;
 
@@ -282,13 +297,14 @@ static void handle_tail(struct object_array *commits, struct rev_info *revs)
 	}
 }
 
-static void handle_tag(const char *name, struct tag *tag)
+static void handle_tag(const char *name, struct tag *tag, struct rev_info *revs)
 {
 	unsigned long size;
 	enum object_type type;
 	char *buf;
 	const char *tagger, *tagger_end, *message;
 	size_t message_size = 0;
+	struct commit *commit;
 
 	buf = read_sha1_file(tag->object.sha1, &type, &size);
 	if (!buf)
@@ -333,10 +349,42 @@ static void handle_tag(const char *name, struct tag *tag)
 			}
 	}
 
+	/* handle tag->tagged having been filtered out due to paths specified */
+	struct object * tagged = tag->tagged;
+	int tagged_mark = get_object_mark(tagged);
+	if (!tagged_mark) {
+		switch(tag_of_filtered_mode) {
+		case ABORT:
+			die ("Tag %s tags unexported commit; use "
+			     "--tag-of-filtered-object=<mode> to handle it.",
+			     sha1_to_hex(tag->object.sha1));
+		case DROP:
+			/* Ignore this tag altogether */
+			return;
+				/* fallthru */
+		case REWRITE:
+			if (tagged->type != OBJ_COMMIT) {
+				die ("Tag %s tags unexported commit; use "
+				     "--tag-of-filtered-object=<mode> to handle it.",
+				     sha1_to_hex(tag->object.sha1));
+			}
+			commit = (struct commit *)tagged;
+			switch (rewrite_one_commit(revs, &commit)) {
+			case rewrite_one_ok:
+				tagged_mark = get_object_mark(&commit->object);
+				break;
+			case rewrite_one_noparents:
+			case rewrite_one_error:
+				die ("Can't find replacement commit for tag %s\n",
+				     sha1_to_hex(tag->object.sha1));
+			}
+		}
+	}
+
 	if (!prefixcmp(name, "refs/tags/"))
 		name += 10;
 	printf("tag %s\nfrom :%d\n%.*s%sdata %d\n%.*s\n",
-	       name, get_object_mark(tag->tagged),
+	       name, tagged_mark,
 	       (int)(tagger_end - tagger), tagger,
 	       tagger == tagger_end ? "" : "\n",
 	       (int)message_size, (int)message_size, message ? message : "");
@@ -399,7 +447,8 @@ static void get_tags_and_duplicates(struct object_array *pending,
 	}
 }
 
-static void handle_tags_and_duplicates(struct string_list *extra_refs)
+static void handle_tags_and_duplicates(struct string_list *extra_refs,
+				       struct rev_info *revs)
 {
 	struct commit *commit;
 	int i;
@@ -415,7 +464,7 @@ static void handle_tags_and_duplicates(struct string_list *extra_refs)
 				/* Ignore this tag altogether */
 				return;
 			}
-			handle_tag(name, tag);
+			handle_tag(name, tag, revs);
 			break;
 		case OBJ_COMMIT:
 			/* create refs pointing to already seen commits */
@@ -504,6 +553,9 @@ int cmd_fast_export(int argc, const char **argv, const char *prefix)
 		OPT_CALLBACK(0, "signed-tags", &signed_tag_mode, "mode",
 			     "select handling of signed tags",
 			     parse_opt_signed_tag_mode),
+		OPT_CALLBACK(0, "tag-of-filtered-object", &tag_of_filtered_mode, "mode",
+			     "select handling of tags that tag filtered objects",
+			     parse_opt_tag_of_filtered_mode),
 		OPT_STRING(0, "export-marks", &export_filename, "FILE",
 			     "Dump marks to this file"),
 		OPT_STRING(0, "import-marks", &import_filename, "FILE",
@@ -551,7 +603,7 @@ int cmd_fast_export(int argc, const char **argv, const char *prefix)
 		}
 	}
 
-	handle_tags_and_duplicates(&extra_refs);
+	handle_tags_and_duplicates(&extra_refs, &revs);
 
 	if (export_filename)
 		export_marks(export_filename);
diff --git a/revision.c b/revision.c
index bf58448..5ed2841 100644
--- a/revision.c
+++ b/revision.c
@@ -1601,13 +1601,7 @@ int prepare_revision_walk(struct rev_info *revs)
 	return 0;
 }
 
-enum rewrite_result {
-	rewrite_one_ok,
-	rewrite_one_noparents,
-	rewrite_one_error,
-};
-
-static enum rewrite_result rewrite_one(struct rev_info *revs, struct commit **pp)
+enum rewrite_result rewrite_one_commit(struct rev_info *revs, struct commit **pp)
 {
 	struct commit_list *cache = NULL;
 
@@ -1633,7 +1627,7 @@ static int rewrite_parents(struct rev_info *revs, struct commit *commit)
 	struct commit_list **pp = &commit->parents;
 	while (*pp) {
 		struct commit_list *parent = *pp;
-		switch (rewrite_one(revs, &parent->item)) {
+		switch (rewrite_one_commit(revs, &parent->item)) {
 		case rewrite_one_ok:
 			break;
 		case rewrite_one_noparents:
diff --git a/revision.h b/revision.h
index 227164c..6bf5b9e 100644
--- a/revision.h
+++ b/revision.h
@@ -158,6 +158,14 @@ extern void add_pending_object(struct rev_info *revs, struct object *obj, const
 
 extern void add_head_to_pending(struct rev_info *);
 
+enum rewrite_result {
+	rewrite_one_ok,
+	rewrite_one_noparents,
+	rewrite_one_error,
+};
+
+extern enum rewrite_result rewrite_one_commit(struct rev_info *revs, struct commit **pp);
+
 enum commit_action {
 	commit_ignore,
 	commit_show,
-- 
1.6.3.2.323.gfb84f

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 6/7] Add new fast-export testcases
  2009-06-20  4:36 A few fast-export fixups newren
                   ` (4 preceding siblings ...)
  2009-06-20  4:37 ` [PATCH 5/7] fast-export: Add a --tag-of-filtered-object option for newly dangling tags newren
@ 2009-06-20  4:37 ` newren
  2009-06-20  4:37 ` [PATCH 7/7] fast-export: Document the fact that git-rev-list arguments are accepted newren
  6 siblings, 0 replies; 14+ messages in thread
From: newren @ 2009-06-20  4:37 UTC (permalink / raw)
  To: git; +Cc: Johannes.Schindelin, kusambite, Elijah Newren

From: Elijah Newren <newren@gmail.com>

The testcases test the new --tag-of-filtered-object option, and test the
output when limiting what to export by path.

Signed-off-by: Elijah Newren <newren@gmail.com>
---
 t/t9301-fast-export.sh |   55 ++++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 55 insertions(+), 0 deletions(-)

diff --git a/t/t9301-fast-export.sh b/t/t9301-fast-export.sh
index d17f0e4..8b165ff 100755
--- a/t/t9301-fast-export.sh
+++ b/t/t9301-fast-export.sh
@@ -262,6 +262,61 @@ test_expect_success 'cope with tagger-less tags' '
 
 '
 
+test_expect_success 'setup for limiting exports by PATH' '
+	mkdir limit-by-paths &&
+	cd limit-by-paths &&
+	git init &&
+	echo hi > there &&
+	git add there &&
+	git commit -m "First file" &&
+	echo foo > bar &&
+	git add bar &&
+	git commit -m "Second file" &&
+	git tag -a -m msg mytag &&
+	cd ..
+'
+
+cat > limit-by-paths/expected << EOF
+blob
+mark :1
+data 3
+hi
+
+reset refs/tags/mytag
+commit refs/tags/mytag
+mark :2
+author A U Thor <author@example.com> 1112912713 -0700
+committer C O Mitter <committer@example.com> 1112912713 -0700
+data 11
+First file
+M 100644 :1 there
+
+EOF
+
+test_expect_success 'dropping tag of filtered out object' '
+	cd limit-by-paths &&
+	git fast-export --tag-of-filtered-object=drop mytag -- there > output &&
+	test_cmp output expected &&
+	cd ..
+'
+
+cat >> limit-by-paths/expected << EOF
+tag mytag
+from :2
+tagger C O Mitter <committer@example.com> 1112912713 -0700
+data 4
+msg
+
+EOF
+
+test_expect_success 'rewriting tag of filtered out object' '
+	cd limit-by-paths &&
+	git fast-export --tag-of-filtered-object=rewrite mytag -- there > output &&
+	test_cmp output expected &&
+	cd ..
+'
+
+
 test_expect_success 'set-up a few more tags for tag export tests' '
 	git checkout -f master &&
 	HEAD_TREE=`git show -s --pretty=raw HEAD | grep tree | sed "s/tree //"` &&
-- 
1.6.3.2.323.gfb84f

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 7/7] fast-export: Document the fact that git-rev-list arguments are accepted
  2009-06-20  4:36 A few fast-export fixups newren
                   ` (5 preceding siblings ...)
  2009-06-20  4:37 ` [PATCH 6/7] Add new fast-export testcases newren
@ 2009-06-20  4:37 ` newren
  6 siblings, 0 replies; 14+ messages in thread
From: newren @ 2009-06-20  4:37 UTC (permalink / raw)
  To: git; +Cc: Johannes.Schindelin, kusambite, Elijah Newren

From: Elijah Newren <newren@gmail.com>


Signed-off-by: Elijah Newren <newren@gmail.com>
---
 Documentation/git-fast-export.txt |    6 ++++++
 1 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/Documentation/git-fast-export.txt b/Documentation/git-fast-export.txt
index 194abde..af2328d 100644
--- a/Documentation/git-fast-export.txt
+++ b/Documentation/git-fast-export.txt
@@ -82,6 +82,12 @@ marks the same across runs.
 	allow that.  So fake a tagger to be able to fast-import the
 	output.
 
+[git-rev-list-args...]::
+       A list of arguments, acceptable to 'git-rev-parse' and
+       'git-rev-list', that specifies the specific objects and references
+       to export.  For example, `master\~10..master` causes the
+       current master reference to be exported along with all objects
+       added since its 10th ancestor commit.
 
 EXAMPLES
 --------
-- 
1.6.3.2.323.gfb84f

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCH 1/7] fast-export: Omit tags that tag trees
  2009-06-20  4:36 ` [PATCH 1/7] fast-export: Omit tags that tag trees newren
@ 2009-06-20 17:31   ` Jeff King
  2009-06-20 18:01     ` Elijah Newren
  0 siblings, 1 reply; 14+ messages in thread
From: Jeff King @ 2009-06-20 17:31 UTC (permalink / raw)
  To: newren; +Cc: git, Johannes.Schindelin, kusambite

On Fri, Jun 19, 2009 at 10:36:57PM -0600, newren@gmail.com wrote:

> From: Elijah Newren <newren@gmail.com>
> 
> Commit c0582c53bcf4e83bba70e1ad23abbad31f96ebc8 introduced logic to just
> omit tags that point to tree objects.  However, a case was missed
> resulting in a tag being output which pointed at "mark :0", which would
> cause fast-import to crash.

Do we really want to disallow tags pointing to trees? There is at least
one well-known case in use (kernel v2.6.11).

Also, (and I haven't investigated at all), this sounds like the same
issue we have with tags pointing to tags. IOW, everything referenceable
should be given a mark, but it is not currently. I posted a "how about
this" patch for the tag case here:

  http://article.gmane.org/gmane.comp.version-control.git/119245

but I never got around to following it up with tests. Could you do
something similar for the tree case?

-Peff

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 1/7] fast-export: Omit tags that tag trees
  2009-06-20 17:31   ` Jeff King
@ 2009-06-20 18:01     ` Elijah Newren
  2009-06-20 18:52       ` Jeff King
  0 siblings, 1 reply; 14+ messages in thread
From: Elijah Newren @ 2009-06-20 18:01 UTC (permalink / raw)
  To: Jeff King; +Cc: git, Johannes.Schindelin, kusmabite

Hi,

On Sat, Jun 20, 2009 at 11:31 AM, Jeff King<peff@peff.net> wrote:
> Do we really want to disallow tags pointing to trees? There is at least
> one well-known case in use (kernel v2.6.11).
>
> Also, (and I haven't investigated at all), this sounds like the same
> issue we have with tags pointing to tags. IOW, everything referenceable
> should be given a mark, but it is not currently. I posted a "how about
> this" patch for the tag case here:
>
>  http://article.gmane.org/gmane.comp.version-control.git/119245
>
> but I never got around to following it up with tests. Could you do
> something similar for the tree case?

I don't see how we could do something similar for the tree case
without making some significant change to the output/input of both
fast-export and fast-import.  Tag objects are part of the output of
fast-export, thus we can add a mark line to give the object a name and
thus provide us a valid mark we can make the outer tag point to.
Trees are not output by fast-export (other than implicitly by
including files in commits), so we have nothing to point such a tag
at.  If we were to do something like use the full sha1sum instead of
marks in such a case (which git-fast-import should currently accept),
then (a) we'd break exporting a repository limited by path ('git
fast-export master -- libfoo"), and (b) we'd break interoperation with
other tools like bzr-fast-import (or a possible future git that uses a
different checksum).

If someone wanted to tackle modifying the output/input syntax of
fast-export and fast-import, maybe something could be done here, but
my patch just tries to make things operate sanely within the given
documented limitations (as found in the git-fast-export manpage --
"Since 'git-fast-import' cannot tag trees, you will not be able to
export the linux-2.6.git repository completely, as it contains a tag
referencing a tree instead of a commit.").


Elijah

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 1/7] fast-export: Omit tags that tag trees
  2009-06-20 18:01     ` Elijah Newren
@ 2009-06-20 18:52       ` Jeff King
  0 siblings, 0 replies; 14+ messages in thread
From: Jeff King @ 2009-06-20 18:52 UTC (permalink / raw)
  To: Elijah Newren; +Cc: git, Johannes.Schindelin, kusmabite

On Sat, Jun 20, 2009 at 12:01:00PM -0600, Elijah Newren wrote:

> I don't see how we could do something similar for the tree case
> without making some significant change to the output/input of both
> fast-export and fast-import.  Tag objects are part of the output of
> fast-export, thus we can add a mark line to give the object a name and
> thus provide us a valid mark we can make the outer tag point to.
> Trees are not output by fast-export (other than implicitly by
> including files in commits), so we have nothing to point such a tag
> at.  If we were to do something like use the full sha1sum instead of

Ah, right. That falls under the "I didn't look at this too closely" part
of my previous message. :)

It does seem a shame not to be able to say "and now here is a tree" in
the same way we say "and now here is a commit". And since that is not
possible without add-ons that might cause compatibility issues, your fix
seems reasonable to at least avoid generating bogus output.

Another option would be to generate a pseudo-commit with no parentage
that holds the tree, and then point the tag at that. It's obviously not
exactly what the user wanted, but neither is simply omitting. You could
even mark the commit message with a "THIS IS REALLY A TREE" magic
string, and then have fast-import recognize and convert those back to a
tree (by just peeling the commit away).

-Peff

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 2/7] Modify fast-export testcase to check that we correctly omit tags of trees
  2009-06-20  4:36 ` [PATCH 2/7] Modify fast-export testcase to check that we correctly omit tags of trees newren
@ 2009-06-21  5:53   ` Stephen Boyd
  2009-06-21  6:17   ` Johannes Sixt
  1 sibling, 0 replies; 14+ messages in thread
From: Stephen Boyd @ 2009-06-21  5:53 UTC (permalink / raw)
  To: newren; +Cc: git, Johannes.Schindelin, kusambite

newren@gmail.com wrote:
> Should I have just squashed this with the previous patch?  Or with the
> other testcase patch?

I think squashing it with the previous one is better as long as this is
directly related to patch 1.

>
>  t/t9301-fast-export.sh |    8 +++++++-
>  1 files changed, 7 insertions(+), 1 deletions(-)
>
> diff --git a/t/t9301-fast-export.sh b/t/t9301-fast-export.sh
> index 8c8a9e6..d17f0e4 100755
> --- a/t/t9301-fast-export.sh
> +++ b/t/t9301-fast-export.sh
> @@ -272,7 +272,13 @@ test_expect_success 'set-up a few more tags for tag export tests' '
>  '
>  
>  # NEEDSWORK: not just check return status, but validate the output

Is this comment still relevant?

> -test_expect_success 'tree_tag'        'git fast-export tree_tag'
> +test_expect_success 'tree_tag'        '
> +	mkdir result &&
> +	cd result &&
> +	git init &&
> +	cd ..
> +	git fast-export tree_tag | (cd result && git fast-import)
> +'
>  test_expect_success 'tree_tag-obj'    'git fast-export tree_tag-obj'
>  test_expect_success 'tag-obj_tag'     'git fast-export tag-obj_tag'
>  test_expect_success 'tag-obj_tag-obj' 'git fast-export tag-obj_tag-obj'

I don't really know, but could test_create_repo() be useful here?

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 2/7] Modify fast-export testcase to check that we correctly omit tags of trees
  2009-06-20  4:36 ` [PATCH 2/7] Modify fast-export testcase to check that we correctly omit tags of trees newren
  2009-06-21  5:53   ` Stephen Boyd
@ 2009-06-21  6:17   ` Johannes Sixt
  2009-06-22 13:12     ` Elijah Newren
  1 sibling, 1 reply; 14+ messages in thread
From: Johannes Sixt @ 2009-06-21  6:17 UTC (permalink / raw)
  To: newren; +Cc: git, Johannes.Schindelin, kusambite

On Samstag, 20. Juni 2009, newren@gmail.com wrote:
> -test_expect_success 'tree_tag'        'git fast-export tree_tag'
> +test_expect_success 'tree_tag'        '
> +	mkdir result &&
> +	cd result &&
> +	git init &&
> +	cd ..
> +	git fast-export tree_tag | (cd result && git fast-import)
> +'

You should make this

test_expect_success 'tree_tag' '
	mkdir result &&
	(cd result && git init) &&
	git fast-export tree_tag > fe-stream &&
	(cd result && git fast-import < fe-stream)
'

in order to (1) not remain in the wrong directory if git init fails, and (2) 
to catch failures in git fast-export.

-- Hannes

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 2/7] Modify fast-export testcase to check that we  correctly omit tags of trees
  2009-06-21  6:17   ` Johannes Sixt
@ 2009-06-22 13:12     ` Elijah Newren
  0 siblings, 0 replies; 14+ messages in thread
From: Elijah Newren @ 2009-06-22 13:12 UTC (permalink / raw)
  To: Johannes Sixt; +Cc: git, Johannes.Schindelin, kusmabite

On Sun, Jun 21, 2009 at 12:17 AM, Johannes Sixt<j6t@kdbg.org> wrote:
> On Samstag, 20. Juni 2009, newren@gmail.com wrote:
>> -test_expect_success 'tree_tag'        'git fast-export tree_tag'
>> +test_expect_success 'tree_tag'        '
>> +     mkdir result &&
>> +     cd result &&
>> +     git init &&
>> +     cd ..
>> +     git fast-export tree_tag | (cd result && git fast-import)
>> +'
>
> You should make this
>
> test_expect_success 'tree_tag' '
>        mkdir result &&
>        (cd result && git init) &&
>        git fast-export tree_tag > fe-stream &&
>        (cd result && git fast-import < fe-stream)
> '
>
> in order to (1) not remain in the wrong directory if git init fails, and (2)
> to catch failures in git fast-export.

Thanks for the tips; I've sent a new patch series with these changes
(except that the second fe-stream needs a ../ in front of it), as well
as the changes suggested by Stephen Boyd in the other email in this
thread.

Elijah

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2009-06-22 13:12 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2009-06-20  4:36 A few fast-export fixups newren
2009-06-20  4:36 ` [PATCH 1/7] fast-export: Omit tags that tag trees newren
2009-06-20 17:31   ` Jeff King
2009-06-20 18:01     ` Elijah Newren
2009-06-20 18:52       ` Jeff King
2009-06-20  4:36 ` [PATCH 2/7] Modify fast-export testcase to check that we correctly omit tags of trees newren
2009-06-21  5:53   ` Stephen Boyd
2009-06-21  6:17   ` Johannes Sixt
2009-06-22 13:12     ` Elijah Newren
2009-06-20  4:36 ` [PATCH 3/7] fast-export: Make sure we show actual ref names instead of "(null)" newren
2009-06-20  4:37 ` [PATCH 4/7] fast-export: Do parent rewriting to avoid dropping relevant commits newren
2009-06-20  4:37 ` [PATCH 5/7] fast-export: Add a --tag-of-filtered-object option for newly dangling tags newren
2009-06-20  4:37 ` [PATCH 6/7] Add new fast-export testcases newren
2009-06-20  4:37 ` [PATCH 7/7] fast-export: Document the fact that git-rev-list arguments are accepted newren

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.