Re: [PATCH v2] git-p4: Fix multi-path changelist empty commits

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



George Vanburgh <george@xxxxxxxxxxx> writes:

> From: George Vanburgh <gvanburgh@xxxxxxxxxxxxx>
>
> When importing from multiple perforce paths - we may attempt to import
> a changelist that contains files from two (or more) of these depot
> paths. Currently, this results in multiple git commits - one
> containing the changes, and the other(s) as empty commit(s). This
> behavior was introduced in commit 1f90a64
> ("git-p4: reduce number of server queries for fetches", 2015-12-19).
>
> Reproduction Steps:
>
> 1. Have a git repo cloned from a perforce repo using multiple depot
> paths (e.g. //depot/foo and //depot/bar).
> 2. Submit a single change to the perforce repo that makes changes in
> both //depot/foo and //depot/bar.
> 3. Run "git p4 sync" to sync the change from #2.
>
> Change is synced as multiple commits, one for each depot path that was
> affected.
>
> Using a set, instead of a list inside p4ChangesForPaths() ensures that
> each changelist is unique to the returned list, and therefore only a
> single commit is generated for each changelist.
>
> Reported-by: James Farwell <jfarwell@xxxxxxxxxx>
> Signed-off-by: George Vanburgh <gvanburgh@xxxxxxxxxxxxx>
> ---

Thanks, George.  Luke, can I add your "Reviewed-by:" here?

>  git-p4.py               |  4 ++--
>  t/t9800-git-p4-basic.sh | 22 +++++++++++++++++++++-
>  2 files changed, 23 insertions(+), 3 deletions(-)
>
> diff --git a/git-p4.py b/git-p4.py
> index fd5ca52..6307bc8 100755
> --- a/git-p4.py
> +++ b/git-p4.py
> @@ -822,7 +822,7 @@ def p4ChangesForPaths(depotPaths, changeRange, requestedBlockSize):
>                  die("cannot use --changes-block-size with non-numeric revisions")
>              block_size = None
>  
> -    changes = []
> +    changes = set()
>  
>      # Retrieve changes a block at a time, to prevent running
>      # into a MaxResults/MaxScanRows error from the server.
> @@ -841,7 +841,7 @@ def p4ChangesForPaths(depotPaths, changeRange, requestedBlockSize):
>  
>          # Insert changes in chronological order
>          for line in reversed(p4_read_pipe_lines(cmd)):
> -            changes.append(int(line.split(" ")[1]))
> +            changes.add(int(line.split(" ")[1]))
>  
>          if not block_size:
>              break
> diff --git a/t/t9800-git-p4-basic.sh b/t/t9800-git-p4-basic.sh
> index 0730f18..4d93522 100755
> --- a/t/t9800-git-p4-basic.sh
> +++ b/t/t9800-git-p4-basic.sh
> @@ -131,6 +131,26 @@ test_expect_success 'clone two dirs, @all, conflicting files' '
>  	)
>  '
>  
> +test_expect_success 'clone two dirs, each edited by submit, single git commit' '
> +	(
> +		cd "$cli" &&
> +		echo sub1/f4 >sub1/f4 &&
> +		p4 add sub1/f4 &&
> +		echo sub2/f4 >sub2/f4 &&
> +		p4 add sub2/f4 &&
> +		p4 submit -d "sub1/f4 and sub2/f4"
> +	) &&
> +	git p4 clone --dest="$git" //depot/sub1@all //depot/sub2@all &&
> +	test_when_finished cleanup_git &&
> +	(
> +		cd "$git" &&
> +		git ls-files >lines &&
> +		test_line_count = 4 lines &&
> +		git log --oneline p4/master >lines &&
> +		test_line_count = 5 lines
> +	)
> +'
> +
>  revision_ranges="2000/01/01,#head \
>  		 1,2080/01/01 \
>  		 2000/01/01,2080/01/01 \
> @@ -147,7 +167,7 @@ test_expect_success 'clone using non-numeric revision ranges' '
>  		(
>  			cd "$git" &&
>  			git ls-files >lines &&
> -			test_line_count = 6 lines
> +			test_line_count = 8 lines
>  		)
>  	done
>  '
>
> --
> https://github.com/git/git/pull/311



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]