{"thread":{"id":"14794","subject":"[PATCH] git-p4: chdir now properly sets PWD environment variable in msysGit","startedAt":"2008-08-01T19:50:03Z","lastAt":"2008-08-16T05:56:26Z","messageCount":8,"participants":["Robert Blum","Junio C Hamano","Han-Wen Nienhuys","Simon Hausmann","Alex Riesen"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"85914","messageId":"bad7471c0808011250v569ffaaby9e20a5ba1f971927@mail.gmail.com","threadId":"14794","inReplyTo":null,"subject":"[PATCH] git-p4: chdir now properly sets PWD environment variable in msysGit","fromName":"Robert Blum","fromEmail":"rob.blum@gmail.com","sentAt":"2008-08-01T19:50:03Z","receivedAt":"2008-08-01T19:50:03Z","isPatch":true,"sender":{"key":"rob.blum@gmail.com","avatar":null},"body":"P4 on Windows expects the PWD environment variable to be set to the\ncurrent working dir, but os.chdir in python doesn't do that by default\n\nSigned-off-by: Robert Blum \n---\n\nPushing it out to the list since I'm not entirely sure who the git-p4 owner\neven is. CC'ed likely suspects for ownership ;)\n\n contrib/fast-import/git-p4 | 15 +++++++++++----\n 1 files changed, 11 insertions(+), 4 deletions(-)\n\ndiff --git a/contrib/fast-import/git-p4 b/contrib/fast-import/git-p4\nindex 6ae0429..b4d0c65 100755\n--- a/contrib/fast-import/git-p4\n+++ b/contrib/fast-import/git-p4\n@@ -16,6 +16,13 @@ from sets import Set;\n\n verbose = False\n\n+if os.name == 'nt':\n+ def os_chdir(dir):\n+ os.environ['PWD']=dir\n+ os.chdir(dir)\n+else:\n+ os_chdir = os.chdir\n+\n def die(msg):\n if verbose:\n raise Exception(msg)\n@@ -712,7 +719,7 @@ class P4Submit(Command):\n print \"Perforce checkout for depot path %s located at %s\" % (self.depot\nPath, self.clientPath)\n self.oldWorkingDirectory = os.getcwd()\n\n- os.chdir(self.clientPath)\n+ os_chdir(self.clientPath)\n print \"Syncronizing p4 checkout...\"\n system(\"p4 sync ...\")\n\n@@ -732,7 +739,7 @@ class P4Submit(Command):\n\n if len(commits) == 0:\n print \"All changes applied!\"\n- os.chdir(self.oldWorkingDirectory)\n+ os_chdir(self.oldWorkingDirectory)\n\n sync = P4Sync()\n sync.run([])\n@@ -1670,7 +1677,7 @@ class P4Clone(P4Sync):\n print \"Importing from %s into %s\" % (', '.join(depotPaths), self.cloneD\nestination)\n if not os.path.exists(self.cloneDestination):\n os.makedirs(self.cloneDestination)\n- os.chdir(self.cloneDestination)\n+ os_chdir(self.cloneDestination)\n system(\"git init\")\n self.gitdir = os.getcwd() + \"/.git\"\n if not P4Sync.run(self, depotPaths):\n@@ -1782,7 +1789,7 @@ def main():\n if os.path.exists(cmd.gitdir):\n cdup = read_pipe(\"git rev-parse --show-cdup\").strip()\n if len(cdup) > 0:\n- os.chdir(cdup);\n+ os_chdir(cdup);\n\n if not isValidGitDir(cmd.gitdir):\n if isValidGitDir(cmd.gitdir + \"/.git\"):\n--\n1.5.5.1015.g9d258\n"},{"id":"86105","messageId":"7v7iaxu7i1.fsf@gitster.siamese.dyndns.org","threadId":"14794","inReplyTo":"bad7471c0808011250v569ffaaby9e20a5ba1f971927@mail.gmail.com","subject":"Re: [PATCH] git-p4: chdir now properly sets PWD environment variable in msysGit","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-08-03T21:13:42Z","receivedAt":"2008-08-03T21:13:42Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Robert Blum\" writes:\n\n> P4 on Windows expects the PWD environment variable to be set to the\n> current working dir, but os.chdir in python doesn't do that by default\n\nMissing full stop at the end of sentence aside, this comment makes me\nwonder if there is an optional way to have it set it, as opposed to the\ninconvenient way it behaves \"by defualt\". If there is none, I think your\npatch, even though it looks ugly, is the least evil approach. Another way\nmight be to wrap callsites of system() by introducing a \"run_p4\" function,\nlike:\n\n\tdef run_p4(arg):\n \tos.environ['PWD'] = os.getcwd() if os.name == 'nt'\n\t\treturn system(arg)\n\n> ---\n>\n> Pushing it out to the list since I'm not entirely sure who the git-p4 owner\n> even is. CC'ed likely suspects for ownership ;)\n\nThanks. I've been waiting for an Ack from somewhere or success reports\nfrom p4 users on Windows.\n"},{"id":"86142","messageId":"g75ror$uav$1@ger.gmane.org","threadId":"14794","inReplyTo":"bad7471c0808011250v569ffaaby9e20a5ba1f971927@mail.gmail.com","subject":"Re: [PATCH] git-p4: chdir now properly sets PWD environment variable in msysGit","fromName":"Han-Wen Nienhuys","fromEmail":"hanwen@xs4all.nl","sentAt":"2008-08-04T03:06:07Z","receivedAt":"2008-08-04T03:06:07Z","isPatch":true,"sender":{"key":"hanwen@google.com","avatar":"https://avatars.githubusercontent.com/u/31547?v=4"},"body":"Robert Blum escreveu:\n> P4 on Windows expects the PWD environment variable to be set to the\n> current working dir, but os.chdir in python doesn't do that by default\n\n> +if os.name == 'nt':\n> + def os_chdir(dir):\n> + os.environ['PWD']=dir\n> + os.chdir(dir)\n> +else:\n> + os_chdir = os.chdir\n> +\n\nStylistic:\n\nI think the naming is a bit ugly (os_); I would write\n\n def chdir(d):\n if os.name == 'nt': .. \n os.chdir(dir)\n\nfor the rest: looks good to me.\n\t\n-- \n Han-Wen Nienhuys - hanwen@xs4all.nl - http://www.xs4all.nl/~hanwen\n"},{"id":"86181","messageId":"bad7471c0808040604p6de570f1k35aa4e76757ca6fd@mail.gmail.com","threadId":"14794","inReplyTo":"bad7471c0808040601y10cceb44idcde5a4a8f415769@mail.gmail.com","subject":"Fwd: [PATCH] git-p4: chdir now properly sets PWD environment variable in msysGit","fromName":"Robert Blum","fromEmail":"rob.blum@gmail.com","sentAt":"2008-08-04T13:04:59Z","receivedAt":"2008-08-04T13:04:59Z","isPatch":true,"sender":{"key":"rob.blum@gmail.com","avatar":null},"body":"Forgot reply-all - forwarding to list....\n\n> Missing full stop at the end of sentence aside, this comment makes me\n> wonder if there is an optional way to have it set it, as opposed to the\n> inconvenient way it behaves \"by defualt\".\n\nNot that I'm aware of. My 'by default' comment refers to a default\nPython installation, i.e. unpatched. As far as I know, the real\nculprit in this is p4. I'd argue it's a bug, since they should be\nusing getcwd(), not getpwd(). Moot point - I don't want to wait for\ngit-p4 until I have convinced perforce to fix this and a new p4 rolls\naround ;)\n\n> If there is none, I think your\n> patch, even though it looks ugly, is the least evil approach.\n\nWarms the cockles of my heart ;)\n\n> Another way\n> might be to wrap callsites of system() by introducing a \"run_p4\" function,\n> like:\n>\n> def run_p4(arg):\n> os.environ['PWD'] = os.getcwd() if os.name == 'nt'\n> return system(arg)\n\nHappy to submit a new patch with that, if that's preferred.\n\n> Thanks. I've been waiting for an Ack from somewhere or success reports\n> from p4 users on Windows.\n\nHan-Wen seems OK with it. (BTW: Who *is* the maintainer of git-p4?)\nBut hold off on applying - I'll resubmit with the run_p4 approach\ntoday.\n\n(Oh, and of course: Works for me ;)\n\n - Robert\n"},{"id":"86265","messageId":"200808051031.00121.simon@lst.de","threadId":"14794","inReplyTo":"7v7iaxu7i1.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] git-p4: chdir now properly sets PWD environment variable in msysGit","fromName":"Simon Hausmann","fromEmail":"simon@lst.de","sentAt":"2008-08-05T08:24:22Z","receivedAt":"2008-08-05T08:24:22Z","isPatch":true,"sender":{"key":"hausmann@kde.org","avatar":"https://gravatar.com/avatar/bc9aad4fb31dce17eb66e690e7b51fe980c62da3c225c785da35dd806b8da778?d=mp&s=160"},"body":"On Sunday 03 August 2008 23:13:42 Junio C Hamano wrote:\n> \"Robert Blum\" writes:\n> > P4 on Windows expects the PWD environment variable to be set to the\n> > current working dir, but os.chdir in python doesn't do that by default\n>\n> Missing full stop at the end of sentence aside, this comment makes me\n> wonder if there is an optional way to have it set it, as opposed to the\n> inconvenient way it behaves \"by defualt\". If there is none, I think your\n> patch, even though it looks ugly, is the least evil approach. Another way\n> might be to wrap callsites of system() by introducing a \"run_p4\" function,\n> like:\n>\n> \tdef run_p4(arg):\n> \tos.environ['PWD'] = os.getcwd() if os.name == 'nt'\n> \t\treturn system(arg)\n>\n> > ---\n> >\n> > Pushing it out to the list since I'm not entirely sure who the git-p4\n> > owner even is. CC'ed likely suspects for ownership ;)\n>\n> Thanks. I've been waiting for an Ack from somewhere or success reports\n> from p4 users on Windows.\n\nAcked-by: Simon Hausmann \n\nIt may not be the prettiest solution, but I agree it needs to be solved :)\n\nSimon\n"},{"id":"86299","messageId":"81b0412b0808051523q71c4ca6ap719f3603aa7b44d4@mail.gmail.com","threadId":"14794","inReplyTo":"bad7471c0808011250v569ffaaby9e20a5ba1f971927@mail.gmail.com","subject":"Re: [PATCH] git-p4: chdir now properly sets PWD environment variable in msysGit","fromName":"Alex Riesen","fromEmail":"raa.lkml@gmail.com","sentAt":"2008-08-05T22:23:54Z","receivedAt":"2008-08-05T22:23:54Z","isPatch":true,"sender":{"key":"raa.lkml@gmail.com","avatar":"https://avatars.githubusercontent.com/u/324101?v=4"},"body":"2008/8/1 Robert Blum :\n> P4 on Windows expects the PWD environment variable to be set to the\n> current working dir, but os.chdir in python doesn't do that by default\n\nTry just removing it (if you can). It worked for me (in bash of cygwin).\n"},{"id":"87365","messageId":"7vskt5jyzh.fsf@gitster.siamese.dyndns.org","threadId":"14794","inReplyTo":"bad7471c0808040604p6de570f1k35aa4e76757ca6fd@mail.gmail.com","subject":"Re: Fwd: [PATCH] git-p4: chdir now properly sets PWD environment variable in msysGit","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-08-16T05:44:02Z","receivedAt":"2008-08-16T05:44:02Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Robert Blum\" writes:\n\n> Forgot reply-all - forwarding to list....\n>\n>> Missing full stop at the end of sentence aside, this comment makes me\n>> wonder if there is an optional way to have it set it, as opposed to the\n>> inconvenient way it behaves \"by defualt\".\n>\n> Not that I'm aware of. My 'by default' comment refers to a default\n> Python installation, i.e. unpatched. As far as I know, the real\n> culprit in this is p4. I'd argue it's a bug, since they should be\n> using getcwd(), not getpwd(). Moot point - I don't want to wait for\n> git-p4 until I have convinced perforce to fix this and a new p4 rolls\n> around ;)\n>\n>> If there is none, I think your\n>> patch, even though it looks ugly, is the least evil approach.\n>\n> Warms the cockles of my heart ;)\n>\n>> Another way\n>> might be to wrap callsites of system() by introducing a \"run_p4\" function,\n>> like:\n>>\n>> def run_p4(arg):\n>> os.environ['PWD'] = os.getcwd() if os.name == 'nt'\n>> return system(arg)\n>\n> Happy to submit a new patch with that, if that's preferred.\n>\n>> Thanks. I've been waiting for an Ack from somewhere or success reports\n>> from p4 users on Windows.\n>\n> Han-Wen seems OK with it. (BTW: Who *is* the maintainer of git-p4?)\n> But hold off on applying - I'll resubmit with the run_p4 approach\n> today.\n>\n> (Oh, and of course: Works for me ;)\n\nI have been wondering what happened to this thread afterwards.\n"},{"id":"87366","messageId":"7vabfdjyet.fsf@gitster.siamese.dyndns.org","threadId":"14794","inReplyTo":"7vskt5jyzh.fsf@gitster.siamese.dyndns.org","subject":"Re: Fwd: [PATCH] git-p4: chdir now properly sets PWD environment variable in msysGit","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-08-16T05:56:26Z","receivedAt":"2008-08-16T05:56:26Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> \"Robert Blum\" writes:\n>\n>> Forgot reply-all - forwarding to list....\n> ...\n> I have been wondering what happened to this thread afterwards.\n\nIn the meantime, here is what I am inclined to apply. The same approach\nas the original, taking stylistic suggestion from Han-Wen.\n\n-- >8 --\nFrom: Robert Blum \nDate: Fri, 1 Aug 2008 12:50:03 -0700\nSubject: [PATCH] git-p4: chdir now properly sets PWD environment variable in msysGit\n\nP4 on Windows expects the PWD environment variable to be set to the\ncurrent working dir, but os.chdir in python doesn't do so.\n\nSigned-off-by: Robert Blum \nAcked-by: Simon Hausmann \nAcked-by: Han-Wen Nienhuys \nSigned-off-by: Junio C Hamano \n---\n contrib/fast-import/git-p4 | 13 +++++++++----\n 1 files changed, 9 insertions(+), 4 deletions(-)\n\ndiff --git a/contrib/fast-import/git-p4 b/contrib/fast-import/git-p4\nindex 6ae0429..3f2303d 100755\n--- a/contrib/fast-import/git-p4\n+++ b/contrib/fast-import/git-p4\n@@ -16,6 +16,11 @@ from sets import Set;\n \n verbose = False\n \n+def chdir(dir):\n+ if os.name == 'nt':\n+ os.environ['PWD']=dir\n+ os.chdir(dir)\n+\n def die(msg):\n if verbose:\n raise Exception(msg)\n@@ -712,7 +717,7 @@ class P4Submit(Command):\n print \"Perforce checkout for depot path %s located at %s\" % (self.depotPath, self.clientPath)\n self.oldWorkingDirectory = os.getcwd()\n \n- os.chdir(self.clientPath)\n+ chdir(self.clientPath)\n print \"Syncronizing p4 checkout...\"\n system(\"p4 sync ...\")\n \n@@ -732,7 +737,7 @@ class P4Submit(Command):\n \n if len(commits) == 0:\n print \"All changes applied!\"\n- os.chdir(self.oldWorkingDirectory)\n+ chdir(self.oldWorkingDirectory)\n \n sync = P4Sync()\n sync.run([])\n@@ -1670,7 +1675,7 @@ class P4Clone(P4Sync):\n print \"Importing from %s into %s\" % (', '.join(depotPaths), self.cloneDestination)\n if not os.path.exists(self.cloneDestination):\n os.makedirs(self.cloneDestination)\n- os.chdir(self.cloneDestination)\n+ chdir(self.cloneDestination)\n system(\"git init\")\n self.gitdir = os.getcwd() + \"/.git\"\n if not P4Sync.run(self, depotPaths):\n@@ -1782,7 +1787,7 @@ def main():\n if os.path.exists(cmd.gitdir):\n cdup = read_pipe(\"git rev-parse --show-cdup\").strip()\n if len(cdup) > 0:\n- os.chdir(cdup);\n+ chdir(cdup);\n \n if not isValidGitDir(cmd.gitdir):\n if isValidGitDir(cmd.gitdir + \"/.git\"):\n-- \n1.6.0.rc3.11.g8134a\n"}]}