Hi Junio,
On Sat, Jan 21, 2012 at 4:55 AM, Junio C Hamano <gitster@xxxxxxxxx> wrote:
> Vitor Antunes <vitor.hda@xxxxxxxxx> writes:
>
>> A fast-import "checkpoint" call is required for each comparison because
>> diff-tree is only able to work with blobs on disk. But most of these
>> commits will not be part of the final imported tree, making fast-import
>> fail. To avoid this, the temporary branches are tracked and then removed
>> at the end of the import process.
>>
>> Signed-off-by: Vitor Antunes <vitor.hda@xxxxxxxxx>
>> ---
>
> It might make sense to squash 1/3 with this patch; the definition alone
> without any actual user would not be very useful and won't help hunting
> for breakages, if exists, in the future.
Makes sense. WIll correct as you suggest in v3.
>> @@ -2012,7 +2014,27 @@ class P4Sync(Command, P4UserMap):
>> parent = self.initialParents[branch]
>> del self.initialParents[branch]
>>
>> - self.commit(description, filesForCommit, branch, [branchPrefix], parent)
>> + parentFound = False
>> + if len(parent) > 0:
>> + tempBranch = os.path.join(self.tempBranchLocation, "%d" % (change))
>> + if self.verbose:
>> + print "Creating temporary branch: " + tempBranch
>> + self.commit(description, filesForCommit, tempBranch, [branchPrefix])
>> + self.tempBranches.append(tempBranch)
>> + self.checkpoint()
>> + for blob in read_pipe_lines("git rev-list --reverse --no-merges %s" % parent):
>> + blob = blob.strip()
>> + if len( read_pipe("git diff-tree %s %s" % (blob, tempBranch)) ) == 0:
>> + parentFound = True
>> + if self.verbose:
>> + print "Found parent of %s in commit %s" % (branch, blob)
>
> ... also this looks excessively deeply nested, which is a sign that it
> might be a good idea to refactor this part into a separate helper function
> or something.
I have to agree with you. Since v3 is required, I'll also include this
fix along.
Thanks,
Vitor
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html