Bug: git-p4 can skip changes when syncing large from multiple depot paths

Bug: git-p4 can skip changes when syncing large from multiple depot paths

[James Farwell]

Reproduction Steps:

1. Have a git repo cloned from a perforce repo using multiple depot paths (e.g. //depot/foo/ and //depot/bar/).
2. Add changes to the perforce repo in both depot paths. (e.g. 5 changes in each)
2. Do a "git p4 sync --changes_block_size n" where n is smaller than the number of changes applied to each depot path. (e.g. 2)


Expected Behavior:

All changes should sync and become commits in the git repo.


Actual Behavior:

All changes from the first depot path (if any) sync. After that only a small subset of changes from the remaining depot paths sync, causing some changes to be skipped entirely.


Best Guess:

I believe this was introduced in commit 1051ef00636357061d72bcf673da86054fb14a12. The important code in question is the p4ChangesForPaths function, which contains a for loop that iterates over the depot paths, which then contains a while loop which iterates over the blocks. This change modified the inner while loop so that with every iteration it modifies changeStart, which causes the original value of changeStart to be lost. The first iteration of the for loop will correctly iterate across all the blocks until changeStart is within one block of the last change number, but then all subsequent iterations of the for loop will use that final changeStart value, causing any changes in those depot paths in earlier blocks to be skipped.

This can probably be easily remedied by using a temporary "start" variable for the block iteration, much like there is already a temporary "end" variable, and resetting it to the value of changeStart at the top of the for loop. (Note: this appears to be how the code prior to 1051ef00636357061d72bcf673da86054fb14a12 functioned).


Thanks!
- James

Re: Bug: git-p4 can skip changes when syncing large from multiple depot paths

[Luke Diamand]

On 12 December 2015 at 02:52, James Farwell <jfarwell@vmware.com> wrote:
>
> Reproduction Steps:
>
> 1. Have a git repo cloned from a perforce repo using multiple depot paths (e.g. //depot/foo/ and //depot/bar/).
> 2. Add changes to the perforce repo in both depot paths. (e.g. 5 changes in each)
> 2. Do a "git p4 sync --changes_block_size n" where n is smaller than the number of changes applied to each depot path. (e.g. 2)

Thanks - this is definitely a bug. Let me see if I can work up a
testcase for it and a fix.

Luke