mbox series

[00/13] git-p4: python3 compatibility

Message ID 20191207003333.3228-1-yang.zhao@skyboxlabs.com (mailing list archive)
Headers show
Series git-p4: python3 compatibility | expand

Message

Yang Zhao Dec. 7, 2019, 12:33 a.m. UTC
This patchset adds python3 compatibility to git-p4.

While some clean-up refactoring would have been nice, I specifically avoided
making any major changes to the internal API, aiming to have passing tests
with as few changes as possible.

CI results can be seen from this GitHub PR: https://github.com/git/git/pull/673

(As of writing, the CI pipelines are intermittently failing due to reasons
that appear unrelated to code. I do have python3 tests passing locally on
a Gentoo host.)

Yang Zhao (13):
  ci: also run linux-gcc pipeline with python-3.7 environment
  git-p4: make python-2.7 the oldest supported version
  git-p4: simplify python version detection
  git-p4: decode response from p4 to str for python3
  git-p4: properly encode/decode communication with git for python 3
  git-p4: convert path to unicode before processing them
  git-p4: open .gitp4-usercache.txt in text mode
  git-p4: use marshal format version 2 when sending to p4
  git-p4: fix freezing while waiting for fast-import progress
  git-p4: use functools.reduce instead of reduce
  git-p4: use dict.items() iteration for python3 compatibility
  git-p4: simplify regex pattern generation for parsing diff-tree
  git-p4: use python3's input() everywhere

 azure-pipelines.yml |  11 +++
 git-p4.py           | 195 ++++++++++++++++++++++++++++----------------
 2 files changed, 136 insertions(+), 70 deletions(-)

Comments

Denton Liu Dec. 7, 2019, 1:09 a.m. UTC | #1
Hi Yang,

On Fri, Dec 06, 2019 at 04:33:18PM -0800, Yang Zhao wrote:
> This patchset adds python3 compatibility to git-p4.
> 
> While some clean-up refactoring would have been nice, I specifically avoided
> making any major changes to the internal API, aiming to have passing tests
> with as few changes as possible.
> 
> CI results can be seen from this GitHub PR: https://github.com/git/git/pull/673
> 
> (As of writing, the CI pipelines are intermittently failing due to reasons
> that appear unrelated to code. I do have python3 tests passing locally on
> a Gentoo host.)

Currently, there's a competing effort to do the same thing[1] by Ben
Keene (CC'd). Like the last time[2] two competing topics arose at the
same time, I'm going to make the same suggestion.

Would it be possible for both of you to join forces?

Thanks,

Denton

[1]: https://lore.kernel.org/git/pull.463.v4.git.1575498577.gitgitgadget@gmail.com/
[2]: https://lore.kernel.org/git/xmqq5zs7oexn.fsf@gitster-ct.c.googlers.com/
Yang Zhao Dec. 7, 2019, 7:29 a.m. UTC | #2
On Fri, Dec 6, 2019 at 5:09 PM Denton Liu <liu.denton@gmail.com> wrote:
> On Fri, Dec 06, 2019 at 04:33:18PM -0800, Yang Zhao wrote:
> > This patchset adds python3 compatibility to git-p4.
> > ...
>
> Currently, there's a competing effort to do the same thing[1] by Ben
> Keene (CC'd). Like the last time[2] two competing topics arose at the
> same time, I'm going to make the same suggestion.
>
> Would it be possible for both of you to join forces?

Yes, I do believe we are aware of each other's efforts. I had submitted
an RFC patch set around the time Ben was preparing his own patchset.

I have not reviewed Ben's first patchset as I did not feel that I understood
the systems well enough at the time. I've briefly skimmed through Ben's latest
iteration and it would appear the general approach is very similar, but there's
more added abstractions and just general code change in his version.

Regardless, I'm open to working together.

Ideally, I would prefer we land something minimal and working in mainline soon,
then further collaborate on changes that clean up code and enable more features.

My end-game is to have P4 Streams working in git-p4, and maybe LFS-like support
that uses p4 as the backend. It would be great to not be the only one
spending effort
in that direction.

Yang
Yang Zhao Dec. 7, 2019, 7:34 a.m. UTC | #3
On Fri, Dec 6, 2019 at 4:33 PM Yang Zhao <yang.zhao@skyboxlabs.com> wrote:
> This patchset adds python3 compatibility to git-p4.
>
> While some clean-up refactoring would have been nice, I specifically avoided
> making any major changes to the internal API, aiming to have passing tests
> with as few changes as possible.
>
> CI results can be seen from this GitHub PR: https://github.com/git/git/pull/673

Looks like p4 LFS tests are failing for python3.  Looks like it's just
more bytes vs str.

Will have to enable LFS tests in my own environment.
Ben Keene Dec. 7, 2019, 4:21 p.m. UTC | #4
On 12/7/2019 2:29 AM, Yang Zhao wrote:
> On Fri, Dec 6, 2019 at 5:09 PM Denton Liu <liu.denton@gmail.com> wrote:
>> On Fri, Dec 06, 2019 at 04:33:18PM -0800, Yang Zhao wrote:
>>> This patchset adds python3 compatibility to git-p4.
>>> ...
>> Currently, there's a competing effort to do the same thing[1] by Ben
>> Keene (CC'd). Like the last time[2] two competing topics arose at the
>> same time, I'm going to make the same suggestion.
>>
>> Would it be possible for both of you to join forces?
> Yes, I do believe we are aware of each other's efforts. I had submitted
> an RFC patch set around the time Ben was preparing his own patchset.
> I have not reviewed Ben's first patchset as I did not feel that I understood
> the systems well enough at the time. I've briefly skimmed through Ben's latest
> iteration and it would appear the general approach is very similar, but there's
> more added abstractions and just general code change in his version.
>
> Regardless, I'm open to working together.

I am also open to working together, and could really use the help, as I'm
not a python developer.

I have taken all the suggestions from my first patch set and have reworked
my code and commits and will submit them now for review.  With the smaller
patches and cleaner commit messages I hope that it will make it easier
to see what I've done so far and what is still open work.

> Ideally, I would prefer we land something minimal and working in mainline soon,
> then further collaborate on changes that clean up code and enable more features.
>
> My end-game is to have P4 Streams working in git-p4, and maybe LFS-like support
> that uses p4 as the backend. It would be great to not be the only one
> spending effort
> in that direction.
>
> Yang


I have similar goals.  I would love to get the smallest set of non-breaking
changes in that allows the program to basically work with Python 3.5+.

My rush has been because I need to use git-p4 for work and have been 
working
on the project at the office.  Once I reach a point where I am able to
generally work (when t9800 is complete) I'll really not be free to spend 
too
much work time on the project, but I am eager to see this through!

As far as status, the last time I ran tests, python 2.7 passed all the tests
and Python 3.5 passed some of the tests.  I know it is not passing t9801
at this time and I'm trying to find out why.

So, Yang, I am very interested in working together.


Kindest regards,

Ben Keene
Yang Zhao Dec. 7, 2019, 7:59 p.m. UTC | #5
On Sat, Dec 7, 2019 at 8:21 AM Ben Keene <seraphire@gmail.com> wrote:
> On 12/7/2019 2:29 AM, Yang Zhao wrote:
> > Ideally, I would prefer we land something minimal and working in mainline soon,
> > then further collaborate on changes that clean up code and enable more features.
> >
> > My end-game is to have P4 Streams working in git-p4, and maybe LFS-like support
> > that uses p4 as the backend. It would be great to not be the only one
> > spending effort
> > in that direction.
>
> I have similar goals.  I would love to get the smallest set of non-breaking
> changes in that allows the program to basically work with Python 3.5+.
>
> My rush has been because I need to use git-p4 for work and have been
> working
> on the project at the office.  Once I reach a point where I am able to
> generally work (when t9800 is complete) I'll really not be free to spend
> too
> much work time on the project, but I am eager to see this through!

I'm in a similar situation, but we use p4 Streams and so I actually need further
development before being able to make a full switch. I am given more liberty
in terms of how much work time I can dedicate to this, though.

Given the situation, can you give my patch set a try in your work environment?
It is currently passing everything except t9824-git-p4-git-lfs.

If you're OK with it, I would prefer that we work from my version as a base and
add some of your quality-of-life enhancements on top. I can do the merges myself
if you are pressed for time.

Thanks,
Yang
Ben Keene Dec. 9, 2019, 3:03 p.m. UTC | #6
On 12/7/2019 2:59 PM, Yang Zhao wrote:
> On Sat, Dec 7, 2019 at 8:21 AM Ben Keene <seraphire@gmail.com> wrote:
>> On 12/7/2019 2:29 AM, Yang Zhao wrote:
>>> Ideally, I would prefer we land something minimal and working in mainline soon,
>>> then further collaborate on changes that clean up code and enable more features.
>>>
>>> My end-game is to have P4 Streams working in git-p4, and maybe LFS-like support
>>> that uses p4 as the backend. It would be great to not be the only one
>>> spending effort
>>> in that direction.
>> I have similar goals.  I would love to get the smallest set of non-breaking
>> changes in that allows the program to basically work with Python 3.5+.
>>
>> My rush has been because I need to use git-p4 for work and have been
>> working
>> on the project at the office.  Once I reach a point where I am able to
>> generally work (when t9800 is complete) I'll really not be free to spend
>> too
>> much work time on the project, but I am eager to see this through!
> I'm in a similar situation, but we use p4 Streams and so I actually need further
> development before being able to make a full switch. I am given more liberty
> in terms of how much work time I can dedicate to this, though.
>
> Given the situation, can you give my patch set a try in your work environment?
> It is currently passing everything except t9824-git-p4-git-lfs.

I downloaded your code and it looks like it works for Python 2.7.  I'm 
seeing errors with the following tests:

* 9816.5

     Traceback (most recent call last):
     File "/home/bkeene/git/git-p4", line 4227, in <module>
         main()
     File "/home/bkeene/git/git-p4", line 4221, in main
         if not cmd.run(args):
     File "/home/bkeene/git/git-p4", line 2381, in run
         ok = self.applyCommit(commit)
     File "/home/bkeene/git/git-p4", line 2106, in applyCommit
         p4_write_pipe(['submit', '-i'], submitTemplate)
     File "/home/bkeene/git/git-p4", line 207, in p4_write_pipe
         return write_pipe(real_cmd, stdin)
     File "/home/bkeene/git/git-p4", line 201, in write_pipe
         die('Command failed: %s' % str(c))
     File "/home/bkeene/git/git-p4", line 158, in die
         raise Exception(msg)
     Exception: Command failed: ['p4', '-r', '3', 'submit', '-i']

* 9816.6

     Traceback (most recent call last):
     File "/home/bkeene/git/git-p4", line 4227, in <module>
         main()
     File "/home/bkeene/git/git-p4", line 4221, in main
         if not cmd.run(args):
     File "/home/bkeene/git/git-p4", line 2381, in run
         ok = self.applyCommit(commit)
     File "/home/bkeene/git/git-p4", line 2106, in applyCommit
         p4_write_pipe(['submit', '-i'], submitTemplate)
     File "/home/bkeene/git/git-p4", line 207, in p4_write_pipe
         return write_pipe(real_cmd, stdin)
     File "/home/bkeene/git/git-p4", line 201, in write_pipe
         die('Command failed: %s' % str(c))
     File "/home/bkeene/git/git-p4", line 158, in die
         raise Exception(msg)
     Exception: Command failed: ['p4', '-r', '3', 'submit', '-i']

* 9816.7

  Traceback (most recent call last):
    File "/home/bkeene/git/git-p4", line 4227, in <module>
      main()
    File "/home/bkeene/git/git-p4", line 4221, in main
      if not cmd.run(args):
    File "/home/bkeene/git/git-p4", line 2381, in run
      ok = self.applyCommit(commit)
    File "/home/bkeene/git/git-p4", line 2106, in applyCommit
      p4_write_pipe(['submit', '-i'], submitTemplate)
    File "/home/bkeene/git/git-p4", line 207, in p4_write_pipe
      return write_pipe(real_cmd, stdin)
    File "/home/bkeene/git/git-p4", line 201, in write_pipe
      die('Command failed: %s' % str(c))
    File "/home/bkeene/git/git-p4", line 158, in die
      raise Exception(msg)
  Exception: Command failed: ['p4', '-r', '3', 'submit', '-i']

* 9816.9

     Traceback (most recent call last):
     File "/home/bkeene/git/git-p4", line 4227, in <module>
         main()
     File "/home/bkeene/git/git-p4", line 4221, in main
         if not cmd.run(args):
     File "/home/bkeene/git/git-p4", line 2381, in run
         ok = self.applyCommit(commit)
     File "/home/bkeene/git/git-p4", line 2106, in applyCommit
         p4_write_pipe(['submit', '-i'], submitTemplate)
     File "/home/bkeene/git/git-p4", line 207, in p4_write_pipe
         return write_pipe(real_cmd, stdin)
     File "/home/bkeene/git/git-p4", line 201, in write_pipe
         die('Command failed: %s' % str(c))
     File "/home/bkeene/git/git-p4", line 158, in die
         raise Exception(msg)
     Exception: Command failed: ['p4', '-r', '3', 'submit', '-i']

* 9810.16

     Traceback (most recent call last):
     File "/home/bkeene/git/git-p4", line 4227, in <module>
         main()
     File "/home/bkeene/git/git-p4", line 4221, in main
         if not cmd.run(args):
     File "/home/bkeene/git/git-p4", line 2436, in run
         rebase.rebase()
     File "/home/bkeene/git/git-p4", line 3913, in rebase
         system("git rebase %s" % upstream)
     File "/home/bkeene/git/git-p4", line 305, in system
         raise CalledProcessError(retcode, cmd)
     subprocess.CalledProcessError: Command 'git rebase 
remotes/p4/master' returned non-zero exit status 1

The last test was a breaking test that stopped the test make.


> If you're OK with it, I would prefer that we work from my version as a base and
> add some of your quality-of-life enhancements on top. I can do the merges myself
> if you are pressed for time.
>
> Thanks,
> Yang

I am not a Python developer and my code is further behind than yours, so
it makes complete sense to use yours as the base.
Ben Keene Dec. 9, 2019, 6:54 p.m. UTC | #7
On 12/9/2019 10:03 AM, Ben Keene wrote:
>
> On 12/7/2019 2:59 PM, Yang Zhao wrote:
>> On Sat, Dec 7, 2019 at 8:21 AM Ben Keene <seraphire@gmail.com> wrote:
>>> On 12/7/2019 2:29 AM, Yang Zhao wrote:
>>>> Ideally, I would prefer we land something minimal and working in 
>>>> mainline soon,
>>>> then further collaborate on changes that clean up code and enable 
>>>> more features.
>>>>
>>>> My end-game is to have P4 Streams working in git-p4, and maybe 
>>>> LFS-like support
>>>> that uses p4 as the backend. It would be great to not be the only one
>>>> spending effort
>>>> in that direction.
>>> I have similar goals.  I would love to get the smallest set of 
>>> non-breaking
>>> changes in that allows the program to basically work with Python 3.5+.
>>>
>>> My rush has been because I need to use git-p4 for work and have been
>>> working
>>> on the project at the office.  Once I reach a point where I am able to
>>> generally work (when t9800 is complete) I'll really not be free to 
>>> spend
>>> too
>>> much work time on the project, but I am eager to see this through!
>> I'm in a similar situation, but we use p4 Streams and so I actually 
>> need further
>> development before being able to make a full switch. I am given more 
>> liberty
>> in terms of how much work time I can dedicate to this, though.
>>
>> Given the situation, can you give my patch set a try in your work 
>> environment?
>> It is currently passing everything except t9824-git-p4-git-lfs.
>
> I downloaded your code and it looks like it works for Python 2.7. I'm 
> seeing errors with the following tests:
>
> * 9816.5
>
>     Traceback (most recent call last):
>     File "/home/bkeene/git/git-p4", line 4227, in <module>
>         main()
>     File "/home/bkeene/git/git-p4", line 4221, in main
>         if not cmd.run(args):
>     File "/home/bkeene/git/git-p4", line 2381, in run
>         ok = self.applyCommit(commit)
>     File "/home/bkeene/git/git-p4", line 2106, in applyCommit
>         p4_write_pipe(['submit', '-i'], submitTemplate)
>     File "/home/bkeene/git/git-p4", line 207, in p4_write_pipe
>         return write_pipe(real_cmd, stdin)
>     File "/home/bkeene/git/git-p4", line 201, in write_pipe
>         die('Command failed: %s' % str(c))
>     File "/home/bkeene/git/git-p4", line 158, in die
>         raise Exception(msg)
>     Exception: Command failed: ['p4', '-r', '3', 'submit', '-i']
>
> * 9816.6
>
>     Traceback (most recent call last):
>     File "/home/bkeene/git/git-p4", line 4227, in <module>
>         main()
>     File "/home/bkeene/git/git-p4", line 4221, in main
>         if not cmd.run(args):
>     File "/home/bkeene/git/git-p4", line 2381, in run
>         ok = self.applyCommit(commit)
>     File "/home/bkeene/git/git-p4", line 2106, in applyCommit
>         p4_write_pipe(['submit', '-i'], submitTemplate)
>     File "/home/bkeene/git/git-p4", line 207, in p4_write_pipe
>         return write_pipe(real_cmd, stdin)
>     File "/home/bkeene/git/git-p4", line 201, in write_pipe
>         die('Command failed: %s' % str(c))
>     File "/home/bkeene/git/git-p4", line 158, in die
>         raise Exception(msg)
>     Exception: Command failed: ['p4', '-r', '3', 'submit', '-i']
>
> * 9816.7
>
>  Traceback (most recent call last):
>    File "/home/bkeene/git/git-p4", line 4227, in <module>
>      main()
>    File "/home/bkeene/git/git-p4", line 4221, in main
>      if not cmd.run(args):
>    File "/home/bkeene/git/git-p4", line 2381, in run
>      ok = self.applyCommit(commit)
>    File "/home/bkeene/git/git-p4", line 2106, in applyCommit
>      p4_write_pipe(['submit', '-i'], submitTemplate)
>    File "/home/bkeene/git/git-p4", line 207, in p4_write_pipe
>      return write_pipe(real_cmd, stdin)
>    File "/home/bkeene/git/git-p4", line 201, in write_pipe
>      die('Command failed: %s' % str(c))
>    File "/home/bkeene/git/git-p4", line 158, in die
>      raise Exception(msg)
>  Exception: Command failed: ['p4', '-r', '3', 'submit', '-i']
>
> * 9816.9
>
>     Traceback (most recent call last):
>     File "/home/bkeene/git/git-p4", line 4227, in <module>
>         main()
>     File "/home/bkeene/git/git-p4", line 4221, in main
>         if not cmd.run(args):
>     File "/home/bkeene/git/git-p4", line 2381, in run
>         ok = self.applyCommit(commit)
>     File "/home/bkeene/git/git-p4", line 2106, in applyCommit
>         p4_write_pipe(['submit', '-i'], submitTemplate)
>     File "/home/bkeene/git/git-p4", line 207, in p4_write_pipe
>         return write_pipe(real_cmd, stdin)
>     File "/home/bkeene/git/git-p4", line 201, in write_pipe
>         die('Command failed: %s' % str(c))
>     File "/home/bkeene/git/git-p4", line 158, in die
>         raise Exception(msg)
>     Exception: Command failed: ['p4', '-r', '3', 'submit', '-i']
>
> * 9810.16
>
>     Traceback (most recent call last):
>     File "/home/bkeene/git/git-p4", line 4227, in <module>
>         main()
>     File "/home/bkeene/git/git-p4", line 4221, in main
>         if not cmd.run(args):
>     File "/home/bkeene/git/git-p4", line 2436, in run
>         rebase.rebase()
>     File "/home/bkeene/git/git-p4", line 3913, in rebase
>         system("git rebase %s" % upstream)
>     File "/home/bkeene/git/git-p4", line 305, in system
>         raise CalledProcessError(retcode, cmd)
>     subprocess.CalledProcessError: Command 'git rebase 
> remotes/p4/master' returned non-zero exit status 1
>
> The last test was a breaking test that stopped the test make.
>
>
>> If you're OK with it, I would prefer that we work from my version as 
>> a base and
>> add some of your quality-of-life enhancements on top. I can do the 
>> merges myself
>> if you are pressed for time.
>>
>> Thanks,
>> Yang
>
> I am not a Python developer and my code is further behind than yours, so
> it makes complete sense to use yours as the base.


So, I just attempted to run a base case on windows: git p4 clone //depot 
and I'm getting an error:

Depot paths must start with "//": /depot
Johannes Schindelin Dec. 9, 2019, 7:48 p.m. UTC | #8
Hi Ben,

On Mon, 9 Dec 2019, Ben Keene wrote:

> So, I just attempted to run a base case on windows: git p4 clone //depot and
> I'm getting an error:
>
> Depot paths must start with "//": /depot

You started this in a Bash, right?

The Git Bash has the very specific problem that many of Git's shell
scripts assume that forward slashes are directory separators, not
backslashes, and that absolute paths start with a single forward slash. In
other words, they expect Unix paths.

But we're on Windows! So the MSYS2 runtime (which is the POSIX emulation
layer derived from Cygwin which allows us to build and run Bash on
Windows) "translates" between the paths. For example, if you pass `/depot`
as a parameter to a Git command, the MSYS2 runtime notices that `git.exe`
is not an MSYS2 program (i.e. it does not understand pseudo-Unix paths),
and translates the path to `C:/Program Files/Git/depot`.

However, your call has _two_ slashes, right? That is unfortunately MSYS2's
trick to say "oh BTW keep the slash, this is not a Unix path".

To avoid this, just set `MSYS_NO_PATHCONV`, like so:

	MSYS_NO_PATHCONV=1 git p4 clone //depot

This behavior is documented in our release notes, by the way:
https://github.com/git-for-windows/build-extra/blob/master/ReleaseNotes.md#known-issues

Ciao,
Johannes
Yang Zhao Dec. 9, 2019, 8:21 p.m. UTC | #9
On Mon, Dec 9, 2019 at 7:03 AM Ben Keene <seraphire@gmail.com> wrote:
> I downloaded your code and it looks like it works for Python 2.7.  I'm
> seeing errors with the following tests:
>
> * 9816.5
> ...

I'm not sure why those would fail for your local environment but not in CI.

I've just pushed an updated PR to GitHub which is now passing all
tests on python-3.5. Give that a go.

If tests are still failing for you, it'd be good to get the verbose
output from the specific test scripts. They don't tell us much without
it.

e.g.:
  ~/git.git/t $ : ./t9816-git-p4-locked.sh --verbose

Thanks,
Yang
Ben Keene Dec. 10, 2019, 2:20 p.m. UTC | #10
On 12/9/2019 2:48 PM, Johannes Schindelin wrote:
> Hi Ben,
>
> On Mon, 9 Dec 2019, Ben Keene wrote:
>
>> So, I just attempted to run a base case on windows: git p4 clone //depot and
>> I'm getting an error:
>>
>> Depot paths must start with "//": /depot
> You started this in a Bash, right?
No, I started it from a windows command cmd.exe prompt.  (I almost never 
use the bash prompt)
>
> The Git Bash has the very specific problem that many of Git's shell
> scripts assume that forward slashes are directory separators, not
> backslashes, and that absolute paths start with a single forward slash. In
> other words, they expect Unix paths.
>
> But we're on Windows! So the MSYS2 runtime (which is the POSIX emulation
> layer derived from Cygwin which allows us to build and run Bash on
> Windows) "translates" between the paths. For example, if you pass `/depot`
> as a parameter to a Git command, the MSYS2 runtime notices that `git.exe`
> is not an MSYS2 program (i.e. it does not understand pseudo-Unix paths),
> and translates the path to `C:/Program Files/Git/depot`.
That is good to know!
>
> However, your call has _two_ slashes, right? That is unfortunately MSYS2's
> trick to say "oh BTW keep the slash, this is not a Unix path".
>
> To avoid this, just set `MSYS_NO_PATHCONV`, like so:

When I first installed git, I didn't read the release notes. (Shame on 
me!) and I installed
python for windows and added an alias for git-p4.py against the windows 
version of
python, so when I run git, it's not performing that conversion.

> 	MSYS_NO_PATHCONV=1 git p4 clone //depot
>
> This behavior is documented in our release notes, by the way:
> https://github.com/git-for-windows/build-extra/blob/master/ReleaseNotes.md#known-issues
>
> Ciao,
> Johannes


I'm starting to run out of time at work, so I'll be slow to try and 
repro this.

Thanks for the info!

- Ben
Ben Keene Dec. 13, 2019, 5:10 p.m. UTC | #11
Here's a patch I have on my tree that I would offer -
it removes references to basestring and should be a drop in patch.


 From 1cc3c0f8570adb1ef2bacc0009aac979a3263d70 Mon Sep 17 00:00:00 2001
From: Ben Keene <seraphire@gmail.com>
Date: Tue, 3 Dec 2019 16:36:26 -0500
Subject: [PATCH] git-p4: change the expansion test from basestring to list

Python 3 handles strings differently than Python 2.7. Since Python 2
is reaching it's end of life, a series of changes are being submitted to
enable python 3.5 and following support. The current code fails basic
tests under python 3.5.

Some codepaths can represent a command line the program
internally prepares to execute either as a single string
(i.e. each token properly quoted, concatenated with $IFS) or
as a list of argv[] elements, and there are 9 places where
we say "if X is isinstance(_, basestring), then do this
thing to handle X as a command line in a single string; if
not, X is a command line in a list form".

This does not work well with Python 3, as there is no
basestring (everything is Unicode now), and even with Python
2, it was not an ideal way to tell the two cases apart,
because an internally formed command line could have been in
a single Unicode string.

Flip the check to say "if X is not a list, then handle X as
a command line in a single string; otherwise treat it as a
command line in a list form".

This will get rid of references to 'basestring', to migrate
the code ready for Python 3.

Thanks-to: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Ben Keene <seraphire@gmail.com>
---
  git-p4.py | 18 +++++++++---------
  1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/git-p4.py b/git-p4.py
index 25d8012e23..d322ae20ef 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -98,7 +98,7 @@ def p4_build_cmd(cmd):
          # Provide a way to not pass this option by setting 
git-p4.retries to 0
          real_cmd += ["-r", str(retries)]

-    if isinstance(cmd,basestring):
+    if not isinstance(cmd, list):
          real_cmd = ' '.join(real_cmd) + ' ' + cmd
      else:
          real_cmd += cmd
@@ -192,7 +192,7 @@ def write_pipe(c, stdin):
      if verbose:
          sys.stderr.write('Writing pipe: %s\n' % str(c))

-    expand = isinstance(c,basestring)
+    expand = not isinstance(c, list)
      p = subprocess.Popen(c, stdin=subprocess.PIPE, shell=expand)
      pipe = p.stdin
      val = pipe.write(stdin)
@@ -214,7 +214,7 @@ def read_pipe_full(c):
      if verbose:
          sys.stderr.write('Reading pipe: %s\n' % str(c))

-    expand = isinstance(c,basestring)
+    expand = not isinstance(c, list)
      p = subprocess.Popen(c, stdout=subprocess.PIPE, 
stderr=subprocess.PIPE, shell=expand)
      (out, err) = p.communicate()
      return (p.returncode, out, decode_text_stream(err))
@@ -254,7 +254,7 @@ def read_pipe_lines(c):
      if verbose:
          sys.stderr.write('Reading pipe: %s\n' % str(c))

-    expand = isinstance(c, basestring)
+    expand = not isinstance(c, list)
      p = subprocess.Popen(c, stdout=subprocess.PIPE, shell=expand)
      pipe = p.stdout
      val = [decode_text_stream(line) for line in pipe.readlines()]
@@ -297,7 +297,7 @@ def p4_has_move_command():
      return True

  def system(cmd, ignore_error=False):
-    expand = isinstance(cmd,basestring)
+    expand = not isinstance(cmd, list)
      if verbose:
          sys.stderr.write("executing %s\n" % str(cmd))
      retcode = subprocess.call(cmd, shell=expand)
@@ -309,7 +309,7 @@ def system(cmd, ignore_error=False):
  def p4_system(cmd):
      """Specifically invoke p4 as the system command. """
      real_cmd = p4_build_cmd(cmd)
-    expand = isinstance(real_cmd, basestring)
+    expand = not isinstance(real_cmd, list)
      retcode = subprocess.call(real_cmd, shell=expand)
      if retcode:
          raise CalledProcessError(retcode, real_cmd)
@@ -547,7 +547,7 @@ def getP4OpenedType(file):
  # Return the set of all p4 labels
  def getP4Labels(depotPaths):
      labels = set()
-    if isinstance(depotPaths,basestring):
+    if not isinstance(depotPaths, list):
          depotPaths = [depotPaths]

      for l in p4CmdList(["labels"] + ["%s..." % p for p in depotPaths]):
@@ -633,7 +633,7 @@ def isModeExecChanged(src_mode, dst_mode):
  def p4CmdList(cmd, stdin=None, stdin_mode='w+b', cb=None, skip_info=False,
          errors_as_exceptions=False):

-    if isinstance(cmd,basestring):
+    if not isinstance(cmd, list):
          cmd = "-G " + cmd
          expand = True
      else:
@@ -650,7 +650,7 @@ def p4CmdList(cmd, stdin=None, stdin_mode='w+b', 
cb=None, skip_info=False,
      stdin_file = None
      if stdin is not None:
          stdin_file = tempfile.TemporaryFile(prefix='p4-stdin', 
mode=stdin_mode)
-        if isinstance(stdin,basestring):
+        if not isinstance(stdin, list):
              stdin_file.write(stdin)
          else:
              for i in stdin: