threads / patch / 51911

patch, 8 partsfast export/import: handle nested tags, improve incremental exports

Subject: [PATCH 0/8] fast export/import: handle nested tags, improve incremental exports

## tl;dr

33 messages between Sep 25, 2019 and Oct 4, 2019. Diffs are folded; open one to read it.

replies: 32people: 3as markdown or json

Elijah Newren· Sep 25, 2019, 01:39 UTC · lore

This series improves the incremental export story for fast-export and fast-import (--export-marks and --import-marks fell a bit short), fixes a couple small export/import bugs, and enables handling nested tags. In particular, the nested tags handling makes it so that fast-export and fast-import can finally handle the git.git repo.

Elijah Newren (8):
  fast-export: fix exporting a tag and nothing else
  fast-import: fix handling of deleted tags
  fast-import: allow tags to be identified by mark labels
  fast-import: add support for new 'alias' command
  fast-export: add support for --import-marks-if-exists
  fast-export: allow user to request tags be marked with --mark-tags
  t9350: add tests for tags of things other than a commit
  fast-export: handle nested tags
 Documentation/git-fast-export.txt | 17 ++++--
 Documentation/git-fast-import.txt | 23 ++++++++
 builtin/fast-export.c             | 67 ++++++++++++++++------
 fast-import.c                     | 94 +++++++++++++++++++++++++++----
 t/t9300-fast-import.sh            | 37 ++++++++++++
 t/t9350-fast-export.sh            | 68 ++++++++++++++++++++--
 6 files changed, 268 insertions(+), 38 deletions(-)
-- 
2.23.0.177.g8af0b3ca64
Elijah Newren· Sep 25, 2019, 01:39 UTC · re: Elijah Newren · lore

[PATCH 1/8] fast-export: fix exporting a tag and nothing else

fast-export allows specifying revision ranges, which can be used to export a tag without exporting the commit it tags. fast-export handled this rather poorly: it would emit a "from :0" directive. Since marks start at 1 and increase, this means it refers to an unknown commit and fast-import will choke on the input.

When we are unable to look up a mark for the object being tagged, use a "from $HASH" directive instead to fix this problem.

Note that this is quite similar to the behavior fast-export exhibits with commits and parents when --reference-excluded-parents is passed along with an excluded commit range. For tags of excluded commits we do not require the --reference-excluded-parents flag because we always have to tag something. By contrast, when dealing with commits, pruning a parent is always a viable option, so we need the flag to specify that parent pruning is not wanted. (It is slightly weird that --reference-excluded-parents isn't the default with a separate --prune-excluded-parents flag, but backward compatibility concerns resulted in the current defaults.)

Signed-off-by: Elijah Newren <newren@gmail.com>
---
 builtin/fast-export.c  |  7 ++++++-
 t/t9350-fast-export.sh | 13 +++++++++++++
 2 files changed, 19 insertions(+), 1 deletion(-)
Show changes to 2 files +19 −1

builtin/fast-export.c, t/t9350-fast-export.sh

diff --git a/builtin/fast-export.c b/builtin/fast-export.c
index f541f55d33..5822271c6b 100644
--- a/builtin/fast-export.c
+++ b/builtin/fast-export.c
@@ -860,7 +860,12 @@ static void handle_tag(const char *name, struct tag *tag)
 
 	if (starts_with(name, "refs/tags/"))
 		name += 10;
-	printf("tag %s\nfrom :%d\n", name, tagged_mark);
+	printf("tag %s\n", name);
+	if (tagged_mark)
+		printf("from :%d\n", tagged_mark);
+	else
+		printf("from %s\n", oid_to_hex(&tagged->oid));
+
 	if (show_original_ids)
 		printf("original-oid %s\n", oid_to_hex(&tag->object.oid));
 	printf("%.*s%sdata %d\n%.*s\n",
diff --git a/t/t9350-fast-export.sh b/t/t9350-fast-export.sh
index b4004e05c2..d32ff41859 100755
--- a/t/t9350-fast-export.sh
+++ b/t/t9350-fast-export.sh
@@ -53,6 +53,19 @@ test_expect_success 'fast-export | fast-import' '
 
 '
 
+test_expect_success 'fast-export ^muss^{commit} muss' '
+	git fast-export --tag-of-filtered-object=rewrite ^muss^{commit} muss >actual &&
+	cat >expected <<-EOF &&
+	tag muss
+	from $(git rev-parse --verify muss^{commit})
+	$(git cat-file tag muss | grep tagger)
+	data 9
+	valentin
+
+	EOF
+	test_cmp expected actual
+'
+
 test_expect_success 'fast-export master~2..master' '
 
 	git fast-export master~2..master >actual &&
-- 
2.23.0.177.g8af0b3ca64
Elijah Newren· Sep 25, 2019, 01:39 UTC · re: Elijah Newren · lore

[PATCH 2/8] fast-import: fix handling of deleted tags

If our input stream includes a tag which is later deleted, we were not properly deleting it. We did have a step which would delete it, but we left a tag in the tag list noting that it needed to be updated, and the updating of annotated tags occurred AFTER ref deletion. So, when we record that a tag needs to be deleted, also remove it from the list of annotated tags to update.

While this has likely been something that has not happened in practice, it will come up more in order to support nested tags. For nested tags, we either need to give temporary names to the intermediate tags and then delete them, or else we need to use the final name for the intermediate tags. If we use the final name for the intermediate tags, then in order to keep the sanity check that someone doesn't try to update the same tag twice, we need to delete the ref after creating the intermediate tag. So, either way nested tags imply the need to delete temporary inner tag references.

Signed-off-by: Elijah Newren <newren@gmail.com>
---
 fast-import.c          | 29 +++++++++++++++++++++++++++++
 t/t9300-fast-import.sh | 13 +++++++++++++
 2 files changed, 42 insertions(+)
Show changes to 2 files +42 −0

fast-import.c, t/t9300-fast-import.sh

diff --git a/fast-import.c b/fast-import.c
index b44d6a467e..dab905d667 100644
--- a/fast-import.c
+++ b/fast-import.c
@@ -2793,6 +2793,35 @@ static void parse_reset_branch(const char *arg)
 		b = new_branch(arg);
 	read_next_command();
 	parse_from(b);
+	if (b->delete && !strncmp(arg, "refs/tags/", 10)) {
+		/*
+		 * Elsewhere, we call dump_branches() before dump_tags(),
+		 * and dump_branches() will handle ref deletions first, so
+		 * in order to make sure the deletion actually takes effect,
+		 * we need to remove the tag from our list of tags to update.
+		 *
+		 * NEEDSWORK: replace list of tags with hashmap for faster
+		 * deletion?
+		 */
+		struct strbuf tag_name = STRBUF_INIT;
+		struct tag *t, *prev = NULL;
+		for (t = first_tag; t; t = t->next_tag) {
+			strbuf_reset(&tag_name);
+			strbuf_addf(&tag_name, "refs/tags/%s", t->name);
+			if (!strcmp(arg, tag_name.buf))
+				break;
+			prev = t;
+		}
+		if (t) {
+			if (prev)
+				prev->next_tag = t->next_tag;
+			else
+				first_tag = t->next_tag;
+			if (!t->next_tag)
+				last_tag = prev;
+			/* There is no mem_pool_free(t) function to call. */
+		}
+	}
 	if (command_buf.len > 0)
 		unread_command_buf = 1;
 }
diff --git a/t/t9300-fast-import.sh b/t/t9300-fast-import.sh
index 141b7fa35e..74bc41333b 100755
--- a/t/t9300-fast-import.sh
+++ b/t/t9300-fast-import.sh
@@ -85,6 +85,15 @@ test_expect_success 'A: create pack from stdin' '
 	An annotated tag that annotates a blob.
 	EOF
 
+	tag to-be-deleted
+	from :3
+	data <<EOF
+	Another annotated tag that annotates a blob.
+	EOF
+
+	reset refs/tags/to-be-deleted
+	from 0000000000000000000000000000000000000000
+
 	INPUT_END
 	git fast-import --export-marks=marks.out <input &&
 	git whatchanged master
@@ -157,6 +166,10 @@ test_expect_success 'A: verify tag/series-A-blob' '
 	test_cmp expect actual
 '
 
+test_expect_success 'A: verify tag deletion is successful' '
+	test_must_fail git rev-parse --verify refs/tags/to-be-deleted
+'
+
 test_expect_success 'A: verify marks output' '
 	cat >expect <<-EOF &&
 	:2 $(git rev-parse --verify master:file2)
-- 
2.23.0.177.g8af0b3ca64
Elijah Newren· Sep 25, 2019, 01:40 UTC · re: Elijah Newren · lore

[PATCH 3/8] fast-import: allow tags to be identified by mark labels

Mark identifiers are used in fast-export and fast-import to provide a label to refer to earlier content. Blobs are given labels because they need to be referenced in the commits where they first appear with a given filename, and commits are given labels because they can be the parents of other commits. Tags were never given labels, probably because they were viewed as unnecessary, but that presents two problems:

   1. It leaves us without a way of referring to previous tags if we
      want to create a tag of a tag (or higher nestings).
   2. It leaves us with no way of recording that a tag has already been
      imported when using --export-marks and --import-marks.
Fix these problems by allowing an optional mark label for tags.
Signed-off-by: Elijah Newren <newren@gmail.com>
---
 Documentation/git-fast-import.txt |  1 +
 fast-import.c                     |  3 ++-
 t/t9300-fast-import.sh            | 19 +++++++++++++++++++
 3 files changed, 22 insertions(+), 1 deletion(-)
Show changes to 3 files +22 −1

Documentation/git-fast-import.txt, fast-import.c, t/t9300-fast-import.sh

diff --git a/Documentation/git-fast-import.txt b/Documentation/git-fast-import.txt
index 0bb276269e..4977869465 100644
--- a/Documentation/git-fast-import.txt
+++ b/Documentation/git-fast-import.txt
@@ -774,6 +774,7 @@ lightweight (non-annotated) tags see the `reset` command below.
 
 ....
 	'tag' SP <name> LF
+	mark?
 	'from' SP <commit-ish> LF
 	original-oid?
 	'tagger' (SP <name>)? SP LT <email> GT SP <when> LF
diff --git a/fast-import.c b/fast-import.c
index dab905d667..0271d81d0d 100644
--- a/fast-import.c
+++ b/fast-import.c
@@ -2713,6 +2713,7 @@ static void parse_new_tag(const char *arg)
 		first_tag = t;
 	last_tag = t;
 	read_next_command();
+	parse_mark();
 
 	/* from ... */
 	if (!skip_prefix(command_buf.buf, "from ", &from))
@@ -2769,7 +2770,7 @@ static void parse_new_tag(const char *arg)
 	strbuf_addbuf(&new_data, &msg);
 	free(tagger);
 
-	if (store_object(OBJ_TAG, &new_data, NULL, &t->oid, 0))
+	if (store_object(OBJ_TAG, &new_data, NULL, &t->oid, next_mark))
 		t->pack_id = MAX_PACK_ID;
 	else
 		t->pack_id = pack_id;
diff --git a/t/t9300-fast-import.sh b/t/t9300-fast-import.sh
index 74bc41333b..3ad2b2f1ba 100755
--- a/t/t9300-fast-import.sh
+++ b/t/t9300-fast-import.sh
@@ -94,6 +94,23 @@ test_expect_success 'A: create pack from stdin' '
 	reset refs/tags/to-be-deleted
 	from 0000000000000000000000000000000000000000
 
+	tag nested
+	mark :6
+	from :4
+	data <<EOF
+	Tag of our lovely commit
+	EOF
+
+	reset refs/tags/nested
+	from 0000000000000000000000000000000000000000
+
+	tag nested
+	mark :7
+	from :6
+	data <<EOF
+	Tag of tag of our lovely commit
+	EOF
+
 	INPUT_END
 	git fast-import --export-marks=marks.out <input &&
 	git whatchanged master
@@ -176,6 +193,8 @@ test_expect_success 'A: verify marks output' '
 	:3 $(git rev-parse --verify master:file3)
 	:4 $(git rev-parse --verify master:file4)
 	:5 $(git rev-parse --verify master^0)
+	:6 $(git cat-file tag nested | grep object | cut -d" " -f 2)
+	:7 $(git rev-parse --verify nested)
 	EOF
 	test_cmp expect marks.out
 '
-- 
2.23.0.177.g8af0b3ca64
Elijah Newren· Sep 25, 2019, 01:40 UTC · re: Elijah Newren · lore

[PATCH 4/8] fast-import: add support for new 'alias' command

fast-export and fast-import have nice --import-marks flags which allow for incremental migrations. However, if there is a mark in fast-export's file of marks without a corresponding mark in the one for fast-import, then we run the risk that fast-export tries to send new objects relative to the mark it knows which fast-import does not, causing fast-import to fail.

This arises in practice when there is a filter of some sort running between the fast-export and fast-import processes which prunes some commits programmatically. Provide such a filter with the ability to alias pruned commits to their most recent non-pruned ancestor.

Signed-off-by: Elijah Newren <newren@gmail.com>
---
 Documentation/git-fast-import.txt | 22 +++++++++++
 fast-import.c                     | 62 ++++++++++++++++++++++++++-----
 t/t9300-fast-import.sh            |  5 +++
 3 files changed, 79 insertions(+), 10 deletions(-)
Show changes to 3 files +79 −10

Documentation/git-fast-import.txt, fast-import.c, t/t9300-fast-import.sh

diff --git a/Documentation/git-fast-import.txt b/Documentation/git-fast-import.txt
index 4977869465..a3f1e0c5e4 100644
--- a/Documentation/git-fast-import.txt
+++ b/Documentation/git-fast-import.txt
@@ -337,6 +337,13 @@ and control the current import process.  More detailed discussion
 	`commit` command.  This command is optional and is not
 	needed to perform an import.
 
+`alias`::
+	Record that a mark refers to a given object without first
+	creating any new object.  Using --import-marks and referring
+	to missing marks will cause fast-import to fail, so aliases
+	can provide a way to set otherwise pruned commits to a valid
+	value (e.g. the nearest non-pruned ancestor).
+
 `checkpoint`::
 	Forces fast-import to close the current packfile, generate its
 	unique SHA-1 checksum and index, and start a new packfile.
@@ -914,6 +921,21 @@ a data chunk which does not have an LF as its last byte.
 +
 The `LF` after `<delim> LF` is optional (it used to be required).
 
+`alias`
+~~~~~~~
+Record that a mark refers to a given object without first creating any
+new object.
+
+....
+	'alias' LF
+	mark
+	'to' SP <commit-ish> LF
+	LF?
+....
+
+For a detailed description of `<commit-ish>` see above under `from`.
+
+
 `checkpoint`
 ~~~~~~~~~~~~
 Forces fast-import to close the current packfile, start a new one, and to
diff --git a/fast-import.c b/fast-import.c
index 0271d81d0d..8228cde759 100644
--- a/fast-import.c
+++ b/fast-import.c
@@ -2491,18 +2491,14 @@ static void parse_from_existing(struct branch *b)
 	}
 }
 
-static int parse_from(struct branch *b)
+static int parse_objectish(struct branch *b, const char *objectish)
 {
-	const char *from;
 	struct branch *s;
 	struct object_id oid;
 
-	if (!skip_prefix(command_buf.buf, "from ", &from))
-		return 0;
-
 	oidcpy(&oid, &b->branch_tree.versions[1].oid);
 
-	s = lookup_branch(from);
+	s = lookup_branch(objectish);
 	if (b == s)
 		die("Can't create a branch from itself: %s", b->name);
 	else if (s) {
@@ -2510,8 +2506,8 @@ static int parse_from(struct branch *b)
 		oidcpy(&b->oid, &s->oid);
 		oidcpy(&b->branch_tree.versions[0].oid, t);
 		oidcpy(&b->branch_tree.versions[1].oid, t);
-	} else if (*from == ':') {
-		uintmax_t idnum = parse_mark_ref_eol(from);
+	} else if (*objectish == ':') {
+		uintmax_t idnum = parse_mark_ref_eol(objectish);
 		struct object_entry *oe = find_mark(idnum);
 		if (oe->type != OBJ_COMMIT)
 			die("Mark :%" PRIuMAX " not a commit", idnum);
@@ -2525,13 +2521,13 @@ static int parse_from(struct branch *b)
 			} else
 				parse_from_existing(b);
 		}
-	} else if (!get_oid(from, &b->oid)) {
+	} else if (!get_oid(objectish, &b->oid)) {
 		parse_from_existing(b);
 		if (is_null_oid(&b->oid))
 			b->delete = 1;
 	}
 	else
-		die("Invalid ref name or SHA1 expression: %s", from);
+		die("Invalid ref name or SHA1 expression: %s", objectish);
 
 	if (b->branch_tree.tree && !oideq(&oid, &b->branch_tree.versions[1].oid)) {
 		release_tree_content_recursive(b->branch_tree.tree);
@@ -2542,6 +2538,26 @@ static int parse_from(struct branch *b)
 	return 1;
 }
 
+static int parse_from(struct branch *b)
+{
+	const char *from;
+
+	if (!skip_prefix(command_buf.buf, "from ", &from))
+		return 0;
+
+	return parse_objectish(b, from);
+}
+
+static int parse_objectish_with_prefix(struct branch *b, const char *prefix)
+{
+	const char *base;
+
+	if (!skip_prefix(command_buf.buf, prefix, &base))
+		return 0;
+
+	return parse_objectish(b, base);
+}
+
 static struct hash_list *parse_merge(unsigned int *count)
 {
 	struct hash_list *list = NULL, **tail = &list, *n;
@@ -3089,6 +3105,28 @@ static void parse_progress(void)
 	skip_optional_lf();
 }
 
+static void parse_alias(void)
+{
+	struct object_entry *e;
+	struct branch b;
+
+	skip_optional_lf();
+	read_next_command();
+
+	/* mark ... */
+	parse_mark();
+	if (!next_mark)
+		die(_("Expected 'mark' command, got %s"), command_buf.buf);
+
+	/* to ... */
+	memset(&b, 0, sizeof(b));
+	if (!parse_objectish_with_prefix(&b, "to "))
+		die(_("Expected 'to' command, got %s"), command_buf.buf);
+	e = find_object(&b.oid);
+	assert(e);
+	insert_mark(next_mark, e);
+}
+
 static char* make_fast_import_path(const char *path)
 {
 	if (!relative_marks_paths || is_absolute_path(path))
@@ -3216,6 +3254,8 @@ static int parse_one_feature(const char *feature, int from_stream)
 		option_import_marks(arg, from_stream, 1);
 	} else if (skip_prefix(feature, "export-marks=", &arg)) {
 		option_export_marks(arg);
+	} else if (!strcmp(feature, "alias")) {
+		; /* Don't die - this feature is supported */
 	} else if (!strcmp(feature, "get-mark")) {
 		; /* Don't die - this feature is supported */
 	} else if (!strcmp(feature, "cat-blob")) {
@@ -3372,6 +3412,8 @@ int cmd_main(int argc, const char **argv)
 			parse_checkpoint();
 		else if (!strcmp("done", command_buf.buf))
 			break;
+		else if (!strcmp("alias", command_buf.buf))
+			parse_alias();
 		else if (starts_with(command_buf.buf, "progress "))
 			parse_progress();
 		else if (skip_prefix(command_buf.buf, "feature ", &v))
diff --git a/t/t9300-fast-import.sh b/t/t9300-fast-import.sh
index 3ad2b2f1ba..41f2a1dad9 100755
--- a/t/t9300-fast-import.sh
+++ b/t/t9300-fast-import.sh
@@ -111,6 +111,10 @@ test_expect_success 'A: create pack from stdin' '
 	Tag of tag of our lovely commit
 	EOF
 
+	alias
+	mark :8
+	to :5
+
 	INPUT_END
 	git fast-import --export-marks=marks.out <input &&
 	git whatchanged master
@@ -195,6 +199,7 @@ test_expect_success 'A: verify marks output' '
 	:5 $(git rev-parse --verify master^0)
 	:6 $(git cat-file tag nested | grep object | cut -d" " -f 2)
 	:7 $(git rev-parse --verify nested)
+	:8 $(git rev-parse --verify master^0)
 	EOF
 	test_cmp expect marks.out
 '
-- 
2.23.0.177.g8af0b3ca64
Elijah Newren· Sep 25, 2019, 01:40 UTC · re: Elijah Newren · lore

[PATCH 6/8] fast-export: allow user to request tags be marked with --mark-tags

Add a new option, --mark-tags, which will output mark identifiers with each tag object. This improves the incremental export story with --export-marks since it will allow us to record that annotated tags have been exported, and it is also needed as a step towards supporting nested tags.

Signed-off-by: Elijah Newren <newren@gmail.com>
---
 Documentation/git-fast-export.txt | 17 +++++++++++++----
 builtin/fast-export.c             |  7 +++++++
 t/t9350-fast-export.sh            | 14 ++++++++++++++
 3 files changed, 34 insertions(+), 4 deletions(-)
Show changes to 3 files +34 −4

Documentation/git-fast-export.txt, builtin/fast-export.c, t/t9350-fast-export.sh

diff --git a/Documentation/git-fast-export.txt b/Documentation/git-fast-export.txt
index cc940eb9ad..c522b34f7b 100644
--- a/Documentation/git-fast-export.txt
+++ b/Documentation/git-fast-export.txt
@@ -75,11 +75,20 @@ produced incorrect results if you gave these options.
 	Before processing any input, load the marks specified in
 	<file>.  The input file must exist, must be readable, and
 	must use the same format as produced by --export-marks.
+
+--mark-tags::
+	In addition to labelling blobs and commits with mark ids, also
+	label tags.  This is useful in conjunction with
+	`--export-marks` and `--import-marks`, and is also useful (and
+	necessary) for exporting of nested tags.  It does not hurt
+	other cases and would be the default, but many fast-import
+	frontends are not prepared to accept tags with mark
+	identifiers.
 +
-Any commits that have already been marked will not be exported again.
-If the backend uses a similar --import-marks file, this allows for
-incremental bidirectional exporting of the repository by keeping the
-marks the same across runs.
+Any commits (or tags) that have already been marked will not be
+exported again.  If the backend uses a similar --import-marks file,
+this allows for incremental bidirectional exporting of the repository
+by keeping the marks the same across runs.
 
 --fake-missing-tagger::
 	Some old repositories have tags without a tagger.  The
diff --git a/builtin/fast-export.c b/builtin/fast-export.c
index 575e47833b..d32e1e9327 100644
--- a/builtin/fast-export.c
+++ b/builtin/fast-export.c
@@ -40,6 +40,7 @@ static int no_data;
 static int full_tree;
 static int reference_excluded_commits;
 static int show_original_ids;
+static int mark_tags;
 static struct string_list extra_refs = STRING_LIST_INIT_NODUP;
 static struct string_list tag_refs = STRING_LIST_INIT_NODUP;
 static struct refspec refspecs = REFSPEC_INIT_FETCH;
@@ -861,6 +862,10 @@ static void handle_tag(const char *name, struct tag *tag)
 	if (starts_with(name, "refs/tags/"))
 		name += 10;
 	printf("tag %s\n", name);
+	if (mark_tags) {
+		mark_next_object(&tag->object);
+		printf("mark :%"PRIu32"\n", last_idnum);
+	}
 	if (tagged_mark)
 		printf("from :%d\n", tagged_mark);
 	else
@@ -1165,6 +1170,8 @@ int cmd_fast_export(int argc, const char **argv, const char *prefix)
 			 &reference_excluded_commits, N_("Reference parents which are not in fast-export stream by object id")),
 		OPT_BOOL(0, "show-original-ids", &show_original_ids,
 			    N_("Show original object ids of blobs/commits")),
+		OPT_BOOL(0, "mark-tags", &mark_tags,
+			    N_("Label tags with mark ids")),
 
 		OPT_END()
 	};
diff --git a/t/t9350-fast-export.sh b/t/t9350-fast-export.sh
index ea84e2f173..b3fca6ffba 100755
--- a/t/t9350-fast-export.sh
+++ b/t/t9350-fast-export.sh
@@ -66,6 +66,20 @@ test_expect_success 'fast-export ^muss^{commit} muss' '
 	test_cmp expected actual
 '
 
+test_expect_success 'fast-export --mark-tags ^muss^{commit} muss' '
+	git fast-export --mark-tags --tag-of-filtered-object=rewrite ^muss^{commit} muss >actual &&
+	cat >expected <<-EOF &&
+	tag muss
+	mark :1
+	from $(git rev-parse --verify muss^{commit})
+	$(git cat-file tag muss | grep tagger)
+	data 9
+	valentin
+
+	EOF
+	test_cmp expected actual
+'
+
 test_expect_success 'fast-export master~2..master' '
 
 	git fast-export master~2..master >actual &&
-- 
2.23.0.177.g8af0b3ca64
Elijah Newren· Sep 25, 2019, 01:40 UTC · re: Elijah Newren · lore

[PATCH 5/8] fast-export: add support for --import-marks-if-exists

fast-import has support for both an --import-marks flag and an --import-marks-if-exists flag; the latter of which will not die() if the file does not exist. fast-export only had support for an --import-marks flag; add an --import-marks-if-exists flag for consistency.

Signed-off-by: Elijah Newren <newren@gmail.com>
---
 builtin/fast-export.c  | 23 +++++++++++++++++++----
 t/t9350-fast-export.sh | 10 ++++------
 2 files changed, 23 insertions(+), 10 deletions(-)
Show changes to 2 files +23 −10

builtin/fast-export.c, t/t9350-fast-export.sh

diff --git a/builtin/fast-export.c b/builtin/fast-export.c
index 5822271c6b..575e47833b 100644
--- a/builtin/fast-export.c
+++ b/builtin/fast-export.c
@@ -1052,11 +1052,16 @@ static void export_marks(char *file)
 		error("Unable to write marks file %s.", file);
 }
 
-static void import_marks(char *input_file)
+static void import_marks(char *input_file, int check_exists)
 {
 	char line[512];
-	FILE *f = xfopen(input_file, "r");
+	FILE *f;
+	struct stat sb;
+
+	if (check_exists && stat(input_file, &sb))
+		return;
 
+	f = xfopen(input_file, "r");
 	while (fgets(line, sizeof(line), f)) {
 		uint32_t mark;
 		char *line_end, *mark_end;
@@ -1120,7 +1125,9 @@ int cmd_fast_export(int argc, const char **argv, const char *prefix)
 	struct rev_info revs;
 	struct object_array commits = OBJECT_ARRAY_INIT;
 	struct commit *commit;
-	char *export_filename = NULL, *import_filename = NULL;
+	char *export_filename = NULL,
+	     *import_filename = NULL,
+	     *import_filename_if_exists = NULL;
 	uint32_t lastimportid;
 	struct string_list refspecs_list = STRING_LIST_INIT_NODUP;
 	struct string_list paths_of_changed_objects = STRING_LIST_INIT_DUP;
@@ -1140,6 +1147,10 @@ int cmd_fast_export(int argc, const char **argv, const char *prefix)
 			     N_("Dump marks to this file")),
 		OPT_STRING(0, "import-marks", &import_filename, N_("file"),
 			     N_("Import marks from this file")),
+		OPT_STRING(0, "import-marks-if-exists",
+			     &import_filename_if_exists,
+			     N_("file"),
+			     N_("Import marks from this file if it exists")),
 		OPT_BOOL(0, "fake-missing-tagger", &fake_missing_tagger,
 			 N_("Fake a tagger when tags lack one")),
 		OPT_BOOL(0, "full-tree", &full_tree,
@@ -1187,8 +1198,12 @@ int cmd_fast_export(int argc, const char **argv, const char *prefix)
 	if (use_done_feature)
 		printf("feature done\n");
 
+	if (import_filename && import_filename_if_exists)
+		die(_("Cannot pass both --import-marks and --import-marks-if-exists"));
 	if (import_filename)
-		import_marks(import_filename);
+		import_marks(import_filename, 0);
+	else if (import_filename_if_exists)
+		import_marks(import_filename_if_exists, 1);
 	lastimportid = last_idnum;
 
 	if (import_filename && revs.prune_data.nr)
diff --git a/t/t9350-fast-export.sh b/t/t9350-fast-export.sh
index d32ff41859..ea84e2f173 100755
--- a/t/t9350-fast-export.sh
+++ b/t/t9350-fast-export.sh
@@ -580,17 +580,15 @@ test_expect_success 'fast-export quotes pathnames' '
 '
 
 test_expect_success 'test bidirectionality' '
-	>marks-cur &&
-	>marks-new &&
 	git init marks-test &&
-	git fast-export --export-marks=marks-cur --import-marks=marks-cur --branches | \
-	git --git-dir=marks-test/.git fast-import --export-marks=marks-new --import-marks=marks-new &&
+	git fast-export --export-marks=marks-cur --import-marks-if-exists=marks-cur --branches | \
+	git --git-dir=marks-test/.git fast-import --export-marks=marks-new --import-marks-if-exists=marks-new &&
 	(cd marks-test &&
 	git reset --hard &&
 	echo Wohlauf > file &&
 	git commit -a -m "back in time") &&
-	git --git-dir=marks-test/.git fast-export --export-marks=marks-new --import-marks=marks-new --branches | \
-	git fast-import --export-marks=marks-cur --import-marks=marks-cur
+	git --git-dir=marks-test/.git fast-export --export-marks=marks-new --import-marks-if-exists=marks-new --branches | \
+	git fast-import --export-marks=marks-cur --import-marks-if-exists=marks-cur
 '
 
 cat > expected << EOF
-- 
2.23.0.177.g8af0b3ca64
Elijah Newren· Sep 25, 2019, 01:40 UTC · re: Elijah Newren · lore

[PATCH 7/8] t9350: add tests for tags of things other than a commit

Multiple changes here:
  * add a test for a tag of a blob
  * add a test for a tag of a tag of a commit
  * add a comment to the tests for (possibly nested) tags of trees,
    making it clear that these tests are doing much less than you might
    expect
Signed-off-by: Elijah Newren <newren@gmail.com>
---
 t/t9350-fast-export.sh | 31 +++++++++++++++++++++++++++++++
 1 file changed, 31 insertions(+)
Show changes to t/t9350-fast-export.sh +31 −0
diff --git a/t/t9350-fast-export.sh b/t/t9350-fast-export.sh
index b3fca6ffba..9ab281e4b9 100755
--- a/t/t9350-fast-export.sh
+++ b/t/t9350-fast-export.sh
@@ -540,10 +540,41 @@ test_expect_success 'tree_tag'        '
 '
 
 # NEEDSWORK: not just check return status, but validate the output
+# Note that these tests DO NOTHING other than print a warning that
+# they are ommitting the one tag we asked them to export (because the
+# tags resolve to a tree).  They exist just to make sure we do not
+# abort but instead just warn.
 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'
 
+test_expect_success 'handling tags of blobs' '
+	git tag -a -m "Tag of a blob" blobtag $(git rev-parse master:file) &&
+	git fast-export blobtag >actual &&
+	cat >expect <<-EOF &&
+	blob
+	mark :1
+	data 9
+	die Luft
+
+	tag blobtag
+	from :1
+	tagger $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> $GIT_COMMITTER_DATE
+	data 14
+	Tag of a blob
+
+	EOF
+	test_cmp expect actual
+'
+
+test_expect_failure 'handling nested tags' '
+	git tag -a -m "This is a nested tag" nested muss &&
+	git fast-export --mark-tags nested >output &&
+	grep "^from $ZERO_OID$" output &&
+	grep "^tag nested$" output >tag_lines &&
+	test_line_count = 2 tag_lines
+'
+
 test_expect_success 'directory becomes symlink'        '
 	git init dirtosymlink &&
 	git init result &&
-- 
2.23.0.177.g8af0b3ca64
Elijah Newren· Sep 25, 2019, 01:40 UTC · re: Elijah Newren · lore

[PATCH 8/8] fast-export: handle nested tags

Signed-off-by: Elijah Newren <newren@gmail.com>
---
 builtin/fast-export.c  | 30 ++++++++++++++++++------------
 t/t9350-fast-export.sh |  2 +-
 2 files changed, 19 insertions(+), 13 deletions(-)
Show changes to 2 files +19 −13

builtin/fast-export.c, t/t9350-fast-export.sh

diff --git a/builtin/fast-export.c b/builtin/fast-export.c
index d32e1e9327..58a74de42a 100644
--- a/builtin/fast-export.c
+++ b/builtin/fast-export.c
@@ -843,22 +843,28 @@ static void handle_tag(const char *name, struct tag *tag)
 			free(buf);
 			return;
 		case REWRITE:
-			if (tagged->type != OBJ_COMMIT) {
-				die("tag %s tags unexported %s!",
-				    oid_to_hex(&tag->object.oid),
-				    type_name(tagged->type));
-			}
-			p = rewrite_commit((struct commit *)tagged);
-			if (!p) {
-				printf("reset %s\nfrom %s\n\n",
-				       name, oid_to_hex(&null_oid));
-				free(buf);
-				return;
+			if (tagged->type == OBJ_TAG && !mark_tags) {
+				die(_("Error: Cannot export nested tags unless --mark-tags is specified."));
+			} else if (tagged->type == OBJ_COMMIT) {
+				p = rewrite_commit((struct commit *)tagged);
+				if (!p) {
+					printf("reset %s\nfrom %s\n\n",
+					       name, oid_to_hex(&null_oid));
+					free(buf);
+					return;
+				}
+				tagged_mark = get_object_mark(&p->object);
+			} else {
+				/* tagged->type is either OBJ_BLOB or OBJ_TAG */
+				tagged_mark = get_object_mark(tagged);
 			}
-			tagged_mark = get_object_mark(&p->object);
 		}
 	}
 
+	if (tagged->type == OBJ_TAG) {
+		printf("reset %s\nfrom %s\n\n",
+		       name, oid_to_hex(&null_oid));
+	}
 	if (starts_with(name, "refs/tags/"))
 		name += 10;
 	printf("tag %s\n", name);
diff --git a/t/t9350-fast-export.sh b/t/t9350-fast-export.sh
index 9ab281e4b9..2e4e214815 100755
--- a/t/t9350-fast-export.sh
+++ b/t/t9350-fast-export.sh
@@ -567,7 +567,7 @@ test_expect_success 'handling tags of blobs' '
 	test_cmp expect actual
 '
 
-test_expect_failure 'handling nested tags' '
+test_expect_success 'handling nested tags' '
 	git tag -a -m "This is a nested tag" nested muss &&
 	git fast-export --mark-tags nested >output &&
 	grep "^from $ZERO_OID$" output &&
-- 
2.23.0.177.g8af0b3ca64
Elijah Newren· Sep 30, 2019, 21:10 UTC · re: Elijah Newren · lore

[PATCH v2 0/8] fast export/import: handle nested tags, improve incremental exports

This series improves the incremental export story for fast-export and fast-import (--export-marks and --import-marks fell a bit short), fixes a couple small export/import bugs, and enables handling nested tags. In particular, the nested tags handling makes it so that fast-export and fast-import can finally handle the git.git repo.

Changes since v1 (full range-diff below):
  - Fixed an issue integrating with next/pu (in particular, with
    jk/fast-import-history-bugfix)
Elijah Newren (8):
  fast-export: fix exporting a tag and nothing else
  fast-import: fix handling of deleted tags
  fast-import: allow tags to be identified by mark labels
  fast-import: add support for new 'alias' command
  fast-export: add support for --import-marks-if-exists
  fast-export: allow user to request tags be marked with --mark-tags
  t9350: add tests for tags of things other than a commit
  fast-export: handle nested tags
 Documentation/git-fast-export.txt | 17 ++++--
 Documentation/git-fast-import.txt | 23 ++++++++
 builtin/fast-export.c             | 67 ++++++++++++++++------
 fast-import.c                     | 94 +++++++++++++++++++++++++++----
 t/t9300-fast-import.sh            | 37 ++++++++++++
 t/t9350-fast-export.sh            | 68 ++++++++++++++++++++--
 6 files changed, 268 insertions(+), 38 deletions(-)
Range-diff:
1:  b751d6c2d6 ! 1:  1d19498bc6 fast-import: fix handling of deleted tags
    @@ fast-import.c: static void parse_reset_branch(const char *arg)
      		b = new_branch(arg);
      	read_next_command();
      	parse_from(b);
    -+	if (b->delete && !strncmp(arg, "refs/tags/", 10)) {
    ++	if (b->delete && !strncmp(b->name, "refs/tags/", 10)) {
     +		/*
     +		 * Elsewhere, we call dump_branches() before dump_tags(),
     +		 * and dump_branches() will handle ref deletions first, so
    @@ fast-import.c: static void parse_reset_branch(const char *arg)
     +		for (t = first_tag; t; t = t->next_tag) {
     +			strbuf_reset(&tag_name);
     +			strbuf_addf(&tag_name, "refs/tags/%s", t->name);
    -+			if (!strcmp(arg, tag_name.buf))
    ++			if (!strcmp(b->name, tag_name.buf))
     +				break;
     +			prev = t;
     +		}
2:  26b77dde15 = 2:  e1fd888e4a fast-import: allow tags to be identified by mark labels
3:  e0d1a1d7aa = 3:  93175f28d9 fast-import: add support for new 'alias' command
4:  edea892661 = 4:  8c8743395c fast-export: add support for --import-marks-if-exists
5:  6af7e1fdd0 = 5:  eebc40df33 fast-export: allow user to request tags be marked with --mark-tags
6:  631ae9a63e = 6:  de39f703c6 t9350: add tests for tags of things other than a commit
7:  c0e932e4da = 7:  ac739dbb79 fast-export: handle nested tags
-- 
2.23.0.264.gac739dbb79
Elijah Newren· Sep 30, 2019, 21:10 UTC · re: Elijah Newren · lore

[PATCH v2 1/8] fast-export: fix exporting a tag and nothing else

fast-export allows specifying revision ranges, which can be used to export a tag without exporting the commit it tags. fast-export handled this rather poorly: it would emit a "from :0" directive. Since marks start at 1 and increase, this means it refers to an unknown commit and fast-import will choke on the input.

When we are unable to look up a mark for the object being tagged, use a "from $HASH" directive instead to fix this problem.

Note that this is quite similar to the behavior fast-export exhibits with commits and parents when --reference-excluded-parents is passed along with an excluded commit range. For tags of excluded commits we do not require the --reference-excluded-parents flag because we always have to tag something. By contrast, when dealing with commits, pruning a parent is always a viable option, so we need the flag to specify that parent pruning is not wanted. (It is slightly weird that --reference-excluded-parents isn't the default with a separate --prune-excluded-parents flag, but backward compatibility concerns resulted in the current defaults.)

Signed-off-by: Elijah Newren <newren@gmail.com>
---
 builtin/fast-export.c  |  7 ++++++-
 t/t9350-fast-export.sh | 13 +++++++++++++
 2 files changed, 19 insertions(+), 1 deletion(-)
Show changes to 2 files +19 −1

builtin/fast-export.c, t/t9350-fast-export.sh

diff --git a/builtin/fast-export.c b/builtin/fast-export.c
index f541f55d33..5822271c6b 100644
--- a/builtin/fast-export.c
+++ b/builtin/fast-export.c
@@ -860,7 +860,12 @@ static void handle_tag(const char *name, struct tag *tag)
 
 	if (starts_with(name, "refs/tags/"))
 		name += 10;
-	printf("tag %s\nfrom :%d\n", name, tagged_mark);
+	printf("tag %s\n", name);
+	if (tagged_mark)
+		printf("from :%d\n", tagged_mark);
+	else
+		printf("from %s\n", oid_to_hex(&tagged->oid));
+
 	if (show_original_ids)
 		printf("original-oid %s\n", oid_to_hex(&tag->object.oid));
 	printf("%.*s%sdata %d\n%.*s\n",
diff --git a/t/t9350-fast-export.sh b/t/t9350-fast-export.sh
index b4004e05c2..d32ff41859 100755
--- a/t/t9350-fast-export.sh
+++ b/t/t9350-fast-export.sh
@@ -53,6 +53,19 @@ test_expect_success 'fast-export | fast-import' '
 
 '
 
+test_expect_success 'fast-export ^muss^{commit} muss' '
+	git fast-export --tag-of-filtered-object=rewrite ^muss^{commit} muss >actual &&
+	cat >expected <<-EOF &&
+	tag muss
+	from $(git rev-parse --verify muss^{commit})
+	$(git cat-file tag muss | grep tagger)
+	data 9
+	valentin
+
+	EOF
+	test_cmp expected actual
+'
+
 test_expect_success 'fast-export master~2..master' '
 
 	git fast-export master~2..master >actual &&
-- 
2.23.0.264.gac739dbb79
Elijah Newren· Sep 30, 2019, 21:10 UTC · re: Elijah Newren · lore

[PATCH v2 2/8] fast-import: fix handling of deleted tags

If our input stream includes a tag which is later deleted, we were not properly deleting it. We did have a step which would delete it, but we left a tag in the tag list noting that it needed to be updated, and the updating of annotated tags occurred AFTER ref deletion. So, when we record that a tag needs to be deleted, also remove it from the list of annotated tags to update.

While this has likely been something that has not happened in practice, it will come up more in order to support nested tags. For nested tags, we either need to give temporary names to the intermediate tags and then delete them, or else we need to use the final name for the intermediate tags. If we use the final name for the intermediate tags, then in order to keep the sanity check that someone doesn't try to update the same tag twice, we need to delete the ref after creating the intermediate tag. So, either way nested tags imply the need to delete temporary inner tag references.

Signed-off-by: Elijah Newren <newren@gmail.com>
---
 fast-import.c          | 29 +++++++++++++++++++++++++++++
 t/t9300-fast-import.sh | 13 +++++++++++++
 2 files changed, 42 insertions(+)
Show changes to 2 files +42 −0

fast-import.c, t/t9300-fast-import.sh

diff --git a/fast-import.c b/fast-import.c
index b44d6a467e..546da3a938 100644
--- a/fast-import.c
+++ b/fast-import.c
@@ -2793,6 +2793,35 @@ static void parse_reset_branch(const char *arg)
 		b = new_branch(arg);
 	read_next_command();
 	parse_from(b);
+	if (b->delete && !strncmp(b->name, "refs/tags/", 10)) {
+		/*
+		 * Elsewhere, we call dump_branches() before dump_tags(),
+		 * and dump_branches() will handle ref deletions first, so
+		 * in order to make sure the deletion actually takes effect,
+		 * we need to remove the tag from our list of tags to update.
+		 *
+		 * NEEDSWORK: replace list of tags with hashmap for faster
+		 * deletion?
+		 */
+		struct strbuf tag_name = STRBUF_INIT;
+		struct tag *t, *prev = NULL;
+		for (t = first_tag; t; t = t->next_tag) {
+			strbuf_reset(&tag_name);
+			strbuf_addf(&tag_name, "refs/tags/%s", t->name);
+			if (!strcmp(b->name, tag_name.buf))
+				break;
+			prev = t;
+		}
+		if (t) {
+			if (prev)
+				prev->next_tag = t->next_tag;
+			else
+				first_tag = t->next_tag;
+			if (!t->next_tag)
+				last_tag = prev;
+			/* There is no mem_pool_free(t) function to call. */
+		}
+	}
 	if (command_buf.len > 0)
 		unread_command_buf = 1;
 }
diff --git a/t/t9300-fast-import.sh b/t/t9300-fast-import.sh
index 141b7fa35e..74bc41333b 100755
--- a/t/t9300-fast-import.sh
+++ b/t/t9300-fast-import.sh
@@ -85,6 +85,15 @@ test_expect_success 'A: create pack from stdin' '
 	An annotated tag that annotates a blob.
 	EOF
 
+	tag to-be-deleted
+	from :3
+	data <<EOF
+	Another annotated tag that annotates a blob.
+	EOF
+
+	reset refs/tags/to-be-deleted
+	from 0000000000000000000000000000000000000000
+
 	INPUT_END
 	git fast-import --export-marks=marks.out <input &&
 	git whatchanged master
@@ -157,6 +166,10 @@ test_expect_success 'A: verify tag/series-A-blob' '
 	test_cmp expect actual
 '
 
+test_expect_success 'A: verify tag deletion is successful' '
+	test_must_fail git rev-parse --verify refs/tags/to-be-deleted
+'
+
 test_expect_success 'A: verify marks output' '
 	cat >expect <<-EOF &&
 	:2 $(git rev-parse --verify master:file2)
-- 
2.23.0.264.gac739dbb79
René Scharfe· Oct 3, 2019, 11:53 UTC · re: Elijah Newren · lore

Re: [PATCH v2 2/8] fast-import: fix handling of deleted tags

Am 30.09.19 um 23:10 schrieb Elijah Newren:
Show 32 quoted lines
> If our input stream includes a tag which is later deleted, we were not
> properly deleting it.  We did have a step which would delete it, but we
> left a tag in the tag list noting that it needed to be updated, and the
> updating of annotated tags occurred AFTER ref deletion.  So, when we
> record that a tag needs to be deleted, also remove it from the list of
> annotated tags to update.
>
> While this has likely been something that has not happened in practice,
> it will come up more in order to support nested tags.  For nested tags,
> we either need to give temporary names to the intermediate tags and then
> delete them, or else we need to use the final name for the intermediate
> tags.  If we use the final name for the intermediate tags, then in order
> to keep the sanity check that someone doesn't try to update the same tag
> twice, we need to delete the ref after creating the intermediate tag.
> So, either way nested tags imply the need to delete temporary inner tag
> references.
>
> Signed-off-by: Elijah Newren <newren@gmail.com>
> ---
>  fast-import.c          | 29 +++++++++++++++++++++++++++++
>  t/t9300-fast-import.sh | 13 +++++++++++++
>  2 files changed, 42 insertions(+)
>
> diff --git a/fast-import.c b/fast-import.c
> index b44d6a467e..546da3a938 100644
> --- a/fast-import.c
> +++ b/fast-import.c
> @@ -2793,6 +2793,35 @@ static void parse_reset_branch(const char *arg)
>  		b = new_branch(arg);
>  	read_next_command();
>  	parse_from(b);
> +	if (b->delete && !strncmp(b->name, "refs/tags/", 10)) {

b->name is a NUL-terminated string; starts_with() could be used to avoid the magic number 10.

Show 10 quoted lines
> +		/*
> +		 * Elsewhere, we call dump_branches() before dump_tags(),
> +		 * and dump_branches() will handle ref deletions first, so
> +		 * in order to make sure the deletion actually takes effect,
> +		 * we need to remove the tag from our list of tags to update.
> +		 *
> +		 * NEEDSWORK: replace list of tags with hashmap for faster
> +		 * deletion?
> +		 */
> +		struct strbuf tag_name = STRBUF_INIT;
This adds a small memory leak.
Show 5 quoted lines
> +		struct tag *t, *prev = NULL;
> +		for (t = first_tag; t; t = t->next_tag) {
> +			strbuf_reset(&tag_name);
> +			strbuf_addf(&tag_name, "refs/tags/%s", t->name);
> +			if (!strcmp(b->name, tag_name.buf))

So the strbuf is used to prefix t->name with "refs/tags/", which we know b->name starts with, and to compare the result with b->name. Removing the "refs/tags/" prefix from b->name using skip_prefix() and comparing the result with t->name would be easier.

Show 16 quoted lines
> +				break;
> +			prev = t;
> +		}
> +		if (t) {
> +			if (prev)
> +				prev->next_tag = t->next_tag;
> +			else
> +				first_tag = t->next_tag;
> +			if (!t->next_tag)
> +				last_tag = prev;
> +			/* There is no mem_pool_free(t) function to call. */
> +		}
> +	}
>  	if (command_buf.len > 0)
>  		unread_command_buf = 1;
>  }
Here's a squashable patch for that:
---
 fast-import.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)
Show changes to fast-import.c +3 −6
diff --git a/fast-import.c b/fast-import.c
index 70cd3f0ff4..a109591406 100644
--- a/fast-import.c
+++ b/fast-import.c
@@ -2779,6 +2779,7 @@ static void parse_new_tag(const char *arg)
 static void parse_reset_branch(const char *arg)
 {
 	struct branch *b;
+	const char *tag_name;

 	b = lookup_branch(arg);
 	if (b) {
@@ -2794,7 +2795,7 @@ static void parse_reset_branch(const char *arg)
 		b = new_branch(arg);
 	read_next_command();
 	parse_from(b);
-	if (b->delete && !strncmp(b->name, "refs/tags/", 10)) {
+	if (b->delete && skip_prefix(b->name, "refs/tags/", &tag_name)) {
 		/*
 		 * Elsewhere, we call dump_branches() before dump_tags(),
 		 * and dump_branches() will handle ref deletions first, so
@@ -2804,12 +2805,9 @@ static void parse_reset_branch(const char *arg)
 		 * NEEDSWORK: replace list of tags with hashmap for faster
 		 * deletion?
 		 */
-		struct strbuf tag_name = STRBUF_INIT;
 		struct tag *t, *prev = NULL;
 		for (t = first_tag; t; t = t->next_tag) {
-			strbuf_reset(&tag_name);
-			strbuf_addf(&tag_name, "refs/tags/%s", t->name);
-			if (!strcmp(b->name, tag_name.buf))
+			if (!strcmp(t->name, tag_name))
 				break;
 			prev = t;
 		}
--
2.23.0
Elijah Newren· Sep 30, 2019, 21:10 UTC · re: Elijah Newren · lore

[PATCH v2 3/8] fast-import: allow tags to be identified by mark labels

Mark identifiers are used in fast-export and fast-import to provide a label to refer to earlier content. Blobs are given labels because they need to be referenced in the commits where they first appear with a given filename, and commits are given labels because they can be the parents of other commits. Tags were never given labels, probably because they were viewed as unnecessary, but that presents two problems:

   1. It leaves us without a way of referring to previous tags if we
      want to create a tag of a tag (or higher nestings).
   2. It leaves us with no way of recording that a tag has already been
      imported when using --export-marks and --import-marks.
Fix these problems by allowing an optional mark label for tags.
Signed-off-by: Elijah Newren <newren@gmail.com>
---
 Documentation/git-fast-import.txt |  1 +
 fast-import.c                     |  3 ++-
 t/t9300-fast-import.sh            | 19 +++++++++++++++++++
 3 files changed, 22 insertions(+), 1 deletion(-)
Show changes to 3 files +22 −1

Documentation/git-fast-import.txt, fast-import.c, t/t9300-fast-import.sh

diff --git a/Documentation/git-fast-import.txt b/Documentation/git-fast-import.txt
index 0bb276269e..4977869465 100644
--- a/Documentation/git-fast-import.txt
+++ b/Documentation/git-fast-import.txt
@@ -774,6 +774,7 @@ lightweight (non-annotated) tags see the `reset` command below.
 
 ....
 	'tag' SP <name> LF
+	mark?
 	'from' SP <commit-ish> LF
 	original-oid?
 	'tagger' (SP <name>)? SP LT <email> GT SP <when> LF
diff --git a/fast-import.c b/fast-import.c
index 546da3a938..0042440487 100644
--- a/fast-import.c
+++ b/fast-import.c
@@ -2713,6 +2713,7 @@ static void parse_new_tag(const char *arg)
 		first_tag = t;
 	last_tag = t;
 	read_next_command();
+	parse_mark();
 
 	/* from ... */
 	if (!skip_prefix(command_buf.buf, "from ", &from))
@@ -2769,7 +2770,7 @@ static void parse_new_tag(const char *arg)
 	strbuf_addbuf(&new_data, &msg);
 	free(tagger);
 
-	if (store_object(OBJ_TAG, &new_data, NULL, &t->oid, 0))
+	if (store_object(OBJ_TAG, &new_data, NULL, &t->oid, next_mark))
 		t->pack_id = MAX_PACK_ID;
 	else
 		t->pack_id = pack_id;
diff --git a/t/t9300-fast-import.sh b/t/t9300-fast-import.sh
index 74bc41333b..3ad2b2f1ba 100755
--- a/t/t9300-fast-import.sh
+++ b/t/t9300-fast-import.sh
@@ -94,6 +94,23 @@ test_expect_success 'A: create pack from stdin' '
 	reset refs/tags/to-be-deleted
 	from 0000000000000000000000000000000000000000
 
+	tag nested
+	mark :6
+	from :4
+	data <<EOF
+	Tag of our lovely commit
+	EOF
+
+	reset refs/tags/nested
+	from 0000000000000000000000000000000000000000
+
+	tag nested
+	mark :7
+	from :6
+	data <<EOF
+	Tag of tag of our lovely commit
+	EOF
+
 	INPUT_END
 	git fast-import --export-marks=marks.out <input &&
 	git whatchanged master
@@ -176,6 +193,8 @@ test_expect_success 'A: verify marks output' '
 	:3 $(git rev-parse --verify master:file3)
 	:4 $(git rev-parse --verify master:file4)
 	:5 $(git rev-parse --verify master^0)
+	:6 $(git cat-file tag nested | grep object | cut -d" " -f 2)
+	:7 $(git rev-parse --verify nested)
 	EOF
 	test_cmp expect marks.out
 '
-- 
2.23.0.264.gac739dbb79
Elijah Newren· Sep 30, 2019, 21:10 UTC · re: Elijah Newren · lore

[PATCH v2 4/8] fast-import: add support for new 'alias' command

fast-export and fast-import have nice --import-marks flags which allow for incremental migrations. However, if there is a mark in fast-export's file of marks without a corresponding mark in the one for fast-import, then we run the risk that fast-export tries to send new objects relative to the mark it knows which fast-import does not, causing fast-import to fail.

This arises in practice when there is a filter of some sort running between the fast-export and fast-import processes which prunes some commits programmatically. Provide such a filter with the ability to alias pruned commits to their most recent non-pruned ancestor.

Signed-off-by: Elijah Newren <newren@gmail.com>
---
 Documentation/git-fast-import.txt | 22 +++++++++++
 fast-import.c                     | 62 ++++++++++++++++++++++++++-----
 t/t9300-fast-import.sh            |  5 +++
 3 files changed, 79 insertions(+), 10 deletions(-)
Show changes to 3 files +79 −10

Documentation/git-fast-import.txt, fast-import.c, t/t9300-fast-import.sh

diff --git a/Documentation/git-fast-import.txt b/Documentation/git-fast-import.txt
index 4977869465..a3f1e0c5e4 100644
--- a/Documentation/git-fast-import.txt
+++ b/Documentation/git-fast-import.txt
@@ -337,6 +337,13 @@ and control the current import process.  More detailed discussion
 	`commit` command.  This command is optional and is not
 	needed to perform an import.
 
+`alias`::
+	Record that a mark refers to a given object without first
+	creating any new object.  Using --import-marks and referring
+	to missing marks will cause fast-import to fail, so aliases
+	can provide a way to set otherwise pruned commits to a valid
+	value (e.g. the nearest non-pruned ancestor).
+
 `checkpoint`::
 	Forces fast-import to close the current packfile, generate its
 	unique SHA-1 checksum and index, and start a new packfile.
@@ -914,6 +921,21 @@ a data chunk which does not have an LF as its last byte.
 +
 The `LF` after `<delim> LF` is optional (it used to be required).
 
+`alias`
+~~~~~~~
+Record that a mark refers to a given object without first creating any
+new object.
+
+....
+	'alias' LF
+	mark
+	'to' SP <commit-ish> LF
+	LF?
+....
+
+For a detailed description of `<commit-ish>` see above under `from`.
+
+
 `checkpoint`
 ~~~~~~~~~~~~
 Forces fast-import to close the current packfile, start a new one, and to
diff --git a/fast-import.c b/fast-import.c
index 0042440487..9a12850d16 100644
--- a/fast-import.c
+++ b/fast-import.c
@@ -2491,18 +2491,14 @@ static void parse_from_existing(struct branch *b)
 	}
 }
 
-static int parse_from(struct branch *b)
+static int parse_objectish(struct branch *b, const char *objectish)
 {
-	const char *from;
 	struct branch *s;
 	struct object_id oid;
 
-	if (!skip_prefix(command_buf.buf, "from ", &from))
-		return 0;
-
 	oidcpy(&oid, &b->branch_tree.versions[1].oid);
 
-	s = lookup_branch(from);
+	s = lookup_branch(objectish);
 	if (b == s)
 		die("Can't create a branch from itself: %s", b->name);
 	else if (s) {
@@ -2510,8 +2506,8 @@ static int parse_from(struct branch *b)
 		oidcpy(&b->oid, &s->oid);
 		oidcpy(&b->branch_tree.versions[0].oid, t);
 		oidcpy(&b->branch_tree.versions[1].oid, t);
-	} else if (*from == ':') {
-		uintmax_t idnum = parse_mark_ref_eol(from);
+	} else if (*objectish == ':') {
+		uintmax_t idnum = parse_mark_ref_eol(objectish);
 		struct object_entry *oe = find_mark(idnum);
 		if (oe->type != OBJ_COMMIT)
 			die("Mark :%" PRIuMAX " not a commit", idnum);
@@ -2525,13 +2521,13 @@ static int parse_from(struct branch *b)
 			} else
 				parse_from_existing(b);
 		}
-	} else if (!get_oid(from, &b->oid)) {
+	} else if (!get_oid(objectish, &b->oid)) {
 		parse_from_existing(b);
 		if (is_null_oid(&b->oid))
 			b->delete = 1;
 	}
 	else
-		die("Invalid ref name or SHA1 expression: %s", from);
+		die("Invalid ref name or SHA1 expression: %s", objectish);
 
 	if (b->branch_tree.tree && !oideq(&oid, &b->branch_tree.versions[1].oid)) {
 		release_tree_content_recursive(b->branch_tree.tree);
@@ -2542,6 +2538,26 @@ static int parse_from(struct branch *b)
 	return 1;
 }
 
+static int parse_from(struct branch *b)
+{
+	const char *from;
+
+	if (!skip_prefix(command_buf.buf, "from ", &from))
+		return 0;
+
+	return parse_objectish(b, from);
+}
+
+static int parse_objectish_with_prefix(struct branch *b, const char *prefix)
+{
+	const char *base;
+
+	if (!skip_prefix(command_buf.buf, prefix, &base))
+		return 0;
+
+	return parse_objectish(b, base);
+}
+
 static struct hash_list *parse_merge(unsigned int *count)
 {
 	struct hash_list *list = NULL, **tail = &list, *n;
@@ -3089,6 +3105,28 @@ static void parse_progress(void)
 	skip_optional_lf();
 }
 
+static void parse_alias(void)
+{
+	struct object_entry *e;
+	struct branch b;
+
+	skip_optional_lf();
+	read_next_command();
+
+	/* mark ... */
+	parse_mark();
+	if (!next_mark)
+		die(_("Expected 'mark' command, got %s"), command_buf.buf);
+
+	/* to ... */
+	memset(&b, 0, sizeof(b));
+	if (!parse_objectish_with_prefix(&b, "to "))
+		die(_("Expected 'to' command, got %s"), command_buf.buf);
+	e = find_object(&b.oid);
+	assert(e);
+	insert_mark(next_mark, e);
+}
+
 static char* make_fast_import_path(const char *path)
 {
 	if (!relative_marks_paths || is_absolute_path(path))
@@ -3216,6 +3254,8 @@ static int parse_one_feature(const char *feature, int from_stream)
 		option_import_marks(arg, from_stream, 1);
 	} else if (skip_prefix(feature, "export-marks=", &arg)) {
 		option_export_marks(arg);
+	} else if (!strcmp(feature, "alias")) {
+		; /* Don't die - this feature is supported */
 	} else if (!strcmp(feature, "get-mark")) {
 		; /* Don't die - this feature is supported */
 	} else if (!strcmp(feature, "cat-blob")) {
@@ -3372,6 +3412,8 @@ int cmd_main(int argc, const char **argv)
 			parse_checkpoint();
 		else if (!strcmp("done", command_buf.buf))
 			break;
+		else if (!strcmp("alias", command_buf.buf))
+			parse_alias();
 		else if (starts_with(command_buf.buf, "progress "))
 			parse_progress();
 		else if (skip_prefix(command_buf.buf, "feature ", &v))
diff --git a/t/t9300-fast-import.sh b/t/t9300-fast-import.sh
index 3ad2b2f1ba..41f2a1dad9 100755
--- a/t/t9300-fast-import.sh
+++ b/t/t9300-fast-import.sh
@@ -111,6 +111,10 @@ test_expect_success 'A: create pack from stdin' '
 	Tag of tag of our lovely commit
 	EOF
 
+	alias
+	mark :8
+	to :5
+
 	INPUT_END
 	git fast-import --export-marks=marks.out <input &&
 	git whatchanged master
@@ -195,6 +199,7 @@ test_expect_success 'A: verify marks output' '
 	:5 $(git rev-parse --verify master^0)
 	:6 $(git cat-file tag nested | grep object | cut -d" " -f 2)
 	:7 $(git rev-parse --verify nested)
+	:8 $(git rev-parse --verify master^0)
 	EOF
 	test_cmp expect marks.out
 '
-- 
2.23.0.264.gac739dbb79
Elijah Newren· Sep 30, 2019, 21:10 UTC · re: Elijah Newren · lore

[PATCH v2 5/8] fast-export: add support for --import-marks-if-exists

fast-import has support for both an --import-marks flag and an --import-marks-if-exists flag; the latter of which will not die() if the file does not exist. fast-export only had support for an --import-marks flag; add an --import-marks-if-exists flag for consistency.

Signed-off-by: Elijah Newren <newren@gmail.com>
---
 builtin/fast-export.c  | 23 +++++++++++++++++++----
 t/t9350-fast-export.sh | 10 ++++------
 2 files changed, 23 insertions(+), 10 deletions(-)
Show changes to 2 files +23 −10

builtin/fast-export.c, t/t9350-fast-export.sh

diff --git a/builtin/fast-export.c b/builtin/fast-export.c
index 5822271c6b..575e47833b 100644
--- a/builtin/fast-export.c
+++ b/builtin/fast-export.c
@@ -1052,11 +1052,16 @@ static void export_marks(char *file)
 		error("Unable to write marks file %s.", file);
 }
 
-static void import_marks(char *input_file)
+static void import_marks(char *input_file, int check_exists)
 {
 	char line[512];
-	FILE *f = xfopen(input_file, "r");
+	FILE *f;
+	struct stat sb;
+
+	if (check_exists && stat(input_file, &sb))
+		return;
 
+	f = xfopen(input_file, "r");
 	while (fgets(line, sizeof(line), f)) {
 		uint32_t mark;
 		char *line_end, *mark_end;
@@ -1120,7 +1125,9 @@ int cmd_fast_export(int argc, const char **argv, const char *prefix)
 	struct rev_info revs;
 	struct object_array commits = OBJECT_ARRAY_INIT;
 	struct commit *commit;
-	char *export_filename = NULL, *import_filename = NULL;
+	char *export_filename = NULL,
+	     *import_filename = NULL,
+	     *import_filename_if_exists = NULL;
 	uint32_t lastimportid;
 	struct string_list refspecs_list = STRING_LIST_INIT_NODUP;
 	struct string_list paths_of_changed_objects = STRING_LIST_INIT_DUP;
@@ -1140,6 +1147,10 @@ int cmd_fast_export(int argc, const char **argv, const char *prefix)
 			     N_("Dump marks to this file")),
 		OPT_STRING(0, "import-marks", &import_filename, N_("file"),
 			     N_("Import marks from this file")),
+		OPT_STRING(0, "import-marks-if-exists",
+			     &import_filename_if_exists,
+			     N_("file"),
+			     N_("Import marks from this file if it exists")),
 		OPT_BOOL(0, "fake-missing-tagger", &fake_missing_tagger,
 			 N_("Fake a tagger when tags lack one")),
 		OPT_BOOL(0, "full-tree", &full_tree,
@@ -1187,8 +1198,12 @@ int cmd_fast_export(int argc, const char **argv, const char *prefix)
 	if (use_done_feature)
 		printf("feature done\n");
 
+	if (import_filename && import_filename_if_exists)
+		die(_("Cannot pass both --import-marks and --import-marks-if-exists"));
 	if (import_filename)
-		import_marks(import_filename);
+		import_marks(import_filename, 0);
+	else if (import_filename_if_exists)
+		import_marks(import_filename_if_exists, 1);
 	lastimportid = last_idnum;
 
 	if (import_filename && revs.prune_data.nr)
diff --git a/t/t9350-fast-export.sh b/t/t9350-fast-export.sh
index d32ff41859..ea84e2f173 100755
--- a/t/t9350-fast-export.sh
+++ b/t/t9350-fast-export.sh
@@ -580,17 +580,15 @@ test_expect_success 'fast-export quotes pathnames' '
 '
 
 test_expect_success 'test bidirectionality' '
-	>marks-cur &&
-	>marks-new &&
 	git init marks-test &&
-	git fast-export --export-marks=marks-cur --import-marks=marks-cur --branches | \
-	git --git-dir=marks-test/.git fast-import --export-marks=marks-new --import-marks=marks-new &&
+	git fast-export --export-marks=marks-cur --import-marks-if-exists=marks-cur --branches | \
+	git --git-dir=marks-test/.git fast-import --export-marks=marks-new --import-marks-if-exists=marks-new &&
 	(cd marks-test &&
 	git reset --hard &&
 	echo Wohlauf > file &&
 	git commit -a -m "back in time") &&
-	git --git-dir=marks-test/.git fast-export --export-marks=marks-new --import-marks=marks-new --branches | \
-	git fast-import --export-marks=marks-cur --import-marks=marks-cur
+	git --git-dir=marks-test/.git fast-export --export-marks=marks-new --import-marks-if-exists=marks-new --branches | \
+	git fast-import --export-marks=marks-cur --import-marks-if-exists=marks-cur
 '
 
 cat > expected << EOF
-- 
2.23.0.264.gac739dbb79
Elijah Newren· Sep 30, 2019, 21:10 UTC · re: Elijah Newren · lore

[PATCH v2 6/8] fast-export: allow user to request tags be marked with --mark-tags

Add a new option, --mark-tags, which will output mark identifiers with each tag object. This improves the incremental export story with --export-marks since it will allow us to record that annotated tags have been exported, and it is also needed as a step towards supporting nested tags.

Signed-off-by: Elijah Newren <newren@gmail.com>
---
 Documentation/git-fast-export.txt | 17 +++++++++++++----
 builtin/fast-export.c             |  7 +++++++
 t/t9350-fast-export.sh            | 14 ++++++++++++++
 3 files changed, 34 insertions(+), 4 deletions(-)
Show changes to 3 files +34 −4

Documentation/git-fast-export.txt, builtin/fast-export.c, t/t9350-fast-export.sh

diff --git a/Documentation/git-fast-export.txt b/Documentation/git-fast-export.txt
index cc940eb9ad..c522b34f7b 100644
--- a/Documentation/git-fast-export.txt
+++ b/Documentation/git-fast-export.txt
@@ -75,11 +75,20 @@ produced incorrect results if you gave these options.
 	Before processing any input, load the marks specified in
 	<file>.  The input file must exist, must be readable, and
 	must use the same format as produced by --export-marks.
+
+--mark-tags::
+	In addition to labelling blobs and commits with mark ids, also
+	label tags.  This is useful in conjunction with
+	`--export-marks` and `--import-marks`, and is also useful (and
+	necessary) for exporting of nested tags.  It does not hurt
+	other cases and would be the default, but many fast-import
+	frontends are not prepared to accept tags with mark
+	identifiers.
 +
-Any commits that have already been marked will not be exported again.
-If the backend uses a similar --import-marks file, this allows for
-incremental bidirectional exporting of the repository by keeping the
-marks the same across runs.
+Any commits (or tags) that have already been marked will not be
+exported again.  If the backend uses a similar --import-marks file,
+this allows for incremental bidirectional exporting of the repository
+by keeping the marks the same across runs.
 
 --fake-missing-tagger::
 	Some old repositories have tags without a tagger.  The
diff --git a/builtin/fast-export.c b/builtin/fast-export.c
index 575e47833b..d32e1e9327 100644
--- a/builtin/fast-export.c
+++ b/builtin/fast-export.c
@@ -40,6 +40,7 @@ static int no_data;
 static int full_tree;
 static int reference_excluded_commits;
 static int show_original_ids;
+static int mark_tags;
 static struct string_list extra_refs = STRING_LIST_INIT_NODUP;
 static struct string_list tag_refs = STRING_LIST_INIT_NODUP;
 static struct refspec refspecs = REFSPEC_INIT_FETCH;
@@ -861,6 +862,10 @@ static void handle_tag(const char *name, struct tag *tag)
 	if (starts_with(name, "refs/tags/"))
 		name += 10;
 	printf("tag %s\n", name);
+	if (mark_tags) {
+		mark_next_object(&tag->object);
+		printf("mark :%"PRIu32"\n", last_idnum);
+	}
 	if (tagged_mark)
 		printf("from :%d\n", tagged_mark);
 	else
@@ -1165,6 +1170,8 @@ int cmd_fast_export(int argc, const char **argv, const char *prefix)
 			 &reference_excluded_commits, N_("Reference parents which are not in fast-export stream by object id")),
 		OPT_BOOL(0, "show-original-ids", &show_original_ids,
 			    N_("Show original object ids of blobs/commits")),
+		OPT_BOOL(0, "mark-tags", &mark_tags,
+			    N_("Label tags with mark ids")),
 
 		OPT_END()
 	};
diff --git a/t/t9350-fast-export.sh b/t/t9350-fast-export.sh
index ea84e2f173..b3fca6ffba 100755
--- a/t/t9350-fast-export.sh
+++ b/t/t9350-fast-export.sh
@@ -66,6 +66,20 @@ test_expect_success 'fast-export ^muss^{commit} muss' '
 	test_cmp expected actual
 '
 
+test_expect_success 'fast-export --mark-tags ^muss^{commit} muss' '
+	git fast-export --mark-tags --tag-of-filtered-object=rewrite ^muss^{commit} muss >actual &&
+	cat >expected <<-EOF &&
+	tag muss
+	mark :1
+	from $(git rev-parse --verify muss^{commit})
+	$(git cat-file tag muss | grep tagger)
+	data 9
+	valentin
+
+	EOF
+	test_cmp expected actual
+'
+
 test_expect_success 'fast-export master~2..master' '
 
 	git fast-export master~2..master >actual &&
-- 
2.23.0.264.gac739dbb79
Elijah Newren· Sep 30, 2019, 21:10 UTC · re: Elijah Newren · lore

[PATCH v2 7/8] t9350: add tests for tags of things other than a commit

Multiple changes here:
  * add a test for a tag of a blob
  * add a test for a tag of a tag of a commit
  * add a comment to the tests for (possibly nested) tags of trees,
    making it clear that these tests are doing much less than you might
    expect
Signed-off-by: Elijah Newren <newren@gmail.com>
---
 t/t9350-fast-export.sh | 31 +++++++++++++++++++++++++++++++
 1 file changed, 31 insertions(+)
Show changes to t/t9350-fast-export.sh +31 −0
diff --git a/t/t9350-fast-export.sh b/t/t9350-fast-export.sh
index b3fca6ffba..9ab281e4b9 100755
--- a/t/t9350-fast-export.sh
+++ b/t/t9350-fast-export.sh
@@ -540,10 +540,41 @@ test_expect_success 'tree_tag'        '
 '
 
 # NEEDSWORK: not just check return status, but validate the output
+# Note that these tests DO NOTHING other than print a warning that
+# they are ommitting the one tag we asked them to export (because the
+# tags resolve to a tree).  They exist just to make sure we do not
+# abort but instead just warn.
 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'
 
+test_expect_success 'handling tags of blobs' '
+	git tag -a -m "Tag of a blob" blobtag $(git rev-parse master:file) &&
+	git fast-export blobtag >actual &&
+	cat >expect <<-EOF &&
+	blob
+	mark :1
+	data 9
+	die Luft
+
+	tag blobtag
+	from :1
+	tagger $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> $GIT_COMMITTER_DATE
+	data 14
+	Tag of a blob
+
+	EOF
+	test_cmp expect actual
+'
+
+test_expect_failure 'handling nested tags' '
+	git tag -a -m "This is a nested tag" nested muss &&
+	git fast-export --mark-tags nested >output &&
+	grep "^from $ZERO_OID$" output &&
+	grep "^tag nested$" output >tag_lines &&
+	test_line_count = 2 tag_lines
+'
+
 test_expect_success 'directory becomes symlink'        '
 	git init dirtosymlink &&
 	git init result &&
-- 
2.23.0.264.gac739dbb79
Elijah Newren· Sep 30, 2019, 21:10 UTC · re: Elijah Newren · lore

[PATCH v2 8/8] fast-export: handle nested tags

Signed-off-by: Elijah Newren <newren@gmail.com>
---
 builtin/fast-export.c  | 30 ++++++++++++++++++------------
 t/t9350-fast-export.sh |  2 +-
 2 files changed, 19 insertions(+), 13 deletions(-)
Show changes to 2 files +19 −13

builtin/fast-export.c, t/t9350-fast-export.sh

diff --git a/builtin/fast-export.c b/builtin/fast-export.c
index d32e1e9327..58a74de42a 100644
--- a/builtin/fast-export.c
+++ b/builtin/fast-export.c
@@ -843,22 +843,28 @@ static void handle_tag(const char *name, struct tag *tag)
 			free(buf);
 			return;
 		case REWRITE:
-			if (tagged->type != OBJ_COMMIT) {
-				die("tag %s tags unexported %s!",
-				    oid_to_hex(&tag->object.oid),
-				    type_name(tagged->type));
-			}
-			p = rewrite_commit((struct commit *)tagged);
-			if (!p) {
-				printf("reset %s\nfrom %s\n\n",
-				       name, oid_to_hex(&null_oid));
-				free(buf);
-				return;
+			if (tagged->type == OBJ_TAG && !mark_tags) {
+				die(_("Error: Cannot export nested tags unless --mark-tags is specified."));
+			} else if (tagged->type == OBJ_COMMIT) {
+				p = rewrite_commit((struct commit *)tagged);
+				if (!p) {
+					printf("reset %s\nfrom %s\n\n",
+					       name, oid_to_hex(&null_oid));
+					free(buf);
+					return;
+				}
+				tagged_mark = get_object_mark(&p->object);
+			} else {
+				/* tagged->type is either OBJ_BLOB or OBJ_TAG */
+				tagged_mark = get_object_mark(tagged);
 			}
-			tagged_mark = get_object_mark(&p->object);
 		}
 	}
 
+	if (tagged->type == OBJ_TAG) {
+		printf("reset %s\nfrom %s\n\n",
+		       name, oid_to_hex(&null_oid));
+	}
 	if (starts_with(name, "refs/tags/"))
 		name += 10;
 	printf("tag %s\n", name);
diff --git a/t/t9350-fast-export.sh b/t/t9350-fast-export.sh
index 9ab281e4b9..2e4e214815 100755
--- a/t/t9350-fast-export.sh
+++ b/t/t9350-fast-export.sh
@@ -567,7 +567,7 @@ test_expect_success 'handling tags of blobs' '
 	test_cmp expect actual
 '
 
-test_expect_failure 'handling nested tags' '
+test_expect_success 'handling nested tags' '
 	git tag -a -m "This is a nested tag" nested muss &&
 	git fast-export --mark-tags nested >output &&
 	grep "^from $ZERO_OID$" output &&
-- 
2.23.0.264.gac739dbb79
Elijah Newren· Oct 2, 2019, 15:54 UTC · re: Elijah Newren · lore

Re: [PATCH v2 0/8] fast export/import: handle nested tags, improve incremental exports

Hi Junio,
On Mon, Sep 30, 2019 at 2:10 PM Elijah Newren <newren@gmail.com> wrote:
Show 6 quoted lines
>
> This series improves the incremental export story for fast-export and
> fast-import (--export-marks and --import-marks fell a bit short),
> fixes a couple small export/import bugs, and enables handling nested
> tags.  In particular, the nested tags handling makes it so that
> fast-export and fast-import can finally handle the git.git repo.

I see you picked up this corrected series in pu; thanks. However, your merge commit, 2a99d6b6ff7c ("Merge branch 'en/fast-imexport-nested-tags' into pu", 2019-10-02), claims "Seems to break t9300 when merged to 'pu'.". I know v1 did that and I could reproduce, but I can't reproduce any failures here. Was this message just left over or is there some problem you are seeing?

Thanks, Elijah

Show 52 quoted lines
>
> Changes since v1 (full range-diff below):
>   - Fixed an issue integrating with next/pu (in particular, with
>     jk/fast-import-history-bugfix)
>
> Elijah Newren (8):
>   fast-export: fix exporting a tag and nothing else
>   fast-import: fix handling of deleted tags
>   fast-import: allow tags to be identified by mark labels
>   fast-import: add support for new 'alias' command
>   fast-export: add support for --import-marks-if-exists
>   fast-export: allow user to request tags be marked with --mark-tags
>   t9350: add tests for tags of things other than a commit
>   fast-export: handle nested tags
>
>  Documentation/git-fast-export.txt | 17 ++++--
>  Documentation/git-fast-import.txt | 23 ++++++++
>  builtin/fast-export.c             | 67 ++++++++++++++++------
>  fast-import.c                     | 94 +++++++++++++++++++++++++++----
>  t/t9300-fast-import.sh            | 37 ++++++++++++
>  t/t9350-fast-export.sh            | 68 ++++++++++++++++++++--
>  6 files changed, 268 insertions(+), 38 deletions(-)
>
> Range-diff:
> 1:  b751d6c2d6 ! 1:  1d19498bc6 fast-import: fix handling of deleted tags
>     @@ fast-import.c: static void parse_reset_branch(const char *arg)
>                 b = new_branch(arg);
>         read_next_command();
>         parse_from(b);
>     -+  if (b->delete && !strncmp(arg, "refs/tags/", 10)) {
>     ++  if (b->delete && !strncmp(b->name, "refs/tags/", 10)) {
>      +          /*
>      +           * Elsewhere, we call dump_branches() before dump_tags(),
>      +           * and dump_branches() will handle ref deletions first, so
>     @@ fast-import.c: static void parse_reset_branch(const char *arg)
>      +          for (t = first_tag; t; t = t->next_tag) {
>      +                  strbuf_reset(&tag_name);
>      +                  strbuf_addf(&tag_name, "refs/tags/%s", t->name);
>     -+                  if (!strcmp(arg, tag_name.buf))
>     ++                  if (!strcmp(b->name, tag_name.buf))
>      +                          break;
>      +                  prev = t;
>      +          }
> 2:  26b77dde15 = 2:  e1fd888e4a fast-import: allow tags to be identified by mark labels
> 3:  e0d1a1d7aa = 3:  93175f28d9 fast-import: add support for new 'alias' command
> 4:  edea892661 = 4:  8c8743395c fast-export: add support for --import-marks-if-exists
> 5:  6af7e1fdd0 = 5:  eebc40df33 fast-export: allow user to request tags be marked with --mark-tags
> 6:  631ae9a63e = 6:  de39f703c6 t9350: add tests for tags of things other than a commit
> 7:  c0e932e4da = 7:  ac739dbb79 fast-export: handle nested tags
> --
> 2.23.0.264.gac739dbb79
>
Junio C Hamano· Oct 2, 2019, 20:10 UTC · re: Elijah Newren · lore

Re: [PATCH v2 0/8] fast export/import: handle nested tags, improve incremental exports

Elijah Newren <newren@gmail.com> writes:
Show 6 quoted lines
> I see you picked up this corrected series in pu; thanks.  However,
> your merge commit, 2a99d6b6ff7c ("Merge branch
> 'en/fast-imexport-nested-tags' into pu", 2019-10-02), claims "Seems to
> break t9300 when merged to 'pu'.".  I know v1 did that and I could
> reproduce, but I can't reproduce any failures here.  Was this message
> just left over or is there some problem you are seeing?

I thought that the latest What's cooking written after you sent the corrected version hasn't been sent yet.

And the draft copy I locally have for the next issue of what's cooking has the comment removed already.

Anything I missed?
Elijah Newren· Oct 2, 2019, 21:05 UTC · re: Junio C Hamano · lore

Re: [PATCH v2 0/8] fast export/import: handle nested tags, improve incremental exports

On Wed, Oct 2, 2019 at 1:10 PM Junio C Hamano <gitster@pobox.com> wrote:
Show 17 quoted lines
>
> Elijah Newren <newren@gmail.com> writes:
>
> > I see you picked up this corrected series in pu; thanks.  However,
> > your merge commit, 2a99d6b6ff7c ("Merge branch
> > 'en/fast-imexport-nested-tags' into pu", 2019-10-02), claims "Seems to
> > break t9300 when merged to 'pu'.".  I know v1 did that and I could
> > reproduce, but I can't reproduce any failures here.  Was this message
> > just left over or is there some problem you are seeing?
>
> I thought that the latest What's cooking written after you sent the
> corrected version hasn't been sent yet.
>
> And the draft copy I locally have for the next issue of what's cooking
> has the comment removed already.
>
> Anything I missed?

I was looking at the commit message for the merge commit where you merged in v2 of the series, and was surprised to see the commit message claim there was still breakage with the new version of the series:

$ git log -1 2a99d6b6ff7c
commit 2a99d6b6ff7cd29cc41d587ae737ffb8e7babea4
Merge: 7d81b61dd0 0e2eeb4cb4
Author: Junio C Hamano <gitster@pobox.com>
Date:   Wed Oct 2 16:08:06 2019 +0900
    Merge branch 'en/fast-imexport-nested-tags' into pu
    Updates to fast-import/export.
    Seems to break t9300 when merged to 'pu'.
    * en/fast-imexport-nested-tags:
      fast-export: handle nested tags
      t9350: add tests for tags of things other than a commit
      fast-export: allow user to request tags be marked with --mark-tags
      fast-export: add support for --import-marks-if-exists
      fast-import: add support for new 'alias' command
      fast-import: allow tags to be identified by mark labels
      fast-import: fix handling of deleted tags
      fast-export: fix exporting a tag and nothing else
Junio C Hamano· Oct 3, 2019, 01:07 UTC · re: Elijah Newren · lore

Re: [PATCH v2 0/8] fast export/import: handle nested tags, improve incremental exports

Elijah Newren <newren@gmail.com> writes:
Show 7 quoted lines
>> And the draft copy I locally have for the next issue of what's cooking
>> has the comment removed already.
>>
>> Anything I missed?
>
> I was looking at the commit message for the merge commit where you
> merged in v2 of the series...

Ah, sorry, yes you are right, the merge messages can lag behind, especially until the topic hits 'next'. It is not a designed goal to work that way, but just a consequene of how my workflow is structured.

 * A merge grabs the message from the then-latest "what's cooking" draft.
 * The "what's cooking" draft is then updated.  This is a two-part
   process:
   - "git log --first-parent master..pu" is scanned, to see which
     parts of what topic are and are not yet in 'next' (this flips
     '-' and '+' prefix in the "what's cooking" report) and marks
     the changed parts for the next part.
   - An editor session looks for these topics with updated status
     and updates the comment in the "what's cooking" draft..

So, the topic with the new fix-up commit was merged to 'pu' with the message taken from the "what's cooking" draft before the draft gets updated with that fix-up commit. When I have sufficient time, I rebuild 'pu' twice (i.e. do the integration once, and then update the "what's cooking" draft, rewind 'pu' to 'master' and do the integration again to pick up messages from the updated "what's cooking" draft), but yesterday was not one of those days X-<.

Elijah Newren· Oct 3, 2019, 20:27 UTC · re: Elijah Newren · lore

[PATCH -v3 0/8] fast export/import: handle nested tags, improve incremental exports

This series improves the incremental export story for fast-export and fast-import (--export-marks and --import-marks fell a bit short), fixes a couple small export/import bugs, and enables handling nested tags. In particular, the nested tags handling makes it so that fast-export and fast-import can finally handle the git.git repo.

Changes since v2 (full range-diff below):
  - Code cleanup of patch 2 suggested by René
Elijah Newren (8):
  fast-export: fix exporting a tag and nothing else
  fast-import: fix handling of deleted tags
  fast-import: allow tags to be identified by mark labels
  fast-import: add support for new 'alias' command
  fast-export: add support for --import-marks-if-exists
  fast-export: allow user to request tags be marked with --mark-tags
  t9350: add tests for tags of things other than a commit
  fast-export: handle nested tags
 Documentation/git-fast-export.txt | 17 ++++--
 Documentation/git-fast-import.txt | 23 ++++++++
 builtin/fast-export.c             | 67 ++++++++++++++++------
 fast-import.c                     | 92 +++++++++++++++++++++++++++----
 t/t9300-fast-import.sh            | 37 +++++++++++++
 t/t9350-fast-export.sh            | 68 +++++++++++++++++++++--
 6 files changed, 266 insertions(+), 38 deletions(-)
Range-diff:
1:  a30cfbbb50 = 1:  a30cfbbb50 fast-export: fix exporting a tag and nothing else
2:  1d19498bc6 ! 2:  36fbf15134 fast-import: fix handling of deleted tags
    @@ Commit message
         So, either way nested tags imply the need to delete temporary inner tag
         references.
     
    +    Helped-by: René Scharfe <l.s.r@web.de>
         Signed-off-by: Elijah Newren <newren@gmail.com>
     
      ## fast-import.c ##
    +@@ fast-import.c: static void parse_new_tag(const char *arg)
    + static void parse_reset_branch(const char *arg)
    + {
    + 	struct branch *b;
    ++	const char *tag_name;
    + 
    + 	b = lookup_branch(arg);
    + 	if (b) {
     @@ fast-import.c: static void parse_reset_branch(const char *arg)
      		b = new_branch(arg);
      	read_next_command();
      	parse_from(b);
    -+	if (b->delete && !strncmp(b->name, "refs/tags/", 10)) {
    ++	if (b->delete && skip_prefix(b->name, "refs/tags/", &tag_name)) {
     +		/*
     +		 * Elsewhere, we call dump_branches() before dump_tags(),
     +		 * and dump_branches() will handle ref deletions first, so
    @@ fast-import.c: static void parse_reset_branch(const char *arg)
     +		 * NEEDSWORK: replace list of tags with hashmap for faster
     +		 * deletion?
     +		 */
    -+		struct strbuf tag_name = STRBUF_INIT;
     +		struct tag *t, *prev = NULL;
     +		for (t = first_tag; t; t = t->next_tag) {
    -+			strbuf_reset(&tag_name);
    -+			strbuf_addf(&tag_name, "refs/tags/%s", t->name);
    -+			if (!strcmp(b->name, tag_name.buf))
    ++			if (!strcmp(t->name, tag_name))
     +				break;
     +			prev = t;
     +		}
3:  e1fd888e4a = 3:  3b5f4270f8 fast-import: allow tags to be identified by mark labels
4:  93175f28d9 = 4:  489c7fd854 fast-import: add support for new 'alias' command
5:  8c8743395c = 5:  38fd19caee fast-export: add support for --import-marks-if-exists
6:  eebc40df33 = 6:  2017b8d9f9 fast-export: allow user to request tags be marked with --mark-tags
7:  de39f703c6 = 7:  0efdbb81b1 t9350: add tests for tags of things other than a commit
8:  ac739dbb79 = 8:  fe7c27d786 fast-export: handle nested tags
-- 
2.23.0.264.g3b9f7f2fc6
Elijah Newren· Oct 3, 2019, 20:27 UTC · re: Elijah Newren · lore

[PATCH -v3 1/8] fast-export: fix exporting a tag and nothing else

fast-export allows specifying revision ranges, which can be used to export a tag without exporting the commit it tags. fast-export handled this rather poorly: it would emit a "from :0" directive. Since marks start at 1 and increase, this means it refers to an unknown commit and fast-import will choke on the input.

When we are unable to look up a mark for the object being tagged, use a "from $HASH" directive instead to fix this problem.

Note that this is quite similar to the behavior fast-export exhibits with commits and parents when --reference-excluded-parents is passed along with an excluded commit range. For tags of excluded commits we do not require the --reference-excluded-parents flag because we always have to tag something. By contrast, when dealing with commits, pruning a parent is always a viable option, so we need the flag to specify that parent pruning is not wanted. (It is slightly weird that --reference-excluded-parents isn't the default with a separate --prune-excluded-parents flag, but backward compatibility concerns resulted in the current defaults.)

Signed-off-by: Elijah Newren <newren@gmail.com>
---
 builtin/fast-export.c  |  7 ++++++-
 t/t9350-fast-export.sh | 13 +++++++++++++
 2 files changed, 19 insertions(+), 1 deletion(-)
Show changes to 2 files +19 −1

builtin/fast-export.c, t/t9350-fast-export.sh

diff --git a/builtin/fast-export.c b/builtin/fast-export.c
index f541f55d33..5822271c6b 100644
--- a/builtin/fast-export.c
+++ b/builtin/fast-export.c
@@ -860,7 +860,12 @@ static void handle_tag(const char *name, struct tag *tag)
 
 	if (starts_with(name, "refs/tags/"))
 		name += 10;
-	printf("tag %s\nfrom :%d\n", name, tagged_mark);
+	printf("tag %s\n", name);
+	if (tagged_mark)
+		printf("from :%d\n", tagged_mark);
+	else
+		printf("from %s\n", oid_to_hex(&tagged->oid));
+
 	if (show_original_ids)
 		printf("original-oid %s\n", oid_to_hex(&tag->object.oid));
 	printf("%.*s%sdata %d\n%.*s\n",
diff --git a/t/t9350-fast-export.sh b/t/t9350-fast-export.sh
index b4004e05c2..d32ff41859 100755
--- a/t/t9350-fast-export.sh
+++ b/t/t9350-fast-export.sh
@@ -53,6 +53,19 @@ test_expect_success 'fast-export | fast-import' '
 
 '
 
+test_expect_success 'fast-export ^muss^{commit} muss' '
+	git fast-export --tag-of-filtered-object=rewrite ^muss^{commit} muss >actual &&
+	cat >expected <<-EOF &&
+	tag muss
+	from $(git rev-parse --verify muss^{commit})
+	$(git cat-file tag muss | grep tagger)
+	data 9
+	valentin
+
+	EOF
+	test_cmp expected actual
+'
+
 test_expect_success 'fast-export master~2..master' '
 
 	git fast-export master~2..master >actual &&
-- 
2.23.0.264.g3b9f7f2fc6
Elijah Newren· Oct 3, 2019, 20:27 UTC · re: Elijah Newren · lore

[PATCH -v3 2/8] fast-import: fix handling of deleted tags

If our input stream includes a tag which is later deleted, we were not properly deleting it. We did have a step which would delete it, but we left a tag in the tag list noting that it needed to be updated, and the updating of annotated tags occurred AFTER ref deletion. So, when we record that a tag needs to be deleted, also remove it from the list of annotated tags to update.

While this has likely been something that has not happened in practice, it will come up more in order to support nested tags. For nested tags, we either need to give temporary names to the intermediate tags and then delete them, or else we need to use the final name for the intermediate tags. If we use the final name for the intermediate tags, then in order to keep the sanity check that someone doesn't try to update the same tag twice, we need to delete the ref after creating the intermediate tag. So, either way nested tags imply the need to delete temporary inner tag references.

Helped-by: René Scharfe <l.s.r@web.de>
Signed-off-by: Elijah Newren <newren@gmail.com>
---
 fast-import.c          | 27 +++++++++++++++++++++++++++
 t/t9300-fast-import.sh | 13 +++++++++++++
 2 files changed, 40 insertions(+)
Show changes to 2 files +40 −0

fast-import.c, t/t9300-fast-import.sh

diff --git a/fast-import.c b/fast-import.c
index b44d6a467e..caae0819f5 100644
--- a/fast-import.c
+++ b/fast-import.c
@@ -2778,6 +2778,7 @@ static void parse_new_tag(const char *arg)
 static void parse_reset_branch(const char *arg)
 {
 	struct branch *b;
+	const char *tag_name;
 
 	b = lookup_branch(arg);
 	if (b) {
@@ -2793,6 +2794,32 @@ static void parse_reset_branch(const char *arg)
 		b = new_branch(arg);
 	read_next_command();
 	parse_from(b);
+	if (b->delete && skip_prefix(b->name, "refs/tags/", &tag_name)) {
+		/*
+		 * Elsewhere, we call dump_branches() before dump_tags(),
+		 * and dump_branches() will handle ref deletions first, so
+		 * in order to make sure the deletion actually takes effect,
+		 * we need to remove the tag from our list of tags to update.
+		 *
+		 * NEEDSWORK: replace list of tags with hashmap for faster
+		 * deletion?
+		 */
+		struct tag *t, *prev = NULL;
+		for (t = first_tag; t; t = t->next_tag) {
+			if (!strcmp(t->name, tag_name))
+				break;
+			prev = t;
+		}
+		if (t) {
+			if (prev)
+				prev->next_tag = t->next_tag;
+			else
+				first_tag = t->next_tag;
+			if (!t->next_tag)
+				last_tag = prev;
+			/* There is no mem_pool_free(t) function to call. */
+		}
+	}
 	if (command_buf.len > 0)
 		unread_command_buf = 1;
 }
diff --git a/t/t9300-fast-import.sh b/t/t9300-fast-import.sh
index 141b7fa35e..74bc41333b 100755
--- a/t/t9300-fast-import.sh
+++ b/t/t9300-fast-import.sh
@@ -85,6 +85,15 @@ test_expect_success 'A: create pack from stdin' '
 	An annotated tag that annotates a blob.
 	EOF
 
+	tag to-be-deleted
+	from :3
+	data <<EOF
+	Another annotated tag that annotates a blob.
+	EOF
+
+	reset refs/tags/to-be-deleted
+	from 0000000000000000000000000000000000000000
+
 	INPUT_END
 	git fast-import --export-marks=marks.out <input &&
 	git whatchanged master
@@ -157,6 +166,10 @@ test_expect_success 'A: verify tag/series-A-blob' '
 	test_cmp expect actual
 '
 
+test_expect_success 'A: verify tag deletion is successful' '
+	test_must_fail git rev-parse --verify refs/tags/to-be-deleted
+'
+
 test_expect_success 'A: verify marks output' '
 	cat >expect <<-EOF &&
 	:2 $(git rev-parse --verify master:file2)
-- 
2.23.0.264.g3b9f7f2fc6
Elijah Newren· Oct 3, 2019, 20:27 UTC · re: Elijah Newren · lore

[PATCH -v3 3/8] fast-import: allow tags to be identified by mark labels

Mark identifiers are used in fast-export and fast-import to provide a label to refer to earlier content. Blobs are given labels because they need to be referenced in the commits where they first appear with a given filename, and commits are given labels because they can be the parents of other commits. Tags were never given labels, probably because they were viewed as unnecessary, but that presents two problems:

   1. It leaves us without a way of referring to previous tags if we
      want to create a tag of a tag (or higher nestings).
   2. It leaves us with no way of recording that a tag has already been
      imported when using --export-marks and --import-marks.
Fix these problems by allowing an optional mark label for tags.
Signed-off-by: Elijah Newren <newren@gmail.com>
---
 Documentation/git-fast-import.txt |  1 +
 fast-import.c                     |  3 ++-
 t/t9300-fast-import.sh            | 19 +++++++++++++++++++
 3 files changed, 22 insertions(+), 1 deletion(-)
Show changes to 3 files +22 −1

Documentation/git-fast-import.txt, fast-import.c, t/t9300-fast-import.sh

diff --git a/Documentation/git-fast-import.txt b/Documentation/git-fast-import.txt
index 0bb276269e..4977869465 100644
--- a/Documentation/git-fast-import.txt
+++ b/Documentation/git-fast-import.txt
@@ -774,6 +774,7 @@ lightweight (non-annotated) tags see the `reset` command below.
 
 ....
 	'tag' SP <name> LF
+	mark?
 	'from' SP <commit-ish> LF
 	original-oid?
 	'tagger' (SP <name>)? SP LT <email> GT SP <when> LF
diff --git a/fast-import.c b/fast-import.c
index caae0819f5..5b9e9e3b02 100644
--- a/fast-import.c
+++ b/fast-import.c
@@ -2713,6 +2713,7 @@ static void parse_new_tag(const char *arg)
 		first_tag = t;
 	last_tag = t;
 	read_next_command();
+	parse_mark();
 
 	/* from ... */
 	if (!skip_prefix(command_buf.buf, "from ", &from))
@@ -2769,7 +2770,7 @@ static void parse_new_tag(const char *arg)
 	strbuf_addbuf(&new_data, &msg);
 	free(tagger);
 
-	if (store_object(OBJ_TAG, &new_data, NULL, &t->oid, 0))
+	if (store_object(OBJ_TAG, &new_data, NULL, &t->oid, next_mark))
 		t->pack_id = MAX_PACK_ID;
 	else
 		t->pack_id = pack_id;
diff --git a/t/t9300-fast-import.sh b/t/t9300-fast-import.sh
index 74bc41333b..3ad2b2f1ba 100755
--- a/t/t9300-fast-import.sh
+++ b/t/t9300-fast-import.sh
@@ -94,6 +94,23 @@ test_expect_success 'A: create pack from stdin' '
 	reset refs/tags/to-be-deleted
 	from 0000000000000000000000000000000000000000
 
+	tag nested
+	mark :6
+	from :4
+	data <<EOF
+	Tag of our lovely commit
+	EOF
+
+	reset refs/tags/nested
+	from 0000000000000000000000000000000000000000
+
+	tag nested
+	mark :7
+	from :6
+	data <<EOF
+	Tag of tag of our lovely commit
+	EOF
+
 	INPUT_END
 	git fast-import --export-marks=marks.out <input &&
 	git whatchanged master
@@ -176,6 +193,8 @@ test_expect_success 'A: verify marks output' '
 	:3 $(git rev-parse --verify master:file3)
 	:4 $(git rev-parse --verify master:file4)
 	:5 $(git rev-parse --verify master^0)
+	:6 $(git cat-file tag nested | grep object | cut -d" " -f 2)
+	:7 $(git rev-parse --verify nested)
 	EOF
 	test_cmp expect marks.out
 '
-- 
2.23.0.264.g3b9f7f2fc6
Elijah Newren· Oct 3, 2019, 20:27 UTC · re: Elijah Newren · lore

[PATCH -v3 4/8] fast-import: add support for new 'alias' command

fast-export and fast-import have nice --import-marks flags which allow for incremental migrations. However, if there is a mark in fast-export's file of marks without a corresponding mark in the one for fast-import, then we run the risk that fast-export tries to send new objects relative to the mark it knows which fast-import does not, causing fast-import to fail.

This arises in practice when there is a filter of some sort running between the fast-export and fast-import processes which prunes some commits programmatically. Provide such a filter with the ability to alias pruned commits to their most recent non-pruned ancestor.

Signed-off-by: Elijah Newren <newren@gmail.com>
---
 Documentation/git-fast-import.txt | 22 +++++++++++
 fast-import.c                     | 62 ++++++++++++++++++++++++++-----
 t/t9300-fast-import.sh            |  5 +++
 3 files changed, 79 insertions(+), 10 deletions(-)
Show changes to 3 files +79 −10

Documentation/git-fast-import.txt, fast-import.c, t/t9300-fast-import.sh

diff --git a/Documentation/git-fast-import.txt b/Documentation/git-fast-import.txt
index 4977869465..a3f1e0c5e4 100644
--- a/Documentation/git-fast-import.txt
+++ b/Documentation/git-fast-import.txt
@@ -337,6 +337,13 @@ and control the current import process.  More detailed discussion
 	`commit` command.  This command is optional and is not
 	needed to perform an import.
 
+`alias`::
+	Record that a mark refers to a given object without first
+	creating any new object.  Using --import-marks and referring
+	to missing marks will cause fast-import to fail, so aliases
+	can provide a way to set otherwise pruned commits to a valid
+	value (e.g. the nearest non-pruned ancestor).
+
 `checkpoint`::
 	Forces fast-import to close the current packfile, generate its
 	unique SHA-1 checksum and index, and start a new packfile.
@@ -914,6 +921,21 @@ a data chunk which does not have an LF as its last byte.
 +
 The `LF` after `<delim> LF` is optional (it used to be required).
 
+`alias`
+~~~~~~~
+Record that a mark refers to a given object without first creating any
+new object.
+
+....
+	'alias' LF
+	mark
+	'to' SP <commit-ish> LF
+	LF?
+....
+
+For a detailed description of `<commit-ish>` see above under `from`.
+
+
 `checkpoint`
 ~~~~~~~~~~~~
 Forces fast-import to close the current packfile, start a new one, and to
diff --git a/fast-import.c b/fast-import.c
index 5b9e9e3b02..ac368b3e2b 100644
--- a/fast-import.c
+++ b/fast-import.c
@@ -2491,18 +2491,14 @@ static void parse_from_existing(struct branch *b)
 	}
 }
 
-static int parse_from(struct branch *b)
+static int parse_objectish(struct branch *b, const char *objectish)
 {
-	const char *from;
 	struct branch *s;
 	struct object_id oid;
 
-	if (!skip_prefix(command_buf.buf, "from ", &from))
-		return 0;
-
 	oidcpy(&oid, &b->branch_tree.versions[1].oid);
 
-	s = lookup_branch(from);
+	s = lookup_branch(objectish);
 	if (b == s)
 		die("Can't create a branch from itself: %s", b->name);
 	else if (s) {
@@ -2510,8 +2506,8 @@ static int parse_from(struct branch *b)
 		oidcpy(&b->oid, &s->oid);
 		oidcpy(&b->branch_tree.versions[0].oid, t);
 		oidcpy(&b->branch_tree.versions[1].oid, t);
-	} else if (*from == ':') {
-		uintmax_t idnum = parse_mark_ref_eol(from);
+	} else if (*objectish == ':') {
+		uintmax_t idnum = parse_mark_ref_eol(objectish);
 		struct object_entry *oe = find_mark(idnum);
 		if (oe->type != OBJ_COMMIT)
 			die("Mark :%" PRIuMAX " not a commit", idnum);
@@ -2525,13 +2521,13 @@ static int parse_from(struct branch *b)
 			} else
 				parse_from_existing(b);
 		}
-	} else if (!get_oid(from, &b->oid)) {
+	} else if (!get_oid(objectish, &b->oid)) {
 		parse_from_existing(b);
 		if (is_null_oid(&b->oid))
 			b->delete = 1;
 	}
 	else
-		die("Invalid ref name or SHA1 expression: %s", from);
+		die("Invalid ref name or SHA1 expression: %s", objectish);
 
 	if (b->branch_tree.tree && !oideq(&oid, &b->branch_tree.versions[1].oid)) {
 		release_tree_content_recursive(b->branch_tree.tree);
@@ -2542,6 +2538,26 @@ static int parse_from(struct branch *b)
 	return 1;
 }
 
+static int parse_from(struct branch *b)
+{
+	const char *from;
+
+	if (!skip_prefix(command_buf.buf, "from ", &from))
+		return 0;
+
+	return parse_objectish(b, from);
+}
+
+static int parse_objectish_with_prefix(struct branch *b, const char *prefix)
+{
+	const char *base;
+
+	if (!skip_prefix(command_buf.buf, prefix, &base))
+		return 0;
+
+	return parse_objectish(b, base);
+}
+
 static struct hash_list *parse_merge(unsigned int *count)
 {
 	struct hash_list *list = NULL, **tail = &list, *n;
@@ -3087,6 +3103,28 @@ static void parse_progress(void)
 	skip_optional_lf();
 }
 
+static void parse_alias(void)
+{
+	struct object_entry *e;
+	struct branch b;
+
+	skip_optional_lf();
+	read_next_command();
+
+	/* mark ... */
+	parse_mark();
+	if (!next_mark)
+		die(_("Expected 'mark' command, got %s"), command_buf.buf);
+
+	/* to ... */
+	memset(&b, 0, sizeof(b));
+	if (!parse_objectish_with_prefix(&b, "to "))
+		die(_("Expected 'to' command, got %s"), command_buf.buf);
+	e = find_object(&b.oid);
+	assert(e);
+	insert_mark(next_mark, e);
+}
+
 static char* make_fast_import_path(const char *path)
 {
 	if (!relative_marks_paths || is_absolute_path(path))
@@ -3214,6 +3252,8 @@ static int parse_one_feature(const char *feature, int from_stream)
 		option_import_marks(arg, from_stream, 1);
 	} else if (skip_prefix(feature, "export-marks=", &arg)) {
 		option_export_marks(arg);
+	} else if (!strcmp(feature, "alias")) {
+		; /* Don't die - this feature is supported */
 	} else if (!strcmp(feature, "get-mark")) {
 		; /* Don't die - this feature is supported */
 	} else if (!strcmp(feature, "cat-blob")) {
@@ -3370,6 +3410,8 @@ int cmd_main(int argc, const char **argv)
 			parse_checkpoint();
 		else if (!strcmp("done", command_buf.buf))
 			break;
+		else if (!strcmp("alias", command_buf.buf))
+			parse_alias();
 		else if (starts_with(command_buf.buf, "progress "))
 			parse_progress();
 		else if (skip_prefix(command_buf.buf, "feature ", &v))
diff --git a/t/t9300-fast-import.sh b/t/t9300-fast-import.sh
index 3ad2b2f1ba..41f2a1dad9 100755
--- a/t/t9300-fast-import.sh
+++ b/t/t9300-fast-import.sh
@@ -111,6 +111,10 @@ test_expect_success 'A: create pack from stdin' '
 	Tag of tag of our lovely commit
 	EOF
 
+	alias
+	mark :8
+	to :5
+
 	INPUT_END
 	git fast-import --export-marks=marks.out <input &&
 	git whatchanged master
@@ -195,6 +199,7 @@ test_expect_success 'A: verify marks output' '
 	:5 $(git rev-parse --verify master^0)
 	:6 $(git cat-file tag nested | grep object | cut -d" " -f 2)
 	:7 $(git rev-parse --verify nested)
+	:8 $(git rev-parse --verify master^0)
 	EOF
 	test_cmp expect marks.out
 '
-- 
2.23.0.264.g3b9f7f2fc6
Elijah Newren· Oct 3, 2019, 20:27 UTC · re: Elijah Newren · lore

[PATCH -v3 5/8] fast-export: add support for --import-marks-if-exists

fast-import has support for both an --import-marks flag and an --import-marks-if-exists flag; the latter of which will not die() if the file does not exist. fast-export only had support for an --import-marks flag; add an --import-marks-if-exists flag for consistency.

Signed-off-by: Elijah Newren <newren@gmail.com>
---
 builtin/fast-export.c  | 23 +++++++++++++++++++----
 t/t9350-fast-export.sh | 10 ++++------
 2 files changed, 23 insertions(+), 10 deletions(-)
Show changes to 2 files +23 −10

builtin/fast-export.c, t/t9350-fast-export.sh

diff --git a/builtin/fast-export.c b/builtin/fast-export.c
index 5822271c6b..575e47833b 100644
--- a/builtin/fast-export.c
+++ b/builtin/fast-export.c
@@ -1052,11 +1052,16 @@ static void export_marks(char *file)
 		error("Unable to write marks file %s.", file);
 }
 
-static void import_marks(char *input_file)
+static void import_marks(char *input_file, int check_exists)
 {
 	char line[512];
-	FILE *f = xfopen(input_file, "r");
+	FILE *f;
+	struct stat sb;
+
+	if (check_exists && stat(input_file, &sb))
+		return;
 
+	f = xfopen(input_file, "r");
 	while (fgets(line, sizeof(line), f)) {
 		uint32_t mark;
 		char *line_end, *mark_end;
@@ -1120,7 +1125,9 @@ int cmd_fast_export(int argc, const char **argv, const char *prefix)
 	struct rev_info revs;
 	struct object_array commits = OBJECT_ARRAY_INIT;
 	struct commit *commit;
-	char *export_filename = NULL, *import_filename = NULL;
+	char *export_filename = NULL,
+	     *import_filename = NULL,
+	     *import_filename_if_exists = NULL;
 	uint32_t lastimportid;
 	struct string_list refspecs_list = STRING_LIST_INIT_NODUP;
 	struct string_list paths_of_changed_objects = STRING_LIST_INIT_DUP;
@@ -1140,6 +1147,10 @@ int cmd_fast_export(int argc, const char **argv, const char *prefix)
 			     N_("Dump marks to this file")),
 		OPT_STRING(0, "import-marks", &import_filename, N_("file"),
 			     N_("Import marks from this file")),
+		OPT_STRING(0, "import-marks-if-exists",
+			     &import_filename_if_exists,
+			     N_("file"),
+			     N_("Import marks from this file if it exists")),
 		OPT_BOOL(0, "fake-missing-tagger", &fake_missing_tagger,
 			 N_("Fake a tagger when tags lack one")),
 		OPT_BOOL(0, "full-tree", &full_tree,
@@ -1187,8 +1198,12 @@ int cmd_fast_export(int argc, const char **argv, const char *prefix)
 	if (use_done_feature)
 		printf("feature done\n");
 
+	if (import_filename && import_filename_if_exists)
+		die(_("Cannot pass both --import-marks and --import-marks-if-exists"));
 	if (import_filename)
-		import_marks(import_filename);
+		import_marks(import_filename, 0);
+	else if (import_filename_if_exists)
+		import_marks(import_filename_if_exists, 1);
 	lastimportid = last_idnum;
 
 	if (import_filename && revs.prune_data.nr)
diff --git a/t/t9350-fast-export.sh b/t/t9350-fast-export.sh
index d32ff41859..ea84e2f173 100755
--- a/t/t9350-fast-export.sh
+++ b/t/t9350-fast-export.sh
@@ -580,17 +580,15 @@ test_expect_success 'fast-export quotes pathnames' '
 '
 
 test_expect_success 'test bidirectionality' '
-	>marks-cur &&
-	>marks-new &&
 	git init marks-test &&
-	git fast-export --export-marks=marks-cur --import-marks=marks-cur --branches | \
-	git --git-dir=marks-test/.git fast-import --export-marks=marks-new --import-marks=marks-new &&
+	git fast-export --export-marks=marks-cur --import-marks-if-exists=marks-cur --branches | \
+	git --git-dir=marks-test/.git fast-import --export-marks=marks-new --import-marks-if-exists=marks-new &&
 	(cd marks-test &&
 	git reset --hard &&
 	echo Wohlauf > file &&
 	git commit -a -m "back in time") &&
-	git --git-dir=marks-test/.git fast-export --export-marks=marks-new --import-marks=marks-new --branches | \
-	git fast-import --export-marks=marks-cur --import-marks=marks-cur
+	git --git-dir=marks-test/.git fast-export --export-marks=marks-new --import-marks-if-exists=marks-new --branches | \
+	git fast-import --export-marks=marks-cur --import-marks-if-exists=marks-cur
 '
 
 cat > expected << EOF
-- 
2.23.0.264.g3b9f7f2fc6
Elijah Newren· Oct 3, 2019, 20:27 UTC · re: Elijah Newren · lore

[PATCH -v3 7/8] t9350: add tests for tags of things other than a commit

Multiple changes here:
  * add a test for a tag of a blob
  * add a test for a tag of a tag of a commit
  * add a comment to the tests for (possibly nested) tags of trees,
    making it clear that these tests are doing much less than you might
    expect
Signed-off-by: Elijah Newren <newren@gmail.com>
---
 t/t9350-fast-export.sh | 31 +++++++++++++++++++++++++++++++
 1 file changed, 31 insertions(+)
Show changes to t/t9350-fast-export.sh +31 −0
diff --git a/t/t9350-fast-export.sh b/t/t9350-fast-export.sh
index b3fca6ffba..9ab281e4b9 100755
--- a/t/t9350-fast-export.sh
+++ b/t/t9350-fast-export.sh
@@ -540,10 +540,41 @@ test_expect_success 'tree_tag'        '
 '
 
 # NEEDSWORK: not just check return status, but validate the output
+# Note that these tests DO NOTHING other than print a warning that
+# they are ommitting the one tag we asked them to export (because the
+# tags resolve to a tree).  They exist just to make sure we do not
+# abort but instead just warn.
 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'
 
+test_expect_success 'handling tags of blobs' '
+	git tag -a -m "Tag of a blob" blobtag $(git rev-parse master:file) &&
+	git fast-export blobtag >actual &&
+	cat >expect <<-EOF &&
+	blob
+	mark :1
+	data 9
+	die Luft
+
+	tag blobtag
+	from :1
+	tagger $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> $GIT_COMMITTER_DATE
+	data 14
+	Tag of a blob
+
+	EOF
+	test_cmp expect actual
+'
+
+test_expect_failure 'handling nested tags' '
+	git tag -a -m "This is a nested tag" nested muss &&
+	git fast-export --mark-tags nested >output &&
+	grep "^from $ZERO_OID$" output &&
+	grep "^tag nested$" output >tag_lines &&
+	test_line_count = 2 tag_lines
+'
+
 test_expect_success 'directory becomes symlink'        '
 	git init dirtosymlink &&
 	git init result &&
-- 
2.23.0.264.g3b9f7f2fc6
Elijah Newren· Oct 3, 2019, 20:27 UTC · re: Elijah Newren · lore

[PATCH -v3 8/8] fast-export: handle nested tags

Signed-off-by: Elijah Newren <newren@gmail.com>
---
 builtin/fast-export.c  | 30 ++++++++++++++++++------------
 t/t9350-fast-export.sh |  2 +-
 2 files changed, 19 insertions(+), 13 deletions(-)
Show changes to 2 files +19 −13

builtin/fast-export.c, t/t9350-fast-export.sh

diff --git a/builtin/fast-export.c b/builtin/fast-export.c
index d32e1e9327..58a74de42a 100644
--- a/builtin/fast-export.c
+++ b/builtin/fast-export.c
@@ -843,22 +843,28 @@ static void handle_tag(const char *name, struct tag *tag)
 			free(buf);
 			return;
 		case REWRITE:
-			if (tagged->type != OBJ_COMMIT) {
-				die("tag %s tags unexported %s!",
-				    oid_to_hex(&tag->object.oid),
-				    type_name(tagged->type));
-			}
-			p = rewrite_commit((struct commit *)tagged);
-			if (!p) {
-				printf("reset %s\nfrom %s\n\n",
-				       name, oid_to_hex(&null_oid));
-				free(buf);
-				return;
+			if (tagged->type == OBJ_TAG && !mark_tags) {
+				die(_("Error: Cannot export nested tags unless --mark-tags is specified."));
+			} else if (tagged->type == OBJ_COMMIT) {
+				p = rewrite_commit((struct commit *)tagged);
+				if (!p) {
+					printf("reset %s\nfrom %s\n\n",
+					       name, oid_to_hex(&null_oid));
+					free(buf);
+					return;
+				}
+				tagged_mark = get_object_mark(&p->object);
+			} else {
+				/* tagged->type is either OBJ_BLOB or OBJ_TAG */
+				tagged_mark = get_object_mark(tagged);
 			}
-			tagged_mark = get_object_mark(&p->object);
 		}
 	}
 
+	if (tagged->type == OBJ_TAG) {
+		printf("reset %s\nfrom %s\n\n",
+		       name, oid_to_hex(&null_oid));
+	}
 	if (starts_with(name, "refs/tags/"))
 		name += 10;
 	printf("tag %s\n", name);
diff --git a/t/t9350-fast-export.sh b/t/t9350-fast-export.sh
index 9ab281e4b9..2e4e214815 100755
--- a/t/t9350-fast-export.sh
+++ b/t/t9350-fast-export.sh
@@ -567,7 +567,7 @@ test_expect_success 'handling tags of blobs' '
 	test_cmp expect actual
 '
 
-test_expect_failure 'handling nested tags' '
+test_expect_success 'handling nested tags' '
 	git tag -a -m "This is a nested tag" nested muss &&
 	git fast-export --mark-tags nested >output &&
 	grep "^from $ZERO_OID$" output &&
-- 
2.23.0.264.g3b9f7f2fc6
Elijah Newren· Oct 3, 2019, 20:27 UTC · re: Elijah Newren · lore

[PATCH -v3 6/8] fast-export: allow user to request tags be marked with --mark-tags

Add a new option, --mark-tags, which will output mark identifiers with each tag object. This improves the incremental export story with --export-marks since it will allow us to record that annotated tags have been exported, and it is also needed as a step towards supporting nested tags.

Signed-off-by: Elijah Newren <newren@gmail.com>
---
 Documentation/git-fast-export.txt | 17 +++++++++++++----
 builtin/fast-export.c             |  7 +++++++
 t/t9350-fast-export.sh            | 14 ++++++++++++++
 3 files changed, 34 insertions(+), 4 deletions(-)
Show changes to 3 files +34 −4

Documentation/git-fast-export.txt, builtin/fast-export.c, t/t9350-fast-export.sh

diff --git a/Documentation/git-fast-export.txt b/Documentation/git-fast-export.txt
index cc940eb9ad..c522b34f7b 100644
--- a/Documentation/git-fast-export.txt
+++ b/Documentation/git-fast-export.txt
@@ -75,11 +75,20 @@ produced incorrect results if you gave these options.
 	Before processing any input, load the marks specified in
 	<file>.  The input file must exist, must be readable, and
 	must use the same format as produced by --export-marks.
+
+--mark-tags::
+	In addition to labelling blobs and commits with mark ids, also
+	label tags.  This is useful in conjunction with
+	`--export-marks` and `--import-marks`, and is also useful (and
+	necessary) for exporting of nested tags.  It does not hurt
+	other cases and would be the default, but many fast-import
+	frontends are not prepared to accept tags with mark
+	identifiers.
 +
-Any commits that have already been marked will not be exported again.
-If the backend uses a similar --import-marks file, this allows for
-incremental bidirectional exporting of the repository by keeping the
-marks the same across runs.
+Any commits (or tags) that have already been marked will not be
+exported again.  If the backend uses a similar --import-marks file,
+this allows for incremental bidirectional exporting of the repository
+by keeping the marks the same across runs.
 
 --fake-missing-tagger::
 	Some old repositories have tags without a tagger.  The
diff --git a/builtin/fast-export.c b/builtin/fast-export.c
index 575e47833b..d32e1e9327 100644
--- a/builtin/fast-export.c
+++ b/builtin/fast-export.c
@@ -40,6 +40,7 @@ static int no_data;
 static int full_tree;
 static int reference_excluded_commits;
 static int show_original_ids;
+static int mark_tags;
 static struct string_list extra_refs = STRING_LIST_INIT_NODUP;
 static struct string_list tag_refs = STRING_LIST_INIT_NODUP;
 static struct refspec refspecs = REFSPEC_INIT_FETCH;
@@ -861,6 +862,10 @@ static void handle_tag(const char *name, struct tag *tag)
 	if (starts_with(name, "refs/tags/"))
 		name += 10;
 	printf("tag %s\n", name);
+	if (mark_tags) {
+		mark_next_object(&tag->object);
+		printf("mark :%"PRIu32"\n", last_idnum);
+	}
 	if (tagged_mark)
 		printf("from :%d\n", tagged_mark);
 	else
@@ -1165,6 +1170,8 @@ int cmd_fast_export(int argc, const char **argv, const char *prefix)
 			 &reference_excluded_commits, N_("Reference parents which are not in fast-export stream by object id")),
 		OPT_BOOL(0, "show-original-ids", &show_original_ids,
 			    N_("Show original object ids of blobs/commits")),
+		OPT_BOOL(0, "mark-tags", &mark_tags,
+			    N_("Label tags with mark ids")),
 
 		OPT_END()
 	};
diff --git a/t/t9350-fast-export.sh b/t/t9350-fast-export.sh
index ea84e2f173..b3fca6ffba 100755
--- a/t/t9350-fast-export.sh
+++ b/t/t9350-fast-export.sh
@@ -66,6 +66,20 @@ test_expect_success 'fast-export ^muss^{commit} muss' '
 	test_cmp expected actual
 '
 
+test_expect_success 'fast-export --mark-tags ^muss^{commit} muss' '
+	git fast-export --mark-tags --tag-of-filtered-object=rewrite ^muss^{commit} muss >actual &&
+	cat >expected <<-EOF &&
+	tag muss
+	mark :1
+	from $(git rev-parse --verify muss^{commit})
+	$(git cat-file tag muss | grep tagger)
+	data 9
+	valentin
+
+	EOF
+	test_cmp expected actual
+'
+
 test_expect_success 'fast-export master~2..master' '
 
 	git fast-export master~2..master >actual &&
-- 
2.23.0.264.g3b9f7f2fc6
Junio C Hamano· Oct 4, 2019, 05:51 UTC · re: Elijah Newren · lore

Re: [PATCH -v3 0/8] fast export/import: handle nested tags, improve incremental exports

Elijah Newren <newren@gmail.com> writes:
Show 18 quoted lines
> This series improves the incremental export story for fast-export and
> fast-import (--export-marks and --import-marks fell a bit short),
> fixes a couple small export/import bugs, and enables handling nested
> tags.  In particular, the nested tags handling makes it so that
> fast-export and fast-import can finally handle the git.git repo.
>
> Changes since v2 (full range-diff below):
>   - Code cleanup of patch 2 suggested by René
>
> Elijah Newren (8):
>   fast-export: fix exporting a tag and nothing else
>   fast-import: fix handling of deleted tags
>   fast-import: allow tags to be identified by mark labels
>   fast-import: add support for new 'alias' command
>   fast-export: add support for --import-marks-if-exists
>   fast-export: allow user to request tags be marked with --mark-tags
>   t9350: add tests for tags of things other than a commit
>   fast-export: handle nested tags
Thanks, both.  Replaced.

← back to recent threads