{"thread":{"id":"18548","subject":"[PATCH] fast-export: Avoid dropping files from commits","startedAt":"2009-03-25T20:55:19Z","lastAt":"2009-03-26T04:02:02Z","messageCount":7,"participants":["newren@gmail.com","Johannes Schindelin","Elijah Newren","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"109414","messageId":"1238014519-11683-1-git-send-email-newren@gmail.com","threadId":"18548","inReplyTo":null,"subject":"[PATCH] fast-export: Avoid dropping files from commits","fromName":"","fromEmail":"newren@gmail.com","sentAt":"2009-03-25T20:55:19Z","receivedAt":"2009-03-25T20:55:19Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"From: Elijah Newren \n\nWhen exporting a subset of commits on a branch that do not go back to a\nroot commit (e.g. master~2..master), we still want each exported commit to\nhave the same files in the exported tree as in the original tree.\n\nSigned-off-by: Elijah Newren \n---\n builtin-fast-export.c | 3 ++-\n t/t9301-fast-export.sh | 7 +++++--\n 2 files changed, 7 insertions(+), 3 deletions(-)\n\ndiff --git a/builtin-fast-export.c b/builtin-fast-export.c\nindex fdf4ae9..34a419c 100644\n--- a/builtin-fast-export.c\n+++ b/builtin-fast-export.c\n@@ -221,7 +221,8 @@ static void handle_commit(struct commit *commit, struct rev_info *rev)\n \tif (message)\n \t\tmessage += 2;\n \n-\tif (commit->parents) {\n+\tif (commit->parents &&\n+\t get_object_mark(&commit->parents->item->object) != 0) {\n \t\tparse_commit(commit->parents->item);\n \t\tdiff_tree_sha1(commit->parents->item->tree->object.sha1,\n \t\t\t commit->tree->object.sha1, \"\", &rev->diffopt);\ndiff --git a/t/t9301-fast-export.sh b/t/t9301-fast-export.sh\nindex 86c3760..b860626 100755\n--- a/t/t9301-fast-export.sh\n+++ b/t/t9301-fast-export.sh\n@@ -8,6 +8,9 @@ test_description='git fast-export'\n \n test_expect_success 'setup' '\n \n+\techo break it > file0 &&\n+\tgit add file0 &&\n+\ttest_tick &&\n \techo Wohlauf > file &&\n \tgit add file &&\n \ttest_tick &&\n@@ -57,8 +60,8 @@ test_expect_success 'fast-export master~2..master' '\n \t\t(cd new &&\n \t\t git fast-import &&\n \t\t test $MASTER != $(git rev-parse --verify refs/heads/partial) &&\n-\t\t git diff master..partial &&\n-\t\t git diff master^..partial^ &&\n+\t\t git diff --exit-code master partial &&\n+\t\t git diff --exit-code master^ partial^ &&\n \t\t test_must_fail git rev-parse partial~2)\n \n '\n-- \n1.6.0.6\n"},{"id":"109438","messageId":"alpine.DEB.1.00.0903252312460.26370@intel-tinevez-2-302","threadId":"18548","inReplyTo":"1238014519-11683-1-git-send-email-newren@gmail.com","subject":"Re: [PATCH] fast-export: Avoid dropping files from commits","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-03-25T22:13:23Z","receivedAt":"2009-03-25T22:13:23Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Wed, 25 Mar 2009, newren@gmail.com wrote:\n\n> From: Elijah Newren \n> \n> When exporting a subset of commits on a branch that do not go back to a\n> root commit (e.g. master~2..master), we still want each exported commit to\n> have the same files in the exported tree as in the original tree.\n> \n> Signed-off-by: Elijah Newren \n> ---\n\nMakes sense.\n\n> builtin-fast-export.c | 3 ++-\n> t/t9301-fast-export.sh | 7 +++++--\n> 2 files changed, 7 insertions(+), 3 deletions(-)\n> \n> diff --git a/builtin-fast-export.c b/builtin-fast-export.c\n> index fdf4ae9..34a419c 100644\n> --- a/builtin-fast-export.c\n> +++ b/builtin-fast-export.c\n> @@ -221,7 +221,8 @@ static void handle_commit(struct commit *commit, struct rev_info *rev)\n> \tif (message)\n> \t\tmessage += 2;\n> \n> -\tif (commit->parents) {\n> +\tif (commit->parents &&\n> +\t get_object_mark(&commit->parents->item->object) != 0) {\n> \t\tparse_commit(commit->parents->item);\n> \t\tdiff_tree_sha1(commit->parents->item->tree->object.sha1,\n> \t\t\t commit->tree->object.sha1, \"\", &rev->diffopt);\n\nI do not understand that change.\n\nA good explanation in the commit message might help this stupid developer.\n\nCiao,\nDscho\n"},{"id":"109445","messageId":"1238025203-27194-1-git-send-email-newren@gmail.com","threadId":"18548","inReplyTo":"alpine.DEB.1.00.0903252312460.26370@intel-tinevez-2-302","subject":"Re: [PATCH] fast-export: Avoid dropping files from commits","fromName":"","fromEmail":"newren@gmail.com","sentAt":"2009-03-25T23:53:23Z","receivedAt":"2009-03-25T23:53:23Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"From: Elijah Newren \n\nWhen exporting a subset of commits on a branch that do not go back to a\nroot commit (e.g. master~2..master), we still want each exported commit to\nhave the same files in the exported tree as in the original tree.\n\nPreviously, when given such a range, we would omit master~2 as a parent of\nmaster~1, but we would still diff against master~2 when selecting the list\nof files to include in master~1. This would result in only files that\nhad changed in the given range showing up in the resulting export. In such\ncases, we should diff master~1 against the root instead (i.e. use\ndiff_root_tree_sha1 instead of diff_tree_sha1).\n\nThere's a special case to consider here: incremental exports (i.e. exports\nwhere the --import-marks flag is specified). If master~2 is an imported\nmark, then we still want to diff master~1 against master~2 when selecting\nthe list of files to include.\n\nWe can handle all cases, including the special case, by just checking\nwhether master~2 corresponds to a known object mark when deciding what to\ndiff against.\n\nSigned-off-by: Elijah Newren \n---\n builtin-fast-export.c | 3 ++-\n t/t9301-fast-export.sh | 7 +++++--\n 2 files changed, 7 insertions(+), 3 deletions(-)\n\ndiff --git a/builtin-fast-export.c b/builtin-fast-export.c\nindex fdf4ae9..34a419c 100644\n--- a/builtin-fast-export.c\n+++ b/builtin-fast-export.c\n@@ -221,7 +221,8 @@ static void handle_commit(struct commit *commit, struct rev_info *rev)\n \tif (message)\n \t\tmessage += 2;\n \n-\tif (commit->parents) {\n+\tif (commit->parents &&\n+\t get_object_mark(&commit->parents->item->object) != 0) {\n \t\tparse_commit(commit->parents->item);\n \t\tdiff_tree_sha1(commit->parents->item->tree->object.sha1,\n \t\t\t commit->tree->object.sha1, \"\", &rev->diffopt);\ndiff --git a/t/t9301-fast-export.sh b/t/t9301-fast-export.sh\nindex 86c3760..b860626 100755\n--- a/t/t9301-fast-export.sh\n+++ b/t/t9301-fast-export.sh\n@@ -8,6 +8,9 @@ test_description='git fast-export'\n \n test_expect_success 'setup' '\n \n+\techo break it > file0 &&\n+\tgit add file0 &&\n+\ttest_tick &&\n \techo Wohlauf > file &&\n \tgit add file &&\n \ttest_tick &&\n@@ -57,8 +60,8 @@ test_expect_success 'fast-export master~2..master' '\n \t\t(cd new &&\n \t\t git fast-import &&\n \t\t test $MASTER != $(git rev-parse --verify refs/heads/partial) &&\n-\t\t git diff master..partial &&\n-\t\t git diff master^..partial^ &&\n+\t\t git diff --exit-code master partial &&\n+\t\t git diff --exit-code master^ partial^ &&\n \t\t test_must_fail git rev-parse partial~2)\n \n '\n-- \n1.6.0.6\n"},{"id":"109446","messageId":"51419b2c0903251706n7cfa053eheff542d20ca9a753@mail.gmail.com","threadId":"18548","inReplyTo":"alpine.DEB.1.00.0903252312460.26370@intel-tinevez-2-302","subject":"Re: [PATCH] fast-export: Avoid dropping files from commits","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2009-03-26T00:06:46Z","receivedAt":"2009-03-26T00:06:46Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"Hi,\n\nOn Wed, Mar 25, 2009 at 4:13 PM, Johannes Schindelin\n wrote:\n>> -     if (commit->parents) {\n>> +     if (commit->parents &&\n>> +         get_object_mark(&commit->parents->item->object) != 0) {\n>>               parse_commit(commit->parents->item);\n>>               diff_tree_sha1(commit->parents->item->tree->object.sha1,\n>>                              commit->tree->object.sha1, \"\", &rev->diffopt);\n>\n> I do not understand that change.\n>\n> A good explanation in the commit message might help this stupid developer.\n\nI resent the patch in another email (sorry for the duplication, but I\ndon't trust gmail to preserve patches, and responding inline to\ncomments via git-send-email isn't so great either). Let me know if\nthe explanation is missing anything, is too detailed, or is using\nincorrect terminology. In two cases I was sufficiently unsure about\nmy wording that I provided extra wording to try to make it clear what\nI was talking about.\n\nThanks,\nElijah\n"},{"id":"109448","messageId":"alpine.DEB.1.00.0903260138410.10279@pacific.mpi-cbg.de","threadId":"18548","inReplyTo":"51419b2c0903251706n7cfa053eheff542d20ca9a753@mail.gmail.com","subject":"Re: [PATCH] fast-export: Avoid dropping files from commits","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-03-26T00:40:14Z","receivedAt":"2009-03-26T00:40:14Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Wed, 25 Mar 2009, Elijah Newren wrote:\n\n> On Wed, Mar 25, 2009 at 4:13 PM, Johannes Schindelin\n> wrote:\n> >> -     if (commit->parents) {\n> >> +     if (commit->parents &&\n> >> +         get_object_mark(&commit->parents->item->object) != 0) {\n> >>               parse_commit(commit->parents->item);\n> >>               diff_tree_sha1(commit->parents->item->tree->object.sha1,\n> >>                              commit->tree->object.sha1, \"\", &rev->diffopt);\n> >\n> > I do not understand that change.\n> >\n> > A good explanation in the commit message might help this stupid \n> > developer.\n> \n> I resent the patch in another email (sorry for the duplication, but I \n> don't trust gmail to preserve patches, and responding inline to comments \n> via git-send-email isn't so great either). Let me know if the \n> explanation is missing anything, is too detailed, or is using incorrect \n> terminology. In two cases I was sufficiently unsure about my wording \n> that I provided extra wording to try to make it clear what I was talking \n> about.\n\nI am pretty tired, but I still have the impression that I understood it, \nso yes, I like it.\n\nYou might want to skip the != 0, though, as we avoid that in the rest of \nGit's source code, too.\n\nThanks,\nDscho\n"},{"id":"109469","messageId":"7vy6utt0op.fsf@gitster.siamese.dyndns.org","threadId":"18548","inReplyTo":"alpine.DEB.1.00.0903252312460.26370@intel-tinevez-2-302","subject":"Re: [PATCH] fast-export: Avoid dropping files from commits","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-03-26T03:36:38Z","receivedAt":"2009-03-26T03:36:38Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin writes:\n\n> Hi,\n>\n> On Wed, 25 Mar 2009, newren@gmail.com wrote:\n>\n>> From: Elijah Newren \n>> \n>> When exporting a subset of commits on a branch that do not go back to a\n>> root commit (e.g. master~2..master), we still want each exported commit to\n>> have the same files in the exported tree as in the original tree.\n>> \n>> Signed-off-by: Elijah Newren \n>> ---\n>\n> Makes sense.\n\nHmm, does it?\n\nShouldn't an export with a bottom commit be always considered an\nincremental? Why special case --import-marks?\n\nWhen you say \"I want to export master~2..master\", isn't the intention\n(unstated, because it is too obvious) that follows it \"... because I do\nhave master~2 already and I would want to replay the export on top of that\nstate\"? If all of master~2, master~1 and master have a file \"frotz\" with\nexactly the same contents, I thought you wouldn't even have to have that\nsame contents repeated in the export datastream.\n\nOr am I (again) entirely misunderstanding the intended use case?\n"},{"id":"109473","messageId":"51419b2c0903252102v71de617aiffad2d4934d4d07f@mail.gmail.com","threadId":"18548","inReplyTo":"7vy6utt0op.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] fast-export: Avoid dropping files from commits","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2009-03-26T04:02:02Z","receivedAt":"2009-03-26T04:02:02Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"Hi,\n\nOn Wed, Mar 25, 2009 at 9:36 PM, Junio C Hamano wrote:\n> Hmm, does it?\n>\n> Shouldn't an export with a bottom commit be always considered an\n> incremental? Why special case --import-marks?\n>\n> When you say \"I want to export master~2..master\", isn't the intention\n> (unstated, because it is too obvious) that follows it \"... because I do\n> have master~2 already and I would want to replay the export on top of that\n> state\"? If all of master~2, master~1 and master have a file \"frotz\" with\n> exactly the same contents, I thought you wouldn't even have to have that\n> same contents repeated in the export datastream.\n\nNo, without the --import-marks it doesn't make sense to make it an\nincremental. Think of it this way: git-fast-export converts all\nsha1's into integer identifiers (marks). When you export\nmaster~2..master, then when fast-export comes across master~1, it\nnotices that there's nothing to convert it's parent sha1sum into since\nno information was output for master~2 in the export stream. The way\naround it would be to simply export master~2 anyway, and master~3,\nand...\n(An alternate way around it, I guess, would be using the sha1sum in\nthe export stream instead of a mark...but do you have any way of\nknowing whether the destination repository you import into will have\nthat commit? Using --import-marks seems like a nice way to specify\nwhether you have that information or not.)\n\nIt's perhaps a gotcha, as I know I had the same initial assumption\neven after reading the manpage. Perhaps\nDocumentation/git-fast-export.txt could be made a bit clearer on this\npoint.\n\n> Or am I (again) entirely misunderstanding the intended use case?\n\nExporting master~2..master without specifying --import-marks is a way\nof squashing history, essentially. Perhaps you don't like it serving\nthis purpose, but it was intentional; looking at the master~2..master\ntestcase in t/t9301-fast-export.sh, you'll see\n\n git fast-export master~2..master | sed \"s/master/partial/\" |\n && test_must_fail git rev-parse partial~2\n"}]}