{"thread":{"id":"19389","subject":"[PATCH 1/3] bisect: rework some rev related functions to make them more reusable","startedAt":"2009-05-17T15:36:44Z","lastAt":"2009-05-27T05:12:36Z","messageCount":13,"participants":["Christian Couder","Junio C Hamano","Jakub Narebski","Miles Bader","Johannes Sixt","Johannes Schindelin"],"isPatch":true,"patchVersion":1,"patchTotal":3},"messages":[{"id":"114132","messageId":"20090517153647.6403.35442.chriscool@tuxfamily.org","threadId":"19389","inReplyTo":"20090517153307.6403.73576.","subject":"[PATCH 1/3] bisect: rework some rev related functions to make them more reusable","fromName":"Christian Couder","fromEmail":"chriscool@tuxfamily.org","sentAt":"2009-05-17T15:36:44Z","receivedAt":"2009-05-17T15:36:44Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"This patches changes the \"bisect_rev_setup\" and \"bisect_common\"\nfunctions to make it easier to reuse them in a later patch.\n\nSigned-off-by: Christian Couder <chriscool@tuxfamily.org>\n---\n bisect.c |   25 ++++++++++++++-----------\n 1 files changed, 14 insertions(+), 11 deletions(-)\n\ndiff --git a/bisect.c b/bisect.c\nindex f57b62c..dc4e1bb 100644\n--- a/bisect.c\n+++ b/bisect.c\n@@ -553,7 +553,9 @@ struct commit_list *filter_skipped(struct commit_list *list,\n \treturn filtered;\n }\n \n-static void bisect_rev_setup(struct rev_info *revs, const char *prefix)\n+static void bisect_rev_setup(struct rev_info *revs, const char *prefix,\n+\t\t\t     const char *bad_format, const char *good_format,\n+\t\t\t     int read_paths)\n {\n \tstruct argv_array rev_argv = { NULL, 0, 0 };\n \tint i;\n@@ -564,26 +566,24 @@ static void bisect_rev_setup(struct rev_info *revs, const char *prefix)\n \n \t/* rev_argv.argv[0] will be ignored by setup_revisions */\n \targv_array_push(&rev_argv, xstrdup(\"bisect_rev_setup\"));\n-\targv_array_push_sha1(&rev_argv, current_bad_sha1, \"%s\");\n+\targv_array_push_sha1(&rev_argv, current_bad_sha1, bad_format);\n \tfor (i = 0; i < good_revs.sha1_nr; i++)\n-\t\targv_array_push_sha1(&rev_argv, good_revs.sha1[i], \"^%s\");\n+\t\targv_array_push_sha1(&rev_argv, good_revs.sha1[i],\n+\t\t\t\t     good_format);\n \targv_array_push(&rev_argv, xstrdup(\"--\"));\n-\tread_bisect_paths(&rev_argv);\n+\tif (read_paths)\n+\t\tread_bisect_paths(&rev_argv);\n \targv_array_push(&rev_argv, NULL);\n \n \tsetup_revisions(rev_argv.argv_nr, rev_argv.argv, revs, NULL);\n-\trevs->limited = 1;\n }\n \n-static void bisect_common(struct rev_info *revs, int *reaches, int *all)\n+static void bisect_common(struct rev_info *revs)\n {\n \tif (prepare_revision_walk(revs))\n \t\tdie(\"revision walk setup failed\");\n \tif (revs->tree_objects)\n \t\tmark_edges_uninteresting(revs->commits, revs, NULL);\n-\n-\trevs->commits = find_bisection(revs->commits, reaches, all,\n-\t\t\t\t       !!skipped_revs.sha1_nr);\n }\n \n static void exit_if_skipped_commits(struct commit_list *tried,\n@@ -843,10 +843,13 @@ int bisect_next_all(const char *prefix)\n \n \tcheck_good_are_ancestors_of_bad(prefix);\n \n-\tbisect_rev_setup(&revs, prefix);\n+\tbisect_rev_setup(&revs, prefix, \"%s\", \"^%s\", 1);\n+\trevs.limited = 1;\n \n-\tbisect_common(&revs, &reaches, &all);\n+\tbisect_common(&revs);\n \n+\trevs.commits = find_bisection(revs.commits, &reaches, &all,\n+\t\t\t\t       !!skipped_revs.sha1_nr);\n \trevs.commits = filter_skipped(revs.commits, &tried, 0);\n \n \tif (!revs.commits) {\n-- \n1.6.3.rc1.112.g17e25\n"},{"id":"114134","messageId":"20090517153647.6403.44036.chriscool@tuxfamily.org","threadId":"19389","inReplyTo":"20090517153307.6403.73576.","subject":"[PATCH 2/3] commit: add function to unparse a commit and its parents","fromName":"Christian Couder","fromEmail":"chriscool@tuxfamily.org","sentAt":"2009-05-17T15:36:45Z","receivedAt":"2009-05-17T15:36:45Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"This patch adds the \"unparse_commit\" function that returns a commit\ninto an unparsed state by freeing its data and resetting its fields\nto 0.\n\nIts parents are recursively unparsed too, because they might have\nbeen changed. But its tree is not unparsed as it should not have\nbeen modifed.\n\nNote that as the \"flags\" and \"used\" fields may be used even if the\nobject is not parsed, we have to reset them anyway.\n\nSigned-off-by: Christian Couder <chriscool@tuxfamily.org>\n---\n commit.c |   20 ++++++++++++++++++++\n commit.h |    2 ++\n 2 files changed, 22 insertions(+), 0 deletions(-)\n\ndiff --git a/commit.c b/commit.c\nindex cf72143..1d4db4d 100644\n--- a/commit.c\n+++ b/commit.c\n@@ -317,6 +317,26 @@ int parse_commit(struct commit *item)\n \treturn ret;\n }\n \n+static void unparse_commit_list(struct commit_list *list)\n+{\n+\tfor (; list; list = list->next)\n+\t\tunparse_commit(list->item);\n+}\n+\n+void unparse_commit(struct commit *item)\n+{\n+\titem->object.flags = 0;\n+\titem->object.used = 0;\n+\tif (item->object.parsed) {\n+\t\titem->object.parsed = 0;\n+\t\tif (item->parents) {\n+\t\t\tunparse_commit_list(item->parents);\n+\t\t\tfree_commit_list(item->parents);\n+\t\t\titem->parents = NULL;\n+\t\t}\n+\t}\n+}\n+\n struct commit_list *commit_list_insert(struct commit *item, struct commit_list **list_p)\n {\n \tstruct commit_list *new_list = xmalloc(sizeof(struct commit_list));\ndiff --git a/commit.h b/commit.h\nindex 8bfdf0e..a8bdd09 100644\n--- a/commit.h\n+++ b/commit.h\n@@ -40,6 +40,8 @@ int parse_commit_buffer(struct commit *item, void *buffer, unsigned long size);\n \n int parse_commit(struct commit *item);\n \n+void unparse_commit(struct commit *item);\n+\n struct commit_list * commit_list_insert(struct commit *item, struct commit_list **list_p);\n unsigned commit_list_count(const struct commit_list *l);\n struct commit_list * insert_by_date(struct commit *item, struct commit_list **list);\n-- \n1.6.3.rc1.112.g17e25\n"},{"id":"114133","messageId":"20090517153647.6403.64994.chriscool@tuxfamily.org","threadId":"19389","inReplyTo":"20090517153307.6403.73576.","subject":"[PATCH 3/3] bisect: check ancestors without forking a \"git rev-list\" process","fromName":"Christian Couder","fromEmail":"chriscool@tuxfamily.org","sentAt":"2009-05-17T15:36:46Z","receivedAt":"2009-05-17T15:36:46Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"We must save the pending commits that will be used during revision\nwalking and unparse them after, because we want to leave a clean\nstate for the next revision walking that will try to find the best\nbisection point.\n\nAs we don't fork a process anymore to call \"git rev-list\", we need\nto remove the use of GIT_TRACE to check how \"git rev-list\" is\ncalled from the t6030 test that uses it.\n\nSigned-off-by: Christian Couder <chriscool@tuxfamily.org>\n---\n bisect.c                    |   54 +++++++++++++++++-------------------------\n t/t6030-bisect-porcelain.sh |   13 +---------\n 2 files changed, 23 insertions(+), 44 deletions(-)\n\ndiff --git a/bisect.c b/bisect.c\nindex dc4e1bb..c43c120 100644\n--- a/bisect.c\n+++ b/bisect.c\n@@ -750,42 +750,31 @@ static void check_merge_bases(void)\n \tfree_commit_list(result);\n }\n \n-/*\n- * This function runs the command \"git rev-list $_good ^$_bad\"\n- * and returns 1 if it produces some output, 0 otherwise.\n- */\n-static int check_ancestors(void)\n+static int check_ancestors(const char *prefix)\n {\n-\tstruct argv_array rev_argv = { NULL, 0, 0 };\n-\tstruct strbuf str = STRBUF_INIT;\n-\tint i, result = 0;\n-\tstruct child_process rls;\n-\tFILE *rls_fout;\n+\tstruct rev_info revs;\n+\tstruct object_array pending_copy;\n+\tint i, res;\n \n-\targv_array_push(&rev_argv, xstrdup(\"rev-list\"));\n-\targv_array_push_sha1(&rev_argv, current_bad_sha1, \"^%s\");\n-\tfor (i = 0; i < good_revs.sha1_nr; i++)\n-\t\targv_array_push_sha1(&rev_argv, good_revs.sha1[i], \"%s\");\n-\targv_array_push(&rev_argv, NULL);\n+\tbisect_rev_setup(&revs, prefix, \"^%s\", \"%s\", 0);\n \n-\tmemset(&rls, 0, sizeof(rls));\n-\trls.argv = rev_argv.argv;\n-\trls.out = -1;\n-\trls.git_cmd = 1;\n-\tif (start_command(&rls))\n-\t\tdie(\"Could not launch 'git rev-list' command.\");\n-\trls_fout = fdopen(rls.out, \"r\");\n-\twhile (strbuf_getline(&str, rls_fout, '\\n') != EOF) {\n-\t\tstrbuf_trim(&str);\n-\t\tif (*str.buf) {\n-\t\t\tresult = 1;\n-\t\t\tbreak;\n-\t\t}\n+\t/* Save pending objects, so they can be cleaned up later. */\n+\tmemset(&pending_copy, 0, sizeof(pending_copy));\n+\tfor (i = 0; i < revs.pending.nr; i++)\n+\t\tadd_object_array(revs.pending.objects[i].item,\n+\t\t\t\t revs.pending.objects[i].name,\n+\t\t\t\t &pending_copy);\n+\n+\tbisect_common(&revs);\n+\tres = (revs.commits != NULL);\n+\n+\t/* Clean up objects used, as they will be reused. */\n+\tfor (i = 0; i < pending_copy.nr; i++) {\n+\t\tstruct object *o = pending_copy.objects[i].item;\n+\t\tunparse_commit((struct commit *)o);\n \t}\n-\tfclose(rls_fout);\n-\tfinish_command(&rls);\n \n-\treturn result;\n+\treturn res;\n }\n \n /*\n@@ -813,7 +802,8 @@ static void check_good_are_ancestors_of_bad(const char *prefix)\n \tif (good_revs.sha1_nr == 0)\n \t\treturn;\n \n-\tif (check_ancestors())\n+\t/* Check if all good revs are ancestor of the bad rev. */\n+\tif (check_ancestors(prefix))\n \t\tcheck_merge_bases();\n \n \t/* Create file BISECT_ANCESTORS_OK. */\ndiff --git a/t/t6030-bisect-porcelain.sh b/t/t6030-bisect-porcelain.sh\nindex 54b7ea6..5254b23 100755\n--- a/t/t6030-bisect-porcelain.sh\n+++ b/t/t6030-bisect-porcelain.sh\n@@ -482,28 +482,17 @@ test_expect_success 'good merge bases when good and bad are siblings' '\n \tgit bisect reset\n '\n \n-check_trace() {\n-\tgrep \"$1\" \"$GIT_TRACE\" | grep \"\\^$2\" | grep \"$3\" >/dev/null\n-}\n-\n test_expect_success 'optimized merge base checks' '\n-\tGIT_TRACE=\"$(pwd)/trace.log\" &&\n-\texport GIT_TRACE &&\n \tgit bisect start \"$HASH7\" \"$SIDE_HASH7\" > my_bisect_log.txt &&\n \tgrep \"merge base must be tested\" my_bisect_log.txt &&\n \tgrep \"$HASH4\" my_bisect_log.txt &&\n-\tcheck_trace \"rev-list\" \"$HASH7\" \"$SIDE_HASH7\" &&\n \tgit bisect good > my_bisect_log2.txt &&\n \ttest -f \".git/BISECT_ANCESTORS_OK\" &&\n \ttest \"$HASH6\" = $(git rev-parse --verify HEAD) &&\n-\t: > \"$GIT_TRACE\" &&\n \tgit bisect bad > my_bisect_log3.txt &&\n-\ttest_must_fail check_trace \"rev-list\" \"$HASH6\" \"$SIDE_HASH7\" &&\n \tgit bisect good \"$A_HASH\" > my_bisect_log4.txt &&\n \tgrep \"merge base must be tested\" my_bisect_log4.txt &&\n-\ttest_must_fail test -f \".git/BISECT_ANCESTORS_OK\" &&\n-\tcheck_trace \"rev-list\" \"$HASH6\" \"$A_HASH\" &&\n-\tunset GIT_TRACE\n+\ttest_must_fail test -f \".git/BISECT_ANCESTORS_OK\"\n '\n \n # This creates another side branch called \"parallel\" with some files\n-- \n1.6.3.rc1.112.g17e25\n"},{"id":"114157","messageId":"7v3ab3exht.fsf@alter.siamese.dyndns.org","threadId":"19389","inReplyTo":"20090517153647.6403.44036.chriscool@tuxfamily.org","subject":"Re: [PATCH 2/3] commit: add function to unparse a commit and its parents","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-05-18T06:27:58Z","receivedAt":"2009-05-18T06:27:58Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Christian Couder <chriscool@tuxfamily.org> writes:\n\n> Its parents are recursively unparsed too, because they might have\n> been changed. But its tree is not unparsed as it should not have\n> been modifed.\n\nIt is a bug in any codepath if it used commit->tree without first checking\nif commit->parsed is true to begin with, so you could NULLify commit->tree\nbut that shouldn't make any difference.  I agree leaving the tree object\nas-is would make sense.\n\nI am not convinced that unparsing all the _remaining_ parents recursively\nlike your patch does is enough.  Isn't there a codepath that\n\n - parses a commit A to find list of true parents X, Y, Z;\n\n - iterates over that list and descend into one of these parents X, doing\n   nasty things such as pruning its parents list after parsing it; and\n\n - decides to prune that parent X from the parent list, making the parent\n   list of A into Y and Z?\n\nAfter such a sequence, calling your unparse_commit(A) will unparse A and\nremaining parents of Y and Z, but will still leave X in the dirty state.\n"},{"id":"114222","messageId":"200905190616.30132.chriscool@tuxfamily.org","threadId":"19389","inReplyTo":"7v3ab3exht.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 2/3] commit: add function to unparse a commit and its parents","fromName":"Christian Couder","fromEmail":"chriscool@tuxfamily.org","sentAt":"2009-05-19T04:16:29Z","receivedAt":"2009-05-19T04:16:29Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"Le lundi 18 mai 2009, Junio C Hamano a écrit :\n> Christian Couder <chriscool@tuxfamily.org> writes:\n> > Its parents are recursively unparsed too, because they might have\n> > been changed. But its tree is not unparsed as it should not have\n> > been modifed.\n>\n> It is a bug in any codepath if it used commit->tree without first\n> checking if commit->parsed is true to begin with, so you could NULLify\n> commit->tree but that shouldn't make any difference.  I agree leaving the\n> tree object as-is would make sense.\n>\n> I am not convinced that unparsing all the _remaining_ parents recursively\n> like your patch does is enough.  Isn't there a codepath that \n>\n>  - parses a commit A to find list of true parents X, Y, Z;\n>\n>  - iterates over that list and descend into one of these parents X, doing\n>    nasty things such as pruning its parents list after parsing it; and\n>\n>  - decides to prune that parent X from the parent list, making the parent\n>    list of A into Y and Z?\n>\n> After such a sequence, calling your unparse_commit(A) will unparse A and\n> remaining parents of Y and Z, but will still leave X in the dirty state.\n\nI agree that unparsing all the remaining parents recursively may not be \nenough in some cases, but I don't know much about these cases. Is it only \nwhen revs->prune is set?\n\nI think it should be ok when checking that all good revs are ancestor of the \nbad rev, but I may be missing something. \n\nIt should be ok too when using \"git replace\" and/or grafts as the parent \nlist of a commit should be changed before its parents are changed.\n\nI also found the clear_commit_marks function that could be used when \nchecking ancestors so I will resend a patch series using it and without \npatch 2/3. But I can continue to work on unparsing commits to improve \nthe \"git replace\" series. I need to find some test cases though, and I \ncannot think of any interesting one right now.\n\nBy the way, when you say that you suspect an attempt to replace an object \nthat is directly listed on the command line would not work very well with \nthe current replace series, is it related to the unparsing commit problem?\n\nThanks,\nChristian.\n"},{"id":"114228","messageId":"7vws8d8y8i.fsf@alter.siamese.dyndns.org","threadId":"19389","inReplyTo":"200905190616.30132.chriscool@tuxfamily.org","subject":"Re: [PATCH 2/3] commit: add function to unparse a commit and its parents","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-05-19T05:20:45Z","receivedAt":"2009-05-19T05:20:45Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Christian Couder <chriscool@tuxfamily.org> writes:\n\n> By the way, when you say that you suspect an attempt to replace an object \n> that is directly listed on the command line would not work very well with \n> the current replace series, is it related to the unparsing commit problem?\n\nWhat I fear is something like this.\n\n\tgit push $there 04a8c^2:master\n\nIt would need to parse 04a8c to find its second parent and then start\ndiscussing what object to send with the other end.  \"04a8c^2\" is a direct\nuser input and should mean the same commit as git show \"04a8c^2\" would\ngive the user, so it obviously needs to obey the replace rules (making\n04a8c parsed), but the object transfer should not look at replace at all\n(that's the whole point of not using the \"graft hack\" that cannot be\nsanely transferred to the other end).\n"},{"id":"114231","messageId":"m3r5ylk347.fsf@localhost.localdomain","threadId":"19389","inReplyTo":"7vws8d8y8i.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 2/3] commit: add function to unparse a commit and its parents","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2009-05-19T06:35:45Z","receivedAt":"2009-05-19T06:35:45Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n> Christian Couder <chriscool@tuxfamily.org> writes:\n> \n> > By the way, when you say that you suspect an attempt to replace an object \n> > that is directly listed on the command line would not work very well with \n> > the current replace series, is it related to the unparsing commit problem?\n> \n> What I fear is something like this.\n> \n> \tgit push $there 04a8c^2:master\n> \n> It would need to parse 04a8c to find its second parent and then start\n> discussing what object to send with the other end.  \"04a8c^2\" is a direct\n> user input and should mean the same commit as git show \"04a8c^2\" would\n> give the user, so it obviously needs to obey the replace rules (making\n> 04a8c parsed), but the object transfer should not look at replace at all\n> (that's the whole point of not using the \"graft hack\" that cannot be\n> sanely transferred to the other end).\n\nFirst, I have always thought that you cannot push arbitrary SHA-1\n(arbitrary commits) in git; you can only push via refs. Isn't it true?\n\nSecond, the \"refs/replace\" mechanism has the advantage over grafts\nthat it is sanely transferrable. Whether \"04a8c^2\"^{replaced} exists\non remote side depends on if other side has the same replacement, or\nif you push replacements in the same push.\n\nSolution here could be to bail out and ask user to confirm whether\nother side has the same replacements... or something like that.\n-- \nJakub Narebski\nPoland\nShadeHawk on #git\n"},{"id":"114232","messageId":"buozld9o9ry.fsf@dhlpc061.dev.necel.com","threadId":"19389","inReplyTo":"m3r5ylk347.fsf@localhost.localdomain","subject":"Re: [PATCH 2/3] commit: add function to unparse a commit and its parents","fromName":"Miles Bader","fromEmail":"miles@gnu.org","sentAt":"2009-05-19T07:02:25Z","receivedAt":"2009-05-19T07:02:25Z","isPatch":true,"sender":{"key":"miles@gnu.org","avatar":"https://gravatar.com/avatar/01069b69593af7bff28e2f97afeb3644ae6fe2f5f56cb3a8cf34c5fb8c36efe5?d=mp&s=160"},"body":"Jakub Narebski <jnareb@gmail.com> writes:\n>> \tgit push $there 04a8c^2:master\n>\n> First, I have always thought that you cannot push arbitrary SHA-1\n> (arbitrary commits) in git; you can only push via refs. Isn't it true?\n\nHmm, you could try it...\n\n[Pushing random commits has always worked for me though.]\n\n-miles\n\n-- \nThe secret to creativity is knowing how to hide your sources.\n  --Albert Einstein\n"},{"id":"114234","messageId":"7vfxf18szq.fsf@alter.siamese.dyndns.org","threadId":"19389","inReplyTo":"m3r5ylk347.fsf@localhost.localdomain","subject":"Re: [PATCH 2/3] commit: add function to unparse a commit and its parents","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-05-19T07:14:01Z","receivedAt":"2009-05-19T07:14:01Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jakub Narebski <jnareb@gmail.com> writes:\n\n> First, I have always thought that you cannot push arbitrary SHA-1\n> (arbitrary commits) in git; you can only push via refs. Isn't it true?\n\nNo.\n\n> Second, the \"refs/replace\" mechanism has the advantage over grafts\n> that it is sanely transferrable. Whether \"04a8c^2\"^{replaced} exists\n> on remote side depends on if other side has the same replacement, or\n> if you push replacements in the same push.\n\nThe reason why replace mechanism could be cleaner than grafts is because\nreachability traversal and transfer do not obey replacements, and local\nancestry traversal will if there are refs/replace entries.\n\nSo \"git log\" and friends will obey refs/replace/*, but between\na repository that replaces a commit and another that doesn't, they will\ntransfer history without replace entries getting in the way.  Whatever\n04a8c^2 resolves to with replacements, the _real_ history between that\ncommit and whatever the tips of refs on the other side has is\ntransferred, and that commit will update the 'master' branch on the other\nside.\n\nIf the other side sees it as the second parent of 04a8c is a different\nmatter.  If refs/replace hierarchy is shared between the repositories, it\nwill; otherwise it won't.  And the beauty of the replace mechanism is,\nunlike grafts, because object transfer will always be done using _real_\nhistory, you can sanely sync refs/replace hierarchy between the\nrepositories via push or fetch.  During the push of the 04a8c^2 object,\nthe other side does not have to worry about the presense/absense of\nreplace that changes the interpretation of that notation on either end.\nThe exact same underlying history is transferred with or without the\nreplacement objects.\n"},{"id":"114237","messageId":"200905190948.09378.jnareb@gmail.com","threadId":"19389","inReplyTo":"7vfxf18szq.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 2/3] commit: add function to unparse a commit and its parents","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2009-05-19T07:48:07Z","receivedAt":"2009-05-19T07:48:07Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"On Tue, 19 May 2009, Junio C Hamano wrote:\n> Jakub Narebski <jnareb@gmail.com> writes:\n> \n> > First, I have always thought that you cannot push arbitrary SHA-1\n> > (arbitrary commits) in git; you can only push via refs. Isn't it true?\n> \n> No.\n\nOh. I must have mistaken it with the protection in the opposite side:\ngit-fetch doesn't allow fetching arbitrary SHA-1 (arbitrary commits),\nisn't it?\n\nSide note: I wonder if any other DVCS has such shotgun of^W^W a feature ;-)\n\n> > Second, the \"refs/replace\" mechanism has the advantage over grafts\n> > that it is sanely transferrable. Whether \"04a8c^2\"^{replaced} exists\n> > on remote side depends on if other side has the same replacement, or\n> > if you push replacements in the same push.\n> \n> The reason why replace mechanism could be cleaner than grafts is because\n> reachability traversal and transfer do not obey replacements, and local\n> ancestry traversal will if there are refs/replace entries.\n[cut]\n\nThanks for an explanation. So \"refs/replace\" is sanely transferrable\nbecause it can be transferred using local reachability only (without\nreplacements turned on), isn't it?\n\nAs I understand the problem with replacement rules is that it cannot be\ntreated simply as 'extended SHA1' syntax; the replacements must be done\nonly for local operations, which probably means opt-in, and pushing it\ndown to the commands itself... well, that or marking commands as local\nor remote...\n\n-- \nJakub Narebski\nPoland\n"},{"id":"114640","messageId":"4A1A621E.10703@viscovery.net","threadId":"19389","inReplyTo":"20090517153647.6403.44036.chriscool@tuxfamily.org","subject":"Re: [PATCH 2/3] commit: add function to unparse a commit and its parents","fromName":"Johannes Sixt","fromEmail":"j.sixt@viscovery.net","sentAt":"2009-05-25T09:17:18Z","receivedAt":"2009-05-25T09:17:18Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Christian Couder schrieb:\n> +static void unparse_commit_list(struct commit_list *list)\n> +{\n> +\tfor (; list; list = list->next)\n> +\t\tunparse_commit(list->item);\n> +}\n> +\n> +void unparse_commit(struct commit *item)\n> +{\n> +\titem->object.flags = 0;\n> +\titem->object.used = 0;\n> +\tif (item->object.parsed) {\n> +\t\titem->object.parsed = 0;\n> +\t\tif (item->parents) {\n> +\t\t\tunparse_commit_list(item->parents);\n> +\t\t\tfree_commit_list(item->parents);\n> +\t\t\titem->parents = NULL;\n> +\t\t}\n> +\t}\n> +}\n\nI see a recursion here. Could this not overflow the stack if there is a\nlong ancestry chain?\n\n-- Hannes\n"},{"id":"114646","messageId":"alpine.DEB.1.00.0905251140320.4288@intel-tinevez-2-302","threadId":"19389","inReplyTo":"4A1A621E.10703@viscovery.net","subject":"Re: [PATCH 2/3] commit: add function to unparse a commit and its parents","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-05-25T09:46:07Z","receivedAt":"2009-05-25T09:46:07Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Mon, 25 May 2009, Johannes Sixt wrote:\n\n> Christian Couder schrieb:\n> > +static void unparse_commit_list(struct commit_list *list)\n> > +{\n> > +\tfor (; list; list = list->next)\n> > +\t\tunparse_commit(list->item);\n> > +}\n> > +\n> > +void unparse_commit(struct commit *item)\n> > +{\n> > +\titem->object.flags = 0;\n> > +\titem->object.used = 0;\n> > +\tif (item->object.parsed) {\n> > +\t\titem->object.parsed = 0;\n> > +\t\tif (item->parents) {\n> > +\t\t\tunparse_commit_list(item->parents);\n> > +\t\t\tfree_commit_list(item->parents);\n> > +\t\t\titem->parents = NULL;\n> > +\t\t}\n> > +\t}\n> > +}\n> \n> I see a recursion here. Could this not overflow the stack if there is a\n> long ancestry chain?\n\nYou mean tail recursion, i.e. something like\n\nvoid unparse_commit(struct commit *item)\n{\n\titem->object.flags = 0;\n\titem->object.used = 0;\n\twhile (item->object.parsed) {\n\t\tstruct commit *first;\n\n\t\titem->object.parsed = 0;\n\t\tif (!item->parents)\n\t\t\tbreak;\n\t\tif (item->parents->next)\n\t\t\tunparse_commit_list(item->parents->next);\n\t\tfirst = item->parents->item;\n\t\tfree_commit_list(item->parents);\n\t\titem->parents = NULL;\n\t\titem = first;\n\t}\n}\n\nHowever, I am a bit concerned that this function is dangerous, as it just \nassumes that there is no reference to the commits left, which assumption \nis _very_ easy to break by mistake.\n\nCiao,\nDscho\n"},{"id":"114764","messageId":"200905270712.36866.chriscool@tuxfamily.org","threadId":"19389","inReplyTo":"alpine.DEB.1.00.0905251140320.4288@intel-tinevez-2-302","subject":"Re: [PATCH 2/3] commit: add function to unparse a commit and its parents","fromName":"Christian Couder","fromEmail":"chriscool@tuxfamily.org","sentAt":"2009-05-27T05:12:36Z","receivedAt":"2009-05-27T05:12:36Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"Hi,\n\nLe Monday 25 May 2009, Johannes Schindelin a écrit :\n> Hi,\n>\n> On Mon, 25 May 2009, Johannes Sixt wrote:\n> > Christian Couder schrieb:\n> > > +static void unparse_commit_list(struct commit_list *list)\n> > > +{\n> > > +\tfor (; list; list = list->next)\n> > > +\t\tunparse_commit(list->item);\n> > > +}\n> > > +\n> > > +void unparse_commit(struct commit *item)\n> > > +{\n> > > +\titem->object.flags = 0;\n> > > +\titem->object.used = 0;\n> > > +\tif (item->object.parsed) {\n> > > +\t\titem->object.parsed = 0;\n> > > +\t\tif (item->parents) {\n> > > +\t\t\tunparse_commit_list(item->parents);\n> > > +\t\t\tfree_commit_list(item->parents);\n> > > +\t\t\titem->parents = NULL;\n> > > +\t\t}\n> > > +\t}\n> > > +}\n> >\n> > I see a recursion here. Could this not overflow the stack if there is a\n> > long ancestry chain?\n>\n> You mean tail recursion, i.e. something like\n>\n> void unparse_commit(struct commit *item)\n> {\n> \titem->object.flags = 0;\n> \titem->object.used = 0;\n> \twhile (item->object.parsed) {\n> \t\tstruct commit *first;\n>\n> \t\titem->object.parsed = 0;\n> \t\tif (!item->parents)\n> \t\t\tbreak;\n> \t\tif (item->parents->next)\n> \t\t\tunparse_commit_list(item->parents->next);\n> \t\tfirst = item->parents->item;\n> \t\tfree_commit_list(item->parents);\n> \t\titem->parents = NULL;\n> \t\titem = first;\n> \t}\n> }\n\nYes, it is better like this. Thanks and sorry about responding late.\n\n> However, I am a bit concerned that this function is dangerous, as it just\n> assumes that there is no reference to the commits left, which assumption\n> is _very_ easy to break by mistake.\n\nAnyway I will send a 2 patch series that will use clear_commit_marks instead \nof this function.\n\nThanks,\nChristian.\n"}]}