Re: [PATCH v2 2/3] git-p4: Search for parent commit on branch creation

From: Junio C Hamano <gitster@pobox.com>
Date: Jan 21, 2012, 04:55 UTC
Message-ID: <7v7h0ld58z.fsf@alter.siamese.dyndns.org>
In-Reply-To: <1327105292-30092-3-git-send-email-vitor.hda@gmail.com>

Vitor Antunes <vitor.hda@gmail.com> writes:

Show 8 quoted lines

> 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@gmail.com>
> ---

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.

Show 19 quoted lines

> @@ -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.