git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
* [PATCH 0/8] fast export/import: handle nested tags, improve incremental exports
@ 2019-09-25  1:39 Elijah Newren
  2019-09-25  1:39 ` [PATCH 1/8] fast-export: fix exporting a tag and nothing else Elijah Newren
                   ` (8 more replies)
  0 siblings, 9 replies; 33+ messages in thread
From: Elijah Newren @ 2019-09-25  1:39 UTC (permalink / raw)
  To: git; +Cc: gitster, Elijah Newren

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

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

* [PATCH 1/8] fast-export: fix exporting a tag and nothing else
  2019-09-25  1:39 [PATCH 0/8] fast export/import: handle nested tags, improve incremental exports Elijah Newren
@ 2019-09-25  1:39 ` Elijah Newren
  2019-09-25  1:39 ` [PATCH 2/8] fast-import: fix handling of deleted tags Elijah Newren
                   ` (7 subsequent siblings)
  8 siblings, 0 replies; 33+ messages in thread
From: Elijah Newren @ 2019-09-25  1:39 UTC (permalink / raw)
  To: git; +Cc: gitster, Elijah Newren

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(-)

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


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

* [PATCH 2/8] fast-import: fix handling of deleted tags
  2019-09-25  1:39 [PATCH 0/8] fast export/import: handle nested tags, improve incremental exports Elijah Newren
  2019-09-25  1:39 ` [PATCH 1/8] fast-export: fix exporting a tag and nothing else Elijah Newren
@ 2019-09-25  1:39 ` Elijah Newren
  2019-09-25  1:40 ` [PATCH 3/8] fast-import: allow tags to be identified by mark labels Elijah Newren
                   ` (6 subsequent siblings)
  8 siblings, 0 replies; 33+ messages in thread
From: Elijah Newren @ 2019-09-25  1:39 UTC (permalink / raw)
  To: git; +Cc: gitster, Elijah Newren

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..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


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

* [PATCH 3/8] fast-import: allow tags to be identified by mark labels
  2019-09-25  1:39 [PATCH 0/8] fast export/import: handle nested tags, improve incremental exports Elijah Newren
  2019-09-25  1:39 ` [PATCH 1/8] fast-export: fix exporting a tag and nothing else Elijah Newren
  2019-09-25  1:39 ` [PATCH 2/8] fast-import: fix handling of deleted tags Elijah Newren
@ 2019-09-25  1:40 ` Elijah Newren
  2019-09-25  1:40 ` [PATCH 4/8] fast-import: add support for new 'alias' command Elijah Newren
                   ` (5 subsequent siblings)
  8 siblings, 0 replies; 33+ messages in thread
From: Elijah Newren @ 2019-09-25  1:40 UTC (permalink / raw)
  To: git; +Cc: gitster, Elijah Newren

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(-)

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


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

* [PATCH 4/8] fast-import: add support for new 'alias' command
  2019-09-25  1:39 [PATCH 0/8] fast export/import: handle nested tags, improve incremental exports Elijah Newren
                   ` (2 preceding siblings ...)
  2019-09-25  1:40 ` [PATCH 3/8] fast-import: allow tags to be identified by mark labels Elijah Newren
@ 2019-09-25  1:40 ` Elijah Newren
  2019-09-25  1:40 ` [PATCH 5/8] fast-export: add support for --import-marks-if-exists Elijah Newren
                   ` (4 subsequent siblings)
  8 siblings, 0 replies; 33+ messages in thread
From: Elijah Newren @ 2019-09-25  1:40 UTC (permalink / raw)
  To: git; +Cc: gitster, Elijah Newren

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(-)

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


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

* [PATCH 5/8] fast-export: add support for --import-marks-if-exists
  2019-09-25  1:39 [PATCH 0/8] fast export/import: handle nested tags, improve incremental exports Elijah Newren
                   ` (3 preceding siblings ...)
  2019-09-25  1:40 ` [PATCH 4/8] fast-import: add support for new 'alias' command Elijah Newren
@ 2019-09-25  1:40 ` Elijah Newren
  2019-09-25  1:40 ` [PATCH 6/8] fast-export: allow user to request tags be marked with --mark-tags Elijah Newren
                   ` (3 subsequent siblings)
  8 siblings, 0 replies; 33+ messages in thread
From: Elijah Newren @ 2019-09-25  1:40 UTC (permalink / raw)
  To: git; +Cc: gitster, Elijah Newren

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(-)

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


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

* [PATCH 6/8] fast-export: allow user to request tags be marked with --mark-tags
  2019-09-25  1:39 [PATCH 0/8] fast export/import: handle nested tags, improve incremental exports Elijah Newren
                   ` (4 preceding siblings ...)
  2019-09-25  1:40 ` [PATCH 5/8] fast-export: add support for --import-marks-if-exists Elijah Newren
@ 2019-09-25  1:40 ` Elijah Newren
  2019-09-25  1:40 ` [PATCH 7/8] t9350: add tests for tags of things other than a commit Elijah Newren
                   ` (2 subsequent siblings)
  8 siblings, 0 replies; 33+ messages in thread
From: Elijah Newren @ 2019-09-25  1:40 UTC (permalink / raw)
  To: git; +Cc: gitster, Elijah Newren

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(-)

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


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

* [PATCH 7/8] t9350: add tests for tags of things other than a commit
  2019-09-25  1:39 [PATCH 0/8] fast export/import: handle nested tags, improve incremental exports Elijah Newren
                   ` (5 preceding siblings ...)
  2019-09-25  1:40 ` [PATCH 6/8] fast-export: allow user to request tags be marked with --mark-tags Elijah Newren
@ 2019-09-25  1:40 ` Elijah Newren
  2019-09-25  1:40 ` [PATCH 8/8] fast-export: handle nested tags Elijah Newren
  2019-09-30 21:10 ` [PATCH v2 0/8] fast export/import: handle nested tags, improve incremental exports Elijah Newren
  8 siblings, 0 replies; 33+ messages in thread
From: Elijah Newren @ 2019-09-25  1:40 UTC (permalink / raw)
  To: git; +Cc: gitster, Elijah Newren

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(+)

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


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

* [PATCH 8/8] fast-export: handle nested tags
  2019-09-25  1:39 [PATCH 0/8] fast export/import: handle nested tags, improve incremental exports Elijah Newren
                   ` (6 preceding siblings ...)
  2019-09-25  1:40 ` [PATCH 7/8] t9350: add tests for tags of things other than a commit Elijah Newren
@ 2019-09-25  1:40 ` Elijah Newren
  2019-09-30 21:10 ` [PATCH v2 0/8] fast export/import: handle nested tags, improve incremental exports Elijah Newren
  8 siblings, 0 replies; 33+ messages in thread
From: Elijah Newren @ 2019-09-25  1:40 UTC (permalink / raw)
  To: git; +Cc: gitster, Elijah Newren

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(-)

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


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

* [PATCH v2 0/8] fast export/import: handle nested tags, improve incremental exports
  2019-09-25  1:39 [PATCH 0/8] fast export/import: handle nested tags, improve incremental exports Elijah Newren
                   ` (7 preceding siblings ...)
  2019-09-25  1:40 ` [PATCH 8/8] fast-export: handle nested tags Elijah Newren
@ 2019-09-30 21:10 ` Elijah Newren
  2019-09-30 21:10   ` [PATCH v2 1/8] fast-export: fix exporting a tag and nothing else Elijah Newren
                     ` (9 more replies)
  8 siblings, 10 replies; 33+ messages in thread
From: Elijah Newren @ 2019-09-30 21:10 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Elijah Newren

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


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

* [PATCH v2 1/8] fast-export: fix exporting a tag and nothing else
  2019-09-30 21:10 ` [PATCH v2 0/8] fast export/import: handle nested tags, improve incremental exports Elijah Newren
@ 2019-09-30 21:10   ` Elijah Newren
  2019-09-30 21:10   ` [PATCH v2 2/8] fast-import: fix handling of deleted tags Elijah Newren
                     ` (8 subsequent siblings)
  9 siblings, 0 replies; 33+ messages in thread
From: Elijah Newren @ 2019-09-30 21:10 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Elijah Newren

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(-)

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


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

* [PATCH v2 2/8] fast-import: fix handling of deleted tags
  2019-09-30 21:10 ` [PATCH v2 0/8] fast export/import: handle nested tags, improve incremental exports Elijah Newren
  2019-09-30 21:10   ` [PATCH v2 1/8] fast-export: fix exporting a tag and nothing else Elijah Newren
@ 2019-09-30 21:10   ` Elijah Newren
  2019-10-03 11:53     ` René Scharfe
  2019-09-30 21:10   ` [PATCH v2 3/8] fast-import: allow tags to be identified by mark labels Elijah Newren
                     ` (7 subsequent siblings)
  9 siblings, 1 reply; 33+ messages in thread
From: Elijah Newren @ 2019-09-30 21:10 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Elijah Newren

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)) {
+		/*
+		 * 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


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

* [PATCH v2 3/8] fast-import: allow tags to be identified by mark labels
  2019-09-30 21:10 ` [PATCH v2 0/8] fast export/import: handle nested tags, improve incremental exports Elijah Newren
  2019-09-30 21:10   ` [PATCH v2 1/8] fast-export: fix exporting a tag and nothing else Elijah Newren
  2019-09-30 21:10   ` [PATCH v2 2/8] fast-import: fix handling of deleted tags Elijah Newren
@ 2019-09-30 21:10   ` Elijah Newren
  2019-09-30 21:10   ` [PATCH v2 4/8] fast-import: add support for new 'alias' command Elijah Newren
                     ` (6 subsequent siblings)
  9 siblings, 0 replies; 33+ messages in thread
From: Elijah Newren @ 2019-09-30 21:10 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Elijah Newren

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(-)

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


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

* [PATCH v2 4/8] fast-import: add support for new 'alias' command
  2019-09-30 21:10 ` [PATCH v2 0/8] fast export/import: handle nested tags, improve incremental exports Elijah Newren
                     ` (2 preceding siblings ...)
  2019-09-30 21:10   ` [PATCH v2 3/8] fast-import: allow tags to be identified by mark labels Elijah Newren
@ 2019-09-30 21:10   ` Elijah Newren
  2019-09-30 21:10   ` [PATCH v2 5/8] fast-export: add support for --import-marks-if-exists Elijah Newren
                     ` (5 subsequent siblings)
  9 siblings, 0 replies; 33+ messages in thread
From: Elijah Newren @ 2019-09-30 21:10 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Elijah Newren

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(-)

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


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

* [PATCH v2 5/8] fast-export: add support for --import-marks-if-exists
  2019-09-30 21:10 ` [PATCH v2 0/8] fast export/import: handle nested tags, improve incremental exports Elijah Newren
                     ` (3 preceding siblings ...)
  2019-09-30 21:10   ` [PATCH v2 4/8] fast-import: add support for new 'alias' command Elijah Newren
@ 2019-09-30 21:10   ` Elijah Newren
  2019-09-30 21:10   ` [PATCH v2 6/8] fast-export: allow user to request tags be marked with --mark-tags Elijah Newren
                     ` (4 subsequent siblings)
  9 siblings, 0 replies; 33+ messages in thread
From: Elijah Newren @ 2019-09-30 21:10 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Elijah Newren

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(-)

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


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

* [PATCH v2 6/8] fast-export: allow user to request tags be marked with --mark-tags
  2019-09-30 21:10 ` [PATCH v2 0/8] fast export/import: handle nested tags, improve incremental exports Elijah Newren
                     ` (4 preceding siblings ...)
  2019-09-30 21:10   ` [PATCH v2 5/8] fast-export: add support for --import-marks-if-exists Elijah Newren
@ 2019-09-30 21:10   ` Elijah Newren
  2019-09-30 21:10   ` [PATCH v2 7/8] t9350: add tests for tags of things other than a commit Elijah Newren
                     ` (3 subsequent siblings)
  9 siblings, 0 replies; 33+ messages in thread
From: Elijah Newren @ 2019-09-30 21:10 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Elijah Newren

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(-)

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


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

* [PATCH v2 7/8] t9350: add tests for tags of things other than a commit
  2019-09-30 21:10 ` [PATCH v2 0/8] fast export/import: handle nested tags, improve incremental exports Elijah Newren
                     ` (5 preceding siblings ...)
  2019-09-30 21:10   ` [PATCH v2 6/8] fast-export: allow user to request tags be marked with --mark-tags Elijah Newren
@ 2019-09-30 21:10   ` Elijah Newren
  2019-09-30 21:10   ` [PATCH v2 8/8] fast-export: handle nested tags Elijah Newren
                     ` (2 subsequent siblings)
  9 siblings, 0 replies; 33+ messages in thread
From: Elijah Newren @ 2019-09-30 21:10 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Elijah Newren

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(+)

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


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

* [PATCH v2 8/8] fast-export: handle nested tags
  2019-09-30 21:10 ` [PATCH v2 0/8] fast export/import: handle nested tags, improve incremental exports Elijah Newren
                     ` (6 preceding siblings ...)
  2019-09-30 21:10   ` [PATCH v2 7/8] t9350: add tests for tags of things other than a commit Elijah Newren
@ 2019-09-30 21:10   ` Elijah Newren
  2019-10-02 15:54   ` [PATCH v2 0/8] fast export/import: handle nested tags, improve incremental exports Elijah Newren
  2019-10-03 20:27   ` [PATCH -v3 " Elijah Newren
  9 siblings, 0 replies; 33+ messages in thread
From: Elijah Newren @ 2019-09-30 21:10 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Elijah Newren

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(-)

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


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

* Re: [PATCH v2 0/8] fast export/import: handle nested tags, improve incremental exports
  2019-09-30 21:10 ` [PATCH v2 0/8] fast export/import: handle nested tags, improve incremental exports Elijah Newren
                     ` (7 preceding siblings ...)
  2019-09-30 21:10   ` [PATCH v2 8/8] fast-export: handle nested tags Elijah Newren
@ 2019-10-02 15:54   ` Elijah Newren
  2019-10-02 20:10     ` Junio C Hamano
  2019-10-03 20:27   ` [PATCH -v3 " Elijah Newren
  9 siblings, 1 reply; 33+ messages in thread
From: Elijah Newren @ 2019-10-02 15:54 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Git Mailing List

Hi Junio,

On Mon, Sep 30, 2019 at 2:10 PM Elijah Newren <newren@gmail.com> wrote:
>
> 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

>
> 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
>

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

* Re: [PATCH v2 0/8] fast export/import: handle nested tags, improve incremental exports
  2019-10-02 15:54   ` [PATCH v2 0/8] fast export/import: handle nested tags, improve incremental exports Elijah Newren
@ 2019-10-02 20:10     ` Junio C Hamano
  2019-10-02 21:05       ` Elijah Newren
  0 siblings, 1 reply; 33+ messages in thread
From: Junio C Hamano @ 2019-10-02 20:10 UTC (permalink / raw)
  To: Elijah Newren; +Cc: Git Mailing List

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?


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

* Re: [PATCH v2 0/8] fast export/import: handle nested tags, improve incremental exports
  2019-10-02 20:10     ` Junio C Hamano
@ 2019-10-02 21:05       ` Elijah Newren
  2019-10-03  1:07         ` Junio C Hamano
  0 siblings, 1 reply; 33+ messages in thread
From: Elijah Newren @ 2019-10-02 21:05 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Git Mailing List

On Wed, Oct 2, 2019 at 1:10 PM Junio C Hamano <gitster@pobox.com> wrote:
>
> 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

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

* Re: [PATCH v2 0/8] fast export/import: handle nested tags, improve incremental exports
  2019-10-02 21:05       ` Elijah Newren
@ 2019-10-03  1:07         ` Junio C Hamano
  0 siblings, 0 replies; 33+ messages in thread
From: Junio C Hamano @ 2019-10-03  1:07 UTC (permalink / raw)
  To: Elijah Newren; +Cc: Git Mailing List

Elijah Newren <newren@gmail.com> writes:

>> 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-<.


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

* Re: [PATCH v2 2/8] fast-import: fix handling of deleted tags
  2019-09-30 21:10   ` [PATCH v2 2/8] fast-import: fix handling of deleted tags Elijah Newren
@ 2019-10-03 11:53     ` René Scharfe
  0 siblings, 0 replies; 33+ messages in thread
From: René Scharfe @ 2019-10-03 11:53 UTC (permalink / raw)
  To: Elijah Newren, git; +Cc: Junio C Hamano

Am 30.09.19 um 23:10 schrieb Elijah Newren:
> 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.

> +		/*
> +		 * 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.

> +		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.

> +				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(-)

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

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

* [PATCH -v3 0/8] fast export/import: handle nested tags, improve incremental exports
  2019-09-30 21:10 ` [PATCH v2 0/8] fast export/import: handle nested tags, improve incremental exports Elijah Newren
                     ` (8 preceding siblings ...)
  2019-10-02 15:54   ` [PATCH v2 0/8] fast export/import: handle nested tags, improve incremental exports Elijah Newren
@ 2019-10-03 20:27   ` Elijah Newren
  2019-10-03 20:27     ` [PATCH -v3 1/8] fast-export: fix exporting a tag and nothing else Elijah Newren
                       ` (8 more replies)
  9 siblings, 9 replies; 33+ messages in thread
From: Elijah Newren @ 2019-10-03 20:27 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, René Scharfe, Elijah Newren

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


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

* [PATCH -v3 1/8] fast-export: fix exporting a tag and nothing else
  2019-10-03 20:27   ` [PATCH -v3 " Elijah Newren
@ 2019-10-03 20:27     ` Elijah Newren
  2019-10-03 20:27     ` [PATCH -v3 2/8] fast-import: fix handling of deleted tags Elijah Newren
                       ` (7 subsequent siblings)
  8 siblings, 0 replies; 33+ messages in thread
From: Elijah Newren @ 2019-10-03 20:27 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, René Scharfe, Elijah Newren

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(-)

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


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

* [PATCH -v3 2/8] fast-import: fix handling of deleted tags
  2019-10-03 20:27   ` [PATCH -v3 " Elijah Newren
  2019-10-03 20:27     ` [PATCH -v3 1/8] fast-export: fix exporting a tag and nothing else Elijah Newren
@ 2019-10-03 20:27     ` Elijah Newren
  2019-10-03 20:27     ` [PATCH -v3 3/8] fast-import: allow tags to be identified by mark labels Elijah Newren
                       ` (6 subsequent siblings)
  8 siblings, 0 replies; 33+ messages in thread
From: Elijah Newren @ 2019-10-03 20:27 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, René Scharfe, Elijah Newren

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(+)

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


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

* [PATCH -v3 3/8] fast-import: allow tags to be identified by mark labels
  2019-10-03 20:27   ` [PATCH -v3 " Elijah Newren
  2019-10-03 20:27     ` [PATCH -v3 1/8] fast-export: fix exporting a tag and nothing else Elijah Newren
  2019-10-03 20:27     ` [PATCH -v3 2/8] fast-import: fix handling of deleted tags Elijah Newren
@ 2019-10-03 20:27     ` Elijah Newren
  2019-10-03 20:27     ` [PATCH -v3 4/8] fast-import: add support for new 'alias' command Elijah Newren
                       ` (5 subsequent siblings)
  8 siblings, 0 replies; 33+ messages in thread
From: Elijah Newren @ 2019-10-03 20:27 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, René Scharfe, Elijah Newren

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(-)

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


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

* [PATCH -v3 4/8] fast-import: add support for new 'alias' command
  2019-10-03 20:27   ` [PATCH -v3 " Elijah Newren
                       ` (2 preceding siblings ...)
  2019-10-03 20:27     ` [PATCH -v3 3/8] fast-import: allow tags to be identified by mark labels Elijah Newren
@ 2019-10-03 20:27     ` Elijah Newren
  2019-10-03 20:27     ` [PATCH -v3 5/8] fast-export: add support for --import-marks-if-exists Elijah Newren
                       ` (4 subsequent siblings)
  8 siblings, 0 replies; 33+ messages in thread
From: Elijah Newren @ 2019-10-03 20:27 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, René Scharfe, Elijah Newren

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(-)

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


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

* [PATCH -v3 5/8] fast-export: add support for --import-marks-if-exists
  2019-10-03 20:27   ` [PATCH -v3 " Elijah Newren
                       ` (3 preceding siblings ...)
  2019-10-03 20:27     ` [PATCH -v3 4/8] fast-import: add support for new 'alias' command Elijah Newren
@ 2019-10-03 20:27     ` Elijah Newren
  2019-10-03 20:27     ` [PATCH -v3 6/8] fast-export: allow user to request tags be marked with --mark-tags Elijah Newren
                       ` (3 subsequent siblings)
  8 siblings, 0 replies; 33+ messages in thread
From: Elijah Newren @ 2019-10-03 20:27 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, René Scharfe, Elijah Newren

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(-)

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


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

* [PATCH -v3 6/8] fast-export: allow user to request tags be marked with --mark-tags
  2019-10-03 20:27   ` [PATCH -v3 " Elijah Newren
                       ` (4 preceding siblings ...)
  2019-10-03 20:27     ` [PATCH -v3 5/8] fast-export: add support for --import-marks-if-exists Elijah Newren
@ 2019-10-03 20:27     ` Elijah Newren
  2019-10-03 20:27     ` [PATCH -v3 7/8] t9350: add tests for tags of things other than a commit Elijah Newren
                       ` (2 subsequent siblings)
  8 siblings, 0 replies; 33+ messages in thread
From: Elijah Newren @ 2019-10-03 20:27 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, René Scharfe, Elijah Newren

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(-)

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


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

* [PATCH -v3 7/8] t9350: add tests for tags of things other than a commit
  2019-10-03 20:27   ` [PATCH -v3 " Elijah Newren
                       ` (5 preceding siblings ...)
  2019-10-03 20:27     ` [PATCH -v3 6/8] fast-export: allow user to request tags be marked with --mark-tags Elijah Newren
@ 2019-10-03 20:27     ` Elijah Newren
  2019-10-03 20:27     ` [PATCH -v3 8/8] fast-export: handle nested tags Elijah Newren
  2019-10-04  5:51     ` [PATCH -v3 0/8] fast export/import: handle nested tags, improve incremental exports Junio C Hamano
  8 siblings, 0 replies; 33+ messages in thread
From: Elijah Newren @ 2019-10-03 20:27 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, René Scharfe, Elijah Newren

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(+)

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


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

* [PATCH -v3 8/8] fast-export: handle nested tags
  2019-10-03 20:27   ` [PATCH -v3 " Elijah Newren
                       ` (6 preceding siblings ...)
  2019-10-03 20:27     ` [PATCH -v3 7/8] t9350: add tests for tags of things other than a commit Elijah Newren
@ 2019-10-03 20:27     ` Elijah Newren
  2019-10-04  5:51     ` [PATCH -v3 0/8] fast export/import: handle nested tags, improve incremental exports Junio C Hamano
  8 siblings, 0 replies; 33+ messages in thread
From: Elijah Newren @ 2019-10-03 20:27 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, René Scharfe, Elijah Newren

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(-)

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


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

* Re: [PATCH -v3 0/8] fast export/import: handle nested tags, improve incremental exports
  2019-10-03 20:27   ` [PATCH -v3 " Elijah Newren
                       ` (7 preceding siblings ...)
  2019-10-03 20:27     ` [PATCH -v3 8/8] fast-export: handle nested tags Elijah Newren
@ 2019-10-04  5:51     ` Junio C Hamano
  8 siblings, 0 replies; 33+ messages in thread
From: Junio C Hamano @ 2019-10-04  5:51 UTC (permalink / raw)
  To: Elijah Newren; +Cc: git, René Scharfe

Elijah Newren <newren@gmail.com> writes:

> 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.

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

end of thread, other threads:[~2019-10-04  5:52 UTC | newest]

Thread overview: 33+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-09-25  1:39 [PATCH 0/8] fast export/import: handle nested tags, improve incremental exports Elijah Newren
2019-09-25  1:39 ` [PATCH 1/8] fast-export: fix exporting a tag and nothing else Elijah Newren
2019-09-25  1:39 ` [PATCH 2/8] fast-import: fix handling of deleted tags Elijah Newren
2019-09-25  1:40 ` [PATCH 3/8] fast-import: allow tags to be identified by mark labels Elijah Newren
2019-09-25  1:40 ` [PATCH 4/8] fast-import: add support for new 'alias' command Elijah Newren
2019-09-25  1:40 ` [PATCH 5/8] fast-export: add support for --import-marks-if-exists Elijah Newren
2019-09-25  1:40 ` [PATCH 6/8] fast-export: allow user to request tags be marked with --mark-tags Elijah Newren
2019-09-25  1:40 ` [PATCH 7/8] t9350: add tests for tags of things other than a commit Elijah Newren
2019-09-25  1:40 ` [PATCH 8/8] fast-export: handle nested tags Elijah Newren
2019-09-30 21:10 ` [PATCH v2 0/8] fast export/import: handle nested tags, improve incremental exports Elijah Newren
2019-09-30 21:10   ` [PATCH v2 1/8] fast-export: fix exporting a tag and nothing else Elijah Newren
2019-09-30 21:10   ` [PATCH v2 2/8] fast-import: fix handling of deleted tags Elijah Newren
2019-10-03 11:53     ` René Scharfe
2019-09-30 21:10   ` [PATCH v2 3/8] fast-import: allow tags to be identified by mark labels Elijah Newren
2019-09-30 21:10   ` [PATCH v2 4/8] fast-import: add support for new 'alias' command Elijah Newren
2019-09-30 21:10   ` [PATCH v2 5/8] fast-export: add support for --import-marks-if-exists Elijah Newren
2019-09-30 21:10   ` [PATCH v2 6/8] fast-export: allow user to request tags be marked with --mark-tags Elijah Newren
2019-09-30 21:10   ` [PATCH v2 7/8] t9350: add tests for tags of things other than a commit Elijah Newren
2019-09-30 21:10   ` [PATCH v2 8/8] fast-export: handle nested tags Elijah Newren
2019-10-02 15:54   ` [PATCH v2 0/8] fast export/import: handle nested tags, improve incremental exports Elijah Newren
2019-10-02 20:10     ` Junio C Hamano
2019-10-02 21:05       ` Elijah Newren
2019-10-03  1:07         ` Junio C Hamano
2019-10-03 20:27   ` [PATCH -v3 " Elijah Newren
2019-10-03 20:27     ` [PATCH -v3 1/8] fast-export: fix exporting a tag and nothing else Elijah Newren
2019-10-03 20:27     ` [PATCH -v3 2/8] fast-import: fix handling of deleted tags Elijah Newren
2019-10-03 20:27     ` [PATCH -v3 3/8] fast-import: allow tags to be identified by mark labels Elijah Newren
2019-10-03 20:27     ` [PATCH -v3 4/8] fast-import: add support for new 'alias' command Elijah Newren
2019-10-03 20:27     ` [PATCH -v3 5/8] fast-export: add support for --import-marks-if-exists Elijah Newren
2019-10-03 20:27     ` [PATCH -v3 6/8] fast-export: allow user to request tags be marked with --mark-tags Elijah Newren
2019-10-03 20:27     ` [PATCH -v3 7/8] t9350: add tests for tags of things other than a commit Elijah Newren
2019-10-03 20:27     ` [PATCH -v3 8/8] fast-export: handle nested tags Elijah Newren
2019-10-04  5:51     ` [PATCH -v3 0/8] fast export/import: handle nested tags, improve incremental exports Junio C Hamano

Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).