{"thread":{"id":"40594","subject":"Git-p4 fails with NameError with python 2.7.2","startedAt":"2015-10-20T10:34:38Z","lastAt":"2015-10-21T20:00:36Z","messageCount":10,"participants":["Etienne Girard","Luke Diamand","Junio C Hamano","Manlio Perillo","Dennis Kaarsemaker","Lars Schneider"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"271962","messageId":"CAJA=mv5Kdsn1YEo4sUAwpTv=0Jc8Xg5V2WPMoCmsxNL4Hnk=kg@mail.gmail.com","threadId":"40594","inReplyTo":null,"subject":"Git-p4 fails with NameError with python 2.7.2","fromName":"Etienne Girard","fromEmail":"etienne.g.girard@gmail.com","sentAt":"2015-10-20T10:34:38Z","receivedAt":"2015-10-20T10:34:38Z","isPatch":false,"sender":{"key":"etienne.g.girard@gmail.com","avatar":null},"body":"Hello,\n\nGit-p4 fail when I try to rebase with the error: \"NameError: global\nname 'ctypes' is not defined\". The error occurs when I use python\n2.7.2 that is installed by default on my company's computers (it goes\nwithout saying that everything works fine with python 2.7.10).\n\nI'm a beginner in python, but simply importing ctypes at the beginning\nof the script does the trick. I was wondering if submitting a patch\nfor this issue is worth the trouble, when a satisfying solution is not\nusing a 4 years old version of python.\n\nBest regards\n"},{"id":"271971","messageId":"CAE5ih793+YDd30rpMSyTHjUNZS+-BLY9D-kJkF9RAogjdctPFw@mail.gmail.com","threadId":"40594","inReplyTo":"CAJA=mv5Kdsn1YEo4sUAwpTv=0Jc8Xg5V2WPMoCmsxNL4Hnk=kg@mail.gmail.com","subject":"Re: Git-p4 fails with NameError with python 2.7.2","fromName":"Luke Diamand","fromEmail":"luke@diamand.org","sentAt":"2015-10-20T13:57:46Z","receivedAt":"2015-10-20T13:57:46Z","isPatch":false,"sender":{"key":"luke@diamand.org","avatar":"https://avatars.githubusercontent.com/u/5330967?v=4"},"body":"On 20 October 2015 at 11:34, Etienne Girard wrote:\n> Hello,\n>\n> Git-p4 fail when I try to rebase with the error: \"NameError: global\n> name 'ctypes' is not defined\". The error occurs when I use python\n> 2.7.2 that is installed by default on my company's computers (it goes\n> without saying that everything works fine with python 2.7.10).\n>\n> I'm a beginner in python, but simply importing ctypes at the beginning\n> of the script does the trick. I was wondering if submitting a patch\n> for this issue is worth the trouble, when a satisfying solution is not\n> using a 4 years old version of python.\n\nIf you're able to submit a patch that would be great!\n\nThanks,\nLuke\n"},{"id":"271975","messageId":"xmqqy4ex8r8k.fsf@gitster.mtv.corp.google.com","threadId":"40594","inReplyTo":"CAE5ih793+YDd30rpMSyTHjUNZS+-BLY9D-kJkF9RAogjdctPFw@mail.gmail.com","subject":"Re: Git-p4 fails with NameError with python 2.7.2","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-10-20T16:00:27Z","receivedAt":"2015-10-20T16:00:27Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Luke Diamand writes:\n\n> On 20 October 2015 at 11:34, Etienne Girard wrote:\n>> Hello,\n>>\n>> Git-p4 fail when I try to rebase with the error: \"NameError: global\n>> name 'ctypes' is not defined\". The error occurs when I use python\n>> 2.7.2 that is installed by default on my company's computers (it goes\n>> without saying that everything works fine with python 2.7.10).\n>>\n>> I'm a beginner in python, but simply importing ctypes at the beginning\n>> of the script does the trick. I was wondering if submitting a patch\n>> for this issue is worth the trouble, when a satisfying solution is not\n>> using a 4 years old version of python.\n>\n> If you're able to submit a patch that would be great!\n\nLars's 4d25dc44 (git-p4: check free space during streaming,\n2015-09-26) introduced two references to ctypes.* and there is no\n'import ctypes' anywhere in the script.\n\nI do not follow Python development, but does the above mean that\nwith recent 2.x you can say ctypes without first saying \"import\nctypes\"? It feels somewhat non-pythonesque that identifiers like\nthis is given to you without you asking with an explicit 'import',\nso I am puzzled.\n"},{"id":"271979","messageId":"CAAToxAHdojvspxdb-=vYx-QY9=MTjnQD29fLTJK3KmY2+To5eQ@mail.gmail.com","threadId":"40594","inReplyTo":"xmqqy4ex8r8k.fsf@gitster.mtv.corp.google.com","subject":"Re: Git-p4 fails with NameError with python 2.7.2","fromName":"Manlio Perillo","fromEmail":"manlio.perillo@gmail.com","sentAt":"2015-10-20T16:42:20Z","receivedAt":"2015-10-20T16:42:20Z","isPatch":false,"sender":{"key":"manlio.perillo@gmail.com","avatar":"https://avatars.githubusercontent.com/u/6217088?v=4"},"body":"On Tue, Oct 20, 2015 at 6:00 PM, Junio C Hamano wrote:\n>\n> Luke Diamand writes:\n>\n> > On 20 October 2015 at 11:34, Etienne Girard wrote:\n> >> Hello,\n> >>\n> >> Git-p4 fail when I try to rebase with the error: \"NameError: global\n> >> name 'ctypes' is not defined\". The error occurs when I use python\n> >> 2.7.2 that is installed by default on my company's computers (it goes\n> >> without saying that everything works fine with python 2.7.10).\n> >>\n> >> I'm a beginner in python, but simply importing ctypes at the beginning\n> >> of the script does the trick. I was wondering if submitting a patch\n> >> for this issue is worth the trouble, when a satisfying solution is not\n> >> using a 4 years old version of python.\n> >\n> > If you're able to submit a patch that would be great!\n>\n> Lars's 4d25dc44 (git-p4: check free space during streaming,\n> 2015-09-26) introduced two references to ctypes.* and there is no\n> 'import ctypes' anywhere in the script.\n>\n> I do not follow Python development, but does the above mean that\n> with recent 2.x you can say ctypes without first saying \"import\n> ctypes\"?\n\n\nNo.\nYou need to import the ctypes module.\n\nHowever in Python it is possible to \"inject\" the ctypes module (and\nany other name) in the builtin namespace.\nThe builtin module contains names that are accessible without importing them:\nhttps://docs.python.org/2/library/__builtin__.html\n\nIMHO, some code is messing with the __builtin__ module.\n\nRunning pyflakes on git-p4.py code I get:\ngit-p4.py:26: 'zlib' imported but unused\ngit-p4.py:640: local variable 'v' is assigned to but never used\ngit-p4.py:2114: local variable 'rhs_index' is assigned to but never used\n\nRunning pylint I get a **lot** of warning and style issues; and the\nfollowing errors:\nE:112,21: Undefined variable 'ctypes' (undefined-variable)\nE:113, 8: Undefined variable 'ctypes' (undefined-variable)\nE:113,51: Undefined variable 'ctypes' (undefined-variable)\nE:113,94: Undefined variable 'ctypes' (undefined-variable)\nE:1002,51: No value for argument 'contentFile' in method call\n(no-value-for-parameter)\n\npyflakes is not reporting an error for ctypes.\nWhatever the cause, the code must be fixed to import the ctypes module.\n\nP.S.:\nSorry for the double message.\nThe first message contained an HTML part and was rejected by vger.kernel.org.\n\n\nRegards Manlio\n"},{"id":"271991","messageId":"1445369506.8543.10.camel@kaarsemaker.net","threadId":"40594","inReplyTo":"xmqqy4ex8r8k.fsf@gitster.mtv.corp.google.com","subject":"[PATCH] git-p4: import the ctypes module","fromName":"Dennis Kaarsemaker","fromEmail":"dennis@kaarsemaker.net","sentAt":"2015-10-20T19:31:46Z","receivedAt":"2015-10-20T19:31:46Z","isPatch":true,"sender":{"key":"dennis@kaarsemaker.net","avatar":"https://avatars.githubusercontent.com/u/200649?v=4"},"body":"The ctypes module is used on windows to calculate free disk space, so it\nmust be imported.\n\nSigned-off-by: Dennis Kaarsemaker \n---\n git-p4.py | 1 +\n 1 file changed, 1 insertion(+)\n\nOn di, 2015-10-20 at 09:00 -0700, Junio C Hamano wrote:\n> Luke Diamand writes:\n> \n> > On 20 October 2015 at 11:34, Etienne Girard <\n> > etienne.g.girard@gmail.com> wrote:\n> > > Hello,\n> > > \n> > > Git-p4 fail when I try to rebase with the error: \"NameError:\n> > > global\n> > > name 'ctypes' is not defined\". The error occurs when I use python\n> > > 2.7.2 that is installed by default on my company's computers (it\n> > > goes\n> > > without saying that everything works fine with python 2.7.10).\n> > > \n> > > I'm a beginner in python, but simply importing ctypes at the\n> > > beginning\n> > > of the script does the trick. I was wondering if submitting a\n> > > patch\n> > > for this issue is worth the trouble, when a satisfying solution\n> > > is not\n> > > using a 4 years old version of python.\n> > \n> > If you're able to submit a patch that would be great!\n> \n> Lars's (git-p4: check free space during streaming,\n> 2015-09-26) introduced two references to ctypes.* and there is no\n> 'import ctypes' anywhere in the script.\n> \n> I do not follow Python development, but does the above mean that\n> with recent 2.x you can say ctypes without first saying \"import\n> ctypes\"? It feels somewhat non-pythonesque that identifiers like\n> this is given to you without you asking with an explicit 'import',\n> so I am puzzled.\n\nNo, you cannot do that. The reason others may not have noticed this bug is that\nin git-p4.py, ctypes is only used on windows.\n\n 111 if platform.system() == 'Windows':\n 112 free_bytes = ctypes.c_ulonglong(0)\n 113 ctypes.windll.kernel32.GetDiskFreeSpaceExW(ctypes.c_wchar_p(os.getcwd()), None, None, ctypes.pointer(free_bytes))\n\nThe fact that it works for the OP with 2.7.10 is puzzling (assuming that it's\non the same system). But this patch should help.\n\ndiff --git a/git-p4.py b/git-p4.py\nindex daa60c6..212ef2b 100755\n--- a/git-p4.py\n+++ b/git-p4.py\n@@ -24,6 +24,7 @@ import shutil\n import stat\n import zipfile\n import zlib\n+import ctypes\n \n try:\n from subprocess import CalledProcessError\n-- \n2.6.2-323-g60bd420\n"},{"id":"271992","messageId":"xmqqwpuh72ot.fsf@gitster.mtv.corp.google.com","threadId":"40594","inReplyTo":"1445369506.8543.10.camel@kaarsemaker.net","subject":"Re: [PATCH] git-p4: import the ctypes module","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-10-20T19:36:02Z","receivedAt":"2015-10-20T19:36:02Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Dennis Kaarsemaker writes:\n\n>> I do not follow Python development, but does the above mean that\n>> with recent 2.x you can say ctypes without first saying \"import\n>> ctypes\"? It feels somewhat non-pythonesque that identifiers like\n>> this is given to you without you asking with an explicit 'import',\n>> so I am puzzled.\n>\n> No, you cannot do that. The reason others may not have noticed this bug is that\n> in git-p4.py, ctypes is only used on windows.\n>\n> 111 if platform.system() == 'Windows':\n> 112 free_bytes = ctypes.c_ulonglong(0)\n> 113 ctypes.windll.kernel32.GetDiskFreeSpaceExW(ctypes.c_wchar_p(os.getcwd()), None, None, ctypes.pointer(free_bytes))\n>\n> The fact that it works for the OP with 2.7.10 is puzzling (assuming that it's\n> on the same system).\n\nExactly. That is where my \"I am puzzled\" comes from.\n\nThe patch looks obviously the right thing to do. Luke? Lars?\n\n>\n> diff --git a/git-p4.py b/git-p4.py\n> index daa60c6..212ef2b 100755\n> --- a/git-p4.py\n> +++ b/git-p4.py\n> @@ -24,6 +24,7 @@ import shutil\n> import stat\n> import zipfile\n> import zlib\n> +import ctypes\n> \n> try:\n> from subprocess import CalledProcessError\n"},{"id":"272011","messageId":"5626C79C.8060503@diamand.org","threadId":"40594","inReplyTo":"xmqqwpuh72ot.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH] git-p4: import the ctypes module","fromName":"Luke Diamand","fromEmail":"luke@diamand.org","sentAt":"2015-10-20T23:00:44Z","receivedAt":"2015-10-20T23:00:44Z","isPatch":true,"sender":{"key":"luke@diamand.org","avatar":"https://avatars.githubusercontent.com/u/5330967?v=4"},"body":"On 20/10/15 20:36, Junio C Hamano wrote:\n> Dennis Kaarsemaker writes:\n>\n>>> I do not follow Python development, but does the above mean that\n>>> with recent 2.x you can say ctypes without first saying \"import\n>>> ctypes\"? It feels somewhat non-pythonesque that identifiers like\n>>> this is given to you without you asking with an explicit 'import',\n>>> so I am puzzled.\n>>\n>> No, you cannot do that. The reason others may not have noticed this bug is that\n>> in git-p4.py, ctypes is only used on windows.\n>>\n>> 111 if platform.system() == 'Windows':\n>> 112 free_bytes = ctypes.c_ulonglong(0)\n>> 113 ctypes.windll.kernel32.GetDiskFreeSpaceExW(ctypes.c_wchar_p(os.getcwd()), None, None, ctypes.pointer(free_bytes))\n>>\n>> The fact that it works for the OP with 2.7.10 is puzzling (assuming that it's\n>> on the same system).\n>\n> Exactly. That is where my \"I am puzzled\" comes from.\n>\n> The patch looks obviously the right thing to do. Luke? Lars?\n\nIt looks sensible to me, and works fine on Linux, thanks. ack.\n\nI can't test on Windows today but I can't see why it wouldn't work.\n\nLuke\n"},{"id":"272030","messageId":"CAJA=mv7N=PmL69kgZRjq8P6J_qsfUbLx76WguhvqPrG2o1UFww@mail.gmail.com","threadId":"40594","inReplyTo":"5626C79C.8060503@diamand.org","subject":"Re: [PATCH] git-p4: import the ctypes module","fromName":"Etienne Girard","fromEmail":"etienne.g.girard@gmail.com","sentAt":"2015-10-21T08:23:43Z","receivedAt":"2015-10-21T08:23:43Z","isPatch":true,"sender":{"key":"etienne.g.girard@gmail.com","avatar":null},"body":"Hello,\n\nI couldn't work further on this yesterday (but I read\nDocumentation/SubmittingPatches, which is a good start I guess). The\ndiff proposed by Dennis works on my machine, I'll try to figure out\nwhy the original script worked with 2.7.10.\n\nThanks\n\n2015-10-21 1:00 GMT+02:00 Luke Diamand :\n> On 20/10/15 20:36, Junio C Hamano wrote:\n>>\n>> Dennis Kaarsemaker writes:\n>>\n>>>> I do not follow Python development, but does the above mean that\n>>>> with recent 2.x you can say ctypes without first saying \"import\n>>>> ctypes\"? It feels somewhat non-pythonesque that identifiers like\n>>>> this is given to you without you asking with an explicit 'import',\n>>>> so I am puzzled.\n>>>\n>>>\n>>> No, you cannot do that. The reason others may not have noticed this bug\n>>> is that\n>>> in git-p4.py, ctypes is only used on windows.\n>>>\n>>> 111 if platform.system() == 'Windows':\n>>> 112 free_bytes = ctypes.c_ulonglong(0)\n>>> 113\n>>> ctypes.windll.kernel32.GetDiskFreeSpaceExW(ctypes.c_wchar_p(os.getcwd()),\n>>> None, None, ctypes.pointer(free_bytes))\n>>>\n>>> The fact that it works for the OP with 2.7.10 is puzzling (assuming that\n>>> it's\n>>> on the same system).\n>>\n>>\n>> Exactly. That is where my \"I am puzzled\" comes from.\n>>\n>> The patch looks obviously the right thing to do. Luke? Lars?\n>\n>\n> It looks sensible to me, and works fine on Linux, thanks. ack.\n>\n> I can't test on Windows today but I can't see why it wouldn't work.\n>\n> Luke\n>\n>\n"},{"id":"272031","messageId":"CAJA=mv5ui6OLc=0OSJg+k5a6bgSxwX=waLmyxtV4=ikWBkXPQg@mail.gmail.com","threadId":"40594","inReplyTo":"CAJA=mv7N=PmL69kgZRjq8P6J_qsfUbLx76WguhvqPrG2o1UFww@mail.gmail.com","subject":"Re: [PATCH] git-p4: import the ctypes module","fromName":"Etienne Girard","fromEmail":"etienne.g.girard@gmail.com","sentAt":"2015-10-21T09:54:18Z","receivedAt":"2015-10-21T09:54:18Z","isPatch":true,"sender":{"key":"etienne.g.girard@gmail.com","avatar":null},"body":"I was wrong, the script doesn't work on my machine if ctypes is not\nimported regardless of python version. I guess I was confused by using\na version of git-p4 before ctypes was introduced, the failing version\nand the patched version, as well as several python versions.\n\nSorry for this misleading claim, and thanks for the quick fix.\n\n2015-10-21 10:23 GMT+02:00 Etienne Girard :\n> Hello,\n>\n> I couldn't work further on this yesterday (but I read\n> Documentation/SubmittingPatches, which is a good start I guess). The\n> diff proposed by Dennis works on my machine, I'll try to figure out\n> why the original script worked with 2.7.10.\n>\n> Thanks\n>\n> 2015-10-21 1:00 GMT+02:00 Luke Diamand :\n>> On 20/10/15 20:36, Junio C Hamano wrote:\n>>>\n>>> Dennis Kaarsemaker writes:\n>>>\n>>>>> I do not follow Python development, but does the above mean that\n>>>>> with recent 2.x you can say ctypes without first saying \"import\n>>>>> ctypes\"? It feels somewhat non-pythonesque that identifiers like\n>>>>> this is given to you without you asking with an explicit 'import',\n>>>>> so I am puzzled.\n>>>>\n>>>>\n>>>> No, you cannot do that. The reason others may not have noticed this bug\n>>>> is that\n>>>> in git-p4.py, ctypes is only used on windows.\n>>>>\n>>>> 111 if platform.system() == 'Windows':\n>>>> 112 free_bytes = ctypes.c_ulonglong(0)\n>>>> 113\n>>>> ctypes.windll.kernel32.GetDiskFreeSpaceExW(ctypes.c_wchar_p(os.getcwd()),\n>>>> None, None, ctypes.pointer(free_bytes))\n>>>>\n>>>> The fact that it works for the OP with 2.7.10 is puzzling (assuming that\n>>>> it's\n>>>> on the same system).\n>>>\n>>>\n>>> Exactly. That is where my \"I am puzzled\" comes from.\n>>>\n>>> The patch looks obviously the right thing to do. Luke? Lars?\n>>\n>>\n>> It looks sensible to me, and works fine on Linux, thanks. ack.\n>>\n>> I can't test on Windows today but I can't see why it wouldn't work.\n>>\n>> Luke\n>>\n>>\n"},{"id":"272058","messageId":"C5786C82-72C3-48C5-A8D0-8DD824F247C9@gmail.com","threadId":"40594","inReplyTo":"CAJA=mv7N=PmL69kgZRjq8P6J_qsfUbLx76WguhvqPrG2o1UFww@mail.gmail.com","subject":"Re: [PATCH] git-p4: import the ctypes module","fromName":"Lars Schneider","fromEmail":"larsxschneider@gmail.com","sentAt":"2015-10-21T20:00:36Z","receivedAt":"2015-10-21T20:00:36Z","isPatch":true,"sender":{"key":"larsxschneider@gmail.com","avatar":"https://avatars.githubusercontent.com/u/477434?v=4"},"body":"Hi Etienne,\n\nthanks for reporting this! Junio is right, I messed that up on my Windows testing box! :-( Sorry! \nIf you have any questions around submitting patches I am happy to help as I just recently went through the learning process myself!\n\n@Dennis: Thanks for the quick patch!\n\nThanks,\nLars\n\nOn 21 Oct 2015, at 10:23, Etienne Girard wrote:\n\n> Hello,\n> \n> I couldn't work further on this yesterday (but I read\n> Documentation/SubmittingPatches, which is a good start I guess). The\n> diff proposed by Dennis works on my machine, I'll try to figure out\n> why the original script worked with 2.7.10.\n> \n> Thanks\n> \n> 2015-10-21 1:00 GMT+02:00 Luke Diamand :\n>> On 20/10/15 20:36, Junio C Hamano wrote:\n>>> \n>>> Dennis Kaarsemaker writes:\n>>> \n>>>>> I do not follow Python development, but does the above mean that\n>>>>> with recent 2.x you can say ctypes without first saying \"import\n>>>>> ctypes\"? It feels somewhat non-pythonesque that identifiers like\n>>>>> this is given to you without you asking with an explicit 'import',\n>>>>> so I am puzzled.\n>>>> \n>>>> \n>>>> No, you cannot do that. The reason others may not have noticed this bug\n>>>> is that\n>>>> in git-p4.py, ctypes is only used on windows.\n>>>> \n>>>> 111 if platform.system() == 'Windows':\n>>>> 112 free_bytes = ctypes.c_ulonglong(0)\n>>>> 113\n>>>> ctypes.windll.kernel32.GetDiskFreeSpaceExW(ctypes.c_wchar_p(os.getcwd()),\n>>>> None, None, ctypes.pointer(free_bytes))\n>>>> \n>>>> The fact that it works for the OP with 2.7.10 is puzzling (assuming that\n>>>> it's\n>>>> on the same system).\n>>> \n>>> \n>>> Exactly. That is where my \"I am puzzled\" comes from.\n>>> \n>>> The patch looks obviously the right thing to do. Luke? Lars?\n>> \n>> \n>> It looks sensible to me, and works fine on Linux, thanks. ack.\n>> \n>> I can't test on Windows today but I can't see why it wouldn't work.\n>> \n>> Luke\n>> \n>> \n"}]}