Re: [PATCH] git-p4: use "p4 fstat" to interpret View setting

Re: [PATCH] git-p4: use "p4 fstat" to interpret View setting

[Pete Wyckoff]

ksaitoh560@gmail.com wrote on Fri, 02 Aug 2013 17:02 +0900:
> I trying clone Perforce project and I found git-p4. It's a great tool!
> 
> And I don't know how to exclude special extension file in a directory?
> (Practically, I want to exclude Excel files at git p4 clone/sync.)
> 
> In Perforce, View setting of p4 client can describe
>   -//depot/project/files/*.xls //client/project/files/*.xls
> to exclude Excel files.
> But "git p4 --use-client-spec" cannot support '*'.
> 
> In git-p4.py, "map_in_client" method analyzes View setting and return
> client file path.
> So I modify the method to just use p4 command, "p4 fstat -T clientFile".
> 
> I'd like to know your opinions about that and what you think about the
> suggestion.

This is either incredibly brilliant or fundamentally broken.  I'm
hoping for the former.  :)

Your theory is:  there is a client spec, and p4 knows how to
interpret these things, so instead of figuring out and
implementing the algorithms for %% and * and ... in git-p4, just
ask p4 directly.

Let me play with this for a bit.  I wonder about the performance
aspects of doing a "p4 fstat" for every file.  Would it be
possible to do one or a few batch queries higher up somewhere?

A few nit-picky questions below, just on the test bits, while I
play with your code.

> diff --git a/t/lib-git-p4.sh b/t/lib-git-p4.sh
> index 2098b9b..fbda1ad 100644
> --- a/t/lib-git-p4.sh
> +++ b/t/lib-git-p4.sh
> @@ -48,6 +48,9 @@ P4DPORT=$((10669 + ($testid - $git_p4_test_start)))
>  P4PORT=localhost:$P4DPORT
>  P4CLIENT=client
>  P4EDITOR=:
> +P4USER=""
> +P4PASS=""
> +P4CHARSET=""

Why do you need these?

> diff --git a/t/t9801-git-p4-branch.sh b/t/t9801-git-p4-branch.sh
> index 2bf142d..b97bdda 100755
> --- a/t/t9801-git-p4-branch.sh
> +++ b/t/t9801-git-p4-branch.sh
> @@ -480,6 +480,7 @@ test_expect_success 'use-client-spec detect-branches
> skips branches setup' '
>  test_expect_success 'use-client-spec detect-branches skips branches' '
>   client_view "//depot/usecs/... //client/..." \
>      "-//depot/usecs/b3/... //client/b3/..." &&
> +    ( p4 sync ) &&
>   test_when_finished cleanup_git &&

How does the p4 sync help here?

> +test_expect_success 'view wildcard *' '
> + client_view "//depot/... //client/..." \
> + "-//depot/dir1/*.junk //client/dir1/*.junk" \
> + "-//depot/dir2/*.junk //client/dir2/*.junk" &&
> + (p4 sync ) &&
> + files="dir1/file11 dir1/file12 dir2/file21 dir2/file22" &&
> + client_verify $files &&
> + git p4 clone --use-client-spec --dest="$git" //depot &&
> + git_verify $files
> +'

It works!  Cool.

		-- Pete

Re: [PATCH] git-p4: use "p4 fstat" to interpret View setting

[kazuki saitoh]

Hi, Pete
Thank you reply.

> Your theory is:  there is a client spec, and p4 knows how to
> interpret these things, so instead of figuring out and
> implementing the algorithms for %% and * and ... in git-p4, just
> ask p4 directly.
That's right.
It is simple way to get my purpose unless break existing function
And passed all existing test.

> Let me play with this for a bit.  I wonder about the performance
> aspects of doing a "p4 fstat" for every file.  Would it be
> possible to do one or a few batch queries higher up somewhere?
Oh yes, thank you advice.
My patch is quick hack to get file info absolutely, but now, I found
directory unit query (e.g.   p4 fstat -T clientFile
//depot/project/... )
Better way is that use directory unit query  then caching.
I will try it.


> > +P4USER=""
> > +P4PASS=""
> > +P4CHARSET=""
>
> Why do you need these?
My environment set P4CHARSET=utf8, but "start_p4d" is failed.
So I undefined P4CHARSET and then passed.
I don't understand source cause clearly, perhaps only Japanese
environment problem.
How about other environment?

P4USER, P4PASS description is no need, just it make sure. I will remove, sorry.


> > +    ( p4 sync ) &&
> >   test_when_finished cleanup_git &&
>
> How does the p4 sync help here?
If remove this line, some test is failed.
Because sometime "p4 fstat" return wrong result after modified p4
client View setting.

I don't understand Perforce behavior clearly.
Maybe after modify View, something commend is need to sync server side
and client side.

This is week point of my suggestion, but I think this situation is rare.