Subject: Re: Cannot push anything via export transport helper after push fails.

From: Andrey Borzenkov <[email-masked]>
Date: Thu, 16 May 2013 21:38:56 +0400

В Sat, 11 May 2013 08:57:14 -0500
Felipe Contreras <[email-masked]> пишет:

> On Sat, May 11, 2013 at 7:29 AM, Andrey Borzenkov <[email-masked]> wrote:
> > I noticed that using git-remote-bzr, but as far as I can tell this is
> > generic for all transport helpers using fast-export.
> >
> >
> >
> > What happened was "git push" failed due to merge conflict. So far so
> > good - but from now on git assumes everything is up to date.
> >
> > bor@opensuse:/tmp/test/git> git push origin master
> > To bzr::bzr+ssh://bor@localhost/tmp/test/bzr
> >  ! [rejected]        master -> master (non-fast-forward)
> > error: failed to push some refs to 'bzr::bzr+ssh://bor@localhost/tmp/test/bzr'
> > hint: Updates were rejected because the tip of your current branch is behind
> > hint: its remote counterpart. Merge the remote changes (e.g. 'git pull')
> > hint: before pushing again.
> > hint: See the 'Note about fast-forwards' in 'git push --help' for details.
> > bor@opensuse:/tmp/test/git> git push origin master
> > Everything up-to-date
> > bor@opensuse:/tmp/test/git>
> >
> > The problem seems to be that git fast-export updates marks
> > unconditionally, whether export actually applied or not. So next time
> > it assumes everything is already exported and does nothing.
> >
> > Is it expected behavior?
> 
> Indeed, this is the way it currently works, and it's not easy to fix.
> We would need some way to make fast-export wait until we know the exit
> status of the remote helper, and then tell it when it failed, so the
> marks are not updated.
> 

Hmm ... actually as far as I understand transport-helper keeps track of
which revisions to push in "remote helper ref" (for the lack of better
word). This makes use of marks as tracking means rather redundant.

What about the idea below? This relies on transport helper to provide
correct revisions and uses marks exclusively as cross-reference between
GIT and remote SCM. It is on top of next branch.

---
 builtin/fast-export.c                 | 15 ++++++++++-----
 contrib/remote-helpers/git-remote-bzr | 11 ++++++++---
 transport-helper.c                    |  7 ++++++-
 3 files changed, 24 insertions(+), 9 deletions(-)

diff --git a/builtin/fast-export.c b/builtin/fast-export.c
index d60d675..5bc4b3c 100644
--- a/builtin/fast-export.c
+++ b/builtin/fast-export.c
@@ -29,6 +29,7 @@ static enum { ERROR, DROP, REWRITE } tag_of_filtered_mode = ERROR;
 static int fake_missing_tagger;
 static int use_done_feature;
 static int no_data;
+static int do_not_skip_marked_commits;
 static int full_tree;
 
 static int parse_opt_signed_tag_mode(const struct option *opt,
@@ -95,7 +96,8 @@ static inline void mark_object(struct object *object, uint32_t mark)
 
 static inline void mark_next_object(struct object *object)
 {
-	mark_object(object, ++last_idnum);
+	if (!(do_not_skip_marked_commits && lookup_decoration(&idnums, object)))
+		mark_object(object, ++last_idnum);
 }
 
 static int get_object_mark(struct object *object)
@@ -144,7 +146,7 @@ static void export_blob(const unsigned char *sha1)
 
 	mark_next_object(object);
 
-	printf("blob\nmark :%"PRIu32"\ndata %lu\n", last_idnum, size);
+	printf("blob\nmark :%"PRIu32"\ndata %lu\n", get_object_mark(object), size);
 	if (size && fwrite(buf, size, 1, stdout) != 1)
 		die_errno ("Could not write blob '%s'", sha1_to_hex(sha1));
 	printf("\n");
@@ -326,7 +328,7 @@ static void handle_commit(struct commit *commit, struct rev_info *rev)
 	if (!commit->parents)
 		printf("reset %s\n", (const char*)commit->util);
 	printf("commit %s\nmark :%"PRIu32"\n%.*s\n%.*s\ndata %u\n%s",
-	       (const char *)commit->util, last_idnum,
+	       (const char *)commit->util, get_object_mark(&commit->object),
 	       (int)(author_end - author), author,
 	       (int)(committer_end - committer), committer,
 	       (unsigned)(reencoded
@@ -631,7 +633,7 @@ static void import_marks(char *input_file)
 		if (!object)
 			continue;
 
-		if (object->flags & SHOWN)
+		if (get_object_mark(object))
 			error("Object %s already has a mark", sha1_to_hex(sha1));
 
 		if (object->type != OBJ_COMMIT)
@@ -640,7 +642,8 @@ static void import_marks(char *input_file)
 
 		mark_object(object, mark);
 
-		object->flags |= SHOWN;
+		if (!do_not_skip_marked_commits)
+			object->flags |= SHOWN;
 	}
 	fclose(f);
 }
@@ -673,6 +676,8 @@ int cmd_fast_export(int argc, const char **argv, const char *prefix)
 		OPT_BOOLEAN(0, "use-done-feature", &use_done_feature,
 			     N_("Use the done feature to terminate the stream")),
 		OPT_BOOL(0, "no-data", &no_data, N_("Skip output of blob data")),
+		OPT_BOOL(0, "do-not-skip-marked-commits", &do_not_skip_marked_commits,
+			     N_("Do not skip marked commits")),
 		OPT_END()
 	};
 
diff --git a/contrib/remote-helpers/git-remote-bzr b/contrib/remote-helpers/git-remote-bzr
index 3e452af..24a9a99 100755
--- a/contrib/remote-helpers/git-remote-bzr
+++ b/contrib/remote-helpers/git-remote-bzr
@@ -629,7 +629,12 @@ def parse_commit(parser):
 
     committer, date, tz = committer
     parents = [mark_to_rev(p) for p in parents]
-    revid = bzrlib.generate_ids.gen_revision_id(committer, date)
+    try:
+        revid = mark_to_rev(commit_mark)
+    except KeyError:
+        revid = bzrlib.generate_ids.gen_revision_id(committer, date)
+        marks.new_mark(revid, commit_mark)
+
     props = {}
     props['branch-nick'] = branch.nick
 
@@ -650,7 +655,6 @@ def parse_commit(parser):
         branch.unlock()
 
     parsed_refs[ref] = revid
-    marks.new_mark(revid, commit_mark)
 
 def parse_reset(parser):
     global parsed_refs
@@ -692,12 +696,12 @@ def do_export(parser):
         if ref.startswith('refs/heads/'):
             name = ref[len('refs/heads/'):]
             branch = bzrlib.branch.Branch.open(branches[name])
-            branch.generate_revision_history(revid, marks.get_tip(name))
 
             if name in peers:
                 peer = bzrlib.branch.Branch.open(peers[name])
                 try:
                     peer.bzrdir.push_branch(branch, revision_id=revid)
+                    branch.generate_revision_history(revid, marks.get_tip(name))
                 except bzrlib.errors.DivergedBranches:
                     print "error %s non-fast forward" % ref
                     continue
@@ -732,6 +736,7 @@ def do_capabilities(parser):
     if os.path.exists(path):
         print "*import-marks %s" % path
     print "*export-marks %s" % path
+    print "*full-marks"
 
     print
 
diff --git a/transport-helper.c b/transport-helper.c
index 2f5ac3f..3f1f2d7 100644
--- a/transport-helper.c
+++ b/transport-helper.c
@@ -27,6 +27,7 @@ struct helper_data {
 		push : 1,
 		connect : 1,
 		signed_tags : 1,
+		full_marks : 1,
 		no_disconnect_req : 1;
 	char *export_marks;
 	char *import_marks;
@@ -195,6 +196,8 @@ static struct child_process *get_helper(struct transport *transport)
 			data->connect = 1;
 		} else if (!strcmp(capname, "signed-tags")) {
 			data->signed_tags = 1;
+		} else if (!strcmp(capname, "full-marks")) {
+			data->full_marks = 1;
 		} else if (!prefixcmp(capname, "export-marks ")) {
 			struct strbuf arg = STRBUF_INIT;
 			strbuf_addstr(&arg, "--export-marks=");
@@ -415,11 +418,13 @@ static int get_exporter(struct transport *transport,
 	/* we need to duplicate helper->in because we want to use it after
 	 * fastexport is done with it. */
 	fastexport->out = dup(helper->in);
-	fastexport->argv = xcalloc(6 + revlist_args->nr, sizeof(*fastexport->argv));
+	fastexport->argv = xcalloc(7 + revlist_args->nr, sizeof(*fastexport->argv));
 	fastexport->argv[argc++] = "fast-export";
 	fastexport->argv[argc++] = "--use-done-feature";
 	fastexport->argv[argc++] = data->signed_tags ?
 		"--signed-tags=verbatim" : "--signed-tags=warn-strip";
+	if (data->full_marks)
+		fastexport->argv[argc++] = "--do-not-skip-marked-commits";
 	if (data->export_marks)
 		fastexport->argv[argc++] = data->export_marks;
 	if (data->import_marks)
-- 
tg: (2a9af4b..) t/marks-shown (depends on: next)