# [PATCH] fast-export: Avoid dropping files from commits 7 messages from 2009-03-25 to 2009-03-26. Participants: newren@gmail.com, Johannes Schindelin, Elijah Newren, Junio C Hamano. Thread: https://gitlist.dev/t/18548 ## newren@gmail.com, 2009-03-25 20:55 Subject: [PATCH] fast-export: Avoid dropping files from commits Message-ID: <1238014519-11683-1-git-send-email-newren@gmail.com> URL: https://gitlist.dev/e/1238014519-11683-1-git-send-email-newren%40gmail.com ``` From: Elijah Newren When exporting a subset of commits on a branch that do not go back to a root commit (e.g. master~2..master), we still want each exported commit to have the same files in the exported tree as in the original tree. Signed-off-by: Elijah Newren --- builtin-fast-export.c | 3 ++- t/t9301-fast-export.sh | 7 +++++-- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/builtin-fast-export.c b/builtin-fast-export.c index fdf4ae9..34a419c 100644 --- a/builtin-fast-export.c +++ b/builtin-fast-export.c @@ -221,7 +221,8 @@ static void handle_commit(struct commit *commit, struct rev_info *rev) if (message) message += 2; - if (commit->parents) { + if (commit->parents && + get_object_mark(&commit->parents->item->object) != 0) { parse_commit(commit->parents->item); diff_tree_sha1(commit->parents->item->tree->object.sha1, commit->tree->object.sha1, "", &rev->diffopt); diff --git a/t/t9301-fast-export.sh b/t/t9301-fast-export.sh index 86c3760..b860626 100755 --- a/t/t9301-fast-export.sh +++ b/t/t9301-fast-export.sh @@ -8,6 +8,9 @@ test_description='git fast-export' test_expect_success 'setup' ' + echo break it > file0 && + git add file0 && + test_tick && echo Wohlauf > file && git add file && test_tick && @@ -57,8 +60,8 @@ test_expect_success 'fast-export master~2..master' ' (cd new && git fast-import && test $MASTER != $(git rev-parse --verify refs/heads/partial) && - git diff master..partial && - git diff master^..partial^ && + git diff --exit-code master partial && + git diff --exit-code master^ partial^ && test_must_fail git rev-parse partial~2) ' -- 1.6.0.6 ``` ## Johannes Schindelin, 2009-03-25 22:13 Subject: Re: [PATCH] fast-export: Avoid dropping files from commits Message-ID: URL: https://gitlist.dev/e/alpine.DEB.1.00.0903252312460.26370%40intel-tinevez-2-302 In-Reply-To: <1238014519-11683-1-git-send-email-newren@gmail.com> ``` Hi, On Wed, 25 Mar 2009, newren@gmail.com wrote: > From: Elijah Newren > > When exporting a subset of commits on a branch that do not go back to a > root commit (e.g. master~2..master), we still want each exported commit to > have the same files in the exported tree as in the original tree. > > Signed-off-by: Elijah Newren > --- Makes sense. > builtin-fast-export.c | 3 ++- > t/t9301-fast-export.sh | 7 +++++-- > 2 files changed, 7 insertions(+), 3 deletions(-) > > diff --git a/builtin-fast-export.c b/builtin-fast-export.c > index fdf4ae9..34a419c 100644 > --- a/builtin-fast-export.c > +++ b/builtin-fast-export.c > @@ -221,7 +221,8 @@ static void handle_commit(struct commit *commit, struct rev_info *rev) > if (message) > message += 2; > > - if (commit->parents) { > + if (commit->parents && > + get_object_mark(&commit->parents->item->object) != 0) { > parse_commit(commit->parents->item); > diff_tree_sha1(commit->parents->item->tree->object.sha1, > commit->tree->object.sha1, "", &rev->diffopt); I do not understand that change. A good explanation in the commit message might help this stupid developer. Ciao, Dscho ``` ## newren@gmail.com, 2009-03-25 23:53 Subject: Re: [PATCH] fast-export: Avoid dropping files from commits Message-ID: <1238025203-27194-1-git-send-email-newren@gmail.com> URL: https://gitlist.dev/e/1238025203-27194-1-git-send-email-newren%40gmail.com In-Reply-To: ``` From: Elijah Newren When exporting a subset of commits on a branch that do not go back to a root commit (e.g. master~2..master), we still want each exported commit to have the same files in the exported tree as in the original tree. Previously, when given such a range, we would omit master~2 as a parent of master~1, but we would still diff against master~2 when selecting the list of files to include in master~1. This would result in only files that had changed in the given range showing up in the resulting export. In such cases, we should diff master~1 against the root instead (i.e. use diff_root_tree_sha1 instead of diff_tree_sha1). There's a special case to consider here: incremental exports (i.e. exports where the --import-marks flag is specified). If master~2 is an imported mark, then we still want to diff master~1 against master~2 when selecting the list of files to include. We can handle all cases, including the special case, by just checking whether master~2 corresponds to a known object mark when deciding what to diff against. Signed-off-by: Elijah Newren --- builtin-fast-export.c | 3 ++- t/t9301-fast-export.sh | 7 +++++-- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/builtin-fast-export.c b/builtin-fast-export.c index fdf4ae9..34a419c 100644 --- a/builtin-fast-export.c +++ b/builtin-fast-export.c @@ -221,7 +221,8 @@ static void handle_commit(struct commit *commit, struct rev_info *rev) if (message) message += 2; - if (commit->parents) { + if (commit->parents && + get_object_mark(&commit->parents->item->object) != 0) { parse_commit(commit->parents->item); diff_tree_sha1(commit->parents->item->tree->object.sha1, commit->tree->object.sha1, "", &rev->diffopt); diff --git a/t/t9301-fast-export.sh b/t/t9301-fast-export.sh index 86c3760..b860626 100755 --- a/t/t9301-fast-export.sh +++ b/t/t9301-fast-export.sh @@ -8,6 +8,9 @@ test_description='git fast-export' test_expect_success 'setup' ' + echo break it > file0 && + git add file0 && + test_tick && echo Wohlauf > file && git add file && test_tick && @@ -57,8 +60,8 @@ test_expect_success 'fast-export master~2..master' ' (cd new && git fast-import && test $MASTER != $(git rev-parse --verify refs/heads/partial) && - git diff master..partial && - git diff master^..partial^ && + git diff --exit-code master partial && + git diff --exit-code master^ partial^ && test_must_fail git rev-parse partial~2) ' -- 1.6.0.6 ``` ## Elijah Newren, 2009-03-26 00:06 Subject: Re: [PATCH] fast-export: Avoid dropping files from commits Message-ID: <51419b2c0903251706n7cfa053eheff542d20ca9a753@mail.gmail.com> URL: https://gitlist.dev/e/51419b2c0903251706n7cfa053eheff542d20ca9a753%40mail.gmail.com In-Reply-To: ``` Hi, On Wed, Mar 25, 2009 at 4:13 PM, Johannes Schindelin wrote: >> -     if (commit->parents) { >> +     if (commit->parents && >> +         get_object_mark(&commit->parents->item->object) != 0) { >>               parse_commit(commit->parents->item); >>               diff_tree_sha1(commit->parents->item->tree->object.sha1, >>                              commit->tree->object.sha1, "", &rev->diffopt); > > I do not understand that change. > > A good explanation in the commit message might help this stupid developer. I resent the patch in another email (sorry for the duplication, but I don't trust gmail to preserve patches, and responding inline to comments via git-send-email isn't so great either). Let me know if the explanation is missing anything, is too detailed, or is using incorrect terminology. In two cases I was sufficiently unsure about my wording that I provided extra wording to try to make it clear what I was talking about. Thanks, Elijah ``` ## Johannes Schindelin, 2009-03-26 00:40 Subject: Re: [PATCH] fast-export: Avoid dropping files from commits Message-ID: URL: https://gitlist.dev/e/alpine.DEB.1.00.0903260138410.10279%40pacific.mpi-cbg.de In-Reply-To: <51419b2c0903251706n7cfa053eheff542d20ca9a753@mail.gmail.com> ``` Hi, On Wed, 25 Mar 2009, Elijah Newren wrote: > On Wed, Mar 25, 2009 at 4:13 PM, Johannes Schindelin > wrote: > >> -     if (commit->parents) { > >> +     if (commit->parents && > >> +         get_object_mark(&commit->parents->item->object) != 0) { > >>               parse_commit(commit->parents->item); > >>               diff_tree_sha1(commit->parents->item->tree->object.sha1, > >>                              commit->tree->object.sha1, "", &rev->diffopt); > > > > I do not understand that change. > > > > A good explanation in the commit message might help this stupid > > developer. > > I resent the patch in another email (sorry for the duplication, but I > don't trust gmail to preserve patches, and responding inline to comments > via git-send-email isn't so great either). Let me know if the > explanation is missing anything, is too detailed, or is using incorrect > terminology. In two cases I was sufficiently unsure about my wording > that I provided extra wording to try to make it clear what I was talking > about. I am pretty tired, but I still have the impression that I understood it, so yes, I like it. You might want to skip the != 0, though, as we avoid that in the rest of Git's source code, too. Thanks, Dscho ``` ## Junio C Hamano, 2009-03-26 03:36 Subject: Re: [PATCH] fast-export: Avoid dropping files from commits Message-ID: <7vy6utt0op.fsf@gitster.siamese.dyndns.org> URL: https://gitlist.dev/e/7vy6utt0op.fsf%40gitster.siamese.dyndns.org In-Reply-To: ``` Johannes Schindelin writes: > Hi, > > On Wed, 25 Mar 2009, newren@gmail.com wrote: > >> From: Elijah Newren >> >> When exporting a subset of commits on a branch that do not go back to a >> root commit (e.g. master~2..master), we still want each exported commit to >> have the same files in the exported tree as in the original tree. >> >> Signed-off-by: Elijah Newren >> --- > > Makes sense. Hmm, does it? Shouldn't an export with a bottom commit be always considered an incremental? Why special case --import-marks? When you say "I want to export master~2..master", isn't the intention (unstated, because it is too obvious) that follows it "... because I do have master~2 already and I would want to replay the export on top of that state"? If all of master~2, master~1 and master have a file "frotz" with exactly the same contents, I thought you wouldn't even have to have that same contents repeated in the export datastream. Or am I (again) entirely misunderstanding the intended use case? ``` ## Elijah Newren, 2009-03-26 04:02 Subject: Re: [PATCH] fast-export: Avoid dropping files from commits Message-ID: <51419b2c0903252102v71de617aiffad2d4934d4d07f@mail.gmail.com> URL: https://gitlist.dev/e/51419b2c0903252102v71de617aiffad2d4934d4d07f%40mail.gmail.com In-Reply-To: <7vy6utt0op.fsf@gitster.siamese.dyndns.org> ``` Hi, On Wed, Mar 25, 2009 at 9:36 PM, Junio C Hamano wrote: > Hmm, does it? > > Shouldn't an export with a bottom commit be always considered an > incremental? Why special case --import-marks? > > When you say "I want to export master~2..master", isn't the intention > (unstated, because it is too obvious) that follows it "... because I do > have master~2 already and I would want to replay the export on top of that > state"? If all of master~2, master~1 and master have a file "frotz" with > exactly the same contents, I thought you wouldn't even have to have that > same contents repeated in the export datastream. No, without the --import-marks it doesn't make sense to make it an incremental. Think of it this way: git-fast-export converts all sha1's into integer identifiers (marks). When you export master~2..master, then when fast-export comes across master~1, it notices that there's nothing to convert it's parent sha1sum into since no information was output for master~2 in the export stream. The way around it would be to simply export master~2 anyway, and master~3, and... (An alternate way around it, I guess, would be using the sha1sum in the export stream instead of a mark...but do you have any way of knowing whether the destination repository you import into will have that commit? Using --import-marks seems like a nice way to specify whether you have that information or not.) It's perhaps a gotcha, as I know I had the same initial assumption even after reading the manpage. Perhaps Documentation/git-fast-export.txt could be made a bit clearer on this point. > Or am I (again) entirely misunderstanding the intended use case? Exporting master~2..master without specifying --import-marks is a way of squashing history, essentially. Perhaps you don't like it serving this purpose, but it was intentional; looking at the master~2..master testcase in t/t9301-fast-export.sh, you'll see git fast-export master~2..master | sed "s/master/partial/" | && test_must_fail git rev-parse partial~2 ```