threads / discuss / 40594

Git-p4 fails with NameError with python 2.7.2

Subject: Git-p4 fails with NameError with python 2.7.2

## tl;dr

10 messages between Oct 20, 2015 and Oct 21, 2015.

replies: 9people: 6as markdown or json

Etienne Girard· Oct 20, 2015, 10:34 UTC · lore
Hello,

Git-p4 fail when I try to rebase with the error: "NameError: global name 'ctypes' is not defined". The error occurs when I use python 2.7.2 that is installed by default on my company's computers (it goes without saying that everything works fine with python 2.7.10).

I'm a beginner in python, but simply importing ctypes at the beginning of the script does the trick. I was wondering if submitting a patch for this issue is worth the trouble, when a satisfying solution is not using a 4 years old version of python.

Best regards
Luke Diamand· Oct 20, 2015, 13:57 UTC · re: Etienne Girard · lore

Re: Git-p4 fails with NameError with python 2.7.2

On 20 October 2015 at 11:34, Etienne Girard <etienne.g.girard@gmail.com> wrote:
Show 11 quoted lines
> Hello,
>
> Git-p4 fail when I try to rebase with the error: "NameError: global
> name 'ctypes' is not defined". The error occurs when I use python
> 2.7.2 that is installed by default on my company's computers (it goes
> without saying that everything works fine with python 2.7.10).
>
> I'm a beginner in python, but simply importing ctypes at the beginning
> of the script does the trick. I was wondering if submitting a patch
> for this issue is worth the trouble, when a satisfying solution is not
> using a 4 years old version of python.
If you're able to submit a patch that would be great!

Thanks, Luke

Junio C Hamano· Oct 20, 2015, 16:00 UTC · re: Luke Diamand · lore

Re: Git-p4 fails with NameError with python 2.7.2

Luke Diamand <luke@diamand.org> writes:
Show 14 quoted lines
> On 20 October 2015 at 11:34, Etienne Girard <etienne.g.girard@gmail.com> wrote:
>> Hello,
>>
>> Git-p4 fail when I try to rebase with the error: "NameError: global
>> name 'ctypes' is not defined". The error occurs when I use python
>> 2.7.2 that is installed by default on my company's computers (it goes
>> without saying that everything works fine with python 2.7.10).
>>
>> I'm a beginner in python, but simply importing ctypes at the beginning
>> of the script does the trick. I was wondering if submitting a patch
>> for this issue is worth the trouble, when a satisfying solution is not
>> using a 4 years old version of python.
>
> If you're able to submit a patch that would be great!

Lars's 4d25dc44 (git-p4: check free space during streaming, 2015-09-26) introduced two references to ctypes.* and there is no 'import ctypes' anywhere in the script.

I do not follow Python development, but does the above mean that with recent 2.x you can say ctypes without first saying "import ctypes"? It feels somewhat non-pythonesque that identifiers like this is given to you without you asking with an explicit 'import', so I am puzzled.

Manlio Perillo· Oct 20, 2015, 16:42 UTC · re: Junio C Hamano · lore

Re: Git-p4 fails with NameError with python 2.7.2

On Tue, Oct 20, 2015 at 6:00 PM, Junio C Hamano <gitster@pobox.com> wrote:
Show 25 quoted lines
>
> Luke Diamand <luke@diamand.org> writes:
>
> > On 20 October 2015 at 11:34, Etienne Girard <etienne.g.girard@gmail.com> wrote:
> >> Hello,
> >>
> >> Git-p4 fail when I try to rebase with the error: "NameError: global
> >> name 'ctypes' is not defined". The error occurs when I use python
> >> 2.7.2 that is installed by default on my company's computers (it goes
> >> without saying that everything works fine with python 2.7.10).
> >>
> >> I'm a beginner in python, but simply importing ctypes at the beginning
> >> of the script does the trick. I was wondering if submitting a patch
> >> for this issue is worth the trouble, when a satisfying solution is not
> >> using a 4 years old version of python.
> >
> > If you're able to submit a patch that would be great!
>
> Lars's 4d25dc44 (git-p4: check free space during streaming,
> 2015-09-26) introduced two references to ctypes.* and there is no
> 'import ctypes' anywhere in the script.
>
> I do not follow Python development, but does the above mean that
> with recent 2.x you can say ctypes without first saying "import
> ctypes"?

No. You need to import the ctypes module.

However in Python it is possible to "inject" the ctypes module (and any other name) in the builtin namespace. The builtin module contains names that are accessible without importing them: https://docs.python.org/2/library/__builtin__.html

IMHO, some code  is messing with the __builtin__ module.

Running pyflakes on git-p4.py code I get: git-p4.py:26: 'zlib' imported but unused git-p4.py:640: local variable 'v' is assigned to but never used git-p4.py:2114: local variable 'rhs_index' is assigned to but never used

Running pylint I get a **lot** of warning and style issues; and the following errors: E:112,21: Undefined variable 'ctypes' (undefined-variable) E:113, 8: Undefined variable 'ctypes' (undefined-variable) E:113,51: Undefined variable 'ctypes' (undefined-variable) E:113,94: Undefined variable 'ctypes' (undefined-variable) E:1002,51: No value for argument 'contentFile' in method call (no-value-for-parameter)

pyflakes is not reporting an error for ctypes. Whatever the cause, the code must be fixed to import the ctypes module.

P.S.: Sorry for the double message. The first message contained an HTML part and was rejected by vger.kernel.org.

Regards  Manlio
Dennis Kaarsemaker· Oct 20, 2015, 19:31 UTC · re: Junio C Hamano · lore

[PATCH] git-p4: import the ctypes module

The ctypes module is used on windows to calculate free disk space, so it must be imported.

Signed-off-by: Dennis Kaarsemaker <dennis@kaarsemaker.net>
---
 git-p4.py | 1 +
 1 file changed, 1 insertion(+)
On di, 2015-10-20 at 09:00 -0700, Junio C Hamano wrote:
Show 32 quoted lines
> Luke Diamand <luke@diamand.org> writes:
> 
> > On 20 October 2015 at 11:34, Etienne Girard <
> > etienne.g.girard@gmail.com> wrote:
> > > Hello,
> > > 
> > > Git-p4 fail when I try to rebase with the error: "NameError:
> > > global
> > > name 'ctypes' is not defined". The error occurs when I use python
> > > 2.7.2 that is installed by default on my company's computers (it
> > > goes
> > > without saying that everything works fine with python 2.7.10).
> > > 
> > > I'm a beginner in python, but simply importing ctypes at the
> > > beginning
> > > of the script does the trick. I was wondering if submitting a
> > > patch
> > > for this issue is worth the trouble, when a satisfying solution
> > > is not
> > > using a 4 years old version of python.
> > 
> > If you're able to submit a patch that would be great!
> 
> Lars's  (git-p4: check free space during streaming,
> 2015-09-26) introduced two references to ctypes.* and there is no
> 'import ctypes' anywhere in the script.
> 
> I do not follow Python development, but does the above mean that
> with recent 2.x you can say ctypes without first saying "import
> ctypes"?  It feels somewhat non-pythonesque that identifiers like
> this is given to you without you asking with an explicit 'import',
> so I am puzzled.

No, you cannot do that. The reason others may not have noticed this bug is that in git-p4.py, ctypes is only used on windows.

 111     if platform.system() == 'Windows':
 112         free_bytes = ctypes.c_ulonglong(0)
 113         ctypes.windll.kernel32.GetDiskFreeSpaceExW(ctypes.c_wchar_p(os.getcwd()), None, None, ctypes.pointer(free_bytes))

The fact that it works for the OP with 2.7.10 is puzzling (assuming that it's on the same system). But this patch should help.

diff --git a/git-p4.py b/git-p4.py
index daa60c6..212ef2b 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -24,6 +24,7 @@ import shutil
 import stat
 import zipfile
 import zlib
+import ctypes
 
 try:
     from subprocess import CalledProcessError
-- 
2.6.2-323-g60bd420
Junio C Hamano· Oct 20, 2015, 19:36 UTC · re: Dennis Kaarsemaker · lore

Re: [PATCH] git-p4: import the ctypes module

Dennis Kaarsemaker <dennis@kaarsemaker.net> writes:
Show 15 quoted lines
>> I do not follow Python development, but does the above mean that
>> with recent 2.x you can say ctypes without first saying "import
>> ctypes"?  It feels somewhat non-pythonesque that identifiers like
>> this is given to you without you asking with an explicit 'import',
>> so I am puzzled.
>
> No, you cannot do that. The reason others may not have noticed this bug is that
> in git-p4.py, ctypes is only used on windows.
>
>  111     if platform.system() == 'Windows':
>  112         free_bytes = ctypes.c_ulonglong(0)
>  113         ctypes.windll.kernel32.GetDiskFreeSpaceExW(ctypes.c_wchar_p(os.getcwd()), None, None, ctypes.pointer(free_bytes))
>
> The fact that it works for the OP with 2.7.10 is puzzling (assuming that it's
> on the same system).
Exactly.  That is where my "I am puzzled" comes from.
The patch looks obviously the right thing to do.  Luke?  Lars?
Show 13 quoted lines
>
> diff --git a/git-p4.py b/git-p4.py
> index daa60c6..212ef2b 100755
> --- a/git-p4.py
> +++ b/git-p4.py
> @@ -24,6 +24,7 @@ import shutil
>  import stat
>  import zipfile
>  import zlib
> +import ctypes
>  
>  try:
>      from subprocess import CalledProcessError
Luke Diamand· Oct 20, 2015, 23:00 UTC · re: Junio C Hamano · lore

Re: [PATCH] git-p4: import the ctypes module

On 20/10/15 20:36, Junio C Hamano wrote:
Show 21 quoted lines
> Dennis Kaarsemaker <dennis@kaarsemaker.net> writes:
>
>>> I do not follow Python development, but does the above mean that
>>> with recent 2.x you can say ctypes without first saying "import
>>> ctypes"?  It feels somewhat non-pythonesque that identifiers like
>>> this is given to you without you asking with an explicit 'import',
>>> so I am puzzled.
>>
>> No, you cannot do that. The reason others may not have noticed this bug is that
>> in git-p4.py, ctypes is only used on windows.
>>
>>   111     if platform.system() == 'Windows':
>>   112         free_bytes = ctypes.c_ulonglong(0)
>>   113         ctypes.windll.kernel32.GetDiskFreeSpaceExW(ctypes.c_wchar_p(os.getcwd()), None, None, ctypes.pointer(free_bytes))
>>
>> The fact that it works for the OP with 2.7.10 is puzzling (assuming that it's
>> on the same system).
>
> Exactly.  That is where my "I am puzzled" comes from.
>
> The patch looks obviously the right thing to do.  Luke?  Lars?
It looks sensible to me, and works fine on Linux, thanks. ack.
I can't test on Windows today but I can't see why it wouldn't work.
Luke
Etienne Girard· Oct 21, 2015, 08:23 UTC · re: Luke Diamand · lore

Re: [PATCH] git-p4: import the ctypes module

Hello,

I couldn't work further on this yesterday (but I read Documentation/SubmittingPatches, which is a good start I guess). The diff proposed by Dennis works on my machine, I'll try to figure out why the original script worked with 2.7.10.

Thanks
2015-10-21 1:00 GMT+02:00 Luke Diamand <luke@diamand.org>:
Show 38 quoted lines
> On 20/10/15 20:36, Junio C Hamano wrote:
>>
>> Dennis Kaarsemaker <dennis@kaarsemaker.net> writes:
>>
>>>> I do not follow Python development, but does the above mean that
>>>> with recent 2.x you can say ctypes without first saying "import
>>>> ctypes"?  It feels somewhat non-pythonesque that identifiers like
>>>> this is given to you without you asking with an explicit 'import',
>>>> so I am puzzled.
>>>
>>>
>>> No, you cannot do that. The reason others may not have noticed this bug
>>> is that
>>> in git-p4.py, ctypes is only used on windows.
>>>
>>>   111     if platform.system() == 'Windows':
>>>   112         free_bytes = ctypes.c_ulonglong(0)
>>>   113
>>> ctypes.windll.kernel32.GetDiskFreeSpaceExW(ctypes.c_wchar_p(os.getcwd()),
>>> None, None, ctypes.pointer(free_bytes))
>>>
>>> The fact that it works for the OP with 2.7.10 is puzzling (assuming that
>>> it's
>>> on the same system).
>>
>>
>> Exactly.  That is where my "I am puzzled" comes from.
>>
>> The patch looks obviously the right thing to do.  Luke?  Lars?
>
>
> It looks sensible to me, and works fine on Linux, thanks. ack.
>
> I can't test on Windows today but I can't see why it wouldn't work.
>
> Luke
>
>
Etienne Girard· Oct 21, 2015, 09:54 UTC · re: Etienne Girard · lore

Re: [PATCH] git-p4: import the ctypes module

I was wrong, the script doesn't work on my machine if ctypes is not imported regardless of python version. I guess I was confused by using a version of git-p4 before ctypes was introduced, the failing version and the patched version, as well as several python versions.

Sorry for this misleading claim, and thanks for the quick fix.
2015-10-21 10:23 GMT+02:00 Etienne Girard <etienne.g.girard@gmail.com>:
Show 48 quoted lines
> Hello,
>
> I couldn't work further on this yesterday (but I read
> Documentation/SubmittingPatches, which is a good start I guess). The
> diff proposed by Dennis works on my machine, I'll try to figure out
> why the original script worked with 2.7.10.
>
> Thanks
>
> 2015-10-21 1:00 GMT+02:00 Luke Diamand <luke@diamand.org>:
>> On 20/10/15 20:36, Junio C Hamano wrote:
>>>
>>> Dennis Kaarsemaker <dennis@kaarsemaker.net> writes:
>>>
>>>>> I do not follow Python development, but does the above mean that
>>>>> with recent 2.x you can say ctypes without first saying "import
>>>>> ctypes"?  It feels somewhat non-pythonesque that identifiers like
>>>>> this is given to you without you asking with an explicit 'import',
>>>>> so I am puzzled.
>>>>
>>>>
>>>> No, you cannot do that. The reason others may not have noticed this bug
>>>> is that
>>>> in git-p4.py, ctypes is only used on windows.
>>>>
>>>>   111     if platform.system() == 'Windows':
>>>>   112         free_bytes = ctypes.c_ulonglong(0)
>>>>   113
>>>> ctypes.windll.kernel32.GetDiskFreeSpaceExW(ctypes.c_wchar_p(os.getcwd()),
>>>> None, None, ctypes.pointer(free_bytes))
>>>>
>>>> The fact that it works for the OP with 2.7.10 is puzzling (assuming that
>>>> it's
>>>> on the same system).
>>>
>>>
>>> Exactly.  That is where my "I am puzzled" comes from.
>>>
>>> The patch looks obviously the right thing to do.  Luke?  Lars?
>>
>>
>> It looks sensible to me, and works fine on Linux, thanks. ack.
>>
>> I can't test on Windows today but I can't see why it wouldn't work.
>>
>> Luke
>>
>>
Lars Schneider· Oct 21, 2015, 20:00 UTC · re: Etienne Girard · lore

Re: [PATCH] git-p4: import the ctypes module

Hi Etienne,

thanks for reporting this! Junio is right, I messed that up on my Windows testing box! :-( Sorry! If you have any questions around submitting patches I am happy to help as I just recently went through the learning process myself!

@Dennis: Thanks for the quick patch!

Thanks, Lars

On 21 Oct 2015, at 10:23, Etienne Girard <etienne.g.girard@gmail.com> wrote:
Show 48 quoted lines
> Hello,
> 
> I couldn't work further on this yesterday (but I read
> Documentation/SubmittingPatches, which is a good start I guess). The
> diff proposed by Dennis works on my machine, I'll try to figure out
> why the original script worked with 2.7.10.
> 
> Thanks
> 
> 2015-10-21 1:00 GMT+02:00 Luke Diamand <luke@diamand.org>:
>> On 20/10/15 20:36, Junio C Hamano wrote:
>>> 
>>> Dennis Kaarsemaker <dennis@kaarsemaker.net> writes:
>>> 
>>>>> I do not follow Python development, but does the above mean that
>>>>> with recent 2.x you can say ctypes without first saying "import
>>>>> ctypes"?  It feels somewhat non-pythonesque that identifiers like
>>>>> this is given to you without you asking with an explicit 'import',
>>>>> so I am puzzled.
>>>> 
>>>> 
>>>> No, you cannot do that. The reason others may not have noticed this bug
>>>> is that
>>>> in git-p4.py, ctypes is only used on windows.
>>>> 
>>>>  111     if platform.system() == 'Windows':
>>>>  112         free_bytes = ctypes.c_ulonglong(0)
>>>>  113
>>>> ctypes.windll.kernel32.GetDiskFreeSpaceExW(ctypes.c_wchar_p(os.getcwd()),
>>>> None, None, ctypes.pointer(free_bytes))
>>>> 
>>>> The fact that it works for the OP with 2.7.10 is puzzling (assuming that
>>>> it's
>>>> on the same system).
>>> 
>>> 
>>> Exactly.  That is where my "I am puzzled" comes from.
>>> 
>>> The patch looks obviously the right thing to do.  Luke?  Lars?
>> 
>> 
>> It looks sensible to me, and works fine on Linux, thanks. ack.
>> 
>> I can't test on Windows today but I can't see why it wouldn't work.
>> 
>> Luke
>> 
>> 

← back to recent threads